From: Wei Liu <wei.liu2@citrix.com>
To: George Dunlap <george.dunlap@citrix.com>
Cc: Juergen Gross <JGross@suse.com>,
wei.liu2@citrix.com, Ian Campbell <ian.campbell@citrix.com>,
george.dunlap@eu.citrix.com, Ian.Jackson@eu.citrix.com,
Chun Yan Liu <cyliu@suse.com>,
xen-devel@lists.xen.org, Jim Fehlig <JFEHLIG@suse.com>,
Simon Cao <caobosimon@gmail.com>
Subject: Re: [PATCH V6 3/7] libxl: add pvusb API
Date: Tue, 29 Sep 2015 18:19:26 +0100 [thread overview]
Message-ID: <20150929171926.GA7370@zion.uk.xensource.com> (raw)
In-Reply-To: <55FA8DCF.5070405@citrix.com>
On Thu, Sep 17, 2015 at 10:54:23AM +0100, George Dunlap wrote:
[...]
> > Hi, George,
> >
> > I'm still confused about the expected look concerning PV/EMU type handling in
> > this patch series.
> >
> > In earlier version, we tried to extract common things in libxl_usb.c and put
> > pvusb specific thing in libxl_pvusb.c, prefixed with pvusb_xxx. As you
> > suggested, we can leave that when EMU USB patch series added.
> >
> > Now, about how to handle PV/EMU type in this patch series, I can think of 3
> > ways:
> >
> > 1. We define the enumeration (contains PV/AUTO only, user interface only allows
> > 'pv' or 'not specified', so we handle everything in 'pv' way without further
> > check. Leave check and other adjusting things when EMU USB patch series added.
> >
> > 2. We check domain type and set proper type if not specified (i.e. 'pv' for PV
> > guest, 'emu' for HVM guest). In add/remove function, check if type='emu', report
> > 'not supported' directly; otherwise, continue do following things. When EMU USB
> > patch serires added, need to extract common things and adjust the check place.
> >
> > 3. Same as 2, but extract common things, only in PV/EMU USB specific part, check
> > type, if type='emu', report 'not supported'; otherwise, do pvusb work. When
> > adding EMU USB patch series, only need to add EMU USB specific things in the
> > type='emu' branch.
> >
> > Which one is expected? Or none?
>
> So there are two questions here, first WRT the code, the second WRT the
> interface.
>
> WRT the code, *normally* the first person to submit the code gets to
> have it easy, and the second person has to do all the work of
> refactoring. So you would be completely within your rights to simply
> submit "libxl_usb.c", and make me refactor that into libxl_pvusb.c and
> whatever else (probably the qemu stuff would go in libxl_qmp.c).
>
> Earlier I asked you as a favor to put things in libxl_pvusb.c, and you
> were kind enough to do so -- so thank you. Just having things roughly
> where they might end up eventually has already been a big help. I'll
> have to move some of the code around pretty much no matter what you do.
> So I don't think it's worth making any more effort wrt the code itself.
>
> WRT the interface -- if we do a release with PV defined, but not
> EMU/DEVICEMODEL, then when we add that option, we'll have to add Yet
> Another LIBL_HAS_BLAH. I would personally like too avoid that.
>
(Note that I didn't go through all emails)
I think adding yet another LIBXL_HAS_BLAH wouldn't be a problem. Chunyan
will have to add one anyway.
> As it happens, if you were to check this in now at the beginning of the
> cycle, it's very likely I could get the EMU side in before the release.
> So it's *probably* OK to just write AUTO and PV.
>
Again, extending the interface shouldn't be a problem -- we do that all
the time. So IMHO having AUTO and PV only is OK.
> I would personally prefer to play it safe and give all three interface
> elements (AUTO, PV, and EMU/DEVICEMODEL), and return ENOTSUPP (or
> whatever) for DEVICEMODEL until it's implemented. But that's really a
> policy decision for the maintianers at this point.
>
This works for me too. I don't really see a problem in choosing one way
or another TBH.
Wei.
> -George
next prev parent reply other threads:[~2015-09-29 17:19 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-10 10:35 [PATCH V6 0/7] xen pvusb toolstack work Chunyan Liu
2015-08-10 10:35 ` [PATCH V6 1/7] libxl: export some functions for pvusb use Chunyan Liu
2015-08-11 11:26 ` Wei Liu
2015-08-10 10:35 ` [PATCH V6 2/7] libxl_read_file_contents: add new entry to read sysfs file Chunyan Liu
2015-08-11 11:26 ` Wei Liu
2015-08-12 2:37 ` Chun Yan Liu
2015-08-13 9:11 ` Wei Liu
2015-08-10 10:35 ` [PATCH V6 3/7] libxl: add pvusb API Chunyan Liu
2015-08-11 11:27 ` Wei Liu
2015-08-12 2:24 ` Chun Yan Liu
2015-08-13 9:09 ` Wei Liu
2015-08-14 1:49 ` Chun Yan Liu
2015-08-18 2:31 ` Chun Yan Liu
2015-08-31 6:10 ` Chun Yan Liu
2015-09-08 14:17 ` Ian Campbell
2015-09-08 16:52 ` George Dunlap
2015-09-09 7:38 ` Chun Yan Liu
2015-09-17 8:19 ` Chun Yan Liu
2015-09-17 9:54 ` George Dunlap
2015-09-29 17:19 ` Wei Liu [this message]
2015-09-17 8:20 ` Chun Yan Liu
2015-09-11 5:42 ` Chun Yan Liu
2015-09-11 13:26 ` Ian Campbell
2015-09-11 13:55 ` Juergen Gross
2015-09-11 14:09 ` Ian Campbell
2015-09-11 14:18 ` Juergen Gross
2015-09-11 14:41 ` Ian Campbell
2015-09-11 15:42 ` Ian Jackson
2015-09-14 3:48 ` Juergen Gross
2015-09-14 10:36 ` George Dunlap
2015-09-14 10:53 ` Juergen Gross
2015-09-14 11:12 ` Ian Jackson
2015-09-14 11:23 ` Juergen Gross
2015-09-14 14:03 ` George Dunlap
2015-09-17 8:24 ` Chun Yan Liu
2015-09-15 8:14 ` Chun Yan Liu
2015-08-10 10:35 ` [PATCH V6 4/7] libxl: add libxl_device_usb_assignable_list API Chunyan Liu
2015-08-10 10:35 ` [PATCH V6 5/7] xl: add pvusb commands Chunyan Liu
2015-08-10 10:35 ` [PATCH V6 6/7] xl: add usb-assignable-list command Chunyan Liu
2015-08-10 10:35 ` [PATCH V6 7/7] domcreate: support pvusb in configuration file Chunyan Liu
2015-08-11 11:27 ` Wei Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150929171926.GA7370@zion.uk.xensource.com \
--to=wei.liu2@citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=JFEHLIG@suse.com \
--cc=JGross@suse.com \
--cc=caobosimon@gmail.com \
--cc=cyliu@suse.com \
--cc=george.dunlap@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=ian.campbell@citrix.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.