[libvirt] [PATCH 0/8] Add iSCSI hostdev pass-through device

https://bugzilla.redhat.com/show_bug.cgi?id=957293 Allow iSCSI <hostdev/> devices to be added similarly to the existing <disk/> devices. Initially do a little bit of refactoring of the hostdev subsys structures to make it easier to model an iSCSI hostdev after the SCSI scsi_host. Although the bulk of the non-structure changes are "technically unnecessary", it just looked nicer to see {usb|pci|scsi}src rather than the longer "->source.subsys.u.{usb|pci|scsi}" in many places in the code. The end result is the guest will have /dev/sdX devices created. I have run this code through my Coverity environment and will be running the virttest suite over the weekend. Patches 1-3 are repetitive moves of the various hostdev subsys types (USB, PCI, and SCSI) into separate typedefs and then modifying code use the Ptr instead of the long union path to each field. I think I got most, but I'm sure there's still a few that could be cleaned up or that were added since I started this. Patch 4 more or less redoes patch 3 and I suppose could be combined. I left it separate because it's showing the progression to patch 6. This patch uses 'scsihost' to reference the specific portions while still using 'scsisrc' to reference the shared portion (which is only sgio). Patch 5 adds a virConnectPtr since patch 6 will need a way to get the secret value for the iSCSI secret/auth in qemuBuildSCSIHostdevDrvStr. Patch 6 adds the qemu code to handle a new hostdev protocol type VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI. The code will mimic the network disk (VIR_STORAGE_NET_PROTOCOL_ISCSI) code when making decisions. The new 'scsisrc' field 'protocol' will be the decision point. The default of VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE was chosen over TYPE_SCSI_HOST. The changes were made in this order to reduce the size of the patch when the XML is added (patch 8). Most of the code was done inline using the iscsisrc->protocol (when the iSCSI code had nothing to do). The remaining changes refactored code into "SCSIHost" and "SCSIiSCSI" to process the differences. Patch 7 just refactors domain_conf to add what will be a common routine to handle the "<host/>" element network disks. The iSCSI code will reuse the code to have it's own element. Patch 8 adds the XML, schema, and updates the docs to describe the <hostdev/> entry as well as of course adding new tests. The tests are a copy/merge of the scsi_host and network iscsi tests to define and describe the expected model. John Ferlan (8): hostdev: Introduce virDomainHostdevSubsysUSB hostdev: Introduce virDomainHostdevSubsysPCI hostdev: Introduce virDomainHostdevSubsysSCSI hostdev: Introduce virDomainHostdevSubsysSCSIHost Add virConnectPtr for qemuBuildSCSIHostdevDrvStr hostdev: Introduce virDomainHostdevSubsysSCSIiSCSI domain_conf: Common routine to handle network storage host xml def hostdev: Add iSCSI hostdev XML docs/formatdomain.html.in | 142 +++++-- docs/schemas/domaincommon.rng | 46 +- src/conf/domain_audit.c | 38 +- src/conf/domain_conf.c | 463 +++++++++++++++------ src/conf/domain_conf.h | 80 +++- src/libxl/libxl_conf.c | 9 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 34 +- src/lxc/lxc_cgroup.c | 4 +- src/lxc/lxc_controller.c | 10 +- src/lxc/lxc_driver.c | 16 +- src/qemu/qemu_cgroup.c | 69 +-- src/qemu/qemu_command.c | 158 ++++--- src/qemu/qemu_command.h | 5 +- src/qemu/qemu_conf.c | 20 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c | 93 +++-- src/qemu/qemu_hotplug.h | 9 +- src/security/security_apparmor.c | 33 +- src/security/security_dac.c | 66 +-- src/security/security_selinux.c | 64 +-- src/security/virt-aa-helper.c | 5 +- src/util/virhostdev.c | 295 +++++++------ .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args | 14 + .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 46 ++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args | 14 + .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml | 40 ++ ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args | 16 + ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 46 ++ .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args | 16 + .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml | 40 ++ tests/qemuxml2argvtest.c | 16 + tests/qemuxml2xmltest.c | 5 + 33 files changed, 1317 insertions(+), 606 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml -- 1.9.3

Create a separate typedef for the hostdev union data describing USB. Then adjust the code to use the new pointer Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 4 ++-- src/conf/domain_conf.c | 50 +++++++++++++++++++--------------------- src/conf/domain_conf.h | 22 ++++++++++-------- src/lxc/lxc_cgroup.c | 4 ++-- src/lxc/lxc_controller.c | 10 ++++---- src/lxc/lxc_driver.c | 16 ++++++------- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_command.c | 26 ++++++++++----------- src/qemu/qemu_hotplug.c | 7 +++--- src/security/security_apparmor.c | 6 ++--- src/security/security_dac.c | 12 ++++------ src/security/security_selinux.c | 10 ++++---- src/security/virt-aa-helper.c | 5 ++-- src/util/virhostdev.c | 50 +++++++++++++++------------------------- 14 files changed, 104 insertions(+), 122 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index a3d6c67..8277b06 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -388,6 +388,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, char *address = NULL; char *device = NULL; const char *virt; + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; virUUIDFormat(vm->def->uuid, uuidstr); if (!(vmname = virAuditEncode("vm", vm->def->name))) { @@ -415,8 +416,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (virAsprintfQuiet(&address, "%.3d.%.3d", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device) < 0) { + usbsrc->bus, usbsrc->device) < 0) { VIR_WARN("OOM while encoding audit message"); goto cleanup; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b91ccf7..046b4f8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3800,6 +3800,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, xmlNodePtr cur; char *startupPolicy = NULL; char *autoAddress; + virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) { def->startupPolicy = @@ -3816,7 +3817,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, if ((autoAddress = virXMLPropString(node, "autoAddress"))) { if (STREQ(autoAddress, "yes")) - def->source.subsys.u.usb.autoAddress = true; + usbsrc->autoAddress = true; VIR_FREE(autoAddress); } @@ -3833,8 +3834,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, if (vendor) { got_vendor = true; - if (virStrToLong_ui(vendor, NULL, 0, - &def->source.subsys.u.usb.vendor) < 0) { + if (virStrToLong_ui(vendor, NULL, 0, &usbsrc->vendor) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse vendor id %s"), vendor); VIR_FREE(vendor); @@ -3852,7 +3852,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, if (product) { got_product = true; if (virStrToLong_ui(product, NULL, 0, - &def->source.subsys.u.usb.product) < 0) { + &usbsrc->product) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse product %s"), product); @@ -3870,8 +3870,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, bus = virXMLPropString(cur, "bus"); if (bus) { - if (virStrToLong_ui(bus, NULL, 0, - &def->source.subsys.u.usb.bus) < 0) { + if (virStrToLong_ui(bus, NULL, 0, &usbsrc->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse bus %s"), bus); VIR_FREE(bus); @@ -3886,8 +3885,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, device = virXMLPropString(cur, "device"); if (device) { - if (virStrToLong_ui(device, NULL, 0, - &def->source.subsys.u.usb.device) < 0) { + if (virStrToLong_ui(device, NULL, 0, &usbsrc->device) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse device %s"), device); @@ -3910,7 +3908,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node, cur = cur->next; } - if (got_vendor && def->source.subsys.u.usb.vendor == 0) { + if (got_vendor && usbsrc->vendor == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vendor cannot be 0.")); goto out; @@ -10104,15 +10102,18 @@ static int virDomainHostdevMatchSubsysUSB(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { - if (a->source.subsys.u.usb.bus && a->source.subsys.u.usb.device) { + virDomainHostdevSubsysUSBPtr ausbsrc = &a->source.subsys.u.usb; + virDomainHostdevSubsysUSBPtr busbsrc = &b->source.subsys.u.usb; + + if (ausbsrc->bus && ausbsrc->device) { /* specified by bus location on host */ - if (a->source.subsys.u.usb.bus == b->source.subsys.u.usb.bus && - a->source.subsys.u.usb.device == b->source.subsys.u.usb.device) + if (ausbsrc->bus == busbsrc->bus && + ausbsrc->device == busbsrc->device) return 1; } else { /* specified by product & vendor id */ - if (a->source.subsys.u.usb.product == b->source.subsys.u.usb.product && - a->source.subsys.u.usb.vendor == b->source.subsys.u.usb.vendor) + if (ausbsrc->product == busbsrc->product && + ausbsrc->vendor == busbsrc->vendor) return 1; } return 0; @@ -15484,6 +15485,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, unsigned int flags, bool includeTypeInAddr) { + virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && def->source.subsys.u.pci.backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { const char *backend = virDomainHostdevSubsysPCIBackendTypeToString(def->source.subsys.u.pci.backend); @@ -15503,8 +15506,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, policy = virDomainStartupPolicyTypeToString(def->startupPolicy); virBufferAsprintf(buf, " startupPolicy='%s'", policy); } - if (def->source.subsys.u.usb.autoAddress && - (flags & VIR_DOMAIN_XML_MIGRATABLE)) + if (usbsrc->autoAddress && (flags & VIR_DOMAIN_XML_MIGRATABLE)) virBufferAddLit(buf, " autoAddress='yes'"); if (def->missing && @@ -15517,18 +15519,14 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, switch (def->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: - if (def->source.subsys.u.usb.vendor) { - virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", - def->source.subsys.u.usb.vendor); - virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", - def->source.subsys.u.usb.product); - } - if (def->source.subsys.u.usb.bus || - def->source.subsys.u.usb.device) { + if (usbsrc->vendor) { + virBufferAsprintf(buf, "<vendor id='0x%.4x'/>\n", usbsrc->vendor); + virBufferAsprintf(buf, "<product id='0x%.4x'/>\n", usbsrc->product); + } + if (usbsrc->bus || usbsrc->device) { virBufferAsprintf(buf, "<address %sbus='%d' device='%d'/>\n", includeTypeInAddr ? "type='usb' " : "", - def->source.subsys.u.usb.bus, - def->source.subsys.u.usb.device); + usbsrc->bus, usbsrc->device); } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 32674e0..d955491 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -390,20 +390,24 @@ typedef enum { VIR_ENUM_DECL(virDomainHostdevSubsysPCIBackend) +typedef struct _virDomainHostdevSubsysUSB virDomainHostdevSubsysUSB; +typedef virDomainHostdevSubsysUSB *virDomainHostdevSubsysUSBPtr; +struct _virDomainHostdevSubsysUSB { + bool autoAddress; /* bus/device were filled automatically based + on vedor/product */ + unsigned bus; + unsigned device; + + unsigned vendor; + unsigned product; +}; + typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys; typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr; struct _virDomainHostdevSubsys { int type; /* enum virDomainHostdevSubsysType */ union { - struct { - bool autoAddress; /* bus/device were filled automatically based - on vedor/product */ - unsigned bus; - unsigned device; - - unsigned vendor; - unsigned product; - } usb; + virDomainHostdevSubsysUSB usb; struct { virDevicePCIAddress addr; /* host address */ int backend; /* enum virDomainHostdevSubsysPCIBackendType */ diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 00ff807..9a86a85 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -403,6 +403,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, VIR_DEBUG("Allowing any hostdev block devs"); for (i = 0; i < def->nhostdevs; i++) { virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; virUSBDevicePtr usb; switch (hostdev->mode) { @@ -412,8 +413,7 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, if (hostdev->missing) continue; - if ((usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device, + if ((usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL)) == NULL) goto cleanup; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 4aa7e02..5ef41a6 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1350,22 +1350,20 @@ virLXCControllerSetupHostdevSubsysUSB(virDomainDefPtr vmDef, char *vroot = NULL; struct stat sb; mode_t mode; + virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; if (virAsprintf(&src, USB_DEVFS "/%03d/%03d", - def->source.subsys.u.usb.bus, - def->source.subsys.u.usb.device) < 0) + usbsrc->bus, usbsrc->device) < 0) goto cleanup; if (virAsprintf(&vroot, "/%s/%s.dev/bus/usb/", LXC_STATE_DIR, vmDef->name) < 0) goto cleanup; - if (virAsprintf(&dstdir, "%s/%03d/", vroot, - def->source.subsys.u.usb.bus) < 0) + if (virAsprintf(&dstdir, "%s/%03d/", vroot, usbsrc->bus) < 0) goto cleanup; - if (virAsprintf(&dstfile, "%s/%03d", dstdir, - def->source.subsys.u.usb.device) < 0) + if (virAsprintf(&dstfile, "%s/%03d", dstdir, usbsrc->device) < 0) goto cleanup; if (stat(src, &sb) < 0) { diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3253211..790a937 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4229,6 +4229,7 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, char *src = NULL; struct stat sb; virUSBDevicePtr usb = NULL; + virDomainHostdevSubsysUSBPtr usbsrc; if (virDomainHostdevFind(vm->def, def, NULL) >= 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", @@ -4236,13 +4237,12 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver, return -1; } + usbsrc = &def->source.subsys.u.usb; if (virAsprintf(&src, "/dev/bus/usb/%03d/%03d", - def->source.subsys.u.usb.bus, - def->source.subsys.u.usb.device) < 0) + usbsrc->bus, usbsrc->device) < 0) goto cleanup; - if (!(usb = virUSBDeviceNew(def->source.subsys.u.usb.bus, - def->source.subsys.u.usb.device, NULL))) + if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL))) goto cleanup; if (stat(src, &sb) < 0) { @@ -4673,6 +4673,7 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, char *dst = NULL; virUSBDevicePtr usb = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; + virDomainHostdevSubsysUSBPtr usbsrc; if ((idx = virDomainHostdevFind(vm->def, dev->data.hostdev, @@ -4682,9 +4683,9 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, goto cleanup; } + usbsrc = &def->source.subsys.u.usb; if (virAsprintf(&dst, "/dev/bus/usb/%03d/%03d", - def->source.subsys.u.usb.bus, - def->source.subsys.u.usb.device) < 0) + usbsrc->bus, usbsrc->device) < 0) goto cleanup; if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { @@ -4693,8 +4694,7 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver, goto cleanup; } - if (!(usb = virUSBDeviceNew(def->source.subsys.u.usb.bus, - def->source.subsys.u.usb.device, NULL))) + if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL))) goto cleanup; if (lxcDomainAttachDeviceUnlink(vm, dst) < 0) { diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 79f5f55..8f03078 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -242,6 +242,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; virPCIDevicePtr pci = NULL; virUSBDevicePtr usb = NULL; virSCSIDevicePtr scsi = NULL; @@ -290,8 +291,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, */ if (dev->missing) break; - if ((usb = virUSBDeviceNew(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, + if ((usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL)) == NULL) { goto cleanup; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2185ef4..208d49d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5035,10 +5035,9 @@ qemuBuildUSBHostdevDevStr(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; - if (!dev->missing && - !dev->source.subsys.u.usb.bus && - !dev->source.subsys.u.usb.device) { + if (!dev->missing && !usbsrc->bus && !usbsrc->device) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("USB host device is missing bus/device information")); return NULL; @@ -5047,8 +5046,7 @@ qemuBuildUSBHostdevDevStr(virDomainDefPtr def, 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); + usbsrc->bus, usbsrc->device); } virBufferAsprintf(&buf, ",id=%s", dev->info->alias); if (dev->info->bootIndex) @@ -5108,6 +5106,7 @@ char * qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) { char *ret = NULL; + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; if (dev->missing) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -5115,16 +5114,13 @@ qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) return NULL; } - if (!dev->source.subsys.u.usb.bus && - !dev->source.subsys.u.usb.device) { + if (!usbsrc->bus && !usbsrc->device) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("USB host device is missing bus/device information")); return NULL; } - ignore_value(virAsprintf(&ret, "host:%d.%d", - dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device)); + ignore_value(virAsprintf(&ret, "host:%d.%d", usbsrc->bus, usbsrc->device)); return ret; } @@ -10083,12 +10079,14 @@ static virDomainHostdevDefPtr qemuParseCommandLineUSB(const char *val) { virDomainHostdevDefPtr def = virDomainHostdevDefAlloc(); + virDomainHostdevSubsysUSBPtr usbsrc; int first = 0, second = 0; const char *start; char *end; if (!def) goto error; + usbsrc = &def->source.subsys.u.usb; if (!STRPREFIX(val, "host:")) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -10127,11 +10125,11 @@ qemuParseCommandLineUSB(const char *val) def->managed = false; def->source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB; if (*end == '.') { - def->source.subsys.u.usb.bus = first; - def->source.subsys.u.usb.device = second; + usbsrc->bus = first; + usbsrc->device = second; } else { - def->source.subsys.u.usb.vendor = first; - def->source.subsys.u.usb.product = second; + usbsrc->vendor = first; + usbsrc->product = second; } return def; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1fc28b8..0427930 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3360,6 +3360,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, { virDomainHostdevDefPtr hostdev = dev->data.hostdev; virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + virDomainHostdevSubsysUSBPtr usbsrc = &subsys->u.usb; virDomainHostdevDefPtr detach = NULL; int idx; @@ -3381,14 +3382,14 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, subsys->u.pci.addr.slot, subsys->u.pci.addr.function); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: - if (subsys->u.usb.bus && subsys->u.usb.device) { + if (usbsrc->bus && usbsrc->device) { virReportError(VIR_ERR_OPERATION_FAILED, _("host usb device %03d.%03d not found"), - subsys->u.usb.bus, subsys->u.usb.device); + usbsrc->bus, usbsrc->device); } else { virReportError(VIR_ERR_OPERATION_FAILED, _("host usb device vendor=0x%.4x product=0x%.4x not found"), - subsys->u.usb.vendor, subsys->u.usb.product); + usbsrc->vendor, usbsrc->product); } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 9603c78..563de4f 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -801,6 +801,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, int ret = -1; virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; if (!secdef) return -1; @@ -822,10 +823,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { virUSBDevicePtr usb = - virUSBDeviceNew(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - vroot); - + virUSBDeviceNew(usbsrc->bus, usbsrc->device, vroot); if (!usb) goto done; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 4d2a9d6..0caeae6 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -466,6 +466,7 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACCallbackData cbdata; + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; int ret = -1; if (!priv->dynamicOwnership) @@ -487,10 +488,7 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (dev->missing) return 0; - usb = virUSBDeviceNew(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - vroot); - if (!usb) + if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, vroot))) goto done; ret = virUSBDeviceFileIterate(usb, @@ -597,6 +595,7 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, { virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDefPtr secdef; + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; int ret = -1; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); @@ -614,10 +613,7 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, if (dev->missing) return 0; - usb = virUSBDeviceNew(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - vroot); - if (!usb) + if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, vroot))) goto done; ret = virUSBDeviceFileIterate(usb, virSecurityDACRestoreSecurityUSBLabel, mgr); diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a0e89b7..a96b224 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1319,6 +1319,7 @@ virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def, const char *vroot) { + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; int ret = -1; switch (dev->source.subsys.type) { @@ -1328,8 +1329,8 @@ virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def, if (dev->missing) return 0; - usb = virUSBDeviceNew(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, + usb = virUSBDeviceNew(usbsrc->bus, + usbsrc->device, vroot); if (!usb) goto done; @@ -1508,6 +1509,7 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, const char *vroot) { + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; int ret = -1; switch (dev->source.subsys.type) { @@ -1517,8 +1519,8 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, if (dev->missing) return 0; - usb = virUSBDeviceNew(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, + usb = virUSBDeviceNew(usbsrc->bus, + usbsrc->device, vroot); if (!usb) goto done; diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index b5f66f3..49b06f6 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1047,12 +1047,11 @@ get_files(vahControl * ctl) for (i = 0; i < ctl->def->nhostdevs; i++) if (ctl->def->hostdevs[i]) { virDomainHostdevDefPtr dev = ctl->def->hostdevs[i]; + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { virUSBDevicePtr usb = - virUSBDeviceNew(dev->source.subsys.u.usb.bus, - dev->source.subsys.u.usb.device, - NULL); + virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL); if (usb == NULL) continue; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 9dd1df2..3c93758 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -897,22 +897,19 @@ virHostdevUpdateActiveUSBDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->activeUSBHostdevs); for (i = 0; i < nhostdevs; i++) { + virDomainHostdevSubsysUSBPtr usbsrc; virUSBDevicePtr usb = NULL; hostdev = hostdevs[i]; + usbsrc = &hostdev->source.subsys.u.usb; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) continue; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) continue; - usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device, - NULL); - if (!usb) { + if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL))) { VIR_WARN("Unable to reattach USB device %03d.%03d on domain %s", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device, - dom_name); + usbsrc->bus, usbsrc->device, dom_name); continue; } @@ -1047,11 +1044,12 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, bool mandatory, virUSBDevicePtr *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; - bool autoAddress = hostdev->source.subsys.u.usb.autoAddress; + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + unsigned vendor = usbsrc->vendor; + unsigned product = usbsrc->product; + unsigned bus = usbsrc->bus; + unsigned device = usbsrc->device; + bool autoAddress = usbsrc->autoAddress; int rc; *usb = NULL; @@ -1106,16 +1104,15 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev, return -1; } - hostdev->source.subsys.u.usb.bus = virUSBDeviceGetBus(*usb); - hostdev->source.subsys.u.usb.device = virUSBDeviceGetDevno(*usb); - hostdev->source.subsys.u.usb.autoAddress = true; + usbsrc->bus = virUSBDeviceGetBus(*usb); + usbsrc->device = virUSBDeviceGetDevno(*usb); + usbsrc->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, + usbsrc->bus, usbsrc->device, bus, device); } } else if (!vendor && bus) { @@ -1332,6 +1329,7 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr hostdev_mgr, virObjectLock(hostdev_mgr->activeUSBHostdevs); for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; virUSBDevicePtr usb, tmp; const char *usedby_drvname; const char *usedby_domname; @@ -1343,15 +1341,9 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr hostdev_mgr, if (hostdev->missing) continue; - usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device, - NULL); - - if (!usb) { + if (!(usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device, NULL))) { VIR_WARN("Unable to reattach USB device %03d.%03d on domain %s", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device, - dom_name); + usbsrc->bus, usbsrc->device, dom_name); continue; } @@ -1367,8 +1359,7 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr hostdev_mgr, if (!tmp) { VIR_WARN("Unable to find device %03d.%03d " "in list of active USB devices", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device); + usbsrc->bus, usbsrc->device); continue; } @@ -1376,10 +1367,7 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr hostdev_mgr, if (STREQ_NULLABLE(drv_name, usedby_drvname) && STREQ_NULLABLE(dom_name, usedby_domname)) { VIR_DEBUG("Removing %03d.%03d dom=%s from activeUSBHostdevs", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device, - dom_name); - + usbsrc->bus, usbsrc->device, dom_name); virUSBDeviceListDel(hostdev_mgr->activeUSBHostdevs, tmp); } } -- 1.9.3

