From: George Dunlap <George.Dunlap@eu.citrix.com>
To: Chunyan Liu <cyliu@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
Ian Campbell <ian.campbell@citrix.com>,
Ian Jackson <Ian.Jackson@eu.citrix.com>,
"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
Jim Fehlig <jfehlig@suse.com>, Simon Cao <caobosimon@gmail.com>
Subject: Re: [PATCH V4 3/7] libxl: add pvusb API
Date: Mon, 15 Jun 2015 15:17:51 +0100 [thread overview]
Message-ID: <CAFLBxZbVXjj-VM4KLdhqBkpC0qSR_sy3mNnSZAJQJmHuo4ZC0Q@mail.gmail.com> (raw)
In-Reply-To: <1433906441-3280-4-git-send-email-cyliu@suse.com>
On Wed, Jun 10, 2015 at 4:20 AM, Chunyan Liu <cyliu@suse.com> wrote:
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 23f27d4..4561e1b 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -541,6 +541,29 @@ libxl_device_pci = Struct("device_pci", [
> ("seize", bool),
> ])
>
> +libxl_usb_protocol = Enumeration("usb_protocol", [
> + (0, "AUTO"),
> + (1, "PV"),
> + (2, "QEMU"),
> + ])
> +
> +libxl_device_usbctrl = Struct("device_usbctrl", [
> + ("protocol", libxl_usb_protocol),
> + ("devid", libxl_devid),
> + ("version", integer),
> + ("ports", integer),
> + ("backend_domid", libxl_domid),
> + ("backend_domname", string),
> + ])
> +
> +libxl_device_usb = Struct("device_usb", [
> + ("ctrl", libxl_devid),
> + ("port", integer),
> + ("busid", string),
> + ("hostbus", integer),
> + ("hostaddr", integer),
> + ])
Ian / Ian / Wei / Jim:
Question about the design of the interface here.
The way that most systems in Linux specify particular USB devices is
with the bus:addr format. It's the output you get when you run tools
like lsusb, for example, and it's the interface that qemu (and thus
KVM) uses when talking about host devices to assign. bus:addr might
look like "002:006".
But the bus:addr is a "public api" for Linux; internally, it has a
more structured format, which contains more about the USB topology.
Chunyan is calling this "busid". An example is something like this:
"2-3.1.1:1.0".
Internally, pvusb needs "busid" in order to find the right sysfs
files. qemu, on the other hand, does not take busid; so the
devicemodel / HVM implementation of USB would need bus:addr
internally.
Converting bus:addr to busid involves looping through all devices and
seeing which one matches, so it's inefficient to do it every time we
need to do some operation. (See libxl_pvusb.c:usb_busaddr_to_busid().)
Converting busid to bus:addr is simpler: you read the bus and addr
keys from sysfs using the busid.
I think for a public UI (i.e., xl), bus:addr is really the only
option; I'm pretty sure that's the main interface libvirt provides as
well.
At the libxl level, we basically have 3 options:
1. Have the libxl layer accept bus:addr. For the pvusb code,
translate bus:addr to busid and place it in an internal structure.
2. Have the libxl layer accept busid. For the devicemodel code,
2a. ...translate busid to bus:addr and place it in an internal structure.
2b. ...translate it to bus:addr when it's used (i.e., when speaking
over qmp to qemu).
3. Have the libxl layer accept both busid and bus:addr. Translate as
necessary and store in the libxl_device_usb struct.
For #2, I think we need to provide a helper function to convert
bus:addr into busid, since bus:addr is the interface most toolstacks
will want to expose to their users.
Chunyan's initial submission had #2 (without a or b, since the
devicemodel code doesn't exist yet). I suggested #1, and she pushed
back by compromising (#3).
The advantage of #3 internally is that the functions can do the
translation once (if necessary), and can then pass around the public
libxl_device_usb struct as-is without needing any extra parameters or
a separate libxl_device_usb_internal. The disadvantage, I think, is
that from an interface perspective, it's fairly pointless to have
both. busid doesn't really give you any better or more control than
the other, and it's not any more convenient for the user (in fact it's
less convenient because it's more difficult to find).
I *think* 2b wouldn't be terrible from a performance point of view,
since (I think) you'd probably only have to do the translation once
before calling the qmp command.
I still prefer #1; but if people thought #3 or #2b would be
preferable, I could live with that.
Any thoughts?
-George
next prev parent reply other threads:[~2015-06-15 14:17 UTC|newest]
Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-10 3:20 [PATCH V4 0/7] xen pvusb toolstack work Chunyan Liu
2015-06-10 3:20 ` [PATCH V4 1/7] libxl: export some functions for pvusb use Chunyan Liu
2015-06-11 16:08 ` Ian Jackson
2015-06-11 16:28 ` Wei Liu
2015-06-12 15:14 ` Ian Jackson
2015-06-10 3:20 ` [PATCH V4 2/7] libxl_read_file_contents: add new entry to read sysfs file Chunyan Liu
2015-06-11 16:16 ` Ian Jackson
2015-06-12 7:00 ` Chun Yan Liu
2015-06-12 15:11 ` Ian Jackson
2015-06-10 3:20 ` [PATCH V4 3/7] libxl: add pvusb API Chunyan Liu
2015-06-11 15:00 ` Juergen Gross
2015-06-11 16:07 ` Ian Jackson
2015-06-11 16:42 ` Ian Jackson
2015-06-12 7:39 ` Chun Yan Liu
2015-06-12 8:06 ` Chun Yan Liu
2015-06-12 11:22 ` Ian Jackson
2015-06-15 14:17 ` George Dunlap [this message]
2015-06-15 14:25 ` Jürgen Groß
2015-06-15 14:34 ` George Dunlap
2015-06-15 18:26 ` Juergen Gross
2015-06-16 10:30 ` George Dunlap
2015-06-16 10:51 ` Juergen Gross
2015-06-16 11:11 ` George Dunlap
2015-06-16 11:19 ` Juergen Gross
2015-06-16 11:23 ` George Dunlap
2015-06-16 11:44 ` Ian Jackson
2015-06-17 11:24 ` Ian Campbell
2015-06-18 11:50 ` George Dunlap
2015-06-18 12:08 ` George Dunlap
2015-06-18 13:03 ` Juergen Gross
2015-06-22 13:29 ` Proposed plan for libxl USB interface (was Re: [PATCH V4 3/7] libxl: add pvusb API) George Dunlap
2015-06-22 14:14 ` Juergen Gross
2015-06-22 14:22 ` Ian Jackson
2015-06-23 2:42 ` Chun Yan Liu
2015-06-23 2:43 ` Chun Yan Liu
2015-06-23 2:44 ` Chun Yan Liu
2015-06-16 10:41 ` [PATCH V4 3/7] libxl: add pvusb API Ian Jackson
2015-06-16 10:56 ` Jürgen Groß
2015-06-16 11:03 ` George Dunlap
2015-06-16 11:10 ` Ian Jackson
2015-06-16 11:25 ` Juergen Gross
2015-06-16 11:45 ` George Dunlap
2015-06-16 12:02 ` Ian Jackson
2015-06-16 13:19 ` George Dunlap
2015-06-16 13:32 ` Juergen Gross
2015-06-16 13:37 ` [PATCH V4 3/7] libxl: add pvusb API [and 1 more messages] Ian Jackson
2015-06-16 14:41 ` George Dunlap
2015-06-16 15:58 ` Sander Eikelenboom
2015-06-16 15:59 ` Ian Jackson
2015-06-16 16:34 ` George Dunlap
2015-06-17 3:59 ` Juergen Gross
2015-06-17 10:27 ` George Dunlap
2015-06-18 6:24 ` Chun Yan Liu
2015-06-16 11:45 ` [PATCH V4 3/7] libxl: add pvusb API Ian Jackson
2015-06-16 13:06 ` Juergen Gross
2015-06-16 13:09 ` George Dunlap
2015-06-16 13:23 ` Juergen Gross
2015-06-16 13:29 ` George Dunlap
2015-06-16 13:49 ` Juergen Gross
2015-06-16 14:06 ` George Dunlap
2015-06-16 14:20 ` Juergen Gross
2015-06-16 14:37 ` George Dunlap
2015-06-17 11:34 ` Ian Campbell
2015-06-17 11:40 ` Juergen Gross
2015-06-18 6:20 ` Chun Yan Liu
2015-06-18 7:02 ` Juergen Gross
2015-06-18 8:50 ` Ian Campbell
2015-06-18 13:02 ` Juergen Gross
2015-06-16 15:38 ` George Dunlap
2015-06-16 11:01 ` George Dunlap
2015-06-16 11:12 ` Ian Jackson
2015-06-16 11:21 ` George Dunlap
2015-06-16 16:32 ` Ian Jackson
2015-06-16 16:39 ` George Dunlap
2015-06-16 16:51 ` Ross Philipson
2015-06-17 4:03 ` Jürgen Groß
2015-06-17 13:48 ` Ross Philipson
2015-06-15 17:47 ` George Dunlap
2015-06-23 10:18 ` Chun Yan Liu
2015-06-23 11:29 ` George Dunlap
2015-06-24 2:26 ` Chun Yan Liu
2015-06-10 3:20 ` [PATCH V4 4/7] libxl: add libxl_device_usb_assignable_list API Chunyan Liu
2015-06-10 3:20 ` [PATCH V4 5/7] xl: add pvusb commands Chunyan Liu
2015-06-12 7:37 ` Juergen Gross
2015-06-12 8:03 ` Chun Yan Liu
2015-06-12 8:22 ` Juergen Gross
2015-06-10 3:20 ` [PATCH V4 6/7] xl: add usb-assignable-list command Chunyan Liu
2015-06-10 3:20 ` [PATCH V4 7/7] domcreate: support pvusb in configuration file Chunyan 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=CAFLBxZbVXjj-VM4KLdhqBkpC0qSR_sy3mNnSZAJQJmHuo4ZC0Q@mail.gmail.com \
--to=george.dunlap@eu.citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=caobosimon@gmail.com \
--cc=cyliu@suse.com \
--cc=ian.campbell@citrix.com \
--cc=jfehlig@suse.com \
--cc=wei.liu2@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.