On Mon, Jul 28, 2008 at 11:02:45AM -0400, Guido G?nther wrote:
> On Fri, Jul 25, 2008 at 04:17:30PM -0400, Guido Günther wrote:
> > attached is some basic support for host device passthrough. It enables
> > you to passthrough usb devices in qemu/kvm via:
> On top of the hostdev passthrough (but it's actually totally
> independent) I added usb massstorage backed by a file. Very handy for
> installer testing where the preseed data is on the USB stick and the
> CD/DVD is the installation medium.
> At the moment I'm using a dummy target "usbdisk" so we don't have to
> check for target == NULL in that many places. Once qemu handles it we
> can fill in bus and device address for unplugging. To add a file as usb
> massstorage to the guest you can use:
>
> <disk type='file' device='disk'>
> <source file='/foo/bar/usbmass.img'/>
> <target bus='usb'/>
> </disk>
The way we do target for other disk types doesn't work so well - others
we typically have a device name, eg hda, sdc, xvdf, etc. For VirtIO we
let people pass in a dummy device name too 'vda', 'vdb', etc. Only some
of the disk buses & guest OS honour these names though - in cases where
they're not honoured, they at most provide a unique key, and allow for
ordering of disks in our XML. USB will be much like SCSI / VirtIO where
the device name in the XML is merely used for ordering, so we should just
allow sda, sdb, etc as the target device name. This will avoid having
to special case the code in the here too much.
> ---
> src/domain_conf.c | 40 ++++++++++++++++++++++++++++------------
> src/domain_conf.h | 1 +
> src/qemu_conf.c | 23 +++++++++++++++++++++--
> src/qemu_driver.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 91 insertions(+), 14 deletions(-)
>
> diff --git a/src/domain_conf.c b/src/domain_conf.c
> index d36caeb..74ceecc 100644
> --- a/src/domain_conf.c
> +++ b/src/domain_conf.c
> @@ -84,7 +84,8 @@ VIR_ENUM_IMPL(virDomainDiskBus, VIR_DOMAIN_DISK_BUS_LAST,
> "fdc",
> "scsi",
> "virtio",
> - "xen")
> + "xen",
> + "usb")
This is fine.
>
> VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
> "user",
> @@ -540,6 +541,14 @@ virDomainDiskDefParseXML(virConnectPtr conn,
> def->device = VIR_DOMAIN_DISK_DEVICE_DISK;
> }
>
> + if (bus) {
> + if ((def->bus = virDomainDiskBusTypeFromString(bus)) < 0) {
> + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> + _("unknown disk bus type '%s'"), bus);
> + goto error;
> + }
> + }
> +
No need to move this - see further down.
> /* Only CDROM and Floppy devices are allowed missing source path
> * to indicate no media present */
> if (source == NULL &&
> @@ -550,10 +559,16 @@ virDomainDiskDefParseXML(virConnectPtr conn,
> goto error;
> }
>
> + /* only USB devices are allowed missing target path since the hypervisor
> + * can assign bus and device number */
> if (target == NULL) {
> - virDomainReportError(conn, VIR_ERR_NO_TARGET,
> - source ? "%s" : NULL, source);
> - goto error;
> + if (def->bus == VIR_DOMAIN_DISK_BUS_USB) {
> + target = strdup("usbdisk");
> + } else {
> + virDomainReportError(conn, VIR_ERR_NO_TARGET,
> + source ? "%s" : NULL, source);
> + goto error;
> + }
No need - we'll just define that 'sdXXXX' is the prefix to use.
> }
>
> if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
> @@ -571,19 +586,14 @@ virDomainDiskDefParseXML(virConnectPtr conn,
> !STRPREFIX((const char *)target, "hd") &&
> !STRPREFIX((const char *)target, "sd") &&
> !STRPREFIX((const char *)target, "vd") &&
> - !STRPREFIX((const char *)target, "xvd")) {
> + !STRPREFIX((const char *)target, "xvd") &&
> + !STREQ((const char*)target, "usbdisk")) {
Again, this is not needed.
> virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> _("Invalid harddisk device name: %s"), target);
> goto error;
> }
>
> - if (bus) {
> - if ((def->bus = virDomainDiskBusTypeFromString(bus)) < 0) {
> - virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> - _("unknown disk bus type '%s'"), bus);
> - goto error;
> - }
> - } else {
> + if (!bus) {
> if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
> def->bus = VIR_DOMAIN_DISK_BUS_FDC;
> } else {
> @@ -612,6 +622,12 @@ virDomainDiskDefParseXML(virConnectPtr conn,
> _("Invalid bus type '%s' for disk"), bus);
> goto error;
> }
> + if (def->device != VIR_DOMAIN_DISK_DEVICE_DISK &&
> + def->bus == VIR_DOMAIN_DISK_BUS_USB) {
> + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
> + _("Invalid bus type '%s' for usb disk"), bus);
> + goto error;
> + }
It is possible to have USB floppys and CDROMs. So this check should be
made in the underlying drivers at time of use, rather than in the
parser. eg, move this into qemudBuildCommandLine() method, or the
device hotplug method for QEMU. Similarly for other hypervisor backends
as needed.
> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
> index 7678ac5..868b3dc 100644
> --- a/src/qemu_conf.c
> +++ b/src/qemu_conf.c
> @@ -55,7 +55,8 @@ VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST,
> "floppy",
> "scsi",
> "virtio",
> - "xen")
> + "xen",
> + "usb")
>
>
> #define qemudLog(level, msg...) fprintf(stderr, msg)
> @@ -772,6 +773,13 @@ int qemudBuildCommandLine(virConnectPtr conn,
> goto no_memory; \
> } while (0)
>
> +#define ADD_USBDISK(thisarg) \
> + do { \
> + ADD_ARG_LIT("-usbdevice"); \
> + if ((asprintf(&qargv[qargc++], "disk:%s", thisarg)) == -1) \
> + goto no_memory; \
> + } while (0)
> +
> snprintf(memory, sizeof(memory), "%lu", vm->def->memory/1024);
> snprintf(vcpus, sizeof(vcpus), "%lu", vm->def->vcpus);
>
> @@ -883,6 +891,12 @@ int qemudBuildCommandLine(virConnectPtr conn,
> int idx = virDiskNameToIndex(disk->dst);
> const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
>
> + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
> + ADD_USBDISK(disk->src);
> + disk = disk->next;
> + continue;
> + }
> +
The indentation is messed up here.
Annoying that QEMU's -drive arg doesn't support USB :-(
> if (idx < 0) {
> qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> _("unsupported disk type '%s'"), disk->dst);
> @@ -924,6 +938,12 @@ int qemudBuildCommandLine(virConnectPtr conn,
> char dev[NAME_MAX];
> char file[PATH_MAX];
>
> + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
> + ADD_USBDISK(disk->src);
> + disk = disk->next;
> + continue;
> + }
More indentation issues.
> +
> if (STREQ(disk->dst, "hdc") &&
> disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
> if (disk->src) {
> @@ -947,7 +967,6 @@ int qemudBuildCommandLine(virConnectPtr conn,
>
> ADD_ARG_LIT(dev);
> ADD_ARG_LIT(file);
> -
> disk = disk->next;
> }
> }
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 3381d10..73b6da4 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -2979,6 +2979,44 @@ static int qemudDomainAttachCdromDevice(virDomainPtr dom,
> return 0;
> }
>
> +static int qemudDomainAttachUsbMassstorageDevice(virDomainPtr dom, virDomainDeviceDefPtr dev)
> +{
> + struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;
> + virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid);
> + int ret;
> + char *cmd, *reply;
> +
> + ret = asprintf(&cmd, "usb_add disk:%s", dev->data.disk->src);
> +
> + if (ret == -1) {
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> + "%s", _("out of memory"));
> + return ret;
> + }
Minor indentation issue there.
> +
> + if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> + "%s", _("cannot attach usb device"));
> + VIR_FREE(cmd);
> + return -1;
> + }
> +
> + DEBUG ("attach_usb reply: %s", reply);
> + /* If the command failed qemu prints:
> + * Could not add ... */
> + if (strstr(reply, "Could not add ")) {
> + qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> + "%s",
> + _("adding usb device failed"));
> + VIR_FREE(reply);
> + VIR_FREE(cmd);
> + return -1;
> + }
> + VIR_FREE(reply);
> + VIR_FREE(cmd);
> + return 0;
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list