On 07/11/2014 06:35 AM, John Ferlan wrote:
Create a separate typedef for the hostdev union data describing USB. Then adjust the code to use the new pointer
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 4 ++-- src/conf/domain_conf.c | 50 +++++++++++++++++++--------------------- src/conf/domain_conf.h | 22 ++++++++++-------- src/lxc/lxc_cgroup.c | 4 ++-- src/lxc/lxc_controller.c | 10 ++++---- src/lxc/lxc_driver.c | 16 ++++++------- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_command.c | 26 ++++++++++----------- src/qemu/qemu_hotplug.c | 7 +++--- src/security/security_apparmor.c | 6 ++--- src/security/security_dac.c | 12 ++++------ src/security/security_selinux.c | 10 ++++---- src/security/virt-aa-helper.c | 5 ++-- src/util/virhostdev.c | 50 +++++++++++++++------------------------- 14 files changed, 104 insertions(+), 122 deletions(-)
Mostly mechanical fallout from the type shuffle.
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index a3d6c67..8277b06 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -388,6 +388,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, char *address = NULL; char *device = NULL; const char *virt; + virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
virUUIDFormat(vm->def->uuid, uuidstr); if (!(vmname = virAuditEncode("vm", vm->def->name))) { @@ -415,8 +416,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (virAsprintfQuiet(&address, "%.3d.%.3d", - hostdev->source.subsys.u.usb.bus, - hostdev->source.subsys.u.usb.device) < 0) { + usbsrc->bus, usbsrc->device) < 0) {
I also find it more legible :)
@@ -10104,15 +10102,18 @@ static int virDomainHostdevMatchSubsysUSB(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { - if (a->source.subsys.u.usb.bus && a->source.subsys.u.usb.device) { + virDomainHostdevSubsysUSBPtr ausbsrc = &a->source.subsys.u.usb; + virDomainHostdevSubsysUSBPtr busbsrc = &b->source.subsys.u.usb;
I read the second variable as bus_b_src, and tried to figure out what the first one was (was "aus" a typo for "bus"?). Until I saw that you meant b_usb_src. Might be worth an _ in the naming to keep it easier to read. Or even different naming conventions: first, second.
+++ b/src/conf/domain_conf.h @@ -390,20 +390,24 @@ typedef enum {
VIR_ENUM_DECL(virDomainHostdevSubsysPCIBackend)
+typedef struct _virDomainHostdevSubsysUSB virDomainHostdevSubsysUSB; +typedef virDomainHostdevSubsysUSB *virDomainHostdevSubsysUSBPtr; +struct _virDomainHostdevSubsysUSB { + bool autoAddress; /* bus/device were filled automatically based + on vedor/product */
Pre-existing typo, but you might as well fix it during the code motion:
+ unsigned bus; + unsigned device; + + unsigned vendor; + unsigned product; +}; + typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys; typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr; struct _virDomainHostdevSubsys { int type; /* enum virDomainHostdevSubsysType */ union { - struct { - bool autoAddress; /* bus/device were filled automatically based - on vedor/product */
s/vedor/vendor/ ACK with nits fixed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Create a separate typedef for the hostdev union data describing PCI. Then adjust the code to use the new pointer Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 9 ++++---- src/conf/domain_conf.c | 25 ++++++++++++--------- src/conf/domain_conf.h | 12 ++++++---- src/libxl/libxl_conf.c | 9 ++++---- src/libxl/libxl_domain.c | 5 +++-- src/libxl/libxl_driver.c | 34 ++++++++++++++--------------- src/qemu/qemu_cgroup.c | 24 ++++++++++---------- src/qemu/qemu_command.c | 47 ++++++++++++++++++---------------------- src/qemu/qemu_hotplug.c | 11 +++++----- src/security/security_apparmor.c | 10 ++++----- src/security/security_dac.c | 20 +++++++---------- src/security/security_selinux.c | 20 +++++++---------- src/util/virhostdev.c | 34 ++++++++++++----------------- 13 files changed, 125 insertions(+), 135 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 8277b06..d3f0449 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -389,6 +389,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, char *device = NULL; const char *virt; virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; virUUIDFormat(vm->def->uuid, uuidstr); if (!(vmname = virAuditEncode("vm", vm->def->name))) { @@ -406,10 +407,10 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, switch (hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: if (virAsprintfQuiet(&address, "%.4x:%.2x:%.2x.%.1x", - hostdev->source.subsys.u.pci.addr.domain, - hostdev->source.subsys.u.pci.addr.bus, - hostdev->source.subsys.u.pci.addr.slot, - hostdev->source.subsys.u.pci.addr.function) < 0) { + pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function) < 0) { VIR_WARN("OOM while encoding audit message"); goto cleanup; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 046b4f8..f1b9a46 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4253,6 +4253,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, char *backendStr = NULL; int backend; int ret = -1; + virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of @@ -4332,7 +4333,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, "has been specified"), backendStr); goto error; } - def->source.subsys.u.pci.backend = backend; + pcisrc->backend = backend; break; @@ -10123,10 +10124,13 @@ static int virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { - if (a->source.subsys.u.pci.addr.domain == b->source.subsys.u.pci.addr.domain && - a->source.subsys.u.pci.addr.bus == b->source.subsys.u.pci.addr.bus && - a->source.subsys.u.pci.addr.slot == b->source.subsys.u.pci.addr.slot && - a->source.subsys.u.pci.addr.function == b->source.subsys.u.pci.addr.function) + virDomainHostdevSubsysPCIPtr apcisrc = &a->source.subsys.u.pci; + virDomainHostdevSubsysPCIPtr bpcisrc = &b->source.subsys.u.pci; + + if (apcisrc->addr.domain == bpcisrc->addr.domain && + apcisrc->addr.bus == bpcisrc->addr.bus && + apcisrc->addr.slot == bpcisrc->addr.slot && + apcisrc->addr.function == bpcisrc->addr.function) return 1; return 0; } @@ -15486,15 +15490,17 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, bool includeTypeInAddr) { virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - def->source.subsys.u.pci.backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { - const char *backend = virDomainHostdevSubsysPCIBackendTypeToString(def->source.subsys.u.pci.backend); + pcisrc->backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { + const char *backend = + virDomainHostdevSubsysPCIBackendTypeToString(pcisrc->backend); if (!backend) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected pci hostdev driver name type %d"), - def->source.subsys.u.pci.backend); + pcisrc->backend); return -1; } virBufferAsprintf(buf, "<driver name='%s'/>\n", backend); @@ -15530,8 +15536,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - if (virDevicePCIAddressFormat(buf, - def->source.subsys.u.pci.addr, + if (virDevicePCIAddressFormat(buf, pcisrc->addr, includeTypeInAddr) != 0) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("PCI address Formatting failed")); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d955491..f1f1929 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -402,16 +402,20 @@ struct _virDomainHostdevSubsysUSB { unsigned product; }; +typedef struct _virDomainHostdevSubsysPCI virDomainHostdevSubsysPCI; +typedef virDomainHostdevSubsysPCI *virDomainHostdevSubsysPCIPtr; +struct _virDomainHostdevSubsysPCI { + virDevicePCIAddress addr; /* host address */ + int backend; /* enum virDomainHostdevSubsysPCIBackendType */ +}; + typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys; typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr; struct _virDomainHostdevSubsys { int type; /* enum virDomainHostdevSubsysType */ union { virDomainHostdevSubsysUSB usb; - struct { - virDevicePCIAddress addr; /* host address */ - int backend; /* enum virDomainHostdevSubsysPCIBackendType */ - } pci; + virDomainHostdevSubsysPCI pci; struct { char *adapter; unsigned bus; diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 0b4a0b5..b52c65f 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1273,15 +1273,16 @@ libxlDriverConfigGet(libxlDriverPrivatePtr driver) int libxlMakePCI(virDomainHostdevDefPtr hostdev, libxl_device_pci *pcidev) { + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) return -1; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) return -1; - pcidev->domain = hostdev->source.subsys.u.pci.addr.domain; - pcidev->bus = hostdev->source.subsys.u.pci.addr.bus; - pcidev->dev = hostdev->source.subsys.u.pci.addr.slot; - pcidev->func = hostdev->source.subsys.u.pci.addr.function; + pcidev->domain = pcisrc->addr.domain; + pcidev->bus = pcisrc->addr.bus; + pcidev->dev = pcisrc->addr.slot; + pcidev->func = pcisrc->addr.function; return 0; } diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0c86601..2596cc7 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -484,6 +484,7 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { virDomainHostdevDefPtr hostdev = dev->data.hostdev; + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; /* forbid capabilities mode hostdev in this kind of hypervisor */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) { @@ -496,8 +497,8 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - hostdev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) - hostdev->source.subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; + pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) + pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; } return 0; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b27581e..ca10f6a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2626,6 +2626,7 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver, libxl_device_pci pcidev; virDomainHostdevDefPtr found; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) return -1; @@ -2633,10 +2634,8 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver, if (virDomainHostdevFind(vm->def, hostdev, &found) >= 0) { virReportError(VIR_ERR_OPERATION_FAILED, _("target pci device %.4x:%.2x:%.2x.%.1x already exists"), - hostdev->source.subsys.u.pci.addr.domain, - hostdev->source.subsys.u.pci.addr.bus, - hostdev->source.subsys.u.pci.addr.slot, - hostdev->source.subsys.u.pci.addr.function); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); return -1; } @@ -2654,10 +2653,8 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver, if (libxl_device_pci_add(priv->ctx, vm->def->id, &pcidev, 0) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to attach pci device %.4x:%.2x:%.2x.%.1x"), - hostdev->source.subsys.u.pci.addr.domain, - hostdev->source.subsys.u.pci.addr.bus, - hostdev->source.subsys.u.pci.addr.slot, - hostdev->source.subsys.u.pci.addr.function); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); goto error; } @@ -2792,6 +2789,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) virDomainDiskDefPtr disk; virDomainHostdevDefPtr hostdev; virDomainHostdevDefPtr found; + virDomainHostdevSubsysPCIPtr pcisrc; switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -2813,13 +2811,12 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) return -1; if (virDomainHostdevFind(vmdef, hostdev, &found) >= 0) { + pcisrc = &hostdev->source.subsys.u.pci; virReportError(VIR_ERR_OPERATION_FAILED, _("target pci device %.4x:%.2x:%.2x.%.1x\ already exists"), - hostdev->source.subsys.u.pci.addr.domain, - hostdev->source.subsys.u.pci.addr.bus, - hostdev->source.subsys.u.pci.addr.slot, - hostdev->source.subsys.u.pci.addr.function); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); return -1; } @@ -2870,6 +2867,7 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver, virDomainHostdevDefPtr hostdev) { virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + virDomainHostdevSubsysPCIPtr pcisrc = &subsys->u.pci; libxl_device_pci pcidev; virDomainHostdevDefPtr detach; int idx; @@ -2882,16 +2880,16 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver, if (idx < 0) { virReportError(VIR_ERR_OPERATION_FAILED, _("host pci device %.4x:%.2x:%.2x.%.1x not found"), - subsys->u.pci.addr.domain, subsys->u.pci.addr.bus, - subsys->u.pci.addr.slot, subsys->u.pci.addr.function); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); return -1; } if (libxlIsMultiFunctionDevice(vm->def, detach->info)) { virReportError(VIR_ERR_OPERATION_FAILED, _("cannot hot unplug multifunction PCI device: %.4x:%.2x:%.2x.%.1x"), - subsys->u.pci.addr.domain, subsys->u.pci.addr.bus, - subsys->u.pci.addr.slot, subsys->u.pci.addr.function); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); goto error; } @@ -2905,8 +2903,8 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver, virReportError(VIR_ERR_INTERNAL_ERROR, _("libxenlight failed to detach pci device\ %.4x:%.2x:%.2x.%.1x"), - subsys->u.pci.addr.domain, subsys->u.pci.addr.bus, - subsys->u.pci.addr.slot, subsys->u.pci.addr.function); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); goto error; } diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 8f03078..c18af84 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -243,6 +243,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; virPCIDevicePtr pci = NULL; virUSBDevicePtr usb = NULL; virSCSIDevicePtr scsi = NULL; @@ -260,14 +261,13 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - if (dev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { int rv; - pci = virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function); + pci = virPCIDeviceNew(pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function); if (!pci) goto cleanup; @@ -340,6 +340,7 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; virPCIDevicePtr pci = NULL; char *path = NULL; @@ -355,14 +356,13 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm, switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - if (dev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { int rv; - pci = virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function); + pci = virPCIDeviceNew(pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function); if (!pci) goto cleanup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 208d49d..d95566f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -549,18 +549,16 @@ qemuNetworkPrepareDevices(virDomainDefPtr def) * separately. */ virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; if (virDomainHostdevFind(def, hostdev, NULL) >= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("PCI device %04x:%02x:%02x.%x " "allocated from network %s is already " "in use by domain %s"), - hostdev->source.subsys.u.pci.addr.domain, - hostdev->source.subsys.u.pci.addr.bus, - hostdev->source.subsys.u.pci.addr.slot, - hostdev->source.subsys.u.pci.addr.function, - net->data.network.name, - def->name); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function, + net->data.network.name, def->name); goto cleanup; } if (virDomainHostdevInsert(def, hostdev) < 0) @@ -4829,14 +4827,13 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, int qemuOpenPCIConfig(virDomainHostdevDefPtr dev) { + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; char *path = NULL; int configfd = -1; if (virAsprintf(&path, "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/config", - dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function) < 0) + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function) < 0) return -1; configfd = open(path, O_RDWR, 0); @@ -4856,7 +4853,8 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; - int backend = dev->source.subsys.u.pci.backend; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; + int backend = pcisrc->backend; /* caller has to assign proper passthrough backend type */ switch ((virDomainHostdevSubsysPCIBackendType) backend) { @@ -4878,20 +4876,19 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def, } virBufferAddLit(&buf, ",host="); - if (dev->source.subsys.u.pci.addr.domain) { + if (pcisrc->addr.domain) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_HOST_PCI_MULTIDOMAIN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("non-zero domain='%.4x' in host device PCI address " "not supported in this QEMU binary"), - dev->source.subsys.u.pci.addr.domain); + pcisrc->addr.domain); goto error; } - virBufferAsprintf(&buf, "%.4x:", dev->source.subsys.u.pci.addr.domain); + virBufferAsprintf(&buf, "%.4x:", pcisrc->addr.domain); } virBufferAsprintf(&buf, "%.2x:%.2x.%.1x", - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function); + pcisrc->addr.bus, pcisrc->addr.slot, + pcisrc->addr.function); virBufferAsprintf(&buf, ",id=%s", dev->info->alias); if (dev->info->bootIndex) virBufferAsprintf(&buf, ",bootindex=%d", dev->info->bootIndex); @@ -4916,25 +4913,23 @@ qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) { char *ret = NULL; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; - if (dev->source.subsys.u.pci.addr.domain) { + if (pcisrc->addr.domain) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_HOST_PCI_MULTIDOMAIN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("non-zero domain='%.4x' in host device PCI address " "not supported in this QEMU binary"), - dev->source.subsys.u.pci.addr.domain); + pcisrc->addr.domain); goto cleanup; } ignore_value(virAsprintf(&ret, "host=%.4x:%.2x:%.2x.%.1x", - dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function)); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function)); } else { ignore_value(virAsprintf(&ret, "host=%.2x:%.2x.%.1x", - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function)); + pcisrc->addr.bus, pcisrc->addr.slot, + pcisrc->addr.function)); } cleanup: return ret; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0427930..f94e42d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3204,14 +3204,14 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr detach) { qemuDomainObjPrivatePtr priv = vm->privateData; - virDomainHostdevSubsysPtr subsys = &detach->source.subsys; + virDomainHostdevSubsysPCIPtr pcisrc = &detach->source.subsys.u.pci; int ret; if (qemuIsMultiFunctionDevice(vm->def, detach->info)) { virReportError(VIR_ERR_OPERATION_FAILED, _("cannot hot unplug multifunction PCI device: %.4x:%.2x:%.2x.%.1x"), - subsys->u.pci.addr.domain, subsys->u.pci.addr.bus, - subsys->u.pci.addr.slot, subsys->u.pci.addr.function); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); return -1; } @@ -3361,6 +3361,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev = dev->data.hostdev; virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; virDomainHostdevSubsysUSBPtr usbsrc = &subsys->u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &subsys->u.pci; virDomainHostdevDefPtr detach = NULL; int idx; @@ -3378,8 +3379,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: virReportError(VIR_ERR_OPERATION_FAILED, _("host pci device %.4x:%.2x:%.2x.%.1x not found"), - subsys->u.pci.addr.domain, subsys->u.pci.addr.bus, - subsys->u.pci.addr.slot, subsys->u.pci.addr.function); + pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (usbsrc->bus && usbsrc->device) { diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 563de4f..c2c4389 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -802,6 +802,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; if (!secdef) return -1; @@ -834,16 +835,13 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { virPCIDevicePtr pci = - virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function); + virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); if (!pci) goto done; - if (dev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); if (!vfioGroupDev) { diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 0caeae6..0844219 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -467,6 +467,7 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityDACCallbackData cbdata; virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; int ret = -1; if (!priv->dynamicOwnership) @@ -500,16 +501,13 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { virPCIDevicePtr pci = - virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function); + virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); if (!pci) goto done; - if (dev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); if (!vfioGroupDev) { @@ -596,6 +594,7 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); virSecurityLabelDefPtr secdef; virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; int ret = -1; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); @@ -624,16 +623,13 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { virPCIDevicePtr pci = - virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function); + virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); if (!pci) goto done; - if (dev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); if (!vfioGroupDev) { diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a96b224..27300ed 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1320,6 +1320,7 @@ virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def, { virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; int ret = -1; switch (dev->source.subsys.type) { @@ -1342,16 +1343,13 @@ virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { virPCIDevicePtr pci = - virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function); + virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); if (!pci) goto done; - if (dev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); if (!vfioGroupDev) { @@ -1510,6 +1508,7 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, { virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; + virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; int ret = -1; switch (dev->source.subsys.type) { @@ -1533,16 +1532,13 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { virPCIDevicePtr pci = - virPCIDeviceNew(dev->source.subsys.u.pci.addr.domain, - dev->source.subsys.u.pci.addr.bus, - dev->source.subsys.u.pci.addr.slot, - dev->source.subsys.u.pci.addr.function); + virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); if (!pci) goto done; - if (dev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { char *vfioGroupDev = virPCIDeviceGetIOMMUGroupDev(pci); if (!vfioGroupDev) { diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 3c93758..f0a1193 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -168,6 +168,7 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; virPCIDevicePtr dev; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) @@ -175,10 +176,8 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - dev = virPCIDeviceNew(hostdev->source.subsys.u.pci.addr.domain, - hostdev->source.subsys.u.pci.addr.bus, - hostdev->source.subsys.u.pci.addr.slot, - hostdev->source.subsys.u.pci.addr.function); + dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); if (!dev) { virObjectUnref(list); return NULL; @@ -191,14 +190,12 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) } virPCIDeviceSetManaged(dev, hostdev->managed); - if (hostdev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { if (virPCIDeviceSetStubDriver(dev, "vfio-pci") < 0) { virObjectUnref(list); return NULL; } - } else if (hostdev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) { + } else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) { if (virPCIDeviceSetStubDriver(dev, "pciback") < 0) { virObjectUnref(list); return NULL; @@ -619,16 +616,15 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr, virPCIDevicePtr dev; virPCIDevicePtr pcidev; virDomainHostdevDefPtr hostdev = hostdevs[i]; + virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) continue; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - dev = virPCIDeviceNew(hostdev->source.subsys.u.pci.addr.domain, - hostdev->source.subsys.u.pci.addr.bus, - hostdev->source.subsys.u.pci.addr.slot, - hostdev->source.subsys.u.pci.addr.function); + dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); /* original states "unbind_from_stub", "remove_slot", * "reprobe" were already set by pciDettachDevice in @@ -832,28 +828,26 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->inactivePCIHostdevs); for (i = 0; i < nhostdevs; i++) { + virDomainHostdevSubsysPCIPtr pcisrc; hostdev = hostdevs[i]; + pcisrc = &hostdev->source.subsys.u.pci; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) continue; if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - dev = virPCIDeviceNew(hostdev->source.subsys.u.pci.addr.domain, - hostdev->source.subsys.u.pci.addr.bus, - hostdev->source.subsys.u.pci.addr.slot, - hostdev->source.subsys.u.pci.addr.function); + dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus, + pcisrc->addr.slot, pcisrc->addr.function); if (!dev) goto cleanup; virPCIDeviceSetManaged(dev, hostdev->managed); - if (hostdev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { if (virPCIDeviceSetStubDriver(dev, "vfio-pci") < 0) goto cleanup; - } else if (hostdev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) { + } else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) { if (virPCIDeviceSetStubDriver(dev, "pciback") < 0) goto cleanup; } else { -- 1.9.3

On 07/11/2014 06:35 AM, John Ferlan wrote:
Create a separate typedef for the hostdev union data describing PCI. Then adjust the code to use the new pointer
Signed-off-by: John Ferlan <jferlan@redhat.com> ---
Starting to get the rebase conflicts here; so yes, reposting the rest of the series would help review.
src/conf/domain_audit.c | 9 ++++---- src/conf/domain_conf.c | 25 ++++++++++++--------- src/conf/domain_conf.h | 12 ++++++---- src/libxl/libxl_conf.c | 9 ++++---- src/libxl/libxl_domain.c | 5 +++-- src/libxl/libxl_driver.c | 34 ++++++++++++++--------------- src/qemu/qemu_cgroup.c | 24 ++++++++++---------- src/qemu/qemu_command.c | 47 ++++++++++++++++++---------------------- src/qemu/qemu_hotplug.c | 11 +++++----- src/security/security_apparmor.c | 10 ++++----- src/security/security_dac.c | 20 +++++++---------- src/security/security_selinux.c | 20 +++++++---------- src/util/virhostdev.c | 34 ++++++++++++----------------- 13 files changed, 125 insertions(+), 135 deletions(-)
Based solely on visual review for this one:
@@ -10123,10 +10124,13 @@ static int virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { - if (a->source.subsys.u.pci.addr.domain == b->source.subsys.u.pci.addr.domain && - a->source.subsys.u.pci.addr.bus == b->source.subsys.u.pci.addr.bus && - a->source.subsys.u.pci.addr.slot == b->source.subsys.u.pci.addr.slot && - a->source.subsys.u.pci.addr.function == b->source.subsys.u.pci.addr.function) + virDomainHostdevSubsysPCIPtr apcisrc = &a->source.subsys.u.pci; + virDomainHostdevSubsysPCIPtr bpcisrc = &b->source.subsys.u.pci;
Similar to 1/8, naming this 'a_pcisrc' or even 'first' might be easier to read. ACK, if the rebase is simple. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Create a separate typedef for the hostdev union data describing SCSI Then adjust the code to use the new pointer Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 7 +++--- src/conf/domain_conf.c | 40 ++++++++++++++++-------------- src/conf/domain_conf.h | 18 ++++++++------ src/qemu/qemu_cgroup.c | 7 +++--- src/qemu/qemu_command.c | 7 +++--- src/qemu/qemu_conf.c | 18 ++++++++------ src/qemu/qemu_hotplug.c | 12 ++++----- src/security/security_apparmor.c | 10 +++----- src/security/security_dac.c | 20 ++++++--------- src/security/security_selinux.c | 20 ++++++--------- src/util/virhostdev.c | 53 ++++++++++++++++------------------------ 11 files changed, 98 insertions(+), 114 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index d3f0449..370dc3a 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -390,6 +390,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, const char *virt; virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; virUUIDFormat(vm->def->uuid, uuidstr); if (!(vmname = virAuditEncode("vm", vm->def->name))) { @@ -424,10 +425,8 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: if (virAsprintfQuiet(&address, "%s:%d:%d:%d", - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit) < 0) { + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit) < 0) { VIR_WARN("OOM while encoding audit message"); goto cleanup; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f1b9a46..e38771c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4025,6 +4025,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, bool got_address = false, got_adapter = false; xmlNodePtr cur; char *bus = NULL, *target = NULL, *unit = NULL; + virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; cur = node->children; while (cur != NULL) { @@ -4046,19 +4047,19 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, goto cleanup; } - if (virStrToLong_ui(bus, NULL, 0, &def->source.subsys.u.scsi.bus) < 0) { + if (virStrToLong_ui(bus, NULL, 0, &scsisrc->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse bus '%s'"), bus); goto cleanup; } - if (virStrToLong_ui(target, NULL, 0, &def->source.subsys.u.scsi.target) < 0) { + if (virStrToLong_ui(target, NULL, 0, &scsisrc->target) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse target '%s'"), target); goto cleanup; } - if (virStrToLong_ui(unit, NULL, 0, &def->source.subsys.u.scsi.unit) < 0) { + if (virStrToLong_ui(unit, NULL, 0, &scsisrc->unit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse unit '%s'"), unit); goto cleanup; @@ -4072,8 +4073,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, "for scsi hostdev source")); goto cleanup; } - if (!(def->source.subsys.u.scsi.adapter = - virXMLPropString(cur, "name"))) { + if (!(scsisrc->adapter = virXMLPropString(cur, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("'adapter' must be specified for scsi hostdev source")); goto cleanup; @@ -4254,6 +4254,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, int backend; int ret = -1; virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; /* @managed can be read from the xml document - it is always an * attribute of the toplevel element, no matter what type of @@ -4311,8 +4312,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, goto error; } - if ((def->source.subsys.u.scsi.sgio = - virDomainDeviceSGIOTypeFromString(sgio)) <= 0) { + if ((scsisrc->sgio = virDomainDeviceSGIOTypeFromString(sgio)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown sgio mode '%s'"), sgio); goto error; @@ -10139,10 +10139,13 @@ static int virDomainHostdevMatchSubsysSCSI(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { - if (STREQ(a->source.subsys.u.scsi.adapter, b->source.subsys.u.scsi.adapter) && - a->source.subsys.u.scsi.bus == b->source.subsys.u.scsi.bus && - a->source.subsys.u.scsi.target == b->source.subsys.u.scsi.target && - a->source.subsys.u.scsi.unit == b->source.subsys.u.scsi.unit) + virDomainHostdevSubsysSCSIPtr ascsisrc = &a->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIPtr bscsisrc = &b->source.subsys.u.scsi; + + if (STREQ(ascsisrc->adapter, bscsisrc->adapter) && + ascsisrc->bus == bscsisrc->bus && + ascsisrc->target == bscsisrc->target && + ascsisrc->unit == bscsisrc->unit) return 1; return 0; } @@ -15491,6 +15494,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, { virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && pcisrc->backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { @@ -15559,12 +15563,10 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: virBufferAsprintf(buf, "<adapter name='%s'/>\n", - def->source.subsys.u.scsi.adapter); + scsisrc->adapter); virBufferAsprintf(buf, "<address %sbus='%d' target='%d' unit='%d'/>\n", includeTypeInAddr ? "type='scsi' " : "", - def->source.subsys.u.scsi.bus, - def->source.subsys.u.scsi.target, - def->source.subsys.u.scsi.unit); + scsisrc->bus, scsisrc->target, scsisrc->unit); break; default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -16960,6 +16962,7 @@ virDomainHostdevDefFormat(virBufferPtr buf, unsigned int flags) { const char *mode = virDomainHostdevModeTypeToString(def->mode); + virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; const char *type; if (!mode) { @@ -16999,11 +17002,10 @@ virDomainHostdevDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " managed='%s'", def->managed ? "yes" : "no"); - if (def->source.subsys.type == - VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - def->source.subsys.u.scsi.sgio) + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + scsisrc->sgio) virBufferAsprintf(buf, " sgio='%s'", - virDomainDeviceSGIOTypeToString(def->source.subsys.u.scsi.sgio)); + virDomainDeviceSGIOTypeToString(scsisrc->sgio)); } virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1f1929..273be7b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -409,6 +409,16 @@ struct _virDomainHostdevSubsysPCI { int backend; /* enum virDomainHostdevSubsysPCIBackendType */ }; +typedef struct _virDomainHostdevSubsysSCSI virDomainHostdevSubsysSCSI; +typedef virDomainHostdevSubsysSCSI *virDomainHostdevSubsysSCSIPtr; +struct _virDomainHostdevSubsysSCSI { + char *adapter; + unsigned bus; + unsigned target; + unsigned unit; + int sgio; /* enum virDomainDeviceSGIO */ +}; + typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys; typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr; struct _virDomainHostdevSubsys { @@ -416,13 +426,7 @@ struct _virDomainHostdevSubsys { union { virDomainHostdevSubsysUSB usb; virDomainHostdevSubsysPCI pci; - struct { - char *adapter; - unsigned bus; - unsigned target; - unsigned unit; - int sgio; /* enum virDomainDeviceSGIO */ - } scsi; + virDomainHostdevSubsysSCSI scsi; } u; }; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index c18af84..0f95cbc 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -244,6 +244,7 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, qemuDomainObjPrivatePtr priv = vm->privateData; virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; virPCIDevicePtr pci = NULL; virUSBDevicePtr usb = NULL; virSCSIDevicePtr scsi = NULL; @@ -307,10 +308,8 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: if ((scsi = virSCSIDeviceNew(NULL, - dev->source.subsys.u.scsi.adapter, - dev->source.subsys.u.scsi.bus, - dev->source.subsys.u.scsi.target, - dev->source.subsys.u.scsi.unit, + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, dev->readonly, dev->shareable)) == NULL) goto cleanup; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d95566f..d990b6a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5125,13 +5125,12 @@ qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, qemuBuildCommandLineCallbacksPtr callbacks) { virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; char *sg = NULL; sg = (callbacks->qemuGetSCSIDeviceSgName)(NULL, - dev->source.subsys.u.scsi.adapter, - dev->source.subsys.u.scsi.bus, - dev->source.subsys.u.scsi.target, - dev->source.subsys.u.scsi.unit); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit); if (!sg) goto error; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e62bec0..9809883 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -927,11 +927,12 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) goto cleanup; } else { + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; if (!(dev_name = virSCSIDeviceGetDevName(NULL, - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit))) + scsisrc->adapter, + scsisrc->bus, + scsisrc->target, + scsisrc->unit))) goto cleanup; if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) @@ -1032,11 +1033,12 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk)))) goto cleanup; } else { + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; if (!(dev_name = virSCSIDeviceGetDevName(NULL, - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit))) + scsisrc->adapter, + scsisrc->bus, + scsisrc->target, + scsisrc->unit))) goto cleanup; if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f94e42d..1807702 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1573,12 +1573,11 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, if (qemuPrepareHostdevSCSIDevices(driver, vm->def->name, &hostdev, 1)) { + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to prepare scsi hostdev: %s:%d:%d:%d"), - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit); return -1; } @@ -3362,6 +3361,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; virDomainHostdevSubsysUSBPtr usbsrc = &subsys->u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &subsys->u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &subsys->u.scsi; virDomainHostdevDefPtr detach = NULL; int idx; @@ -3396,8 +3396,8 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: virReportError(VIR_ERR_OPERATION_FAILED, _("host scsi device %s:%d:%d.%d not found"), - subsys->u.scsi.adapter, subsys->u.scsi.bus, - subsys->u.scsi.target, subsys->u.scsi.unit); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit); break; default: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index c2c4389..93841d1 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -803,6 +803,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME); virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; if (!secdef) return -1; @@ -860,12 +861,9 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - dev->source.subsys.u.scsi.adapter, - dev->source.subsys.u.scsi.bus, - dev->source.subsys.u.scsi.target, - dev->source.subsys.u.scsi.unit, - dev->readonly, - dev->shareable); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, + dev->readonly, dev->shareable); if (!scsi) goto done; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 0844219..968568e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -468,6 +468,7 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, virSecurityDACCallbackData cbdata; virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; int ret = -1; if (!priv->dynamicOwnership) @@ -529,12 +530,9 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - dev->source.subsys.u.scsi.adapter, - dev->source.subsys.u.scsi.bus, - dev->source.subsys.u.scsi.target, - dev->source.subsys.u.scsi.unit, - dev->readonly, - dev->shareable); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, + dev->readonly, dev->shareable); if (!scsi) goto done; @@ -595,6 +593,7 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virSecurityLabelDefPtr secdef; virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; int ret = -1; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); @@ -648,12 +647,9 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - dev->source.subsys.u.scsi.adapter, - dev->source.subsys.u.scsi.bus, - dev->source.subsys.u.scsi.target, - dev->source.subsys.u.scsi.unit, - dev->readonly, - dev->shareable); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, + dev->readonly, dev->shareable); if (!scsi) goto done; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 27300ed..a33ab45 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1321,6 +1321,7 @@ virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def, { virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; int ret = -1; switch (dev->source.subsys.type) { @@ -1368,12 +1369,9 @@ virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - dev->source.subsys.u.scsi.adapter, - dev->source.subsys.u.scsi.bus, - dev->source.subsys.u.scsi.target, - dev->source.subsys.u.scsi.unit, - dev->readonly, - dev->shareable); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, + dev->readonly, dev->shareable); if (!scsi) goto done; @@ -1509,6 +1507,7 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, { virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; int ret = -1; switch (dev->source.subsys.type) { @@ -1557,12 +1556,9 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - dev->source.subsys.u.scsi.adapter, - dev->source.subsys.u.scsi.bus, - dev->source.subsys.u.scsi.target, - dev->source.subsys.u.scsi.unit, - dev->readonly, - dev->shareable); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, + dev->readonly, dev->shareable); if (!scsi) goto done; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index f0a1193..1e52cc9 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -938,19 +938,18 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->activeSCSIHostdevs); for (i = 0; i < nhostdevs; i++) { + virDomainHostdevSubsysSCSIPtr scsisrc; hostdev = hostdevs[i]; + scsisrc = &hostdev->source.subsys.u.scsi; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) continue; if (!(scsi = virSCSIDeviceNew(NULL, - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit, - hostdev->readonly, - hostdev->shareable))) + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, + hostdev->readonly, hostdev->shareable))) goto cleanup; if ((tmp = virSCSIDeviceListFind(mgr->activeSCSIHostdevs, scsi))) { @@ -1220,6 +1219,8 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr hostdev_mgr, /* Loop 1: build temporary list */ for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; + virDomainHostdevSubsysSCSIPtr scsisrc = + &hostdev->source.subsys.u.scsi; virSCSIDevicePtr scsi; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || @@ -1233,12 +1234,9 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr hostdev_mgr, } if (!(scsi = virSCSIDeviceNew(NULL, - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit, - hostdev->readonly, - hostdev->shareable))) + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, + hostdev->readonly, hostdev->shareable))) goto cleanup; if (scsi && virSCSIDeviceListAdd(list, scsi) < 0) { @@ -1383,6 +1381,8 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, virObjectLock(hostdev_mgr->activeSCSIHostdevs); for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; + virDomainHostdevSubsysSCSIPtr scsisrc = + &hostdev->source.subsys.u.scsi; virSCSIDevicePtr scsi; virSCSIDevicePtr tmp; @@ -1391,18 +1391,12 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, continue; if (!(scsi = virSCSIDeviceNew(NULL, - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit, - hostdev->readonly, - hostdev->shareable))) { + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit, + hostdev->readonly, hostdev->shareable))) { VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%d on domain %s", - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit, - dom_name); + scsisrc->adapter, scsisrc->bus, scsisrc->target, + scsisrc->unit, dom_name); continue; } @@ -1412,20 +1406,15 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, if (!(tmp = virSCSIDeviceListFind(hostdev_mgr->activeSCSIHostdevs, scsi))) { VIR_WARN("Unable to find device %s:%d:%d:%d " "in list of active SCSI devices", - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit); + scsisrc->adapter, scsisrc->bus, + scsisrc->target, scsisrc->unit); virSCSIDeviceFree(scsi); continue; } VIR_DEBUG("Removing %s:%d:%d:%d dom=%s from activeSCSIHostdevs", - hostdev->source.subsys.u.scsi.adapter, - hostdev->source.subsys.u.scsi.bus, - hostdev->source.subsys.u.scsi.target, - hostdev->source.subsys.u.scsi.unit, - dom_name); + scsisrc->adapter, scsisrc->bus, scsisrc->target, + scsisrc->unit, dom_name); virSCSIDeviceListDel(hostdev_mgr->activeSCSIHostdevs, tmp, drv_name, dom_name); -- 1.9.3

