[libvirt] [PATCH v2 0/8] Add iSCSI hostdev pass-through device
Followup/rebase to partially ACK'd pile of changes - figure it's just as easy for me to keep things together and it's better to see things in totality... See: http://www.redhat.com/archives/libvir-list/2014-July/msg00592.html * Patches 1-5 were adjusted to use the first_/second_ naming scheme rather than a/b. * Typo for 'vedor' in patch 1 adjusted. * Changed name of iSCSIFree to iSCSIClear. 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 | 471 ++++++++++++++------- src/conf/domain_conf.h | 80 +++- src/libxl/libxl_conf.c | 9 +- src/libxl/libxl_domain.c | 6 +- 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, 1322 insertions(+), 610 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 | 54 +++++++++++++++++++--------------------- 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, 106 insertions(+), 124 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 1e27165..f23f271 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3807,6 +3807,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 = @@ -3823,7 +3824,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); } @@ -3840,8 +3841,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); @@ -3859,7 +3859,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); @@ -3877,8 +3877,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); @@ -3893,8 +3892,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); @@ -3917,7 +3915,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; @@ -10205,18 +10203,21 @@ virDomainHostdevRemove(virDomainDefPtr def, size_t i) static int -virDomainHostdevMatchSubsysUSB(virDomainHostdevDefPtr a, - virDomainHostdevDefPtr b) +virDomainHostdevMatchSubsysUSB(virDomainHostdevDefPtr first, + virDomainHostdevDefPtr second) { - if (a->source.subsys.u.usb.bus && a->source.subsys.u.usb.device) { + virDomainHostdevSubsysUSBPtr first_usbsrc = &first->source.subsys.u.usb; + virDomainHostdevSubsysUSBPtr second_usbsrc = &second->source.subsys.u.usb; + + if (first_usbsrc->bus && first_usbsrc->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 (first_usbsrc->bus == second_usbsrc->bus && + first_usbsrc->device == second_usbsrc->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 (first_usbsrc->product == second_usbsrc->product && + first_usbsrc->vendor == second_usbsrc->vendor) return 1; } return 0; @@ -15469,6 +15470,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); @@ -15488,8 +15491,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 && @@ -15502,18 +15504,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 cd3293b..47b739c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -396,20 +396,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 vendor/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 395ea05..0b60f5a 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -393,6 +393,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) { @@ -402,8 +403,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 fd93f47..2d220eb 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1347,22 +1347,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 b7b4b02..43a10bf 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -4264,6 +4264,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", @@ -4271,13 +4272,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) { @@ -4708,6 +4708,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, @@ -4717,9 +4718,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)) { @@ -4728,8 +4729,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 62a8f81..1b2c633 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 8062510..8b0d400 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5040,10 +5040,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; @@ -5052,8 +5051,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) @@ -5113,6 +5111,7 @@ char * qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) { char *ret = NULL; + virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb; if (dev->missing) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -5120,16 +5119,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; } @@ -10216,12 +10212,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, @@ -10260,11 +10258,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 e6563de..43f86ce 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -812,6 +812,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; @@ -833,10 +834,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 fe9ad59..f25a7df7 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1026,12 +1026,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/21/2014 02:47 PM, 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 | 54 +++++++++++++++++++--------------------- 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, 106 insertions(+), 124 deletions(-)
ACK -- 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 | 29 +++++++++++++++---------- src/conf/domain_conf.h | 12 ++++++---- src/libxl/libxl_conf.c | 9 ++++---- src/libxl/libxl_domain.c | 6 +++-- 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, 128 insertions(+), 137 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 f23f271..1c8b85a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4260,6 +4260,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 @@ -4339,7 +4340,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, "has been specified"), backendStr); goto error; } - def->source.subsys.u.pci.backend = backend; + pcisrc->backend = backend; break; @@ -10224,13 +10225,16 @@ virDomainHostdevMatchSubsysUSB(virDomainHostdevDefPtr first, } static int -virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr a, - virDomainHostdevDefPtr b) +virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr first, + virDomainHostdevDefPtr second) { - 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 first_pcisrc = &first->source.subsys.u.pci; + virDomainHostdevSubsysPCIPtr second_pcisrc = &second->source.subsys.u.pci; + + if (first_pcisrc->addr.domain == second_pcisrc->addr.domain && + first_pcisrc->addr.bus == second_pcisrc->addr.bus && + first_pcisrc->addr.slot == second_pcisrc->addr.slot && + first_pcisrc->addr.function == second_pcisrc->addr.function) return 1; return 0; } @@ -15471,15 +15475,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); @@ -15515,8 +15521,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 47b739c..62d0f36 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -408,16 +408,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 488ce2a..3afce69 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1274,15 +1274,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 6780e34..cdac82c 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -487,11 +487,13 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)) { virDomainHostdevDefPtr hostdev; + virDomainHostdevSubsysPCIPtr pcisrc; if (dev->type == VIR_DOMAIN_DEVICE_NET) hostdev = &(dev->data.net)->data.hostdev.def; else hostdev = dev->data.hostdev; + pcisrc = &hostdev->source.subsys.u.pci; /* forbid capabilities mode hostdev in this kind of hypervisor */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) { @@ -504,8 +506,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 5fbff1c..67fd7bc6 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2627,6 +2627,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; @@ -2634,10 +2635,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; } @@ -2655,10 +2654,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; } @@ -2854,6 +2851,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) virDomainNetDefPtr net; virDomainHostdevDefPtr hostdev; virDomainHostdevDefPtr found; + virDomainHostdevSubsysPCIPtr pcisrc; switch (dev->type) { case VIR_DOMAIN_DEVICE_DISK: @@ -2883,13 +2881,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; } @@ -2940,6 +2937,7 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver, virDomainHostdevDefPtr hostdev) { virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + virDomainHostdevSubsysPCIPtr pcisrc = &subsys->u.pci; libxl_device_pci pcidev; virDomainHostdevDefPtr detach; int idx; @@ -2952,16 +2950,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; } @@ -2975,8 +2973,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 1b2c633..8a7e640 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 8b0d400..5362b09 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -554,18 +554,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) @@ -4834,14 +4832,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); @@ -4861,7 +4858,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) { @@ -4883,20 +4881,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); @@ -4921,25 +4918,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 43f86ce..a45ca6f 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -813,6 +813,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; @@ -845,16 +846,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/21/2014 02:47 PM, 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> --- src/conf/domain_audit.c | 9 ++++---- src/conf/domain_conf.c | 29 +++++++++++++++---------- src/conf/domain_conf.h | 12 ++++++---- src/libxl/libxl_conf.c | 9 ++++---- src/libxl/libxl_domain.c | 6 +++-- 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, 128 insertions(+), 137 deletions(-)
ACK. -- 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 | 45 ++++++++++++++++++---------------- 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, 101 insertions(+), 116 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 1c8b85a..837f4a8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4032,6 +4032,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) { @@ -4053,19 +4054,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; @@ -4079,8 +4080,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; @@ -4261,6 +4261,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 @@ -4318,8 +4319,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; @@ -10240,13 +10240,17 @@ virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr first, } static int -virDomainHostdevMatchSubsysSCSI(virDomainHostdevDefPtr a, - virDomainHostdevDefPtr b) +virDomainHostdevMatchSubsysSCSI(virDomainHostdevDefPtr first, + virDomainHostdevDefPtr second) { - 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 first_scsisrc = &first->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIPtr second_scsisrc = + &second->source.subsys.u.scsi; + + if (STREQ(first_scsisrc->adapter, second_scsisrc->adapter) && + first_scsisrc->bus == second_scsisrc->bus && + first_scsisrc->target == second_scsisrc->target && + first_scsisrc->unit == second_scsisrc->unit) return 1; return 0; } @@ -15476,6 +15480,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) { @@ -15544,12 +15549,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, @@ -16967,6 +16970,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) { @@ -17006,11 +17010,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 62d0f36..ecb92b2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -415,6 +415,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 { @@ -422,13 +432,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 8a7e640..9155b9a 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 5362b09..c640e59 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5130,13 +5130,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 a45ca6f..7796121 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -814,6 +814,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; @@ -871,12 +872,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/21/2014 02:47 PM, 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 | 45 ++++++++++++++++++---------------- 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, 101 insertions(+), 116 deletions(-)
ACK -- 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 | 59 ++++++++++++++++++++++++---------------- 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(+), 76 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 837f4a8..53bd1d5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1740,7 +1740,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; } } @@ -4025,16 +4025,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")) { @@ -4054,19 +4054,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; @@ -4080,7 +4081,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; @@ -4112,6 +4113,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. */ @@ -4350,7 +4358,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; @@ -10240,17 +10248,18 @@ virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr first, } static int -virDomainHostdevMatchSubsysSCSI(virDomainHostdevDefPtr first, - virDomainHostdevDefPtr second) -{ - virDomainHostdevSubsysSCSIPtr first_scsisrc = &first->source.subsys.u.scsi; - virDomainHostdevSubsysSCSIPtr second_scsisrc = - &second->source.subsys.u.scsi; - - if (STREQ(first_scsisrc->adapter, second_scsisrc->adapter) && - first_scsisrc->bus == second_scsisrc->bus && - first_scsisrc->target == second_scsisrc->target && - first_scsisrc->unit == second_scsisrc->unit) +virDomainHostdevMatchSubsysSCSIHost(virDomainHostdevDefPtr first, + virDomainHostdevDefPtr second) +{ + virDomainHostdevSubsysSCSIHostPtr first_scsihostsrc = + &first->source.subsys.u.scsi.u.host; + virDomainHostdevSubsysSCSIHostPtr second_scsihostsrc = + &second->source.subsys.u.scsi.u.host; + + if (STREQ(first_scsihostsrc->adapter, second_scsihostsrc->adapter) && + first_scsihostsrc->bus == second_scsihostsrc->bus && + first_scsihostsrc->target == second_scsihostsrc->target && + first_scsihostsrc->unit == second_scsihostsrc->unit) return 1; return 0; } @@ -10268,7 +10277,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; } @@ -15481,6 +15490,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) { @@ -15549,10 +15559,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 ecb92b2..56f6735 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -415,14 +415,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 9155b9a..3d69b09 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 c640e59..fa9c72f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5131,11 +5131,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 7796121..a3f1025 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -870,10 +870,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/21/2014 02:47 PM, 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> --- src/conf/domain_audit.c | 8 ++++-- src/conf/domain_conf.c | 59 ++++++++++++++++++++++++---------------- 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(+), 76 deletions(-)
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 fa9c72f..372ad22 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5125,7 +5125,8 @@ qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) } char * -qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev, +qemuBuildSCSIHostdevDrvStr(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, qemuBuildCommandLineCallbacksPtr callbacks) { @@ -8999,7 +9000,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 1782913..77af34d 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/21/2014 02:47 PM, 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(-)
+++ 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,
While touching these two declarations, you could get rid of the space after * to be consistent to the style. ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 07/23/2014 09:31 PM, Eric Blake wrote:
On 07/21/2014 02:47 PM, 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(-)
+++ 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,
While touching these two declarations, you could get rid of the space after * to be consistent to the style.
ACK
The rest of the module used "char * " (19 definitions) - doing so creates inconsistency in the module. I'll send a follow-up patch to reformat all calls and adjust spacing accordingly in the module (there's a few others in src/qemu/*.h as well). 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 53bd1d5..a80530f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1709,6 +1709,17 @@ virDomainHostdevDefPtr virDomainHostdevDefAlloc(void) return def; } +static void +virDomainHostdevSubsysSCSIiSCSIClear(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) @@ -1739,8 +1750,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) { + virDomainHostdevSubsysSCSIiSCSIClear(&scsisrc->u.iscsi); + } else { + VIR_FREE(scsisrc->u.host.adapter); + } + } break; } } @@ -4320,8 +4338,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; @@ -10265,6 +10282,22 @@ virDomainHostdevMatchSubsysSCSIHost(virDomainHostdevDefPtr first, } static int +virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first, + virDomainHostdevDefPtr second) +{ + virDomainHostdevSubsysSCSIiSCSIPtr first_iscsisrc = + &first->source.subsys.u.scsi.u.iscsi; + virDomainHostdevSubsysSCSIiSCSIPtr second_iscsisrc = + &second->source.subsys.u.scsi.u.iscsi; + + if (STREQ(first_iscsisrc->hosts[0].name, second_iscsisrc->hosts[0].name) && + STREQ(first_iscsisrc->hosts[0].port, second_iscsisrc->hosts[0].port) && + STREQ(first_iscsisrc->path, second_iscsisrc->path)) + return 1; + return 0; +} + +static int virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a, virDomainHostdevDefPtr b) { @@ -10277,7 +10310,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 56f6735..569b2a0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -396,6 +396,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 { @@ -424,12 +433,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 3d69b09..795ad90 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 372ad22..1cf7ba2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5124,13 +5124,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; @@ -5140,10 +5138,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); @@ -5162,10 +5214,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 a3f1025..041ce65 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -825,6 +825,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/21/2014 02:47 PM, 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(-)
static int +virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first, + virDomainHostdevDefPtr second) +{ + virDomainHostdevSubsysSCSIiSCSIPtr first_iscsisrc = + &first->source.subsys.u.scsi.u.iscsi; + virDomainHostdevSubsysSCSIiSCSIPtr second_iscsisrc = + &second->source.subsys.u.scsi.u.iscsi; + + if (STREQ(first_iscsisrc->hosts[0].name, second_iscsisrc->hosts[0].name) &&
Do you need to match nhosts, or is nhosts always 1?
+ + /* Only delete the devices which are marked as being used by @name, + * because qemuProcessStart could fail on the half way. */
Sounds funny; maybe "could fail half way through"
- - /* Only delete the devices which are marked as being used by @name, - * because qemuProcessStart could fail on the half way. */
Then again, it's just code motion. Conditional ACK, if the match function doesn't need to track nhosts. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 07/23/2014 10:08 PM, Eric Blake wrote:
On 07/21/2014 02:47 PM, 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(-)
static int +virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first, + virDomainHostdevDefPtr second) +{ + virDomainHostdevSubsysSCSIiSCSIPtr first_iscsisrc = + &first->source.subsys.u.scsi.u.iscsi; + virDomainHostdevSubsysSCSIiSCSIPtr second_iscsisrc = + &second->source.subsys.u.scsi.u.iscsi; + + if (STREQ(first_iscsisrc->hosts[0].name, second_iscsisrc->hosts[0].name) &&
Do you need to match nhosts, or is nhosts always 1?
Only 1 host is supported (same as for a disk XML iSCSI resource). This will be seen in patch 8 in virDomainHostdevSubsysSCSIiSCSIDefParseXML() John Going to push the first 6 patches shortly - thanks for the reviews so far.
+ + /* Only delete the devices which are marked as being used by @name, + * because qemuProcessStart could fail on the half way. */
Sounds funny; maybe "could fail half way through"
- - /* Only delete the devices which are marked as being used by @name, - * because qemuProcessStart could fail on the half way. */
Then again, it's just code motion.
Conditional ACK, if the match function doesn't need to track nhosts.
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 a80530f..0665c27 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4043,6 +4043,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) { @@ -5033,13 +5106,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: @@ -5092,59 +5160,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) @@ -5167,9 +5184,7 @@ virDomainDiskSourceParse(xmlNodePtr node, ret = 0; cleanup: - virStorageNetHostDefClear(&host); VIR_FREE(protocol); - VIR_FREE(transport); return ret; } -- 1.9.3
On 07/21/2014 02:47 PM, John Ferlan wrote:
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(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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 8950959..918ba3c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2802,57 +2802,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 835bd3c..c59e3a9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3563,13 +3563,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 0665c27..8fcb58c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -605,6 +605,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", @@ -4205,10 +4210,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 @@ -4353,6 +4441,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, virDomainHostdevDefPtr def, unsigned int flags) { + xmlNodePtr authnode; xmlNodePtr sourcenode; char *managed = NULL; char *sgio = NULL; @@ -4448,7 +4537,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; @@ -15543,6 +15634,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) { @@ -15558,18 +15650,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"); @@ -15610,12 +15720,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 28436f2..e72c131 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1341,6 +1341,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 cefe05b..81c5d32 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -357,6 +357,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/21/2014 02:47 PM, John Ferlan wrote:
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
+ <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
The (preferred) virsh command is nodedev-detach (we can't correct the API typo, but did correct the virsh typo).
+ <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>
1.2.8 now.
+++ b/docs/schemas/domaincommon.rng @@ -3563,13 +3563,47 @@ </choice> </attribute> </optional> + <optional> + <ref name='diskAuth'/> + </optional>
Does diskAuth work for all configurations, or only for the new iscsi configuration? By putting it here, you are allowing it for all users,...
<element name="source"> - <interleave> - <ref name="sourceinfoadapter"/> - <element name="address"> - <ref name="scsiaddress"/> - </element> - </interleave> + <optional> + <attribute name="protocol"> + <choice> + <value>iscsi</value> + </choice>
...whereas using some <group> magic could make it valid only when protocol is iscsi.
+ </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>
This <interleave> adds nothing. It doesn't hurt to leave it in, but when there is only one possible (repetition of a) child "host" <element>, there is nothing to be interleaved.
@@ -4205,10 +4210,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 */
Is there enough commonality to be worth a common shared function? I'm okay if there isn't, but want to make sure we aren't doing unneeded duplication.
+ } + if (iscsisrc->nhosts > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one source host address may be specified " + "for the iSCSI hostdev")); + goto cleanup;
This doesn't match the .rng file which said <oneOrMore>.
+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'"),
s/protcol/protocol/ -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/04/2014 02:21 PM, Eric Blake wrote:
On 07/21/2014 02:47 PM, John Ferlan wrote:
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
+ <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
The (preferred) virsh command is nodedev-detach (we can't correct the API typo, but did correct the virsh typo).
Fixed (plus another such typo)
+ <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>
1.2.8 now.
Updated...
+++ b/docs/schemas/domaincommon.rng @@ -3563,13 +3563,47 @@ </choice> </attribute> </optional> + <optional> + <ref name='diskAuth'/> + </optional>
Does diskAuth work for all configurations, or only for the new iscsi configuration? By putting it here, you are allowing it for all users,...
<element name="source"> - <interleave> - <ref name="sourceinfoadapter"/> - <element name="address"> - <ref name="scsiaddress"/> - </element> - </interleave> + <optional> + <attribute name="protocol"> + <choice> + <value>iscsi</value> + </choice>
...whereas using some <group> magic could make it valid only when protocol is iscsi.
So clearly this RNG syntax is BFM and confusing... The "goal" is/was to copy the same element logic from the disk element, but unlike disk it will only be valid for iscsi. It should have the following format: <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> ... where source would be reqd, auth would be option It's not very clear to me where/how to place it in the rng file...
+ </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>
This <interleave> adds nothing. It doesn't hurt to leave it in, but when there is only one possible (repetition of a) child "host" <element>, there is nothing to be interleaved.
Like <group>, the <interleave> didn't quite make sense in how/when it should be used... obviously :-) I was trying to follow 'diskSource' and 'diskSourceNetwork' in order to allow the definition of the <host name="string" port="string/number">. In any case, I went back and forth on this. On one hand, it's been allowed by the disk syntax, while on the other hand it doesn't make sense to allow more than one, but yet - I'm trying to mimic the disk syntax. I think
@@ -4205,10 +4210,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 */
Is there enough commonality to be worth a common shared function? I'm okay if there isn't, but want to make sure we aren't doing unneeded duplication.
I think I've already cut down as much as I can...
+ } + if (iscsisrc->nhosts > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one source host address may be specified " + "for the iSCSI hostdev")); + goto cleanup;
This doesn't match the .rng file which said <oneOrMore>.
Right - so should I follow the logic of <disk> and allow the check in the qemu code (qemuBuildNetworkDriveURI()) be the final arbiter? Yet another struggle - where to cause failure... I think the < 0 would have to stay since <host> would be required.
+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'"),
s/protcol/protocol/
Fixed. John
On 08/04/2014 02:21 PM, Eric Blake wrote: <snip>
+++ b/docs/schemas/domaincommon.rng @@ -3563,13 +3563,47 @@ </choice> </attribute> </optional> + <optional> + <ref name='diskAuth'/> + </optional>
Does diskAuth work for all configurations, or only for the new iscsi configuration? By putting it here, you are allowing it for all users,...
<element name="source"> - <interleave> - <ref name="sourceinfoadapter"/> - <element name="address"> - <ref name="scsiaddress"/> - </element> - </interleave> + <optional> + <attribute name="protocol"> + <choice> + <value>iscsi</value> + </choice>
...whereas using some <group> magic could make it valid only when protocol is iscsi.
+ </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>
This <interleave> adds nothing. It doesn't hurt to leave it in, but when there is only one possible (repetition of a) child "host" <element>, there is nothing to be interleaved.
Maybe perhaps sleep helped me figure this out... Is the following what you had in mind? (and it passes make check too) <define name="hostdevsubsysscsi"> <attribute name="type"> <value>scsi</value> </attribute> <optional> <attribute name="sgio"> <choice> <value>filtered</value> <value>unfiltered</value> </choice> </attribute> </optional> <choice> <group> <element name="source"> <interleave> <ref name="sourceinfoadapter"/> <element name="address"> <ref name="scsiaddress"/> </element> </interleave> </element> </group> <group> <optional> <ref name='diskAuth'/> </optional> <element name="source"> <attribute name="protocol"> <choice> <value>iscsi</value> </choice> </attribute> <attribute name="name"> <text/> </attribute> <interleave> <oneOrMore> <element name='host'> <attribute name='name'> <text/> </attribute> <optional> <attribute name='port'> <ref name="PortNumber"/> </attribute> </optional> <empty/> </element> </oneOrMore> </interleave> </element> </group> </choice> </define> I suppose I could/should follow the disk and put auth after the source... Would you like to see a v3? also including the extraneous spacing change from the initial review of 5/8 and I had created a separate patch on (which wasn't technically acked, so I didn't push): http://www.redhat.com/archives/libvir-list/2014-July/msg01268.html John
On 08/05/2014 06:00 AM, John Ferlan wrote:
Maybe perhaps sleep helped me figure this out... Is the following what you had in mind? (and it passes make check too)
<define name="hostdevsubsysscsi"> <attribute name="type"> <value>scsi</value> </attribute> <optional> <attribute name="sgio"> <choice> <value>filtered</value> <value>unfiltered</value> </choice> </attribute> </optional> <choice> <group> <element name="source"> <interleave> <ref name="sourceinfoadapter"/> <element name="address"> <ref name="scsiaddress"/> </element> </interleave> </element> </group>
Looks better - this says that the caller that uses hostdevsubsysscsi either has sourceinfoadapter (with no auth, and no protocol)...
<group> <optional> <ref name='diskAuth'/> </optional> <element name="source"> <attribute name="protocol"> <choice> <value>iscsi</value> </choice> </attribute> <attribute name="name"> <text/> </attribute> <interleave> <oneOrMore> <element name='host'> <attribute name='name'> <text/> </attribute> <optional> <attribute name='port'> <ref name="PortNumber"/> </attribute> </optional> <empty/> </element> </oneOrMore> </interleave> </element> </group>
or <source protocol='iscsi'> with optional diskAuth. It still looks a bit odd, though. Normally, when we have two different <source> children, the parent element contains an attribute that says which child syntax we will be looking for. Something like: <define name="hostdevsubsysscsi"> <attribute name="type"> <value>scsi</value> </attribute> <choice> <group> <!-- default to adapter --> <optional> <attribute name='???'> <value>adapter</value> </attribute> </optional> <element name='source'> <ref name="sourceinfoadapter"/> </element> </group> <group> <!-- new code --> <attribute name='???'> <value>iscsi</value> </attribute> <element name="source"> <attribute name="protocol"> <value>iscsi</value> </attribute> <attribute name="name"> <text/> </attribute> ... but in writing that, I don't know what to call the new attribute (the name='???' above), since we already have type='scsi'. Does that mean we need TWO types, type='scsi' for the old code, and type='iscsi' for the new additions? I'm typing this email without looking at the full context of the patch; maybe seeing it inline with the added test cases will help make it obvious how we know which style of <source> is in use.
I suppose I could/should follow the disk and put auth after the source...
Yeah. The <interleave> will let it come first when the user writes it that way, but for readability, output auth after source makes more sense.
Would you like to see a v3? also including the extraneous spacing change from the initial review of 5/8 and I had created a separate patch on (which wasn't technically acked, so I didn't push):
http://www.redhat.com/archives/libvir-list/2014-July/msg01268.html
Yes, I think that's probably wise. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/05/2014 11:20 AM, Eric Blake wrote:
On 08/05/2014 06:00 AM, John Ferlan wrote:
Maybe perhaps sleep helped me figure this out... Is the following what you had in mind? (and it passes make check too)
<define name="hostdevsubsysscsi"> <attribute name="type"> <value>scsi</value> </attribute> <optional> <attribute name="sgio"> <choice> <value>filtered</value> <value>unfiltered</value> </choice> </attribute> </optional> <choice> <group> <element name="source"> <interleave> <ref name="sourceinfoadapter"/> <element name="address"> <ref name="scsiaddress"/> </element> </interleave> </element> </group>
Looks better - this says that the caller that uses hostdevsubsysscsi either has sourceinfoadapter (with no auth, and no protocol)...
<group> <optional> <ref name='diskAuth'/> </optional> <element name="source"> <attribute name="protocol"> <choice> <value>iscsi</value> </choice> </attribute> <attribute name="name"> <text/> </attribute> <interleave> <oneOrMore> <element name='host'> <attribute name='name'> <text/> </attribute> <optional> <attribute name='port'> <ref name="PortNumber"/> </attribute> </optional> <empty/> </element> </oneOrMore> </interleave> </element> </group>
or <source protocol='iscsi'> with optional diskAuth.
It still looks a bit odd, though. Normally, when we have two different <source> children, the parent element contains an attribute that says which child syntax we will be looking for. Something like:
<define name="hostdevsubsysscsi"> <attribute name="type"> <value>scsi</value> </attribute> <choice> <group> <!-- default to adapter --> <optional> <attribute name='???'> <value>adapter</value> </attribute> </optional> <element name='source'> <ref name="sourceinfoadapter"/> </element> </group> <group> <!-- new code --> <attribute name='???'> <value>iscsi</value> </attribute> <element name="source"> <attribute name="protocol"> <value>iscsi</value> </attribute> <attribute name="name"> <text/> </attribute> ...
but in writing that, I don't know what to call the new attribute (the name='???' above), since we already have type='scsi'. Does that mean we need TWO types, type='scsi' for the old code, and type='iscsi' for the new additions?
Both are 'scsi' adapters - existing is 'scsi_host' and the new one is 'iscsi'. I'm unsure about putting the 'optional' attribute name really does. Not to confuse things further, but the next step in the evolution of this is to add a fiber channel adapter (eg, fc_host). The "first" format is for the "scsi_host" with the valid XML being: <hostdev mode='subsystem' type='scsi'> <source> <adapter name='scsi_host0'/> <address type='scsi' bus='0' target='0' unit='0'/> </source> <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </hostdev> The "second" format is for the "iscsi" valid XML for 'iscsi' is: <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> I'll send a v3 shortly John
I'm typing this email without looking at the full context of the patch; maybe seeing it inline with the added test cases will help make it obvious how we know which style of <source> is in use.
I suppose I could/should follow the disk and put auth after the source...
Yeah. The <interleave> will let it come first when the user writes it that way, but for readability, output auth after source makes more sense.
Would you like to see a v3? also including the extraneous spacing change from the initial review of 5/8 and I had created a separate patch on (which wasn't technically acked, so I didn't push):
http://www.redhat.com/archives/libvir-list/2014-July/msg01268.html
Yes, I think that's probably wise.
On 08/05/2014 01:59 PM, John Ferlan wrote:
Both are 'scsi' adapters - existing is 'scsi_host' and the new one is 'iscsi'. I'm unsure about putting the 'optional' attribute name really does. Not to confuse things further, but the next step in the evolution of this is to add a fiber channel adapter (eg, fc_host).
The "first" format is for the "scsi_host" with the valid XML being:
Seeing it in context helps.
<hostdev mode='subsystem' type='scsi'> <source> <adapter name='scsi_host0'/>
Here, I think I see the solution to my earlier confusion. If _this_ were written: <source protocol='adapter'> <adapter... where the protocol='adapter' is optional (default, for back-compat), then you know that <source>'s children will be <adapter> and <address>...
<address type='scsi' bus='0' target='0' unit='0'/> </source> <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </hostdev>
The "second" format is for the "iscsi" valid XML for 'iscsi' is:
<hostdev mode='subsystem' type='scsi'> <source protocol='iscsi' name='iqn.2014-08.com.example:iscsi-nopool/1'>
...and here, protocol='iscsi' is mandatory, to describe that that <source>'s children will be <host>.
<host name='example.com' port='3260'/> </source> <auth username='myuser'> <secret type='iscsi' usage='libvirtiscsi'/> </auth>
Is the <auth> something inherent to the overall hostdev, or something only to the <source protocol='iscsi'>? I think I'd rather see this <auth> be a child of <source>, rather than a sibling. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/05/2014 04:24 PM, Eric Blake wrote:
On 08/05/2014 01:59 PM, John Ferlan wrote:
Both are 'scsi' adapters - existing is 'scsi_host' and the new one is 'iscsi'. I'm unsure about putting the 'optional' attribute name really does. Not to confuse things further, but the next step in the evolution of this is to add a fiber channel adapter (eg, fc_host).
The "first" format is for the "scsi_host" with the valid XML being:
Seeing it in context helps.
<hostdev mode='subsystem' type='scsi'> <source> <adapter name='scsi_host0'/>
Here, I think I see the solution to my earlier confusion. If _this_ were written:
<source protocol='adapter'> <adapter...
where the protocol='adapter' is optional (default, for back-compat), then you know that <source>'s children will be <adapter> and <address>...
<address type='scsi' bus='0' target='0' unit='0'/> </source> <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </hostdev>
The "second" format is for the "iscsi" valid XML for 'iscsi' is:
<hostdev mode='subsystem' type='scsi'> <source protocol='iscsi' name='iqn.2014-08.com.example:iscsi-nopool/1'>
...and here, protocol='iscsi' is mandatory, to describe that that <source>'s children will be <host>.
<host name='example.com' port='3260'/> </source> <auth username='myuser'> <secret type='iscsi' usage='libvirtiscsi'/> </auth>
Is the <auth> something inherent to the overall hostdev, or something only to the <source protocol='iscsi'>? I think I'd rather see this <auth> be a child of <source>, rather than a sibling.
OK - I'll 'repost' v3 2/3... This could also make adding adapter=fc_host a bit simpler. Moving <auth> under <source> is more like storage pool syntax which is fine - I guess I was trying to as much like the <disk> as I could be. John
participants (2)
-
Eric Blake -
John Ferlan