4:13 p.m.
USB devices can disappear without OS being mad about it, which makes
them ideal for startupPolicy so that a domain with USB devices can be
migrated to a host that lacks the USB devices. Moreover, this series
allows USB devices to be re-plugged or a host to be rebooted while
domains with USB devices are (managed)saved.
Jiri Denemark (6):
conf: Add support for startupPolicy for USB devices
qemu: Introduce qemuFindHostdevUSBDevice
qemu: Add option to treat missing USB devices as success
qemu: Implement startupPolicy for USB passed through devices
Add MIGRATABLE flag for virDomainGetXMLDesc
qemu: Make save/restore with USB devices usable
docs/formatdomain.html.in | 28 ++++++--
docs/schemas/domaincommon.rng | 3 +
include/libvirt/libvirt.h.in | 1 +
src/conf/domain_conf.c | 45 ++++++++++++-
src/conf/domain_conf.h | 4 ++
src/qemu/qemu_cgroup.c | 2 +
src/qemu/qemu_command.c | 20 ++++--
src/qemu/qemu_domain.c | 21 +++---
src/qemu/qemu_domain.h | 11 ++--
src/qemu/qemu_driver.c | 19 +++---
src/qemu/qemu_hostdev.c | 137 +++++++++++++++++++++++++++++-----------
src/qemu/qemu_hostdev.h | 6 +-
src/qemu/qemu_hotplug.c | 33 +---------
src/qemu/qemu_migration.c | 12 ++--
src/qemu/qemu_process.c | 16 ++---
src/security/security_dac.c | 16 +++--
src/security/security_selinux.c | 16 +++--
src/util/hostusb.c | 91 ++++++++++++++++++--------
src/util/hostusb.h | 22 ++++---
tools/virsh-domain.c | 4 ++
tools/virsh.pod | 7 +-
21 files changed, 351 insertions(+), 163 deletions(-)
--
1.7.12
4:13 p.m.
New subject: [libvirt] [PATCH 1/6] conf: Add support for startupPolicy for USB devices
USB devices can disappear without OS being mad about it, which makes
them ideal for startupPolicy. With this attribute, USB devices can be
configured to be mandatory (the default), requisite (will disappear
during migration if they cannot be found), or completely optional.
---
docs/formatdomain.html.in | 28 ++++++++++++++++++++++++----
docs/schemas/domaincommon.rng | 3 +++
src/conf/domain_conf.c | 32 +++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 1 +
4 files changed, 59 insertions(+), 5 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3cc6c51..44fa0d9 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2056,7 +2056,7 @@
...
<devices>
<hostdev mode='subsystem' type='usb'>
- <source>
+ <source startupPolicy='optional'>
<vendor id='0x1234'/>
<product id='0xbeef'/>
</source>
@@ -2100,9 +2100,29 @@
<dd>The source element describes the device as seen from the host.
The USB device can either be addressed by vendor / product id using the
<code>vendor</code> and <code>product</code> elements or by
the device's
- address on the hosts using the <code>address</code> element.
- PCI devices on the other hand can only be described by their
- <code>address</code></dd>
+ address on the hosts using the <code>address</code> element. PCI
devices
+ on the other hand can only be described by their <code>address</code>.
+
+ <span class="since">Since 0.10.3</span>, the
<code>source</code> element
+ of USB devices may contain <code>startupPolicy</code> attribute which
can
+ be used to define policy what to do if the specified host USB device is
+ not found. The attribute accepts the following values:
+ <table class="top_table">
+ <tr>
+ <td> mandatory </td>
+ <td> fail if missing for any reason (the default) </td>
+ </tr>
+ <tr>
+ <td> requisite </td>
+ <td> fail if missing on boot up,
+ drop if missing on migrate/restore/revert </td>
+ </tr>
+ <tr>
+ <td> optional </td>
+ <td> drop if missing at any start attempt </td>
+ </tr>
+ </table>
+ </dd>
<dt><code>vendor</code>,
<code>product</code></dt>
<dd>The <code>vendor</code> and <code>product</code>
elements each have an
<code>id</code> attribute that specifies the USB vendor and product
id.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f47fdad..38dab7e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2763,6 +2763,9 @@
</optional>
<group>
<element name="source">
+ <optional>
+ <ref name="startupPolicy"/>
+ </optional>
<choice>
<group>
<ref name="usbproduct"/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 33e1e7f..da2d86d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2694,6 +2694,20 @@ virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node,
int ret = -1;
int got_product, got_vendor;
xmlNodePtr cur;
+ char *startupPolicy = NULL;
+
+ if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
+ def->startupPolicy =
+ virDomainStartupPolicyTypeFromString(startupPolicy);
+ if (def->startupPolicy <= 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Unknown startup policy '%s'"),
+ startupPolicy);
+ VIR_FREE(startupPolicy);
+ goto out;
+ }
+ VIR_FREE(startupPolicy);
+ }
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized*/
@@ -2957,6 +2971,15 @@ virDomainHostdevPartsParse(xmlNodePtr node,
_("Missing <source> element in hostdev device"));
goto error;
}
+
+ if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB &&
+ virXPathBoolean("boolean(./source/@startupPolicy)", ctxt)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Setting startupPolicy is only allowed for USB"
+ " devices"));
+ goto error;
+ }
+
switch (def->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
if (virDomainHostdevSubsysPciDefParseXML(sourcenode, def, flags) < 0)
@@ -12043,7 +12066,14 @@ virDomainHostdevSourceFormat(virBufferPtr buf,
unsigned int flags,
bool includeTypeInAddr)
{
- virBufferAddLit(buf, "<source>\n");
+ virBufferAddLit(buf, "<source");
+ if (def->startupPolicy) {
+ const char *policy;
+ policy = virDomainStartupPolicyTypeToString(def->startupPolicy);
+ virBufferAsprintf(buf, " startupPolicy='%s'", policy);
+ }
+ virBufferAddLit(buf, ">\n");
+
virBufferAdjustIndent(buf, 2);
switch (def->source.subsys.type)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 14dead3..fa5d1f4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -384,6 +384,7 @@ struct _virDomainHostdevSubsys {
struct _virDomainHostdevDef {
virDomainDeviceDef parent; /* higher level Def containing this */
int mode; /* enum virDomainHostdevMode */
+ int startupPolicy; /* enum virDomainStartupPolicy */
unsigned int managed : 1;
union {
virDomainHostdevSubsys subsys;
--
1.7.12
7:31 p.m.
New subject: [libvirt] [PATCH 1/6] conf: Add support for startupPolicy for USB devices
On Tue, Oct 09, 2012 at 02:13:22PM +0200, Jiri Denemark wrote:
USB devices can disappear without OS being mad about it, which makes
them ideal for startupPolicy. With this attribute, USB devices can be
configured to be mandatory (the default), requisite (will disappear
during migration if they cannot be found), or completely optional.
---
docs/formatdomain.html.in | 28 ++++++++++++++++++++++++----
docs/schemas/domaincommon.rng | 3 +++
src/conf/domain_conf.c | 32 +++++++++++++++++++++++++++++++-
src/conf/domain_conf.h | 1 +
4 files changed, 59 insertions(+), 5 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:13 p.m.
New subject: [libvirt] [PATCH 2/6] qemu: Introduce qemuFindHostdevUSBDevice
The code which looks up a USB device specified by hostdev is duplicated
in two places. This patch creates a dedicated function that can be
called in both places.
---
src/qemu/qemu_hostdev.c | 73 +++++++++++++++++++++++++++----------------------
src/qemu/qemu_hostdev.h | 1 +
src/qemu/qemu_hotplug.c | 33 +---------------------
3 files changed, 42 insertions(+), 65 deletions(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 7375d26..017f0ec 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -641,6 +641,44 @@ error:
return -1;
}
+usbDevice *
+qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev)
+{
+ usbDevice *usb = NULL;
+ unsigned vendor = hostdev->source.subsys.u.usb.vendor;
+ unsigned product = hostdev->source.subsys.u.usb.product;
+ unsigned bus = hostdev->source.subsys.u.usb.bus;
+ unsigned device = hostdev->source.subsys.u.usb.device;
+
+ if (vendor && bus) {
+ usb = usbFindDevice(vendor, product, bus, device);
+
+ } else if (vendor && !bus) {
+ usbDeviceList *devs = usbFindDeviceByVendor(vendor, product);
+ if (!devs)
+ return NULL;
+
+ if (usbDeviceListCount(devs) > 1) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("multiple USB devices for %x:%x, "
+ "use <address> to specify one"), vendor,
product);
+ usbDeviceListFree(devs);
+ return NULL;
+ }
+ usb = usbDeviceListGet(devs, 0);
+ usbDeviceListSteal(devs, usb);
+ usbDeviceListFree(devs);
+
+ hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
+ hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
+
+ } else if (!vendor && bus) {
+ usb = usbFindDeviceByBus(bus, device);
+ }
+
+ return usb;
+}
+
static int
qemuPrepareHostUSBDevices(struct qemud_driver *driver,
virDomainDefPtr def)
@@ -663,45 +701,14 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver,
*/
for (i = 0 ; i < nhostdevs ; i++) {
virDomainHostdevDefPtr hostdev = hostdevs[i];
- usbDevice *usb = NULL;
+ usbDevice *usb;
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
continue;
if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
continue;
- unsigned vendor = hostdev->source.subsys.u.usb.vendor;
- unsigned product = hostdev->source.subsys.u.usb.product;
- unsigned bus = hostdev->source.subsys.u.usb.bus;
- unsigned device = hostdev->source.subsys.u.usb.device;
-
- if (vendor && bus) {
- usb = usbFindDevice(vendor, product, bus, device);
-
- } else if (vendor && !bus) {
- usbDeviceList *devs = usbFindDeviceByVendor(vendor, product);
- if (!devs)
- goto cleanup;
-
- if (usbDeviceListCount(devs) > 1) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("multiple USB devices for %x:%x, "
- "use <address> to specify one"), vendor,
product);
- usbDeviceListFree(devs);
- goto cleanup;
- }
- usb = usbDeviceListGet(devs, 0);
- usbDeviceListSteal(devs, usb);
- usbDeviceListFree(devs);
-
- hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
- hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
-
- } else if (!vendor && bus) {
- usb = usbFindDeviceByBus(bus, device);
- }
-
- if (!usb)
+ if (!(usb = qemuFindHostdevUSBDevice(hostdev)))
goto cleanup;
if (usbDeviceListAdd(list, usb) < 0) {
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 14f1fca..204b2d4 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -36,6 +36,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
const unsigned char *uuid,
virDomainHostdevDefPtr *hostdevs,
int nhostdevs);
+usbDevice *qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev);
int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
const char *name,
usbDeviceList *list);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a738b19..59de8e4 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1149,38 +1149,7 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
goto cleanup;
if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
- unsigned vendor = hostdev->source.subsys.u.usb.vendor;
- unsigned product = hostdev->source.subsys.u.usb.product;
- unsigned bus = hostdev->source.subsys.u.usb.bus;
- unsigned device = hostdev->source.subsys.u.usb.device;
-
- if (vendor && bus) {
- usb = usbFindDevice(vendor, product, bus, device);
-
- } else if (vendor && !bus) {
- usbDeviceList *devs = usbFindDeviceByVendor(vendor, product);
- if (!devs)
- goto cleanup;
-
- if (usbDeviceListCount(devs) > 1) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("multiple USB devices for %x:%x, "
- "use <address> to specify one"), vendor,
product);
- usbDeviceListFree(devs);
- goto cleanup;
- }
- usb = usbDeviceListGet(devs, 0);
- usbDeviceListSteal(devs, usb);
- usbDeviceListFree(devs);
-
- hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
- hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
-
- } else if (!vendor && bus) {
- usb = usbFindDeviceByBus(bus, device);
- }
-
- if (!usb)
+ if (!(usb = qemuFindHostdevUSBDevice(hostdev)))
goto cleanup;
if (usbDeviceListAdd(list, usb) < 0) {
--
1.7.12
7:32 p.m.
New subject: [libvirt] [PATCH 2/6] qemu: Introduce qemuFindHostdevUSBDevice
On Tue, Oct 09, 2012 at 02:13:23PM +0200, Jiri Denemark wrote:
The code which looks up a USB device specified by hostdev is
duplicated
in two places. This patch creates a dedicated function that can be
called in both places.
---
src/qemu/qemu_hostdev.c | 73 +++++++++++++++++++++++++++----------------------
src/qemu/qemu_hostdev.h | 1 +
src/qemu/qemu_hotplug.c | 33 +---------------------
3 files changed, 42 insertions(+), 65 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:13 p.m.
New subject: [libvirt] [PATCH 3/6] qemu: Add option to treat missing USB devices as success
All USB device lookup functions emit an error when they cannot find the
requested device. With this patch, their caller can choose if a missing
device is an error or normal condition.
---
src/conf/domain_conf.h | 1 +
src/qemu/qemu_hostdev.c | 57 ++++++++++++++++----------
src/qemu/qemu_hostdev.h | 4 +-
src/qemu/qemu_hotplug.c | 2 +-
src/security/security_dac.c | 16 ++++++--
src/security/security_selinux.c | 16 ++++++--
src/util/hostusb.c | 91 ++++++++++++++++++++++++++++++-----------
src/util/hostusb.h | 22 ++++++----
8 files changed, 145 insertions(+), 64 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fa5d1f4..3cb1193 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -386,6 +386,7 @@ struct _virDomainHostdevDef {
int mode; /* enum virDomainHostdevMode */
int startupPolicy; /* enum virDomainStartupPolicy */
unsigned int managed : 1;
+ unsigned int missing : 1;
union {
virDomainHostdevSubsys subsys;
struct {
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 017f0ec..32c858b 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -641,42 +641,57 @@ error:
return -1;
}
-usbDevice *
-qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev)
+int
+qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev,
+ bool mandatory,
+ usbDevice **usb)
{
- usbDevice *usb = NULL;
unsigned vendor = hostdev->source.subsys.u.usb.vendor;
unsigned product = hostdev->source.subsys.u.usb.product;
unsigned bus = hostdev->source.subsys.u.usb.bus;
unsigned device = hostdev->source.subsys.u.usb.device;
+ int rc;
- if (vendor && bus) {
- usb = usbFindDevice(vendor, product, bus, device);
+ *usb = NULL;
+ if (vendor && bus) {
+ rc = usbFindDevice(vendor, product, bus, device, mandatory, usb);
+ if (rc < 0)
+ return -1;
} else if (vendor && !bus) {
- usbDeviceList *devs = usbFindDeviceByVendor(vendor, product);
- if (!devs)
- return NULL;
+ usbDeviceList *devs;
- if (usbDeviceListCount(devs) > 1) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("multiple USB devices for %x:%x, "
- "use <address> to specify one"), vendor,
product);
- usbDeviceListFree(devs);
- return NULL;
+ rc = usbFindDeviceByVendor(vendor, product, mandatory, &devs);
+ if (rc < 0)
+ return -1;
+
+ if (rc == 1) {
+ *usb = usbDeviceListGet(devs, 0);
+ usbDeviceListSteal(devs, *usb);
}
- usb = usbDeviceListGet(devs, 0);
- usbDeviceListSteal(devs, usb);
usbDeviceListFree(devs);
- hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(usb);
- hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(usb);
+ if (rc == 0) {
+ goto out;
+ } else if (rc > 1) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("multiple USB devices for %x:%x, "
+ "use <address> to specify one"),
+ vendor, product);
+ return -1;
+ }
+ hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(*usb);
+ hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(*usb);
} else if (!vendor && bus) {
- usb = usbFindDeviceByBus(bus, device);
+ if (usbFindDeviceByBus(bus, device, mandatory, usb) < 0)
+ return -1;
}
- return usb;
+out:
+ if (!*usb)
+ hostdev->missing = 1;
+ return 0;
}
static int
@@ -708,7 +723,7 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver,
if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
continue;
- if (!(usb = qemuFindHostdevUSBDevice(hostdev)))
+ if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0)
goto cleanup;
if (usbDeviceListAdd(list, usb) < 0) {
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 204b2d4..74dd2ce 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -36,7 +36,9 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
const unsigned char *uuid,
virDomainHostdevDefPtr *hostdevs,
int nhostdevs);
-usbDevice *qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev);
+int qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev,
+ bool mandatory,
+ usbDevice **usb);
int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
const char *name,
usbDeviceList *list);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 59de8e4..2ff9e7a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1149,7 +1149,7 @@ int qemuDomainAttachHostDevice(struct qemud_driver *driver,
goto cleanup;
if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
- if (!(usb = qemuFindHostdevUSBDevice(hostdev)))
+ if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0)
goto cleanup;
if (usbDeviceListAdd(list, usb) < 0) {
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 22edba2..f126aa5 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -495,9 +495,13 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
switch (dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus,
- dev->source.subsys.u.usb.device);
+ usbDevice *usb;
+ if (dev->missing)
+ return 0;
+
+ usb = usbGetDevice(dev->source.subsys.u.usb.bus,
+ dev->source.subsys.u.usb.device);
if (!usb)
goto done;
@@ -568,9 +572,13 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
switch (dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus,
- dev->source.subsys.u.usb.device);
+ usbDevice *usb;
+
+ if (dev->missing)
+ return 0;
+ usb = usbGetDevice(dev->source.subsys.u.usb.bus,
+ dev->source.subsys.u.usb.device);
if (!usb)
goto done;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 883a82b..d55c60d 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1101,9 +1101,13 @@ virSecuritySELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UN
switch (dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus,
- dev->source.subsys.u.usb.device);
+ usbDevice *usb;
+ if (dev->missing)
+ return 0;
+
+ usb = usbGetDevice(dev->source.subsys.u.usb.bus,
+ dev->source.subsys.u.usb.device);
if (!usb)
goto done;
@@ -1174,9 +1178,13 @@ virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr
mgr ATTRIBUT
switch (dev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
- usbDevice *usb = usbGetDevice(dev->source.subsys.u.usb.bus,
- dev->source.subsys.u.usb.device);
+ usbDevice *usb;
+
+ if (dev->missing)
+ return 0;
+ usb = usbGetDevice(dev->source.subsys.u.usb.bus,
+ dev->source.subsys.u.usb.device);
if (!usb)
goto done;
diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index f0196a8..81a9f5a 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -186,76 +186,117 @@ cleanup:
return ret;
}
-usbDeviceList *
-usbFindDeviceByVendor(unsigned int vendor, unsigned product)
+int
+usbFindDeviceByVendor(unsigned int vendor,
+ unsigned product,
+ bool mandatory,
+ usbDeviceList **devices)
{
-
usbDeviceList *list;
+ int count;
+
if (!(list = usbDeviceSearch(vendor, product, 0 , 0,
USB_DEVICE_FIND_BY_VENDOR)))
- return NULL;
+ return -1;
if (list->count == 0) {
+ usbDeviceListFree(list);
+ if (!mandatory) {
+ VIR_DEBUG("Did not find USB device %x:%x",
+ vendor, product);
+ if (devices)
+ *devices = NULL;
+ return 0;
+ }
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Did not find USB device %x:%x"), vendor, product);
- usbDeviceListFree(list);
- return NULL;
+ return -1;
}
- return list;
+ count = list->count;
+ if (devices)
+ *devices = list;
+ else
+ usbDeviceListFree(list);
+
+ return count;
}
-usbDevice *
-usbFindDeviceByBus(unsigned int bus, unsigned devno)
+int
+usbFindDeviceByBus(unsigned int bus,
+ unsigned devno,
+ bool mandatory,
+ usbDevice **usb)
{
- usbDevice *usb;
usbDeviceList *list;
if (!(list = usbDeviceSearch(0, 0, bus, devno,
USB_DEVICE_FIND_BY_BUS)))
- return NULL;
+ return -1;
if (list->count == 0) {
+ usbDeviceListFree(list);
+ if (!mandatory) {
+ VIR_DEBUG("Did not find USB device bus:%u device:%u",
+ bus, devno);
+ if (usb)
+ *usb = NULL;
+ return 0;
+ }
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Did not find USB device bus:%u device:%u"),
bus, devno);
- usbDeviceListFree(list);
- return NULL;
+ return -1;
}
- usb = usbDeviceListGet(list, 0);
- usbDeviceListSteal(list, usb);
+ if (usb) {
+ *usb = usbDeviceListGet(list, 0);
+ usbDeviceListSteal(list, *usb);
+ }
usbDeviceListFree(list);
- return usb;
+ return 0;
}
-usbDevice *
+int
usbFindDevice(unsigned int vendor,
unsigned int product,
unsigned int bus,
- unsigned int devno)
+ unsigned int devno,
+ bool mandatory,
+ usbDevice **usb)
{
- usbDevice *usb;
usbDeviceList *list;
unsigned int flags = USB_DEVICE_FIND_BY_VENDOR|USB_DEVICE_FIND_BY_BUS;
if (!(list = usbDeviceSearch(vendor, product, bus, devno, flags)))
- return NULL;
+ return -1;
if (list->count == 0) {
+ usbDeviceListFree(list);
+ if (!mandatory) {
+ VIR_DEBUG("Did not find USB device %x:%x bus:%u device:%u",
+ vendor, product, bus, devno);
+ if (usb)
+ *usb = NULL;
+ return 0;
+ }
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Did not find USB device %x:%x bus:%u device:%u"),
vendor, product, bus, devno);
- usbDeviceListFree(list);
- return NULL;
+ return -1;
}
- usb = usbDeviceListGet(list, 0);
- usbDeviceListSteal(list, usb);
+ if (usb) {
+ *usb = usbDeviceListGet(list, 0);
+ usbDeviceListSteal(list, *usb);
+ }
usbDeviceListFree(list);
- return usb;
+ return 0;
}
usbDevice *
diff --git a/src/util/hostusb.h b/src/util/hostusb.h
index 6f5067c..4f55fdc 100644
--- a/src/util/hostusb.h
+++ b/src/util/hostusb.h
@@ -31,16 +31,22 @@ typedef struct _usbDeviceList usbDeviceList;
usbDevice *usbGetDevice(unsigned int bus,
unsigned int devno);
-usbDevice *usbFindDeviceByBus(unsigned int bus,
- unsigned int devno);
+int usbFindDeviceByBus(unsigned int bus,
+ unsigned int devno,
+ bool mandatory,
+ usbDevice **usb);
-usbDeviceList *usbFindDeviceByVendor(unsigned int vendor,
- unsigned int product);
+int usbFindDeviceByVendor(unsigned int vendor,
+ unsigned int product,
+ bool mandatory,
+ usbDeviceList **devices);
-usbDevice *usbFindDevice(unsigned int vendor,
- unsigned int product,
- unsigned int bus,
- unsigned int devno);
+int usbFindDevice(unsigned int vendor,
+ unsigned int product,
+ unsigned int bus,
+ unsigned int devno,
+ bool mandatory,
+ usbDevice **usb);
void usbFreeDevice (usbDevice *dev);
void usbDeviceSetUsedBy(usbDevice *dev, const char *name);
--
1.7.12
7:35 p.m.
New subject: [libvirt] [PATCH 3/6] qemu: Add option to treat missing USB devices as success
On Tue, Oct 09, 2012 at 02:13:24PM +0200, Jiri Denemark wrote:
All USB device lookup functions emit an error when they cannot find
the
requested device. With this patch, their caller can choose if a missing
device is an error or normal condition.
---
src/conf/domain_conf.h | 1 +
src/qemu/qemu_hostdev.c | 57 ++++++++++++++++----------
src/qemu/qemu_hostdev.h | 4 +-
src/qemu/qemu_hotplug.c | 2 +-
src/security/security_dac.c | 16 ++++++--
src/security/security_selinux.c | 16 ++++++--
src/util/hostusb.c | 91 ++++++++++++++++++++++++++++++-----------
src/util/hostusb.h | 22 ++++++----
8 files changed, 145 insertions(+), 64 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fa5d1f4..3cb1193 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -386,6 +386,7 @@ struct _virDomainHostdevDef {
int mode; /* enum virDomainHostdevMode */
int startupPolicy; /* enum virDomainStartupPolicy */
unsigned int managed : 1;
+ unsigned int missing : 1;
union {
virDomainHostdevSubsys subsys;
struct {
I was wondering if we should actually expose this in the
XML, as a read-only attribute, only shown when requesting
the live XML. Otherwise there's no way for an application
to determine whether the USB device they requested was
actually given to the guest, without looking inside the
guest.
ACK to the other parts of the patch
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:35 p.m.
New subject: [libvirt] [PATCH 3/6] qemu: Add option to treat missing USB devices as success
On Wed, Oct 10, 2012 at 16:35:05 +0100, Daniel P. Berrange wrote:
On Tue, Oct 09, 2012 at 02:13:24PM +0200, Jiri Denemark wrote:
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index fa5d1f4..3cb1193 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -386,6 +386,7 @@ struct _virDomainHostdevDef {
> int mode; /* enum virDomainHostdevMode */
> int startupPolicy; /* enum virDomainStartupPolicy */
> unsigned int managed : 1;
> + unsigned int missing : 1;
> union {
> virDomainHostdevSubsys subsys;
> struct {
I was wondering if we should actually expose this in the
XML, as a read-only attribute, only shown when requesting
the live XML. Otherwise there's no way for an application
to determine whether the USB device they requested was
actually given to the guest, without looking inside the
guest.
Yeah, this sounds like a useful thing. Could that be a follow up patch to
avoid holding up the whole series just for this? :-)
ACK to the other parts of the patch
Jirka
5:42 p.m.
New subject: [libvirt] [PATCH 3/6] qemu: Add option to treat missing USB devices as success
On Thu, Oct 11, 2012 at 03:35:27PM +0200, Jiri Denemark wrote:
On Wed, Oct 10, 2012 at 16:35:05 +0100, Daniel P. Berrange wrote:
> On Tue, Oct 09, 2012 at 02:13:24PM +0200, Jiri Denemark wrote:
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index fa5d1f4..3cb1193 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -386,6 +386,7 @@ struct _virDomainHostdevDef {
> > int mode; /* enum virDomainHostdevMode */
> > int startupPolicy; /* enum virDomainStartupPolicy */
> > unsigned int managed : 1;
> > + unsigned int missing : 1;
> > union {
> > virDomainHostdevSubsys subsys;
> > struct {
>
> I was wondering if we should actually expose this in the
> XML, as a read-only attribute, only shown when requesting
> the live XML. Otherwise there's no way for an application
> to determine whether the USB device they requested was
> actually given to the guest, without looking inside the
> guest.
Yeah, this sounds like a useful thing. Could that be a follow up patch to
avoid holding up the whole series just for this? :-)
As long as we do the followup now, I'm fine with it being separate.
Just don't want us to release anything without it.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:38 p.m.
New subject: [libvirt] [PATCH 3/6] qemu: Add option to treat missing USB devices as success
On Thu, Oct 11, 2012 at 14:42:07 +0100, Daniel P. Berrange wrote:
On Thu, Oct 11, 2012 at 03:35:27PM +0200, Jiri Denemark wrote:
> On Wed, Oct 10, 2012 at 16:35:05 +0100, Daniel P. Berrange wrote:
> >
> > I was wondering if we should actually expose this in the
> > XML, as a read-only attribute, only shown when requesting
> > the live XML. Otherwise there's no way for an application
> > to determine whether the USB device they requested was
> > actually given to the guest, without looking inside the
> > guest.
>
> Yeah, this sounds like a useful thing. Could that be a follow up patch to
> avoid holding up the whole series just for this? :-)
As long as we do the followup now, I'm fine with it being separate.
Just don't want us to release anything without it.
Sure, I'm already working on it. I just wanted to push this since Peter is
afraid of it conflicting with his work.
Jirka
4:13 p.m.
New subject: [libvirt] [PATCH 4/6] qemu: Implement startupPolicy for USB passed through devices
---
src/qemu/qemu_cgroup.c | 2 ++
src/qemu/qemu_command.c | 20 +++++++++++++++-----
src/qemu/qemu_hostdev.c | 20 +++++++++++++++-----
src/qemu/qemu_hostdev.h | 3 ++-
src/qemu/qemu_process.c | 2 +-
5 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 79faf8e..166f9b9 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -289,6 +289,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
continue;
if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
continue;
+ if (hostdev->missing)
+ continue;
if ((usb = usbGetDevice(hostdev->source.subsys.u.usb.bus,
hostdev->source.subsys.u.usb.device)) == NULL)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 09f412e..d590df6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3507,17 +3507,21 @@ qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (!dev->source.subsys.u.usb.bus &&
+ if (!dev->missing &&
+ !dev->source.subsys.u.usb.bus &&
!dev->source.subsys.u.usb.device) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("USB host device is missing bus/device
information"));
return NULL;
}
- virBufferAsprintf(&buf, "usb-host,hostbus=%d,hostaddr=%d,id=%s",
- dev->source.subsys.u.usb.bus,
- dev->source.subsys.u.usb.device,
- dev->info->alias);
+ virBufferAddLit(&buf, "usb-host");
+ if (!dev->missing) {
+ virBufferAsprintf(&buf, ",hostbus=%d,hostaddr=%d",
+ dev->source.subsys.u.usb.bus,
+ dev->source.subsys.u.usb.device);
+ }
+ virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
if (qemuBuildDeviceAddressStr(&buf, dev->info, caps) < 0)
goto error;
@@ -3577,6 +3581,12 @@ qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev)
{
char *ret = NULL;
+ if (dev->missing) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't not support missing USB
devices"));
+ return NULL;
+ }
+
if (!dev->source.subsys.u.usb.bus &&
!dev->source.subsys.u.usb.device) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 32c858b..90dfd28 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -696,7 +696,8 @@ out:
static int
qemuPrepareHostUSBDevices(struct qemud_driver *driver,
- virDomainDefPtr def)
+ virDomainDefPtr def,
+ bool coldBoot)
{
int i, ret = -1;
usbDeviceList *list;
@@ -716,6 +717,7 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver,
*/
for (i = 0 ; i < nhostdevs ; i++) {
virDomainHostdevDefPtr hostdev = hostdevs[i];
+ bool required = true;
usbDevice *usb;
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
@@ -723,10 +725,15 @@ qemuPrepareHostUSBDevices(struct qemud_driver *driver,
if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
continue;
- if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0)
+ if (hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_OPTIONAL ||
+ (hostdev->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_REQUISITE &&
+ !coldBoot))
+ required = false;
+
+ if (qemuFindHostdevUSBDevice(hostdev, required, &usb) < 0)
goto cleanup;
- if (usbDeviceListAdd(list, usb) < 0) {
+ if (usb && usbDeviceListAdd(list, usb) < 0) {
usbFreeDevice(usb);
goto cleanup;
}
@@ -756,7 +763,8 @@ cleanup:
}
int qemuPrepareHostDevices(struct qemud_driver *driver,
- virDomainDefPtr def)
+ virDomainDefPtr def,
+ bool coldBoot)
{
if (!def->nhostdevs)
return 0;
@@ -764,7 +772,7 @@ int qemuPrepareHostDevices(struct qemud_driver *driver,
if (qemuPrepareHostPCIDevices(driver, def) < 0)
return -1;
- if (qemuPrepareHostUSBDevices(driver, def) < 0)
+ if (qemuPrepareHostUSBDevices(driver, def, coldBoot) < 0)
return -1;
return 0;
@@ -891,6 +899,8 @@ qemuDomainReAttachHostUsbDevices(struct qemud_driver *driver,
continue;
if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
continue;
+ if (hostdev->missing)
+ continue;
usb = usbGetDevice(hostdev->source.subsys.u.usb.bus,
hostdev->source.subsys.u.usb.device);
diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
index 74dd2ce..0da25f9 100644
--- a/src/qemu/qemu_hostdev.h
+++ b/src/qemu/qemu_hostdev.h
@@ -43,7 +43,8 @@ int qemuPrepareHostdevUSBDevices(struct qemud_driver *driver,
const char *name,
usbDeviceList *list);
int qemuPrepareHostDevices(struct qemud_driver *driver,
- virDomainDefPtr def);
+ virDomainDefPtr def,
+ bool coldBoot);
void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver);
void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver,
const char *name,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 31909b7..0a50c78 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3382,7 +3382,7 @@ int qemuProcessStart(virConnectPtr conn,
/* Must be run before security labelling */
VIR_DEBUG("Preparing host devices");
- if (qemuPrepareHostDevices(driver, vm->def) < 0)
+ if (qemuPrepareHostDevices(driver, vm->def, !migrateFrom) < 0)
goto cleanup;
VIR_DEBUG("Preparing chr devices");
--
1.7.12
7:39 p.m.
New subject: [libvirt] [PATCH 4/6] qemu: Implement startupPolicy for USB passed through devices
On Tue, Oct 09, 2012 at 02:13:25PM +0200, Jiri Denemark wrote:
---
src/qemu/qemu_cgroup.c | 2 ++
src/qemu/qemu_command.c | 20 +++++++++++++++-----
src/qemu/qemu_hostdev.c | 20 +++++++++++++++-----
src/qemu/qemu_hostdev.h | 3 ++-
src/qemu/qemu_process.c | 2 +-
5 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 09f412e..d590df6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3507,17 +3507,21 @@ qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (!dev->source.subsys.u.usb.bus &&
+ if (!dev->missing &&
+ !dev->source.subsys.u.usb.bus &&
!dev->source.subsys.u.usb.device) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("USB host device is missing bus/device
information"));
return NULL;
}
- virBufferAsprintf(&buf, "usb-host,hostbus=%d,hostaddr=%d,id=%s",
- dev->source.subsys.u.usb.bus,
- dev->source.subsys.u.usb.device,
- dev->info->alias);
+ virBufferAddLit(&buf, "usb-host");
+ if (!dev->missing) {
+ virBufferAsprintf(&buf, ",hostbus=%d,hostaddr=%d",
+ dev->source.subsys.u.usb.bus,
+ dev->source.subsys.u.usb.device);
+ }
+ virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
I'm curious about this - it will result in
-device usb-host,id=XXXXX
shouldn't we actually just leave out the entire arg. I'm not sure
what QEMU would do with such a device specification.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:39 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: Implement startupPolicy for USB passed through devices
On Wed, Oct 10, 2012 at 16:39:06 +0100, Daniel P. Berrange wrote:
On Tue, Oct 09, 2012 at 02:13:25PM +0200, Jiri Denemark wrote:
>
> + virBufferAddLit(&buf, "usb-host");
> + if (!dev->missing) {
> + virBufferAsprintf(&buf, ",hostbus=%d,hostaddr=%d",
> + dev->source.subsys.u.usb.bus,
> + dev->source.subsys.u.usb.device);
> + }
> + virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
I'm curious about this - it will result in
-device usb-host,id=XXXXX
shouldn't we actually just leave out the entire arg. I'm not sure
what QEMU would do with such a device specification.
According to Gerd (and my testing) it's the desired command line. QEMU needs
to know about the usb-host device during incoming migration to avoid aborting
on unknown section in migration data. It's not necessary when starting a
domain from scratch but it doesn't seem to cause any harm.
Jirka
1:13 p.m.
New subject: [libvirt] [PATCH 4/6] qemu: Implement startupPolicy for USB passed through devices
On Wed, Oct 10, 2012 at 10:39:53PM +0200, Jiri Denemark wrote:
On Wed, Oct 10, 2012 at 16:39:06 +0100, Daniel P. Berrange wrote:
> On Tue, Oct 09, 2012 at 02:13:25PM +0200, Jiri Denemark wrote:
> >
> > + virBufferAddLit(&buf, "usb-host");
> > + if (!dev->missing) {
> > + virBufferAsprintf(&buf, ",hostbus=%d,hostaddr=%d",
> > + dev->source.subsys.u.usb.bus,
> > + dev->source.subsys.u.usb.device);
> > + }
> > + virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
>
> I'm curious about this - it will result in
>
> -device usb-host,id=XXXXX
>
> shouldn't we actually just leave out the entire arg. I'm not sure
> what QEMU would do with such a device specification.
According to Gerd (and my testing) it's the desired command line. QEMU needs
to know about the usb-host device during incoming migration to avoid aborting
on unknown section in migration data. It's not necessary when starting a
domain from scratch but it doesn't seem to cause any harm.
Ok, that makes sense now. ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:13 p.m.
New subject: [libvirt] [PATCH 5/6] Add MIGRATABLE flag for virDomainGetXMLDesc
Using VIR_DOMAIN_XML_MIGRATABLE flag, one can request domain's XML
configuration that is suitable for migration or save/restore. Such XML
may contain extra run-time stuff internal to libvirt and some default
configuration may be removed for better compatibility of the XML with
older libvirt releases.
This flag may serve as an easy way to get the XML that can be passed
(after desired modifications) to APIs that accept custom XMLs, such as
virDomainMigrate{,ToURI}2 or virDomainSaveFlags.
---
include/libvirt/libvirt.h.in | 1 +
src/conf/domain_conf.c | 3 ++-
src/qemu/qemu_domain.c | 21 ++++++++-------------
src/qemu/qemu_domain.h | 11 ++++++-----
src/qemu/qemu_driver.c | 16 +++++++++-------
src/qemu/qemu_migration.c | 11 +++++------
src/qemu/qemu_process.c | 14 +++++++-------
tools/virsh-domain.c | 4 ++++
tools/virsh.pod | 7 +++++--
9 files changed, 47 insertions(+), 41 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 81f12a4..b1a3e25 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1651,6 +1651,7 @@ typedef enum {
VIR_DOMAIN_XML_SECURE = (1 << 0), /* dump security sensitive information
too */
VIR_DOMAIN_XML_INACTIVE = (1 << 1), /* dump inactive domain information */
VIR_DOMAIN_XML_UPDATE_CPU = (1 << 2), /* update guest CPU requirements
according to host CPU */
+ VIR_DOMAIN_XML_MIGRATABLE = (1 << 3), /* dump XML suitable for migration */
} virDomainXMLFlags;
char * virDomainGetXMLDesc (virDomainPtr domain,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index da2d86d..c32ce8d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13294,7 +13294,8 @@ virDomainHubDefFormat(virBufferPtr buf,
#define DUMPXML_FLAGS \
(VIR_DOMAIN_XML_SECURE | \
VIR_DOMAIN_XML_INACTIVE | \
- VIR_DOMAIN_XML_UPDATE_CPU)
+ VIR_DOMAIN_XML_UPDATE_CPU | \
+ VIR_DOMAIN_XML_MIGRATABLE)
verify(((VIR_DOMAIN_XML_INTERNAL_STATUS |
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 17ae3b9..ff56c46 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -46,10 +46,6 @@
#define QEMU_NAMESPACE_HREF "http://libvirt.org/schemas/domain/qemu/1.0"
-#define QEMU_DOMAIN_FORMAT_LIVE_FLAGS \
- (VIR_DOMAIN_XML_SECURE | \
- VIR_DOMAIN_XML_UPDATE_CPU)
-
VIR_ENUM_IMPL(qemuDomainJob, QEMU_JOB_LAST,
"none",
"query",
@@ -1220,7 +1216,6 @@ int
qemuDomainDefFormatBuf(struct qemud_driver *driver,
virDomainDefPtr def,
unsigned int flags,
- bool compatible,
virBuffer *buf)
{
int ret = -1;
@@ -1245,7 +1240,7 @@ qemuDomainDefFormatBuf(struct qemud_driver *driver,
def->cpu = cpu;
}
- if (compatible) {
+ if ((flags & VIR_DOMAIN_XML_MIGRATABLE)) {
int i;
virDomainControllerDefPtr usb = NULL;
@@ -1297,12 +1292,11 @@ cleanup:
char *qemuDomainDefFormatXML(struct qemud_driver *driver,
virDomainDefPtr def,
- unsigned int flags,
- bool compatible)
+ unsigned int flags)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- if (qemuDomainDefFormatBuf(driver, def, flags, compatible, &buf) < 0) {
+ if (qemuDomainDefFormatBuf(driver, def, flags, &buf) < 0) {
virBufferFreeAndReset(&buf);
return NULL;
}
@@ -1318,8 +1312,7 @@ char *qemuDomainDefFormatXML(struct qemud_driver *driver,
char *qemuDomainFormatXML(struct qemud_driver *driver,
virDomainObjPtr vm,
- unsigned int flags,
- bool compatible)
+ unsigned int flags)
{
virDomainDefPtr def;
@@ -1328,7 +1321,7 @@ char *qemuDomainFormatXML(struct qemud_driver *driver,
else
def = vm->def;
- return qemuDomainDefFormatXML(driver, def, flags, compatible);
+ return qemuDomainDefFormatXML(driver, def, flags);
}
char *
@@ -1341,8 +1334,10 @@ qemuDomainDefFormatLive(struct qemud_driver *driver,
if (inactive)
flags |= VIR_DOMAIN_XML_INACTIVE;
+ if (compatible)
+ flags |= VIR_DOMAIN_XML_MIGRATABLE;
- return qemuDomainDefFormatXML(driver, def, flags, compatible);
+ return qemuDomainDefFormatXML(driver, def, flags);
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 97e4346..26b6c55 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -39,6 +39,10 @@
(1 << VIR_DOMAIN_VIRT_KVM) | \
(1 << VIR_DOMAIN_VIRT_XEN))
+# define QEMU_DOMAIN_FORMAT_LIVE_FLAGS \
+ (VIR_DOMAIN_XML_SECURE | \
+ VIR_DOMAIN_XML_UPDATE_CPU)
+
# if ULONG_MAX == 4294967295
/* Qemu has a 64-bit limit, but we are limited by our historical choice of
* representing bandwidth in a long instead of a 64-bit int. */
@@ -253,18 +257,15 @@ void qemuDomainObjExitRemoteWithDriver(struct qemud_driver *driver,
int qemuDomainDefFormatBuf(struct qemud_driver *driver,
virDomainDefPtr vm,
unsigned int flags,
- bool compatible,
virBuffer *buf);
char *qemuDomainDefFormatXML(struct qemud_driver *driver,
virDomainDefPtr vm,
- unsigned int flags,
- bool compatible);
+ unsigned int flags);
char *qemuDomainFormatXML(struct qemud_driver *driver,
virDomainObjPtr vm,
- unsigned int flags,
- bool compatible);
+ unsigned int flags);
char *qemuDomainDefFormatLive(struct qemud_driver *driver,
virDomainDefPtr def,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 97ad23e..094e9ab 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4966,7 +4966,7 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path,
if (fd < 0)
goto cleanup;
- ret = qemuDomainDefFormatXML(driver, def, flags, false);
+ ret = qemuDomainDefFormatXML(driver, def, flags);
cleanup:
virDomainDefFree(def);
@@ -5010,8 +5010,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
xml = qemuDomainDefFormatXML(driver, def,
VIR_DOMAIN_XML_INACTIVE |
- VIR_DOMAIN_XML_SECURE,
- true);
+ VIR_DOMAIN_XML_SECURE |
+ VIR_DOMAIN_XML_MIGRATABLE);
if (!xml)
goto cleanup;
len = strlen(xml) + 1;
@@ -5161,7 +5161,10 @@ endjob:
}
}
- ret = qemuDomainFormatXML(driver, vm, flags, false);
+ if ((flags & VIR_DOMAIN_XML_MIGRATABLE))
+ flags |= QEMU_DOMAIN_FORMAT_LIVE_FLAGS;
+
+ ret = qemuDomainFormatXML(driver, vm, flags);
cleanup:
if (vm)
@@ -5201,7 +5204,7 @@ static char *qemuDomainXMLFromNative(virConnectPtr conn,
goto cleanup;
}
- xml = qemuDomainDefFormatXML(driver, def, VIR_DOMAIN_XML_INACTIVE, false);
+ xml = qemuDomainDefFormatXML(driver, def, VIR_DOMAIN_XML_INACTIVE);
cleanup:
virDomainDefFree(def);
@@ -11731,8 +11734,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
if (!(xml = qemuDomainDefFormatXML(driver,
snap->def->dom,
VIR_DOMAIN_XML_INACTIVE |
- VIR_DOMAIN_XML_SECURE,
- false)))
+ VIR_DOMAIN_XML_SECURE)))
goto cleanup;
config = virDomainDefParseString(driver->caps, xml,
QEMU_EXPECTED_VIRT_TYPES,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index db69a0a..7cc1f98 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -432,8 +432,8 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
if (qemuDomainDefFormatBuf(driver,
mig->persistent,
VIR_DOMAIN_XML_INACTIVE |
- VIR_DOMAIN_XML_SECURE,
- true,
+ VIR_DOMAIN_XML_SECURE |
+ VIR_DOMAIN_XML_MIGRATABLE,
buf) < 0)
return -1;
virBufferAdjustIndent(buf, -2);
@@ -1264,7 +1264,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
int hookret;
if (!(xml = qemuDomainDefFormatXML(driver, def,
- VIR_DOMAIN_XML_SECURE, false)))
+ VIR_DOMAIN_XML_SECURE)))
goto cleanup;
hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def->name,
@@ -2213,9 +2213,8 @@ static int doPeer2PeerMigrate2(struct qemud_driver *driver,
* and pass it to Prepare2.
*/
if (!(dom_xml = qemuDomainFormatXML(driver, vm,
- VIR_DOMAIN_XML_SECURE |
- VIR_DOMAIN_XML_UPDATE_CPU,
- true)))
+ QEMU_DOMAIN_FORMAT_LIVE_FLAGS |
+ VIR_DOMAIN_XML_MIGRATABLE)))
return -1;
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0a50c78..26374e4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3137,7 +3137,7 @@ qemuProcessReconnect(void *opaque)
/* Run an hook to allow admins to do some magic */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
- char *xml = qemuDomainDefFormatXML(driver, obj->def, 0, false);
+ char *xml = qemuDomainDefFormatXML(driver, obj->def, 0);
int hookret;
hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, obj->def->name,
@@ -3365,7 +3365,7 @@ int qemuProcessStart(virConnectPtr conn,
/* Run an early hook to set-up missing devices */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
- char *xml = qemuDomainDefFormatXML(driver, vm->def, 0, false);
+ char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
int hookret;
hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
@@ -3582,7 +3582,7 @@ int qemuProcessStart(virConnectPtr conn,
/* now that we know it is about to start call the hook if present */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
- char *xml = qemuDomainDefFormatXML(driver, vm->def, 0, false);
+ char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
int hookret;
hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
@@ -3823,7 +3823,7 @@ int qemuProcessStart(virConnectPtr conn,
/* finally we can call the 'started' hook script if any */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
- char *xml = qemuDomainDefFormatXML(driver, vm->def, 0, false);
+ char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
int hookret;
hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
@@ -4003,7 +4003,7 @@ void qemuProcessStop(struct qemud_driver *driver,
/* now that we know it's stopped call the hook if present */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
- char *xml = qemuDomainDefFormatXML(driver, vm->def, 0, false);
+ char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
/* we can't stop the operation even if the script raised an error */
virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
@@ -4098,7 +4098,7 @@ retry:
/* The "release" hook cleans up additional resources */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
- char *xml = qemuDomainDefFormatXML(driver, vm->def, 0, false);
+ char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
/* we can't stop the operation even if the script raised an error */
virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
@@ -4300,7 +4300,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
/* Run an hook to allow admins to do some magic */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
- char *xml = qemuDomainDefFormatXML(driver, vm->def, 0, false);
+ char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
int hookret;
hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 505169b..1df0872 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6399,6 +6399,7 @@ static const vshCmdOptDef opts_dumpxml[] = {
{"inactive", VSH_OT_BOOL, 0, N_("show inactive defined XML")},
{"security-info", VSH_OT_BOOL, 0, N_("include security sensitive
information in XML dump")},
{"update-cpu", VSH_OT_BOOL, 0, N_("update guest CPU according to host
CPU")},
+ {"migratable", VSH_OT_BOOL, 0, N_("provide XML suitable for
migrations")},
{NULL, 0, 0, NULL}
};
@@ -6412,6 +6413,7 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
bool inactive = vshCommandOptBool(cmd, "inactive");
bool secure = vshCommandOptBool(cmd, "security-info");
bool update = vshCommandOptBool(cmd, "update-cpu");
+ bool migratable = vshCommandOptBool(cmd, "migratable");
if (inactive)
flags |= VIR_DOMAIN_XML_INACTIVE;
@@ -6419,6 +6421,8 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_XML_SECURE;
if (update)
flags |= VIR_DOMAIN_XML_UPDATE_CPU;
+ if (migratable)
+ flags |= VIR_DOMAIN_XML_MIGRATABLE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index ac8a00f..ea5060c 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -948,7 +948,7 @@ NOTE: Some hypervisors may require the user to manually ensure proper
permissions on file and path specified by argument I<corefilepath>.
=item B<dumpxml> I<domain> [I<--inactive>] [I<--security-info>]
-[I<--update-cpu>]
+[I<--update-cpu>] [I<--migratable>]
Output the domain information as an XML dump to stdout, this format can be used
by the B<create> command. Additional options affecting the XML dump may be
@@ -956,7 +956,10 @@ used. I<--inactive> tells virsh to dump domain configuration
that will be used
on next start of the domain as opposed to the current domain configuration.
Using I<--security-info> will also include security sensitive information
in the XML dump. I<--update-cpu> updates domain CPU requirements according to
-host CPU.
+host CPU. With I<--migratable> one can request an XML that is suitable for
+migrations, i.e., compatible with older libvirt releases and possibly amended
+with internal run-time options. This option may automatically enable other
+options (I<--update-cpu>, I<--security-info>, ...) as necessary.
=item B<edit> I<domain>
--
1.7.12
4:03 p.m.
New subject: [libvirt] [PATCH 5/6] Add MIGRATABLE flag for virDomainGetXMLDesc
On 10/09/12 14:13, Jiri Denemark wrote:
Using VIR_DOMAIN_XML_MIGRATABLE flag, one can request domain's
XML
configuration that is suitable for migration or save/restore. Such XML
may contain extra run-time stuff internal to libvirt and some default
configuration may be removed for better compatibility of the XML with
older libvirt releases.
This flag may serve as an easy way to get the XML that can be passed
(after desired modifications) to APIs that accept custom XMLs, such as
virDomainMigrate{,ToURI}2 or virDomainSaveFlags.
---
include/libvirt/libvirt.h.in | 1 +
src/conf/domain_conf.c | 3 ++-
src/qemu/qemu_domain.c | 21 ++++++++-------------
src/qemu/qemu_domain.h | 11 ++++++-----
src/qemu/qemu_driver.c | 16 +++++++++-------
src/qemu/qemu_migration.c | 11 +++++------
src/qemu/qemu_process.c | 14 +++++++-------
tools/virsh-domain.c | 4 ++++
tools/virsh.pod | 7 +++++--
9 files changed, 47 insertions(+), 41 deletions(-)
ACK.
Peter
4:13 p.m.
New subject: [libvirt] [PATCH 6/6] qemu: Make save/restore with USB devices usable
Save/restore with passed through USB devices currently only works if the
USB device can be found at the same USB address where it used to be
before saving a domain. This makes sense in case a user explicitly
configure the USB address in domain XML. However, if the device was
found automatically by vendor/product identification, we should try to
search for that device when restoring the domain and use any device we
find as long as there is only one available. In other words, the USB
device can now be removed and plugged again or the host can be rebooted
between saving and restoring the domain.
---
src/conf/domain_conf.c | 10 ++++++++++
src/conf/domain_conf.h | 2 ++
src/qemu/qemu_driver.c | 5 +++--
src/qemu/qemu_hostdev.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_migration.c | 3 ++-
5 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c32ce8d..0cb686d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2695,6 +2695,7 @@ virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node,
int got_product, got_vendor;
xmlNodePtr cur;
char *startupPolicy = NULL;
+ char *autoAddress;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
def->startupPolicy =
@@ -2709,6 +2710,12 @@ virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node,
VIR_FREE(startupPolicy);
}
+ if ((autoAddress = virXMLPropString(node, "autoAddress"))) {
+ if (STREQ(autoAddress, "yes"))
+ def->source.subsys.u.usb.autoAddress = true;
+ VIR_FREE(autoAddress);
+ }
+
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized*/
got_product = 0;
@@ -12072,6 +12079,9 @@ virDomainHostdevSourceFormat(virBufferPtr buf,
policy = virDomainStartupPolicyTypeToString(def->startupPolicy);
virBufferAsprintf(buf, " startupPolicy='%s'", policy);
}
+ if (def->source.subsys.u.usb.autoAddress &&
+ (flags & VIR_DOMAIN_XML_MIGRATABLE))
+ virBufferAddLit(buf, " autoAddress='yes'");
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3cb1193..27f9538 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -370,6 +370,8 @@ struct _virDomainHostdevSubsys {
int type; /* enum virDomainHostdevSubsysType */
union {
struct {
+ bool autoAddress; /* bus/device were filled automatically based
+ on vedor/product */
unsigned bus;
unsigned device;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 094e9ab..6022f97 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11183,7 +11183,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
} else {
/* Easiest way to clone inactive portion of vm->def is via
* conversion in and back out of xml. */
- if (!(xml = qemuDomainDefFormatLive(driver, vm->def, true, false)) ||
+ if (!(xml = qemuDomainDefFormatLive(driver, vm->def, true, true)) ||
!(def->dom = virDomainDefParseString(driver->caps, xml,
QEMU_EXPECTED_VIRT_TYPES,
VIR_DOMAIN_XML_INACTIVE)))
@@ -11734,7 +11734,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
if (!(xml = qemuDomainDefFormatXML(driver,
snap->def->dom,
VIR_DOMAIN_XML_INACTIVE |
- VIR_DOMAIN_XML_SECURE)))
+ VIR_DOMAIN_XML_SECURE |
+ VIR_DOMAIN_XML_MIGRATABLE)))
goto cleanup;
config = virDomainDefParseString(driver->caps, xml,
QEMU_EXPECTED_VIRT_TYPES,
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 90dfd28..e24c022 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -650,15 +650,31 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev,
unsigned product = hostdev->source.subsys.u.usb.product;
unsigned bus = hostdev->source.subsys.u.usb.bus;
unsigned device = hostdev->source.subsys.u.usb.device;
+ bool autoAddress = hostdev->source.subsys.u.usb.autoAddress;
int rc;
*usb = NULL;
if (vendor && bus) {
- rc = usbFindDevice(vendor, product, bus, device, mandatory, usb);
- if (rc < 0)
+ rc = usbFindDevice(vendor, product, bus, device,
+ autoAddress ? false : mandatory,
+ usb);
+ if (rc < 0) {
return -1;
- } else if (vendor && !bus) {
+ } else if (!autoAddress) {
+ goto out;
+ } else {
+ VIR_INFO("USB device %x:%x could not be found at previous"
+ " address (bus:%u device:%u)",
+ vendor, product, bus, device);
+ }
+ }
+
+ /* When vendor is specified, its USB address is either unspecified or the
+ * device could not be found at the USB device where it had been
+ * automatically found before.
+ */
+ if (vendor) {
usbDeviceList *devs;
rc = usbFindDeviceByVendor(vendor, product, mandatory, &devs);
@@ -674,15 +690,32 @@ qemuFindHostdevUSBDevice(virDomainHostdevDefPtr hostdev,
if (rc == 0) {
goto out;
} else if (rc > 1) {
- virReportError(VIR_ERR_OPERATION_FAILED,
- _("multiple USB devices for %x:%x, "
- "use <address> to specify one"),
- vendor, product);
+ if (autoAddress) {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Multiple USB devices for %x:%x were found,"
+ " but none of them is at bus:%u device:%u"),
+ vendor, product, bus, device);
+ } else {
+ virReportError(VIR_ERR_OPERATION_FAILED,
+ _("Multiple USB devices for %x:%x, "
+ "use <address> to specify one"),
+ vendor, product);
+ }
return -1;
}
hostdev->source.subsys.u.usb.bus = usbDeviceGetBus(*usb);
hostdev->source.subsys.u.usb.device = usbDeviceGetDevno(*usb);
+ hostdev->source.subsys.u.usb.autoAddress = true;
+
+ if (autoAddress) {
+ VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved"
+ " from bus:%u device:%u)",
+ vendor, product,
+ hostdev->source.subsys.u.usb.bus,
+ hostdev->source.subsys.u.usb.device,
+ bus, device);
+ }
} else if (!vendor && bus) {
if (usbFindDeviceByBus(bus, device, mandatory, usb) < 0)
return -1;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 7cc1f98..68d614d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1264,7 +1264,8 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
int hookret;
if (!(xml = qemuDomainDefFormatXML(driver, def,
- VIR_DOMAIN_XML_SECURE)))
+ VIR_DOMAIN_XML_SECURE |
+ VIR_DOMAIN_XML_MIGRATABLE)))
goto cleanup;
hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def->name,
--
1.7.12
7:40 p.m.
New subject: [libvirt] [PATCH 6/6] qemu: Make save/restore with USB devices usable
On Tue, Oct 09, 2012 at 02:13:27PM +0200, Jiri Denemark wrote:
Save/restore with passed through USB devices currently only works if
the
USB device can be found at the same USB address where it used to be
before saving a domain. This makes sense in case a user explicitly
configure the USB address in domain XML. However, if the device was
found automatically by vendor/product identification, we should try to
search for that device when restoring the domain and use any device we
find as long as there is only one available. In other words, the USB
device can now be removed and plugged again or the host can be rebooted
between saving and restoring the domain.
---
src/conf/domain_conf.c | 10 ++++++++++
src/conf/domain_conf.h | 2 ++
src/qemu/qemu_driver.c | 5 +++--
src/qemu/qemu_hostdev.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_migration.c | 3 ++-
5 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c32ce8d..0cb686d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2695,6 +2695,7 @@ virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node,
int got_product, got_vendor;
xmlNodePtr cur;
char *startupPolicy = NULL;
+ char *autoAddress;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
def->startupPolicy =
@@ -2709,6 +2710,12 @@ virDomainHostdevSubsysUsbDefParseXML(const xmlNodePtr node,
VIR_FREE(startupPolicy);
}
+ if ((autoAddress = virXMLPropString(node, "autoAddress"))) {
+ if (STREQ(autoAddress, "yes"))
+ def->source.subsys.u.usb.autoAddress = true;
+ VIR_FREE(autoAddress);
+ }
+
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized*/
got_product = 0;
@@ -12072,6 +12079,9 @@ virDomainHostdevSourceFormat(virBufferPtr buf,
policy = virDomainStartupPolicyTypeToString(def->startupPolicy);
virBufferAsprintf(buf, " startupPolicy='%s'", policy);
}
+ if (def->source.subsys.u.usb.autoAddress &&
+ (flags & VIR_DOMAIN_XML_MIGRATABLE))
+ virBufferAddLit(buf, " autoAddress='yes'");
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
Do we really need to add a new attribute for this. IMHO if the user
has specified a vendor+product, then it is always auto-address, even
if they have also given a dev+address.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:42 p.m.
New subject: [libvirt] [PATCH 6/6] qemu: Make save/restore with USB devices usable
On Wed, Oct 10, 2012 at 16:40:59 +0100, Daniel P. Berrange wrote:
On Tue, Oct 09, 2012 at 02:13:27PM +0200, Jiri Denemark wrote:
> @@ -12072,6 +12079,9 @@ virDomainHostdevSourceFormat(virBufferPtr buf,
> policy = virDomainStartupPolicyTypeToString(def->startupPolicy);
> virBufferAsprintf(buf, " startupPolicy='%s'", policy);
> }
> + if (def->source.subsys.u.usb.autoAddress &&
> + (flags & VIR_DOMAIN_XML_MIGRATABLE))
> + virBufferAddLit(buf, " autoAddress='yes'");
> virBufferAddLit(buf, ">\n");
>
> virBufferAdjustIndent(buf, 2);
Do we really need to add a new attribute for this. IMHO if the user
has specified a vendor+product, then it is always auto-address, even
if they have also given a dev+address.
We do need it. If both vendor/product and bus/dev are specified, the code
doesn't try to find the device at different address. See the following code
snippet from usbDeviceSearch:
if ((flags & USB_DEVICE_FIND_BY_VENDOR) &&
(found_prod != product || found_vend != vendor))
continue;
if (flags & USB_DEVICE_FIND_BY_BUS) {
if (found_bus != bus || found_devno != devno)
continue;
found = true;
}
And it also make sense because if there are more devices with the same
vendor/product id, we require bus/dev to be configured so that we can assign
the right device and don't mess with other similar devices. However, if the
address was autofilled by libvirt, we can try to look at other addresses if
the device cannot be found at its original address (i.e., it was unplugged and
plugged again).
Jirka
2:20 p.m.
New subject: [libvirt] [PATCH 6/6] qemu: Make save/restore with USB devices usable
On Thu, Oct 11, 2012 at 11:42:50AM +0200, Jiri Denemark wrote:
On Wed, Oct 10, 2012 at 16:40:59 +0100, Daniel P. Berrange wrote:
> On Tue, Oct 09, 2012 at 02:13:27PM +0200, Jiri Denemark wrote:
> > @@ -12072,6 +12079,9 @@ virDomainHostdevSourceFormat(virBufferPtr buf,
> > policy = virDomainStartupPolicyTypeToString(def->startupPolicy);
> > virBufferAsprintf(buf, " startupPolicy='%s'",
policy);
> > }
> > + if (def->source.subsys.u.usb.autoAddress &&
> > + (flags & VIR_DOMAIN_XML_MIGRATABLE))
> > + virBufferAddLit(buf, " autoAddress='yes'");
> > virBufferAddLit(buf, ">\n");
> >
> > virBufferAdjustIndent(buf, 2);
>
> Do we really need to add a new attribute for this. IMHO if the user
> has specified a vendor+product, then it is always auto-address, even
> if they have also given a dev+address.
We do need it. If both vendor/product and bus/dev are specified, the code
doesn't try to find the device at different address. See the following code
snippet from usbDeviceSearch:
if ((flags & USB_DEVICE_FIND_BY_VENDOR) &&
(found_prod != product || found_vend != vendor))
continue;
if (flags & USB_DEVICE_FIND_BY_BUS) {
if (found_bus != bus || found_devno != devno)
continue;
found = true;
}
And it also make sense because if there are more devices with the same
vendor/product id, we require bus/dev to be configured so that we can assign
the right device and don't mess with other similar devices. However, if the
address was autofilled by libvirt, we can try to look at other addresses if
the device cannot be found at its original address (i.e., it was unplugged and
plugged again).
Ok, ACK to the patch
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:41 p.m.
On Tue, Oct 09, 2012 at 14:13:21 +0200, Jiri Denemark wrote:
USB devices can disappear without OS being mad about it, which makes
them ideal for startupPolicy so that a domain with USB devices can be
migrated to a host that lacks the USB devices. Moreover, this series
allows USB devices to be re-plugged or a host to be rebooted while
domains with USB devices are (managed)saved.
Jiri Denemark (6):
conf: Add support for startupPolicy for USB devices
qemu: Introduce qemuFindHostdevUSBDevice
qemu: Add option to treat missing USB devices as success
qemu: Implement startupPolicy for USB passed through devices
Add MIGRATABLE flag for virDomainGetXMLDesc
qemu: Make save/restore with USB devices usable
I pushed this series. Thanks for the reviews.
Jirka
4500
days inactive
4502
days old
20 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Jiri Denemark -
Peter Krempa