On 07/11/2014 06:35 AM, John Ferlan wrote:
Create a separate typedef for the hostdev union data describing SCSI Then adjust the code to use the new pointer
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 7 +++--- src/conf/domain_conf.c | 40 ++++++++++++++++-------------- src/conf/domain_conf.h | 18 ++++++++------ src/qemu/qemu_cgroup.c | 7 +++--- src/qemu/qemu_command.c | 7 +++--- src/qemu/qemu_conf.c | 18 ++++++++------ src/qemu/qemu_hotplug.c | 12 ++++----- src/security/security_apparmor.c | 10 +++----- src/security/security_dac.c | 20 ++++++--------- src/security/security_selinux.c | 20 ++++++--------- src/util/virhostdev.c | 53 ++++++++++++++++------------------------ 11 files changed, 98 insertions(+), 114 deletions(-)
@@ -10139,10 +10139,13 @@ static int virDomainHostdevMatchSubsysSCSI(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { - if (STREQ(a->source.subsys.u.scsi.adapter, b->source.subsys.u.scsi.adapter) && - a->source.subsys.u.scsi.bus == b->source.subsys.u.scsi.bus && - a->source.subsys.u.scsi.target == b->source.subsys.u.scsi.target && - a->source.subsys.u.scsi.unit == b->source.subsys.u.scsi.unit) + virDomainHostdevSubsysSCSIPtr ascsisrc = &a->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIPtr bscsisrc = &b->source.subsys.u.scsi; + + if (STREQ(ascsisrc->adapter, bscsisrc->adapter) && + ascsisrc->bus == bscsisrc->bus && + ascsisrc->target == bscsisrc->target && + ascsisrc->unit == bscsisrc->unit) return 1;
Same as 1/8 on naming 'first', 'second' ACK assuming rebase is trivial. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Split virDomainHostdevSubsysSCSI further. In preparation for having either SCSI or iSCSI data, create a union in virDomainHostdevSubsysSCSI to contain just a virDomainHostdevSubsysSCSIHost to describe the 'scsi_host' host device Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 8 +++--- src/conf/domain_conf.c | 58 ++++++++++++++++++++++++---------------- src/conf/domain_conf.h | 14 +++++++--- src/qemu/qemu_cgroup.c | 9 ++++--- src/qemu/qemu_command.c | 7 +++-- src/qemu/qemu_conf.c | 18 +++++++------ src/qemu/qemu_hotplug.c | 13 +++++---- src/security/security_apparmor.c | 5 ++-- src/security/security_dac.c | 10 ++++--- src/security/security_selinux.c | 10 ++++--- src/util/virhostdev.c | 36 ++++++++++++------------- 11 files changed, 113 insertions(+), 75 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 370dc3a..ee9baa2 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -423,14 +423,16 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, goto cleanup; } break; - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; if (virAsprintfQuiet(&address, "%s:%d:%d:%d", - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit) < 0) { + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit) < 0) { VIR_WARN("OOM while encoding audit message"); goto cleanup; } break; + } default: VIR_WARN("Unexpected hostdev type while encoding audit message: %d", hostdev->source.subsys.type); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e38771c..55c0822 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1733,7 +1733,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) break; case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) - VIR_FREE(def->source.subsys.u.scsi.adapter); + VIR_FREE(def->source.subsys.u.scsi.u.host.adapter); break; } } @@ -4018,16 +4018,16 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, } static int -virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, - virDomainHostdevDefPtr def) +virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode, + virDomainHostdevSubsysSCSIPtr scsisrc) { int ret = -1; bool got_address = false, got_adapter = false; xmlNodePtr cur; char *bus = NULL, *target = NULL, *unit = NULL; - virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - cur = node->children; + cur = sourcenode->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { if (xmlStrEqual(cur->name, BAD_CAST "address")) { @@ -4047,19 +4047,20 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, goto cleanup; } - if (virStrToLong_ui(bus, NULL, 0, &scsisrc->bus) < 0) { + if (virStrToLong_ui(bus, NULL, 0, &scsihostsrc->bus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse bus '%s'"), bus); goto cleanup; } - if (virStrToLong_ui(target, NULL, 0, &scsisrc->target) < 0) { + if (virStrToLong_ui(target, NULL, 0, + &scsihostsrc->target) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse target '%s'"), target); goto cleanup; } - if (virStrToLong_ui(unit, NULL, 0, &scsisrc->unit) < 0) { + if (virStrToLong_ui(unit, NULL, 0, &scsihostsrc->unit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse unit '%s'"), unit); goto cleanup; @@ -4073,7 +4074,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, "for scsi hostdev source")); goto cleanup; } - if (!(scsisrc->adapter = virXMLPropString(cur, "name"))) { + if (!(scsihostsrc->adapter = virXMLPropString(cur, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("'adapter' must be specified for scsi hostdev source")); goto cleanup; @@ -4105,6 +4106,13 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node, return ret; } +static int +virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode, + virDomainHostdevSubsysSCSIPtr scsisrc) +{ + return virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, scsisrc); +} + /* Check if a drive type address $controller:0:0:$unit is already * taken by a disk or not. */ @@ -4343,7 +4351,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - if (virDomainHostdevSubsysSCSIDefParseXML(sourcenode, def) < 0) + if (virDomainHostdevSubsysSCSIDefParseXML(sourcenode, scsisrc) < 0) goto error; break; @@ -10136,16 +10144,18 @@ virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr a, } static int -virDomainHostdevMatchSubsysSCSI(virDomainHostdevDefPtr a, - virDomainHostdevDefPtr b) -{ - virDomainHostdevSubsysSCSIPtr ascsisrc = &a->source.subsys.u.scsi; - virDomainHostdevSubsysSCSIPtr bscsisrc = &b->source.subsys.u.scsi; - - if (STREQ(ascsisrc->adapter, bscsisrc->adapter) && - ascsisrc->bus == bscsisrc->bus && - ascsisrc->target == bscsisrc->target && - ascsisrc->unit == bscsisrc->unit) +virDomainHostdevMatchSubsysSCSIHost(virDomainHostdevDefPtr a, + virDomainHostdevDefPtr b) +{ + virDomainHostdevSubsysSCSIHostPtr ascsihostsrc = + &a->source.subsys.u.scsi.u.host; + virDomainHostdevSubsysSCSIHostPtr bscsihostsrc = + &b->source.subsys.u.scsi.u.host; + + if (STREQ(ascsihostsrc->adapter, bscsihostsrc->adapter) && + ascsihostsrc->bus == bscsihostsrc->bus && + ascsihostsrc->target == bscsihostsrc->target && + ascsihostsrc->unit == bscsihostsrc->unit) return 1; return 0; } @@ -10163,7 +10173,7 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: return virDomainHostdevMatchSubsysUSB(a, b); case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - return virDomainHostdevMatchSubsysSCSI(a, b); + return virDomainHostdevMatchSubsysSCSIHost(a, b); } return 0; } @@ -15495,6 +15505,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb; virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && pcisrc->backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { @@ -15563,10 +15574,11 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: virBufferAsprintf(buf, "<adapter name='%s'/>\n", - scsisrc->adapter); + scsihostsrc->adapter); virBufferAsprintf(buf, "<address %sbus='%d' target='%d' unit='%d'/>\n", includeTypeInAddr ? "type='scsi' " : "", - scsisrc->bus, scsisrc->target, scsisrc->unit); + scsihostsrc->bus, scsihostsrc->target, + scsihostsrc->unit); break; default: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 273be7b..e9a4214 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -409,14 +409,22 @@ struct _virDomainHostdevSubsysPCI { int backend; /* enum virDomainHostdevSubsysPCIBackendType */ }; -typedef struct _virDomainHostdevSubsysSCSI virDomainHostdevSubsysSCSI; -typedef virDomainHostdevSubsysSCSI *virDomainHostdevSubsysSCSIPtr; -struct _virDomainHostdevSubsysSCSI { +typedef struct _virDomainHostdevSubsysSCSIHost virDomainHostdevSubsysSCSIHost; +typedef virDomainHostdevSubsysSCSIHost *virDomainHostdevSubsysSCSIHostPtr; +struct _virDomainHostdevSubsysSCSIHost { char *adapter; unsigned bus; unsigned target; unsigned unit; +}; + +typedef struct _virDomainHostdevSubsysSCSI virDomainHostdevSubsysSCSI; +typedef virDomainHostdevSubsysSCSI *virDomainHostdevSubsysSCSIPtr; +struct _virDomainHostdevSubsysSCSI { int sgio; /* enum virDomainDeviceSGIO */ + union { + virDomainHostdevSubsysSCSIHost host; + } u; }; typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 0f95cbc..9a5459b 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -306,10 +306,11 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, } break; - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; if ((scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, dev->readonly, dev->shareable)) == NULL) goto cleanup; @@ -318,6 +319,8 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, qemuSetupHostSCSIDeviceCgroup, vm) < 0) goto cleanup; + break; + } default: break; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d990b6a..dbdb871 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5126,11 +5126,14 @@ qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, { virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; char *sg = NULL; sg = (callbacks->qemuGetSCSIDeviceSgName)(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit); + scsihostsrc->adapter, + scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit); if (!sg) goto error; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 9809883..9636a87 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -928,11 +928,12 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, goto cleanup; } else { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; if (!(dev_name = virSCSIDeviceGetDevName(NULL, - scsisrc->adapter, - scsisrc->bus, - scsisrc->target, - scsisrc->unit))) + scsihostsrc->adapter, + scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit))) goto cleanup; if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) @@ -1034,11 +1035,12 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver, goto cleanup; } else { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; if (!(dev_name = virSCSIDeviceGetDevName(NULL, - scsisrc->adapter, - scsisrc->bus, - scsisrc->target, - scsisrc->unit))) + scsihostsrc->adapter, + scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit))) goto cleanup; if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1807702..7df5832 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1574,10 +1574,11 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, if (qemuPrepareHostdevSCSIDevices(driver, vm->def->name, &hostdev, 1)) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to prepare scsi hostdev: %s:%d:%d:%d"), - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit); + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit); return -1; } @@ -3393,12 +3394,14 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, usbsrc->vendor, usbsrc->product); } break; - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virReportError(VIR_ERR_OPERATION_FAILED, _("host scsi device %s:%d:%d.%d not found"), - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit); + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit); break; + } default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected hostdev type %d"), subsys->type); diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 93841d1..40c667e 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -859,10 +859,11 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, } case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, dev->readonly, dev->shareable); if (!scsi) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 968568e..08f5041 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -528,10 +528,11 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, } case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, dev->readonly, dev->shareable); if (!scsi) @@ -645,10 +646,11 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, } case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, dev->readonly, dev->shareable); if (!scsi) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a33ab45..f32816f 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1367,10 +1367,11 @@ virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def, } case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, dev->readonly, dev->shareable); if (!scsi) @@ -1554,10 +1555,11 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, } case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; virSCSIDevicePtr scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, dev->readonly, dev->shareable); if (!scsi) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 1e52cc9..83f85aa 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -938,17 +938,17 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->activeSCSIHostdevs); for (i = 0; i < nhostdevs; i++) { - virDomainHostdevSubsysSCSIPtr scsisrc; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc; hostdev = hostdevs[i]; - scsisrc = &hostdev->source.subsys.u.scsi; + scsihostsrc = &hostdev->source.subsys.u.scsi.u.host; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) continue; if (!(scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, hostdev->readonly, hostdev->shareable))) goto cleanup; @@ -1219,8 +1219,8 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr hostdev_mgr, /* Loop 1: build temporary list */ for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; - virDomainHostdevSubsysSCSIPtr scsisrc = - &hostdev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = + &hostdev->source.subsys.u.scsi.u.host; virSCSIDevicePtr scsi; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || @@ -1234,8 +1234,8 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr hostdev_mgr, } if (!(scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, hostdev->readonly, hostdev->shareable))) goto cleanup; @@ -1381,8 +1381,8 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, virObjectLock(hostdev_mgr->activeSCSIHostdevs); for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; - virDomainHostdevSubsysSCSIPtr scsisrc = - &hostdev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = + &hostdev->source.subsys.u.scsi.u.host; virSCSIDevicePtr scsi; virSCSIDevicePtr tmp; @@ -1391,12 +1391,12 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, continue; if (!(scsi = virSCSIDeviceNew(NULL, - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, hostdev->readonly, hostdev->shareable))) { VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%d on domain %s", - scsisrc->adapter, scsisrc->bus, scsisrc->target, - scsisrc->unit, dom_name); + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, dom_name); continue; } @@ -1406,15 +1406,15 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, if (!(tmp = virSCSIDeviceListFind(hostdev_mgr->activeSCSIHostdevs, scsi))) { VIR_WARN("Unable to find device %s:%d:%d:%d " "in list of active SCSI devices", - scsisrc->adapter, scsisrc->bus, - scsisrc->target, scsisrc->unit); + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit); virSCSIDeviceFree(scsi); continue; } VIR_DEBUG("Removing %s:%d:%d:%d dom=%s from activeSCSIHostdevs", - scsisrc->adapter, scsisrc->bus, scsisrc->target, - scsisrc->unit, dom_name); + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, dom_name); virSCSIDeviceListDel(hostdev_mgr->activeSCSIHostdevs, tmp, drv_name, dom_name); -- 1.9.3

