[libvirt] [PATCH 1/2] qemu: hotplug: Mark 2 private functions as static
They aren't used outside of qemu_hotplug.c --- src/qemu/qemu_hotplug.c | 14 ++++++++------ src/qemu/qemu_hotplug.h | 6 ------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c136232..f4fc723 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1141,9 +1141,10 @@ try_remove: } -int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainHostdevDefPtr hostdev) +static int +qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret; @@ -1433,9 +1434,10 @@ cleanup: return ret; } -int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainHostdevDefPtr hostdev) +static int +qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainHostdevDefPtr hostdev) { qemuDomainObjPrivatePtr priv = vm->privateData; virUSBDeviceList *list = NULL; diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 75789d6..ffc435c 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -47,12 +47,6 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainNetDefPtr net); -int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainHostdevDefPtr hostdev); -int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainHostdevDefPtr hostdev); int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainRedirdevDefPtr hostdev); -- 1.8.4.2
Since setting security label is dependent bus/addr being available. This fixes hotplugging a USB device that is referenced only by product/vendor (virt-manager's default). https://bugzilla.redhat.com/show_bug.cgi?id=1016511 --- src/qemu/qemu_hotplug.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f4fc723..d93fef9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1437,19 +1437,16 @@ cleanup: static int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainHostdevDefPtr hostdev) + virDomainHostdevDefPtr hostdev, + virUSBDevicePtr usb) { qemuDomainObjPrivatePtr priv = vm->privateData; virUSBDeviceList *list = NULL; - virUSBDevicePtr usb = NULL; char *devstr = NULL; bool added = false; bool teardowncgroup = false; int ret = -1; - if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0) - return -1; - if (!(list = virUSBDeviceListNew())) goto cleanup; @@ -1594,6 +1591,8 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { + virUSBDevicePtr usb = NULL; + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("hostdev mode '%s' not supported"), @@ -1601,6 +1600,11 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, return -1; } + /* We need to fill in USB values before the security labeling */ + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && + qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0) + return -1; + if (virSecurityManagerSetHostdevLabel(driver->securityManager, vm->def, hostdev, NULL) < 0) return -1; @@ -1614,7 +1618,7 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (qemuDomainAttachHostUsbDevice(driver, vm, - hostdev) < 0) + hostdev, usb) < 0) goto error; break; -- 1.8.4.2
On Thu, Dec 05, 2013 at 13:18:57 -0500, Cole Robinson wrote:
Since setting security label is dependent bus/addr being available. This fixes hotplugging a USB device that is referenced only by product/vendor (virt-manager's default).
https://bugzilla.redhat.com/show_bug.cgi?id=1016511
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f4fc723..d93fef9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c ... @@ -1601,6 +1600,11 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, return -1; }
+ /* We need to fill in USB values before the security labeling */ + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && + qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0) + return -1; + if (virSecurityManagerSetHostdevLabel(driver->securityManager, vm->def, hostdev, NULL) < 0) return -1; @@ -1614,7 +1618,7 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (qemuDomainAttachHostUsbDevice(driver, vm, - hostdev) < 0) + hostdev, usb) < 0) goto error; break;
Shouldn't we rather move virSecurityManagerSetHostdevLabel further in device-type specific functions similarly to how I fixed this issue for qemuSetupHostdevCGroup in 05e149f94cbd34e4c3d4e9c7f6871e13cfe03d8c? I think it makes sense to label devices only after we know they are not used by other domains and after we know we can really attach them. Jirka
On 12/05/2013 01:29 PM, Jiri Denemark wrote:
On Thu, Dec 05, 2013 at 13:18:57 -0500, Cole Robinson wrote:
Since setting security label is dependent bus/addr being available. This fixes hotplugging a USB device that is referenced only by product/vendor (virt-manager's default).
https://bugzilla.redhat.com/show_bug.cgi?id=1016511
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f4fc723..d93fef9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c ... @@ -1601,6 +1600,11 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, return -1; }
+ /* We need to fill in USB values before the security labeling */ + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && + qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0) + return -1; + if (virSecurityManagerSetHostdevLabel(driver->securityManager, vm->def, hostdev, NULL) < 0) return -1; @@ -1614,7 +1618,7 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (qemuDomainAttachHostUsbDevice(driver, vm, - hostdev) < 0) + hostdev, usb) < 0) goto error; break;
Shouldn't we rather move virSecurityManagerSetHostdevLabel further in device-type specific functions similarly to how I fixed this issue for qemuSetupHostdevCGroup in 05e149f94cbd34e4c3d4e9c7f6871e13cfe03d8c? I think it makes sense to label devices only after we know they are not used by other domains and after we know we can really attach them.
Good points, I'll take a stab at it if you aren't already working on it. - Cole
On Thu, Dec 05, 2013 at 13:35:48 -0500, Cole Robinson wrote:
On 12/05/2013 01:29 PM, Jiri Denemark wrote:
On Thu, Dec 05, 2013 at 13:18:57 -0500, Cole Robinson wrote:
Since setting security label is dependent bus/addr being available. This fixes hotplugging a USB device that is referenced only by product/vendor (virt-manager's default).
https://bugzilla.redhat.com/show_bug.cgi?id=1016511
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f4fc723..d93fef9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c ... @@ -1601,6 +1600,11 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, return -1; }
+ /* We need to fill in USB values before the security labeling */ + if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && + qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0) + return -1; + if (virSecurityManagerSetHostdevLabel(driver->securityManager, vm->def, hostdev, NULL) < 0) return -1; @@ -1614,7 +1618,7 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (qemuDomainAttachHostUsbDevice(driver, vm, - hostdev) < 0) + hostdev, usb) < 0) goto error; break;
Shouldn't we rather move virSecurityManagerSetHostdevLabel further in device-type specific functions similarly to how I fixed this issue for qemuSetupHostdevCGroup in 05e149f94cbd34e4c3d4e9c7f6871e13cfe03d8c? I think it makes sense to label devices only after we know they are not used by other domains and after we know we can really attach them.
Good points, I'll take a stab at it if you aren't already working on it.
It's in my queue but I'm not working on it right now :-) Jirka
On 12/05/2013 02:05 PM, Jiri Denemark wrote:
On Thu, Dec 05, 2013 at 13:18:56 -0500, Cole Robinson wrote:
They aren't used outside of qemu_hotplug.c --- src/qemu/qemu_hotplug.c | 14 ++++++++------ src/qemu/qemu_hotplug.h | 6 ------ 2 files changed, 8 insertions(+), 12 deletions(-)
ACK
Jirka
Thanks, pushed now. - Cole
participants (2)
-
Cole Robinson -
Jiri Denemark