On 07/11/2014 06:35 AM, John Ferlan wrote:
Split virDomainHostdevSubsysSCSI further. In preparation for having either SCSI or iSCSI data, create a union in virDomainHostdevSubsysSCSI to contain just a virDomainHostdevSubsysSCSIHost to describe the 'scsi_host' host device
Signed-off-by: John Ferlan <jferlan@redhat.com> ---
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Add a conn for future patches to be able to grab the secret when authenticating an iSCSI host device Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 5 +++-- src/qemu/qemu_command.h | 5 +++-- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_hotplug.c | 34 ++++++++++++++++++++-------------- src/qemu/qemu_hotplug.h | 9 ++++++--- 5 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dbdb871..1ab3ade 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5120,7 +5120,8 @@ qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) } char * -qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, +qemuBuildSCSIHostdevDrvStr(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, qemuBuildCommandLineCallbacksPtr callbacks) { @@ -8866,7 +8867,7 @@ qemuBuildCommandLine(virConnectPtr conn, char *drvstr; virCommandAddArg(cmd, "-drive"); - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps, callbacks))) + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, qemuCaps, callbacks))) goto error; virCommandAddArg(cmd, drvstr); VIR_FREE(drvstr); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index cf51056..b71e964 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -170,10 +170,11 @@ char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); -char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, +char * qemuBuildSCSIHostdevDrvStr(virConnectPtr conn, + virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps, qemuBuildCommandLineCallbacksPtr callbacks) - ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(4); char * qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..3c6fef7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6472,7 +6472,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, break; case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = qemuDomainAttachHostDevice(driver, vm, + ret = qemuDomainAttachHostDevice(dom->conn, driver, vm, dev->data.hostdev); if (!ret) dev->data.hostdev = NULL; @@ -6544,10 +6544,10 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm, ret = qemuDomainDetachLease(driver, vm, dev->data.lease); break; case VIR_DOMAIN_DEVICE_NET: - ret = qemuDomainDetachNetDevice(driver, vm, dev); + ret = qemuDomainDetachNetDevice(dom->conn, driver, vm, dev); break; case VIR_DOMAIN_DEVICE_HOSTDEV: - ret = qemuDomainDetachHostDevice(driver, vm, dev); + ret = qemuDomainDetachHostDevice(dom->conn, driver, vm, dev); break; case VIR_DOMAIN_DEVICE_CHR: ret = qemuDomainDetachChrDevice(driver, vm, dev->data.chr); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7df5832..6773d50 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -860,7 +860,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, * netdev-specific code as appropriate), then also added to * the nets list (see cleanup:) if successful. */ - ret = qemuDomainAttachHostDevice(driver, vm, + ret = qemuDomainAttachHostDevice(conn, driver, vm, virDomainNetGetActualHostdev(net)); goto cleanup; } @@ -1547,7 +1547,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, } static int -qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, +qemuDomainAttachHostSCSIDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { @@ -1594,7 +1595,7 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0) goto cleanup; - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps, + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, priv->qemuCaps, &buildCommandLineCallbacks))) goto cleanup; @@ -1642,7 +1643,8 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, return ret; } -int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, +int qemuDomainAttachHostDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev) { @@ -1667,7 +1669,7 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - if (qemuDomainAttachHostSCSIDevice(driver, vm, + if (qemuDomainAttachHostSCSIDevice(conn, driver, vm, hostdev) < 0) goto error; break; @@ -3265,7 +3267,8 @@ qemuDomainDetachHostUSBDevice(virQEMUDriverPtr driver, } static int -qemuDomainDetachHostSCSIDevice(virQEMUDriverPtr driver, +qemuDomainDetachHostSCSIDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr detach) { @@ -3286,7 +3289,7 @@ qemuDomainDetachHostSCSIDevice(virQEMUDriverPtr driver, return -1; } - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(detach, priv->qemuCaps, + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, detach, priv->qemuCaps, &buildCommandLineCallbacks))) goto cleanup; if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, detach, priv->qemuCaps))) @@ -3317,7 +3320,8 @@ qemuDomainDetachHostSCSIDevice(virQEMUDriverPtr driver, } static int -qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver, +qemuDomainDetachThisHostDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr detach) { @@ -3331,7 +3335,7 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver, ret = qemuDomainDetachHostUSBDevice(driver, vm, detach); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - ret = qemuDomainDetachHostSCSIDevice(driver, vm, detach); + ret = qemuDomainDetachHostSCSIDevice(conn, driver, vm, detach); break; default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -3354,7 +3358,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver, } /* search for a hostdev matching dev and detach it */ -int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, +int qemuDomainDetachHostDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -3414,13 +3419,14 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, * function so that mac address / virtualport are reset */ if (detach->parent.type == VIR_DOMAIN_DEVICE_NET) - return qemuDomainDetachNetDevice(driver, vm, &detach->parent); + return qemuDomainDetachNetDevice(conn, driver, vm, &detach->parent); else - return qemuDomainDetachThisHostDevice(driver, vm, detach); + return qemuDomainDetachThisHostDevice(conn, driver, vm, detach); } int -qemuDomainDetachNetDevice(virQEMUDriverPtr driver, +qemuDomainDetachNetDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev) { @@ -3436,7 +3442,7 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, if (virDomainNetGetActualType(detach) == VIR_DOMAIN_NET_TYPE_HOSTDEV) { /* coverity[negative_returns] */ - ret = qemuDomainDetachThisHostDevice(driver, vm, + ret = qemuDomainDetachThisHostDevice(conn, driver, vm, virDomainNetGetActualHostdev(detach)); goto cleanup; } diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index beaa332..6192973 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -50,7 +50,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainRedirdevDefPtr hostdev); -int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, +int qemuDomainAttachHostDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainHostdevDefPtr hostdev); int qemuDomainChangeGraphics(virQEMUDriverPtr driver, @@ -75,10 +76,12 @@ int qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver, int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev); -int qemuDomainDetachNetDevice(virQEMUDriverPtr driver, +int qemuDomainDetachNetDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev); -int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, +int qemuDomainDetachHostDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev); int qemuDomainAttachLease(virQEMUDriverPtr driver, -- 1.9.3

On 07/11/2014 06:35 AM, John Ferlan wrote:
Add a conn for future patches to be able to grab the secret when authenticating an iSCSI host device
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 5 +++-- src/qemu/qemu_command.h | 5 +++-- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_hotplug.c | 34 ++++++++++++++++++++-------------- src/qemu/qemu_hotplug.h | 9 ++++++--- 5 files changed, 35 insertions(+), 24 deletions(-)
I'm not sure if our goal has been to get rid of passing the conn through by having smarter callbacks, but if it is, this moves in the opposite direction. But this patch is clean code-wise, as long as there are no major design concerns. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 07/21/2014 04:14 PM, Eric Blake wrote:
On 07/11/2014 06:35 AM, John Ferlan wrote:
Add a conn for future patches to be able to grab the secret when authenticating an iSCSI host device
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 5 +++-- src/qemu/qemu_command.h | 5 +++-- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_hotplug.c | 34 ++++++++++++++++++++-------------- src/qemu/qemu_hotplug.h | 9 ++++++--- 5 files changed, 35 insertions(+), 24 deletions(-)
I'm not sure if our goal has been to get rid of passing the conn through by having smarter callbacks, but if it is, this moves in the opposite direction. But this patch is clean code-wise, as long as there are no major design concerns.
The code follows the VIR_DOMAIN_DEVICE_NET: option. It's needed for the secret driver. I do recall we wanted to do something within the code regarding this type of path, but I forget the details. Going to post a v2 shortly. John

Create the structures and API's to hold and manage the iSCSI host device. This extends the 'scsi_host' definitions added in commit id '5c811dce'. A future patch will add the XML parsing, but that code requires some infrastructure to be in place first in order to handle the differences between a 'scsi_host' and an 'iSCSI host' device. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 20 +++- src/conf/domain_conf.c | 47 ++++++++- src/conf/domain_conf.h | 20 ++++ src/qemu/qemu_cgroup.c | 35 ++++--- src/qemu/qemu_command.c | 74 ++++++++++++--- src/qemu/qemu_hotplug.c | 36 +++++-- src/security/security_apparmor.c | 6 ++ src/security/security_dac.c | 12 +++ src/security/security_selinux.c | 12 +++ src/util/virhostdev.c | 200 +++++++++++++++++++++++++-------------- 10 files changed, 349 insertions(+), 113 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index ee9baa2..3ad58b0 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -424,12 +424,22 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev, } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - if (virAsprintfQuiet(&address, "%s:%d:%d:%d", - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit) < 0) { - VIR_WARN("OOM while encoding audit message"); + if (scsisrc->protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + /* Follow virDomainAuditDisk && virDomainAuditGenericDev + * and don't audit the networked device. + */ goto cleanup; + } else { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = + &scsisrc->u.host; + if (virAsprintfQuiet(&address, "%s:%d:%d:%d", + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit) < 0) { + VIR_WARN("OOM while encoding audit message"); + goto cleanup; + } } break; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 55c0822..758b907 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1702,6 +1702,17 @@ virDomainHostdevDefPtr virDomainHostdevDefAlloc(void) return def; } +static void +virDomainHostdevSubsysSCSIiSCSIFree(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc) +{ + if (!iscsisrc) + return; + VIR_FREE(iscsisrc->path); + virStorageNetHostDefFree(iscsisrc->nhosts, iscsisrc->hosts); + virStorageAuthDefFree(iscsisrc->auth); + iscsisrc->auth = NULL; +} + void virDomainHostdevDefClear(virDomainHostdevDefPtr def) { if (!def) @@ -1732,8 +1743,15 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) } break; case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: - if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) - VIR_FREE(def->source.subsys.u.scsi.u.host.adapter); + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { + virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; + if (scsisrc->protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + virDomainHostdevSubsysSCSIiSCSIFree(&scsisrc->u.iscsi); + } else { + VIR_FREE(scsisrc->u.host.adapter); + } + } break; } } @@ -4313,8 +4331,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, } if (sgio) { - if (def->source.subsys.type != - VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { + if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { virReportError(VIR_ERR_XML_ERROR, "%s", _("sgio is only supported for scsi host device")); goto error; @@ -10161,6 +10178,22 @@ virDomainHostdevMatchSubsysSCSIHost(virDomainHostdevDefPtr a, } static int +virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr a, + virDomainHostdevDefPtr b) +{ + virDomainHostdevSubsysSCSIiSCSIPtr aiscsisrc = + &a->source.subsys.u.scsi.u.iscsi; + virDomainHostdevSubsysSCSIiSCSIPtr biscsisrc = + &b->source.subsys.u.scsi.u.iscsi; + + if (STREQ(aiscsisrc->hosts[0].name, biscsisrc->hosts[0].name) && + STREQ(aiscsisrc->hosts[0].port, biscsisrc->hosts[0].port) && + STREQ(aiscsisrc->path, biscsisrc->path)) + return 1; + return 0; +} + +static int virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { @@ -10173,7 +10206,11 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: return virDomainHostdevMatchSubsysUSB(a, b); case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - return virDomainHostdevMatchSubsysSCSIHost(a, b); + if (a->source.subsys.u.scsi.protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + return virDomainHostdevMatchSubsysSCSIiSCSI(a, b); + else + return virDomainHostdevMatchSubsysSCSIHost(a, b); } return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e9a4214..e6aa1db 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -390,6 +390,15 @@ typedef enum { VIR_ENUM_DECL(virDomainHostdevSubsysPCIBackend) +typedef enum { + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE, + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI, + + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST, +} virDomainHostdevSCSIProtocolType; + +VIR_ENUM_DECL(virDomainHostdevSubsysSCSIProtocol) + typedef struct _virDomainHostdevSubsysUSB virDomainHostdevSubsysUSB; typedef virDomainHostdevSubsysUSB *virDomainHostdevSubsysUSBPtr; struct _virDomainHostdevSubsysUSB { @@ -418,12 +427,23 @@ struct _virDomainHostdevSubsysSCSIHost { unsigned unit; }; +typedef struct _virDomainHostdevSubsysSCSIiSCSI virDomainHostdevSubsysSCSIiSCSI; +typedef virDomainHostdevSubsysSCSIiSCSI *virDomainHostdevSubsysSCSIiSCSIPtr; +struct _virDomainHostdevSubsysSCSIiSCSI { + char *path; + size_t nhosts; + virStorageNetHostDefPtr hosts; + virStorageAuthDefPtr auth; +}; + typedef struct _virDomainHostdevSubsysSCSI virDomainHostdevSubsysSCSI; typedef virDomainHostdevSubsysSCSI *virDomainHostdevSubsysSCSIPtr; struct _virDomainHostdevSubsysSCSI { + int protocol; /* enum virDomainHostdevSCSIProtocolType */ int sgio; /* enum virDomainDeviceSGIO */ union { virDomainHostdevSubsysSCSIHost host; + virDomainHostdevSubsysSCSIiSCSI iscsi; } u; }; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9a5459b..e109c33 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -307,18 +307,31 @@ qemuSetupHostdevCGroup(virDomainObjPtr vm, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - if ((scsi = virSCSIDeviceNew(NULL, - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit, - dev->readonly, - dev->shareable)) == NULL) - goto cleanup; + if (scsisrc->protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; + /* Follow qemuSetupDiskCgroup() and qemuSetImageCgroupInternal() + * which does nothing for non local storage + */ + VIR_DEBUG("Not updating cgroups for hostdev iSCSI path '%s'", + iscsisrc->path); + } else { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = + &scsisrc->u.host; + if ((scsi = virSCSIDeviceNew(NULL, + scsihostsrc->adapter, + scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit, + dev->readonly, + dev->shareable)) == NULL) + goto cleanup; - if (virSCSIDeviceFileIterate(scsi, - qemuSetupHostSCSIDeviceCgroup, - vm) < 0) - goto cleanup; + if (virSCSIDeviceFileIterate(scsi, + qemuSetupHostSCSIDeviceCgroup, + vm) < 0) + goto cleanup; + } break; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1ab3ade..4a2a5ea 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5119,13 +5119,11 @@ qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) return ret; } -char * -qemuBuildSCSIHostdevDrvStr(virConnectPtr conn ATTRIBUTE_UNUSED, - virDomainHostdevDefPtr dev, - virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, - qemuBuildCommandLineCallbacksPtr callbacks) +static char * +qemuBuildSCSIHostHostdevDrvStr(virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, + qemuBuildCommandLineCallbacksPtr callbacks) { - virBuffer buf = VIR_BUFFER_INITIALIZER; virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; char *sg = NULL; @@ -5135,10 +5133,64 @@ qemuBuildSCSIHostdevDrvStr(virConnectPtr conn ATTRIBUTE_UNUSED, scsihostsrc->bus, scsihostsrc->target, scsihostsrc->unit); - if (!sg) - goto error; + return sg; +} + +static char * +qemuBuildSCSIiSCSIHostdevDrvStr(virConnectPtr conn, + virDomainHostdevDefPtr dev) +{ + char *source = NULL; + char *secret = NULL; + char *username = NULL; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; + + if (conn && iscsisrc->auth) { + const char *protocol = + virStorageNetProtocolTypeToString(VIR_STORAGE_NET_PROTOCOL_ISCSI); + bool encode = false; + int secretType = VIR_SECRET_USAGE_TYPE_ISCSI; + + username = iscsisrc->auth->username; + if (!(secret = qemuGetSecretString(conn, protocol, encode, + iscsisrc->auth, secretType))) + goto cleanup; + } + + /* Rather than pull what we think we want - use the network disk code */ + source = qemuBuildNetworkDriveURI(VIR_STORAGE_NET_PROTOCOL_ISCSI, + iscsisrc->path, + NULL, /* volume */ + iscsisrc->nhosts, + iscsisrc->hosts, + username, secret); - virBufferAsprintf(&buf, "file=/dev/%s,if=none", sg); + cleanup: + VIR_FREE(secret); + return source; +} + +char * +qemuBuildSCSIHostdevDrvStr(virConnectPtr conn, + virDomainHostdevDefPtr dev, + virQEMUCapsPtr qemuCaps, + qemuBuildCommandLineCallbacksPtr callbacks) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *source = NULL; + virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; + + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + if (!(source = qemuBuildSCSIiSCSIHostdevDrvStr(conn, dev))) + goto error; + virBufferAsprintf(&buf, "file=%s,if=none,format=raw", source); + } else { + if (!(source = qemuBuildSCSIHostHostdevDrvStr(dev, qemuCaps, + callbacks))) + goto error; + virBufferAsprintf(&buf, "file=/dev/%s,if=none", source); + } virBufferAsprintf(&buf, ",id=%s-%s", virDomainDeviceAddressTypeToString(dev->info->type), dev->info->alias); @@ -5157,10 +5209,10 @@ qemuBuildSCSIHostdevDrvStr(virConnectPtr conn ATTRIBUTE_UNUSED, if (virBufferCheckError(&buf) < 0) goto error; - VIR_FREE(sg); + VIR_FREE(source); return virBufferContentAndReset(&buf); error: - VIR_FREE(sg); + VIR_FREE(source); virBufferFreeAndReset(&buf); return NULL; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6773d50..729744c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1575,11 +1575,18 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn, if (qemuPrepareHostdevSCSIDevices(driver, vm->def->name, &hostdev, 1)) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to prepare scsi hostdev: %s:%d:%d:%d"), - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit); + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to prepare scsi hostdev for iSCSI: %s"), + iscsisrc->path); + } else { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to prepare scsi hostdev: %s:%d:%d:%d"), + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit); + } return -1; } @@ -3400,11 +3407,20 @@ int qemuDomainDetachHostDevice(virConnectPtr conn, } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: { - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - virReportError(VIR_ERR_OPERATION_FAILED, - _("host scsi device %s:%d:%d.%d not found"), - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit); + if (scsisrc->protocol == + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; + virReportError(VIR_ERR_OPERATION_FAILED, + _("host scsi iSCSI path %s not found"), + iscsisrc->path); + } else { + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = + &scsisrc->u.host; + virReportError(VIR_ERR_OPERATION_FAILED, + _("host scsi device %s:%d:%d.%d not found"), + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit); + } break; } default: diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 40c667e..af9de2d 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -814,6 +814,12 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) return 0; + /* Like AppArmorRestoreSecurityImageLabel() for a networked disk, + * do nothing for an iSCSI hostdev + */ + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + return 0; + if (profile_loaded(secdef->imagelabel) < 0) return 0; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 08f5041..0f92194 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -477,6 +477,12 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr mgr, if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) return 0; + /* Like virSecurityDACSetSecurityImageLabel() for a networked disk, + * do nothing for an iSCSI hostdev + */ + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + return 0; + cbdata.manager = mgr; cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); @@ -605,6 +611,12 @@ virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) return 0; + /* Like virSecurityDACRestoreSecurityImageLabelInt() for a networked disk, + * do nothing for an iSCSI hostdev + */ + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + return 0; + switch ((virDomainHostdevSubsysType) dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { virUSBDevicePtr usb; diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index f32816f..c078cab 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1324,6 +1324,12 @@ virSecuritySELinuxSetSecurityHostdevSubsysLabel(virDomainDefPtr def, virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; int ret = -1; + /* Like virSecuritySELinuxSetSecurityImageLabelInternal() for a networked + * disk, do nothing for an iSCSI hostdev + */ + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + return 0; + switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { virUSBDevicePtr usb; @@ -1511,6 +1517,12 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; int ret = -1; + /* Like virSecuritySELinuxRestoreSecurityImageLabelInt() for a networked + * disk, do nothing for an iSCSI hostdev + */ + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + return 0; + switch (dev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: { virUSBDevicePtr usb; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 83f85aa..53da626 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -920,6 +920,43 @@ virHostdevUpdateActiveUSBDevices(virHostdevManagerPtr mgr, return ret; } +static int +virHostdevUpdateActiveSCSIHostDevices(virHostdevManagerPtr mgr, + virDomainHostdevDefPtr hostdev, + virDomainHostdevSubsysSCSIPtr scsisrc, + const char *drv_name, + const char *dom_name) +{ + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; + virSCSIDevicePtr scsi = NULL; + virSCSIDevicePtr tmp = NULL; + int ret = -1; + + if (!(scsi = virSCSIDeviceNew(NULL, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, + hostdev->readonly, hostdev->shareable))) + goto cleanup; + + if ((tmp = virSCSIDeviceListFind(mgr->activeSCSIHostdevs, scsi))) { + if (virSCSIDeviceSetUsedBy(tmp, drv_name, dom_name) < 0) { + virSCSIDeviceFree(scsi); + goto cleanup; + } + virSCSIDeviceFree(scsi); + } else { + if (virSCSIDeviceSetUsedBy(scsi, drv_name, dom_name) < 0 || + virSCSIDeviceListAdd(mgr->activeSCSIHostdevs, scsi) < 0) { + virSCSIDeviceFree(scsi); + goto cleanup; + } + } + ret = 0; + + cleanup: + return ret; +} + int virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr, virDomainHostdevDefPtr *hostdevs, @@ -930,40 +967,26 @@ virHostdevUpdateActiveSCSIDevices(virHostdevManagerPtr mgr, virDomainHostdevDefPtr hostdev = NULL; size_t i; int ret = -1; - virSCSIDevicePtr scsi = NULL; - virSCSIDevicePtr tmp = NULL; if (!nhostdevs) return 0; virObjectLock(mgr->activeSCSIHostdevs); for (i = 0; i < nhostdevs; i++) { - virDomainHostdevSubsysSCSIHostPtr scsihostsrc; + virDomainHostdevSubsysSCSIPtr scsisrc; hostdev = hostdevs[i]; - scsihostsrc = &hostdev->source.subsys.u.scsi.u.host; + scsisrc = &hostdev->source.subsys.u.scsi; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) continue; - if (!(scsi = virSCSIDeviceNew(NULL, - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit, - hostdev->readonly, hostdev->shareable))) - goto cleanup; - - if ((tmp = virSCSIDeviceListFind(mgr->activeSCSIHostdevs, scsi))) { - if (virSCSIDeviceSetUsedBy(tmp, drv_name, dom_name) < 0) { - virSCSIDeviceFree(scsi); - goto cleanup; - } - virSCSIDeviceFree(scsi); + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + continue; /* Not supported for iSCSI */ } else { - if (virSCSIDeviceSetUsedBy(scsi, drv_name, dom_name) < 0 || - virSCSIDeviceListAdd(mgr->activeSCSIHostdevs, scsi) < 0) { - virSCSIDeviceFree(scsi); + if (virHostdevUpdateActiveSCSIHostDevices(mgr, hostdev, scsisrc, + drv_name, dom_name) < 0) goto cleanup; - } } } ret = 0; @@ -1193,6 +1216,38 @@ virHostdevPrepareUSBDevices(virHostdevManagerPtr hostdev_mgr, return ret; } +static int +virHostdevPrepareSCSIHostDevices(virDomainHostdevDefPtr hostdev, + virDomainHostdevSubsysSCSIPtr scsisrc, + virSCSIDeviceListPtr list) +{ + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; + virSCSIDevicePtr scsi; + int ret = -1; + + if (hostdev->managed) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("SCSI host device doesn't support managed mode")); + goto cleanup; + } + + if (!(scsi = virSCSIDeviceNew(NULL, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, + hostdev->readonly, hostdev->shareable))) + goto cleanup; + + if (virSCSIDeviceListAdd(list, scsi) < 0) { + virSCSIDeviceFree(scsi); + goto cleanup; + } + + ret = 0; + + cleanup: + return ret; +} + int virHostdevPrepareSCSIDevices(virHostdevManagerPtr hostdev_mgr, const char *drv_name, @@ -1219,29 +1274,17 @@ virHostdevPrepareSCSIDevices(virHostdevManagerPtr hostdev_mgr, /* Loop 1: build temporary list */ for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = - &hostdev->source.subsys.u.scsi.u.host; - virSCSIDevicePtr scsi; + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) continue; - if (hostdev->managed) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("SCSI host device doesn't support managed mode")); - goto cleanup; - } - - if (!(scsi = virSCSIDeviceNew(NULL, - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit, - hostdev->readonly, hostdev->shareable))) - goto cleanup; - - if (scsi && virSCSIDeviceListAdd(list, scsi) < 0) { - virSCSIDeviceFree(scsi); - goto cleanup; + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + continue; /* Not supported for iSCSI */ + } else { + if (virHostdevPrepareSCSIHostDevices(hostdev, scsisrc, list) < 0) + goto cleanup; } } @@ -1366,6 +1409,48 @@ virHostdevReAttachUSBDevices(virHostdevManagerPtr hostdev_mgr, virObjectUnlock(hostdev_mgr->activeUSBHostdevs); } +static void +virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr hostdev_mgr, + virDomainHostdevDefPtr hostdev, + virDomainHostdevSubsysSCSIPtr scsisrc, + const char *drv_name, + const char *dom_name) +{ + virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; + virSCSIDevicePtr scsi; + virSCSIDevicePtr tmp; + + if (!(scsi = virSCSIDeviceNew(NULL, + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit, + hostdev->readonly, hostdev->shareable))) { + VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%d on domain %s", + scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target, + scsihostsrc->unit, dom_name); + return; + } + + /* Only delete the devices which are marked as being used by @name, + * because qemuProcessStart could fail on the half way. */ + + if (!(tmp = virSCSIDeviceListFind(hostdev_mgr->activeSCSIHostdevs, scsi))) { + VIR_WARN("Unable to find device %s:%d:%d:%d " + "in list of active SCSI devices", + scsihostsrc->adapter, scsihostsrc->bus, + scsihostsrc->target, scsihostsrc->unit); + virSCSIDeviceFree(scsi); + return; + } + + VIR_DEBUG("Removing %s:%d:%d:%d dom=%s from activeSCSIHostdevs", + scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target, + scsihostsrc->unit, dom_name); + + virSCSIDeviceListDel(hostdev_mgr->activeSCSIHostdevs, tmp, + drv_name, dom_name); + virSCSIDeviceFree(scsi); +} + void virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, const char *drv_name, @@ -1381,44 +1466,17 @@ virHostdevReAttachSCSIDevices(virHostdevManagerPtr hostdev_mgr, virObjectLock(hostdev_mgr->activeSCSIHostdevs); for (i = 0; i < nhostdevs; i++) { virDomainHostdevDefPtr hostdev = hostdevs[i]; - virDomainHostdevSubsysSCSIHostPtr scsihostsrc = - &hostdev->source.subsys.u.scsi.u.host; - virSCSIDevicePtr scsi; - virSCSIDevicePtr tmp; + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) continue; - if (!(scsi = virSCSIDeviceNew(NULL, - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit, - hostdev->readonly, hostdev->shareable))) { - VIR_WARN("Unable to reattach SCSI device %s:%d:%d:%d on domain %s", - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit, dom_name); - continue; - } - - /* Only delete the devices which are marked as being used by @name, - * because qemuProcessStart could fail on the half way. */ - - if (!(tmp = virSCSIDeviceListFind(hostdev_mgr->activeSCSIHostdevs, scsi))) { - VIR_WARN("Unable to find device %s:%d:%d:%d " - "in list of active SCSI devices", - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit); - virSCSIDeviceFree(scsi); - continue; - } - - VIR_DEBUG("Removing %s:%d:%d:%d dom=%s from activeSCSIHostdevs", - scsihostsrc->adapter, scsihostsrc->bus, - scsihostsrc->target, scsihostsrc->unit, dom_name); - - virSCSIDeviceListDel(hostdev_mgr->activeSCSIHostdevs, tmp, - drv_name, dom_name); - virSCSIDeviceFree(scsi); + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + continue; /* Not supported for iSCSI */ + else + virHostdevReAttachSCSIHostDevices(hostdev_mgr, hostdev, scsisrc, + drv_name, dom_name); } virObjectUnlock(hostdev_mgr->activeSCSIHostdevs); } -- 1.9.3

On 07/11/2014 06:35 AM, John Ferlan wrote:
Create the structures and API's to hold and manage the iSCSI host device. This extends the 'scsi_host' definitions added in commit id '5c811dce'. A future patch will add the XML parsing, but that code requires some infrastructure to be in place first in order to handle the differences between a 'scsi_host' and an 'iSCSI host' device.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_audit.c | 20 +++- src/conf/domain_conf.c | 47 ++++++++- src/conf/domain_conf.h | 20 ++++ src/qemu/qemu_cgroup.c | 35 ++++--- src/qemu/qemu_command.c | 74 ++++++++++++--- src/qemu/qemu_hotplug.c | 36 +++++-- src/security/security_apparmor.c | 6 ++ src/security/security_dac.c | 12 +++ src/security/security_selinux.c | 12 +++ src/util/virhostdev.c | 200 +++++++++++++++++++++++++-------------- 10 files changed, 349 insertions(+), 113 deletions(-)
At this point, it's big enough that I'll wait for a respin to make sure I'm reviewing it correctly on the latest tree. But quick comments:
+static void +virDomainHostdevSubsysSCSIiSCSIFree(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc) +{ + if (!iscsisrc) + return; + VIR_FREE(iscsisrc->path); + virStorageNetHostDefFree(iscsisrc->nhosts, iscsisrc->hosts); + virStorageAuthDefFree(iscsisrc->auth); + iscsisrc->auth = NULL; +}
This function doesn't free iscsisrc; typically, we name this type of function Clear instead of Free.
static int +virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr a, + virDomainHostdevDefPtr b) +{ + virDomainHostdevSubsysSCSIiSCSIPtr aiscsisrc = + &a->source.subsys.u.scsi.u.iscsi; + virDomainHostdevSubsysSCSIiSCSIPtr biscsisrc = + &b->source.subsys.u.scsi.u.iscsi; + + if (STREQ(aiscsisrc->hosts[0].name, biscsisrc->hosts[0].name) && + STREQ(aiscsisrc->hosts[0].port, biscsisrc->hosts[0].port) && + STREQ(aiscsisrc->path, biscsisrc->path))
'first' and 'second' naming, as in 1/8. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

In preparation for hostdev support for iSCSI and a virStorageNetHostDefPtr, split out the network disk storage parsing of the 'host' element into a separate routine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 137 +++++++++++++++++++++++++++---------------------- 1 file changed, 76 insertions(+), 61 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 758b907..814ab87 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4036,6 +4036,79 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node, } static int +virDomainStorageHostParse(xmlNodePtr node, + virStorageNetHostDefPtr *hosts, + size_t *nhosts) +{ + int ret = -1; + xmlNodePtr child; + char *transport = NULL; + virStorageNetHostDef host; + + memset(&host, 0, sizeof(host)); + + child = node->children; + while (child != NULL) { + if (child->type == XML_ELEMENT_NODE && + xmlStrEqual(child->name, BAD_CAST "host")) { + + host.transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + + /* transport can be tcp (default), unix or rdma. */ + if ((transport = virXMLPropString(child, "transport"))) { + host.transport = virStorageNetHostTransportTypeFromString(transport); + if (host.transport < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown protocol transport type '%s'"), + transport); + goto cleanup; + } + } + + host.socket = virXMLPropString(child, "socket"); + + if (host.transport == VIR_STORAGE_NET_HOST_TRANS_UNIX && + host.socket == NULL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing socket for unix transport")); + goto cleanup; + } + + if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX && + host.socket != NULL) { + virReportError(VIR_ERR_XML_ERROR, + _("transport '%s' does not support " + "socket attribute"), + transport); + goto cleanup; + } + + VIR_FREE(transport); + + if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX) { + if (!(host.name = virXMLPropString(child, "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing name for host")); + goto cleanup; + } + + host.port = virXMLPropString(child, "port"); + } + + if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host) < 0) + goto cleanup; + } + child = child->next; + } + ret = 0; + + cleanup: + virStorageNetHostDefClear(&host); + VIR_FREE(transport); + return ret; +} + +static int virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode, virDomainHostdevSubsysSCSIPtr scsisrc) { @@ -5014,13 +5087,8 @@ int virDomainDiskSourceParse(xmlNodePtr node, virStorageSourcePtr src) { - char *protocol = NULL; - char *transport = NULL; - virStorageNetHostDef host; - xmlNodePtr child; int ret = -1; - - memset(&host, 0, sizeof(host)); + char *protocol = NULL; switch ((virStorageType)src->type) { case VIR_STORAGE_TYPE_FILE: @@ -5073,59 +5141,8 @@ virDomainDiskSourceParse(xmlNodePtr node, tmp[0] = '\0'; } - child = node->children; - while (child != NULL) { - if (child->type == XML_ELEMENT_NODE && - xmlStrEqual(child->name, BAD_CAST "host")) { - - host.transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - - /* transport can be tcp (default), unix or rdma. */ - if ((transport = virXMLPropString(child, "transport"))) { - host.transport = virStorageNetHostTransportTypeFromString(transport); - if (host.transport < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown protocol transport type '%s'"), - transport); - goto cleanup; - } - } - - host.socket = virXMLPropString(child, "socket"); - - if (host.transport == VIR_STORAGE_NET_HOST_TRANS_UNIX && - host.socket == NULL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing socket for unix transport")); - goto cleanup; - } - - if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX && - host.socket != NULL) { - virReportError(VIR_ERR_XML_ERROR, - _("transport '%s' does not support " - "socket attribute"), - transport); - goto cleanup; - } - - VIR_FREE(transport); - - if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX) { - if (!(host.name = virXMLPropString(child, "name"))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing name for host")); - goto cleanup; - } - - host.port = virXMLPropString(child, "port"); - } - - if (VIR_APPEND_ELEMENT(src->hosts, src->nhosts, host) < 0) - goto cleanup; - } - child = child->next; - } + if (virDomainStorageHostParse(node, &src->hosts, &src->nhosts) < 0) + goto cleanup; break; case VIR_STORAGE_TYPE_VOLUME: if (virDomainDiskSourcePoolDefParse(node, &src->srcpool) < 0) @@ -5148,9 +5165,7 @@ virDomainDiskSourceParse(xmlNodePtr node, ret = 0; cleanup: - virStorageNetHostDefClear(&host); VIR_FREE(protocol); - VIR_FREE(transport); return ret; } -- 1.9.3

Introduce a new structure to handle an iSCSI host device based on the existing virDomainHostdevSubsysSCSI by adding a "protocol='iscsi'" to the <source/> element. The hostdev structure mimics the existing <disk/> element for an iSCSI device (network) device. New XML is: <hostdev mode='subsystem' type='scsi' managed='yes'> <auth username='myname'> <secret type='iscsi' usage='mycluster_myname'/> </auth> <source protocol='iscsi' name='iqn.1992-01.com.example'> <host name='example.org' port='3260'/> </source> <address type='drive' controller='0' bus='0' target='2' unit='5'/> </hostdev> The controller element will mimic the existing scsi_host code insomuch as when 'lsi' and 'virtio-scsi' are used. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 142 ++++++++++++------- docs/schemas/domaincommon.rng | 46 ++++++- src/conf/domain_conf.c | 152 ++++++++++++++++++--- .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args | 14 ++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 46 +++++++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args | 14 ++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml | 40 ++++++ ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args | 16 +++ ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 46 +++++++ .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args | 16 +++ .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml | 40 ++++++ tests/qemuxml2argvtest.c | 16 +++ tests/qemuxml2xmltest.c | 5 + 13 files changed, 524 insertions(+), 69 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b69da4c..82b9091 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2781,57 +2781,107 @@ </devices> ...</pre> + + <p>or:</p> + +<pre> + ... + <devices> + <hostdev mode='subsystem' type='scsi'> + <source protocol='iscsi' name='iqn.2014-08.com.example:iscsi-nopool/1'> + <host name='example.com' port='3260'/> + </source> + <auth username='myuser'> + <secret type='iscsi' usage='libvirtiscsi'/> + </auth> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </hostdev> + </devices> + ...</pre> + <dl> <dt><code>hostdev</code></dt> <dd>The <code>hostdev</code> element is the main container for describing - host devices. For usb device passthrough <code>mode</code> is always - "subsystem" and <code>type</code> is "usb" for a USB device, "pci" - for a PCI device and "scsi" for a SCSI device. When - <code>managed</code> is "yes" for a PCI - device, it is detached from the host before being passed on to - the guest, and reattached to the host after the guest exits. - If <code>managed</code> is omitted or "no", and for USB - devices, the user is responsible to - call <code>virNodeDeviceDettach</code> (or <code>virsh - nodedev-dettach</code>) before starting the guest or - hot-plugging the device, - and <code>virNodeDeviceReAttach</code> (or <code>virsh - nodedev-reattach</code>) after hot-unplug or stopping the - guest. For SCSI device, user is responsible to make sure the device - is not used by host. - The optional <code>sgio</code> (<span class="since">since 1.0.6</span>) - attribute indicates whether the kernel will filter unprivileged - SG_IO commands for the disk, valid settings are "filtered" or - "unfiltered". Defaults to "filtered". + host devices. For each device, the <code>mode</code> is always + "subsystem" and the <code>type</code> is one of the following values + with additional attributes noted. + <dl> + <dt>usb</dt> + <dd>For USB devices, the user is responsible to call + <code>virNodeDeviceDettach</code> (or + <code>virsh nodedev-dettach</code>) before starting the guest + or hot-plugging the device and <code>virNodeDeviceReAttach</code> + (or <code>virsh nodedev-reattach</code>) after hot-unplug or + stopping the guest. + </dd> + <dt>pci</dt> + <dd>For PCI devices, when <code>managed</code> is "yes" it is + detached from the host before being passed on to the guest + and reattached to the host after the guest exits. If + <code>managed</code> is omitted or "no", follow the steps + described for a USB device to detach before starting the + guest or hot-plugging and reattach after stopping the guest + or hot-unplug. + </dd> + <dt>scsi</dt> + <dd>For SCSI devices, user is responsible to make sure the device + is not used by host. The optional <code>sgio</code> + (<span class="since">since 1.0.6</span>) attribute indicates + whether the kernel will filter unprivileged SG_IO commands for + the disk, valid settings are "filtered" or "unfiltered". + The default is "filtered". + </dd> + </dl> </dd> <dt><code>source</code></dt> - <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>. - SCSI devices are described by both the <code>adapter</code> and - <code>address</code> elements. - - <span class="since">Since 1.0.0</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>The source element describes the device as seen from the host using + the following mechanism to describe: + <dl> + <dt>usb</dt> + <dd>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 host using the + <code>address</code> element. + <p> + <span class="since">Since 1.0.0</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: + </p> + <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>pci</dt> + <dd>PCI devices can only be described by their <code>address</code>. + </dd> + <dt>scsi</dt> + <dd>SCSI devices are described by both the <code>adapter</code> + and <code>address</code> elements. + <p> + <span class="since">Since 1.2.7</span>, the <code>source</code> + element of a SCSI device may contain the <code>protocol</code> + attribute. When the attribute is set to "iscsi", the host + device XML follows the network <a href="#elementsDisks">disk</a> + device using the same <code>name</code> attribute and optionally + using the <code>auth</code> element to provide the authentication + credentials to the iSCSI server. + </p> + </dd> + </dl> </dd> <dt><code>vendor</code>, <code>product</code></dt> <dd>The <code>vendor</code> and <code>product</code> elements each have an diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7be028d..e54f30a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3502,13 +3502,47 @@ </choice> </attribute> </optional> + <optional> + <ref name='diskAuth'/> + </optional> <element name="source"> - <interleave> - <ref name="sourceinfoadapter"/> - <element name="address"> - <ref name="scsiaddress"/> - </element> - </interleave> + <optional> + <attribute name="protocol"> + <choice> + <value>iscsi</value> + </choice> + </attribute> + <attribute name="name"> + <text/> + </attribute> + </optional> + <choice> + <group> + <interleave> + <ref name="sourceinfoadapter"/> + <element name="address"> + <ref name="scsiaddress"/> + </element> + </interleave> + </group> + <group> + <interleave> + <oneOrMore> + <element name='host'> + <attribute name='name'> + <text/> + </attribute> + <optional> + <attribute name='port'> + <ref name="PortNumber"/> + </attribute> + </optional> + <empty/> + </element> + </oneOrMore> + </interleave> + </group> + </choice> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 814ab87..3bba887 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -604,6 +604,11 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysPCIBackend, "vfio", "xen") +VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIProtocol, + VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST, + "default", + "iscsi") + VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST, "storage", "misc", @@ -4198,10 +4203,93 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode, } static int +virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, + xmlNodePtr authnode, + virDomainHostdevSubsysSCSIPtr def) +{ + int ret = -1; + int auth_secret_usage = -1; + virStorageAuthDefPtr authdef = NULL; + virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &def->u.iscsi; + + /* Similar to virDomainDiskSourceParse for a VIR_STORAGE_TYPE_NETWORK */ + + if (!(iscsisrc->path = virXMLPropString(sourcenode, "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing iSCSI hostdev source path name")); + goto cleanup; + } + + if (virDomainStorageHostParse(sourcenode, &iscsisrc->hosts, + &iscsisrc->nhosts) < 0) + goto cleanup; + + if (iscsisrc->nhosts < 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing the host address for the iSCSI hostdev")); + goto cleanup; + } + if (iscsisrc->nhosts > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one source host address may be specified " + "for the iSCSI hostdev")); + goto cleanup; + } + + if (authnode) { + if (!(authdef = virStorageAuthDefParse(sourcenode->doc, authnode))) + goto cleanup; + if ((auth_secret_usage = + virSecretUsageTypeFromString(authdef->secrettype)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid secret type %s"), + authdef->secrettype); + goto cleanup; + } + if (auth_secret_usage != VIR_SECRET_USAGE_TYPE_ISCSI) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("hostdev invalid secret type '%s'"), + authdef->secrettype); + goto cleanup; + } + iscsisrc->auth = authdef; + authdef = NULL; + } + ret = 0; + + cleanup: + virStorageAuthDefFree(authdef); + return ret; +} + +static int virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode, + xmlNodePtr authnode, virDomainHostdevSubsysSCSIPtr scsisrc) { - return virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, scsisrc); + char *protocol = NULL; + int ret = -1; + + if ((protocol = virXMLPropString(sourcenode, "protocol"))) { + scsisrc->protocol = + virDomainHostdevSubsysSCSIProtocolTypeFromString(protocol); + if (scsisrc->protocol < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown SCSI subsystem protcol '%s'"), + protocol); + goto cleanup; + } + } + + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) + ret = virDomainHostdevSubsysSCSIiSCSIDefParseXML(sourcenode, authnode, + scsisrc); + else + ret = virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, scsisrc); + + cleanup: + VIR_FREE(protocol); + return ret; } /* Check if a drive type address $controller:0:0:$unit is already @@ -4346,6 +4434,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, virDomainHostdevDefPtr def, unsigned int flags) { + xmlNodePtr authnode; xmlNodePtr sourcenode; char *managed = NULL; char *sgio = NULL; @@ -4441,7 +4530,9 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - if (virDomainHostdevSubsysSCSIDefParseXML(sourcenode, scsisrc) < 0) + authnode = virXPathNode("./auth", ctxt); + if (virDomainHostdevSubsysSCSIDefParseXML(sourcenode, authnode, + scsisrc) < 0) goto error; break; @@ -15558,6 +15649,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci; virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi; virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; + virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && pcisrc->backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { @@ -15573,18 +15665,36 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virBufferAsprintf(buf, "<driver name='%s'/>\n", backend); } + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + if (iscsisrc->auth) { + if (virStorageAuthDefFormat(buf, iscsisrc->auth) < 0) + return -1; + } + } + virBufferAddLit(buf, "<source"); - if (def->startupPolicy) { - const char *policy; - policy = virDomainStartupPolicyTypeToString(def->startupPolicy); - virBufferAsprintf(buf, " startupPolicy='%s'", policy); + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { + if (def->startupPolicy) { + const char *policy; + policy = virDomainStartupPolicyTypeToString(def->startupPolicy); + virBufferAsprintf(buf, " startupPolicy='%s'", policy); + } + if (usbsrc->autoAddress && (flags & VIR_DOMAIN_XML_MIGRATABLE)) + virBufferAddLit(buf, " autoAddress='yes'"); + + if (def->missing && !(flags & VIR_DOMAIN_XML_INACTIVE)) + virBufferAddLit(buf, " missing='yes'"); } - if (usbsrc->autoAddress && (flags & VIR_DOMAIN_XML_MIGRATABLE)) - virBufferAddLit(buf, " autoAddress='yes'"); - if (def->missing && - !(flags & VIR_DOMAIN_XML_INACTIVE)) - virBufferAddLit(buf, " missing='yes'"); + if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + const char *protocol = + virDomainHostdevSubsysSCSIProtocolTypeToString(scsisrc->protocol); + + virBufferAsprintf(buf, " protocol='%s' name='%s'", + protocol, iscsisrc->path); + } virBufferAddLit(buf, ">\n"); @@ -15625,12 +15735,20 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, } break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - virBufferAsprintf(buf, "<adapter name='%s'/>\n", - scsihostsrc->adapter); - virBufferAsprintf(buf, "<address %sbus='%d' target='%d' unit='%d'/>\n", - includeTypeInAddr ? "type='scsi' " : "", - scsihostsrc->bus, scsihostsrc->target, - scsihostsrc->unit); + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { + virBufferAddLit(buf, "<host"); + virBufferEscapeString(buf, " name='%s'", iscsisrc->hosts[0].name); + virBufferEscapeString(buf, " port='%s'", iscsisrc->hosts[0].port); + virBufferAddLit(buf, "/>\n"); + } else { + virBufferAsprintf(buf, "<adapter name='%s'/>\n", + scsihostsrc->adapter); + virBufferAsprintf(buf, + "<address %sbus='%d' target='%d' unit='%d'/>\n", + includeTypeInAddr ? "type='scsi' " : "", + scsihostsrc->bus, scsihostsrc->target, + scsihostsrc->unit); + } break; default: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args new file mode 100644 index 0000000..6638dce --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args @@ -0,0 +1,14 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device lsi,id=scsi0,bus=pci.0,addr=0x3 -usb \ +-drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=iscsi://myname:AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A@example.org\ +:3260/iqn.1992-01.com.example,if=none,format=raw,id=drive-hostdev0 \ +-device scsi-generic,bus=scsi0.0,scsi-id=4,drive=drive-hostdev0,id=hostdev0 \ +-drive file=iscsi://myname:AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A@example.org\ +:3260/iqn.1992-01.com.example/1,if=none,format=raw,id=drive-hostdev1 \ +-device scsi-generic,bus=scsi0.0,scsi-id=5,drive=drive-hostdev1,id=hostdev1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml new file mode 100644 index 0000000..e5b7172 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml @@ -0,0 +1,46 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='scsi' managed='yes'> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org' port='3260'/> + </source> + <address type='drive' controller='0' bus='0' target='0' unit='4'/> + </hostdev> + <hostdev mode='subsystem' type='scsi' managed='yes'> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + <source protocol='iscsi' name='iqn.1992-01.com.example/1'> + <host name='example.org' port='3260'/> + </source> + <address type='drive' controller='0' bus='0' target='0' unit='5'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args new file mode 100644 index 0000000..2aebe9c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args @@ -0,0 +1,14 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device lsi,id=scsi0,bus=pci.0,addr=0x3 -usb \ +-drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=iscsi://example.org:3260/iqn.1992-01.com.example,if=none,\ +format=raw,id=drive-hostdev0 \ +-device scsi-generic,bus=scsi0.0,scsi-id=4,drive=drive-hostdev0,id=hostdev0 \ +-drive file=iscsi://example.org:3260/iqn.1992-01.com.example/1,if=none,\ +format=raw,id=drive-hostdev1 \ +-device scsi-generic,bus=scsi0.0,scsi-id=5,drive=drive-hostdev1,id=hostdev1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml new file mode 100644 index 0000000..8a05099 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='scsi' managed='yes'> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org' port='3260'/> + </source> + <address type='drive' controller='0' bus='0' target='0' unit='4'/> + </hostdev> + <hostdev mode='subsystem' type='scsi' managed='yes'> + <source protocol='iscsi' name='iqn.1992-01.com.example/1'> + <host name='example.org' port='3260'/> + </source> + <address type='drive' controller='0' bus='0' target='0' unit='5'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args new file mode 100644 index 0000000..7fd3a00 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args @@ -0,0 +1,16 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -usb \ +-drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=iscsi://myname:AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A@example.org\ +:3260/iqn.1992-01.com.example,if=none,format=raw,id=drive-hostdev0 \ +-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=4,\ +drive=drive-hostdev0,id=hostdev0 \ +-drive file=iscsi://myname:AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A@example.org\ +:3260/iqn.1992-01.com.example/1,if=none,format=raw,id=drive-hostdev1 \ +-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=5,\ +drive=drive-hostdev1,id=hostdev1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml new file mode 100644 index 0000000..f504e88 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml @@ -0,0 +1,46 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='scsi' managed='yes'> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org' port='3260'/> + </source> + <address type='drive' controller='0' bus='0' target='2' unit='4'/> + </hostdev> + <hostdev mode='subsystem' type='scsi' managed='yes'> + <auth username='myname'> + <secret type='iscsi' usage='mycluster_myname'/> + </auth> + <source protocol='iscsi' name='iqn.1992-01.com.example/1'> + <host name='example.org' port='3260'/> + </source> + <address type='drive' controller='0' bus='0' target='2' unit='5'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args new file mode 100644 index 0000000..e4b6e97 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args @@ -0,0 +1,16 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 -usb \ +-drive file=/dev/HostVG/QEMUGuest2,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-drive file=iscsi://example.org:3260/iqn.1992-01.com.example,if=none,\ +format=raw,id=drive-hostdev0 \ +-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=4,\ +drive=drive-hostdev0,id=hostdev0 \ +-drive file=iscsi://example.org:3260/iqn.1992-01.com.example/1,if=none,\ +format=raw,id=drive-hostdev1 \ +-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=2,lun=5,\ +drive=drive-hostdev1,id=hostdev1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml new file mode 100644 index 0000000..13c0930 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest2</name> + <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'/> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='scsi' managed='yes'> + <source protocol='iscsi' name='iqn.1992-01.com.example'> + <host name='example.org' port='3260'/> + </source> + <address type='drive' controller='0' bus='0' target='2' unit='4'/> + </hostdev> + <hostdev mode='subsystem' type='scsi' managed='yes'> + <source protocol='iscsi' name='iqn.1992-01.com.example/1'> + <host name='example.org' port='3260'/> + </source> + <address type='drive' controller='0' bus='0' target='2' unit='5'/> + </hostdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a841adb..cee309d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1328,6 +1328,22 @@ mymain(void) QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_SCSI_GENERIC, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX); + DO_TEST("hostdev-scsi-lsi-iscsi", QEMU_CAPS_DRIVE, + QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI, + QEMU_CAPS_DEVICE_SCSI_GENERIC); + DO_TEST("hostdev-scsi-lsi-iscsi-auth", QEMU_CAPS_DRIVE, + QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI, + QEMU_CAPS_DEVICE_SCSI_GENERIC); + DO_TEST("hostdev-scsi-virtio-iscsi", QEMU_CAPS_DRIVE, + QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_DEVICE_SCSI_GENERIC); + DO_TEST("hostdev-scsi-virtio-iscsi-auth", QEMU_CAPS_DRIVE, + QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI, + QEMU_CAPS_DEVICE_SCSI_GENERIC); DO_TEST("mlock-on", QEMU_CAPS_MLOCK); DO_TEST_FAILURE("mlock-on", NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 26e3cad..a6fca91 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -355,6 +355,11 @@ mymain(void) DO_TEST_DIFFERENT("hostdev-scsi-autogen-address"); + DO_TEST("hostdev-scsi-lsi-iscsi"); + DO_TEST("hostdev-scsi-lsi-iscsi-auth"); + DO_TEST("hostdev-scsi-virtio-iscsi"); + DO_TEST("hostdev-scsi-virtio-iscsi-auth"); + DO_TEST_DIFFERENT("s390-defaultconsole"); DO_TEST("pcihole64"); -- 1.9.3

On 07/11/2014 08:35 AM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=957293
Allow iSCSI <hostdev/> devices to be added similarly to the existing <disk/> devices. Initially do a little bit of refactoring of the hostdev subsys structures to make it easier to model an iSCSI hostdev after the SCSI scsi_host. Although the bulk of the non-structure changes are "technically unnecessary", it just looked nicer to see {usb|pci|scsi}src rather than the longer "->source.subsys.u.{usb|pci|scsi}" in many places in the code.
The end result is the guest will have /dev/sdX devices created.
I have run this code through my Coverity environment and will be running the virttest suite over the weekend.
Patches 1-3 are repetitive moves of the various hostdev subsys types (USB, PCI, and SCSI) into separate typedefs and then modifying code use the Ptr instead of the long union path to each field. I think I got most, but I'm sure there's still a few that could be cleaned up or that were added since I started this.
Patch 4 more or less redoes patch 3 and I suppose could be combined. I left it separate because it's showing the progression to patch 6. This patch uses 'scsihost' to reference the specific portions while still using 'scsisrc' to reference the shared portion (which is only sgio).
Patch 5 adds a virConnectPtr since patch 6 will need a way to get the secret value for the iSCSI secret/auth in qemuBuildSCSIHostdevDrvStr.
Patch 6 adds the qemu code to handle a new hostdev protocol type VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI. The code will mimic the network disk (VIR_STORAGE_NET_PROTOCOL_ISCSI) code when making decisions. The new 'scsisrc' field 'protocol' will be the decision point. The default of VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE was chosen over TYPE_SCSI_HOST. The changes were made in this order to reduce the size of the patch when the XML is added (patch 8). Most of the code was done inline using the iscsisrc->protocol (when the iSCSI code had nothing to do). The remaining changes refactored code into "SCSIHost" and "SCSIiSCSI" to process the differences.
Patch 7 just refactors domain_conf to add what will be a common routine to handle the "<host/>" element network disks. The iSCSI code will reuse the code to have it's own element.
Patch 8 adds the XML, schema, and updates the docs to describe the <hostdev/> entry as well as of course adding new tests. The tests are a copy/merge of the scsi_host and network iscsi tests to define and describe the expected model.
John Ferlan (8): hostdev: Introduce virDomainHostdevSubsysUSB hostdev: Introduce virDomainHostdevSubsysPCI hostdev: Introduce virDomainHostdevSubsysSCSI hostdev: Introduce virDomainHostdevSubsysSCSIHost Add virConnectPtr for qemuBuildSCSIHostdevDrvStr hostdev: Introduce virDomainHostdevSubsysSCSIiSCSI domain_conf: Common routine to handle network storage host xml def hostdev: Add iSCSI hostdev XML
docs/formatdomain.html.in | 142 +++++-- docs/schemas/domaincommon.rng | 46 +- src/conf/domain_audit.c | 38 +- src/conf/domain_conf.c | 463 +++++++++++++++------ src/conf/domain_conf.h | 80 +++- src/libxl/libxl_conf.c | 9 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 34 +- src/lxc/lxc_cgroup.c | 4 +- src/lxc/lxc_controller.c | 10 +- src/lxc/lxc_driver.c | 16 +- src/qemu/qemu_cgroup.c | 69 +-- src/qemu/qemu_command.c | 158 ++++--- src/qemu/qemu_command.h | 5 +- src/qemu/qemu_conf.c | 20 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c | 93 +++-- src/qemu/qemu_hotplug.h | 9 +- src/security/security_apparmor.c | 33 +- src/security/security_dac.c | 66 +-- src/security/security_selinux.c | 64 +-- src/security/virt-aa-helper.c | 5 +- src/util/virhostdev.c | 295 +++++++------ .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args | 14 + .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 46 ++ .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args | 14 + .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml | 40 ++ ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args | 16 + ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 46 ++ .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args | 16 + .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml | 40 ++ tests/qemuxml2argvtest.c | 16 + tests/qemuxml2xmltest.c | 5 + 33 files changed, 1317 insertions(+), 606 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml
ping? Should I repost - using git am against top of tree will most assuredly have conflicts... Tks, John
participants (2)
-
Eric Blake
-
John Ferlan