Re: [libvirt] [PATCH 18/25] qemu: Refactor the helpers to track shared scsi host device

This changes the helpers qemu{Add,Remove}SharedDisk into qemu{Add,Remove}SharedDevice, as most of the code in the helpers can be reused for scsi host device. To track the shared scsi host device, first it finds out the device path (e.g. /dev/s[dr]*) which is mapped to the sg device, and use device ID of the found device path (/dev/s[dr]*) as the hash key. This is because of the device ID is not unique between between /dev/s[dr]* and /dev/sg*, e.g. % sg_map /dev/sg0 /dev/sda /dev/sg1 /dev/sr0 % ls -l /dev/sda brw-rw----. 1 root disk 8, 0 May 2 19:26 /dev/sda %ls -l /dev/sg0 crw-rw----. 1 root disk 21, 0 May 2 19:26 /dev/sg0 --- src/qemu/qemu_conf.c | 143 ++++++++++++++++++++++++++++++++++++------------ src/qemu/qemu_conf.h | 20 +++---- src/qemu/qemu_driver.c | 16 +++--- src/qemu/qemu_process.c | 19 +++++-- 4 files changed, 140 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 244795d..ebcd176 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1084,47 +1084,80 @@ cleanup: return NULL; } -/* qemuAddSharedDisk: +/* qemuAddSharedDevice: * @driver: Pointer to qemu driver struct - * @disk: The disk def + * @dev: The device def * @name: The domain name * * Increase ref count and add the domain name into the list which - * records all the domains that use the shared disk if the entry + * records all the domains that use the shared device if the entry * already exists, otherwise add a new entry. */ int -qemuAddSharedDisk(virQEMUDriverPtr driver, - virDomainDiskDefPtr disk, - const char *name) +qemuAddSharedDevice(virQEMUDriverPtr driver, + virDomainDeviceDefPtr dev, + const char *name) { qemuSharedDeviceEntry *entry = NULL; qemuSharedDeviceEntry *new_entry = NULL; + virDomainDiskDefPtr disk = NULL; + virDomainHostdevDefPtr hostdev = NULL; + char *dev_name = NULL; + char *dev_path = NULL; char *key = NULL; int ret = -1; - /* Currently the only conflicts we have to care about - * for the shared disk is "sgio" setting, which is only - * valid for block disk. + /* Currently the only conflicts we have to care about for + * the shared disk and shared host device is "sgio" setting, + * which is only valid for block disk and scsi host device. */ - if (!disk->shared || - !disk->src || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + disk = dev->data.disk; + + if (disk->shared || + !disk->src || + (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && + !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && + disk->srcpool && + disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + return 0; + } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { + hostdev = dev->data.hostdev; + + if (!hostdev->shareable || + (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI))

+ return 0; + } else { return 0; + } qemuDriverLock(driver); - if (qemuCheckSharedDisk(driver->sharedDevices, disk) < 0) - goto cleanup; + if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (qemuCheckSharedDisk(driver->sharedDevices, disk) < 0) + goto cleanup; - if (!(key = qemuGetSharedDeviceKey(disk->src))) - goto cleanup; + if (!(key = qemuGetSharedDeviceKey(disk->src))) + goto cleanup; + } else { + if (!(dev_name = virSCSIDeviceGetDevName(hostdev->source.subsys.u.scsi.adapter, + hostdev->source.subsys.u.scsi.bus, + hostdev->source.subsys.u.scsi.target, + hostdev->source.subsys.u.scsi.unit))) + goto cleanup; + + if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!(key = qemuGetSharedDeviceKey(dev_path))) + goto cleanup; + } if ((entry = virHashLookup(driver->sharedDevices, key))) { - /* Nothing to do if the shared disk is already recorded - * in the table. + /* Nothing to do if the shared scsi host device is already + * recorded in the table. */ if (qemuSharedDeviceEntryDomainExists(entry, name, NULL)) { ret = 0; @@ -1163,41 +1196,77 @@ qemuAddSharedDisk(virQEMUDriverPtr driver, ret = 0; cleanup: qemuDriverUnlock(driver); + VIR_FREE(dev_name); + VIR_FREE(dev_path); VIR_FREE(key); return ret; } -/* qemuRemoveSharedDisk: +/* qemuRemoveSharedDevice: * @driver: Pointer to qemu driver struct - * @disk: The disk def + * @device: The device def * @name: The domain name * * Decrease ref count and remove the domain name from the list which - * records all the domains that use the shared disk if ref is not 1, - * otherwise remove the entry. + * records all the domains that use the shared device if ref is not + * 1, otherwise remove the entry. */ int -qemuRemoveSharedDisk(virQEMUDriverPtr driver, - virDomainDiskDefPtr disk, - const char *name) +qemuRemoveSharedDevice(virQEMUDriverPtr driver, + virDomainDeviceDefPtr dev, + const char *name) { qemuSharedDeviceEntryPtr entry = NULL; qemuSharedDeviceEntryPtr new_entry = NULL; + virDomainDiskDefPtr disk = NULL; + virDomainHostdevDefPtr hostdev = NULL; char *key = NULL; + char *dev_name = NULL; + char *dev_path = NULL; int ret = -1; int idx; - if (!disk->shared || - !disk->src || - (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && - !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && - disk->srcpool && - disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + disk = dev->data.disk; + + if (!disk->shared || + !disk->src || + (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK && + !(disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME && + disk->srcpool && + disk->srcpool->voltype == VIR_STORAGE_VOL_BLOCK))) + return 0; + } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { + hostdev = dev->data.hostdev; + + if (!hostdev->shareable || + (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + return 0;

+ } else { return 0; + } qemuDriverLock(driver); - if (!(key = qemuGetSharedDeviceKey(disk->src))) - goto cleanup; + + if (dev->type == VIR_DOMAIN_DEVICE_DISK) { + if (!(key = qemuGetSharedDeviceKey(disk->src))) + goto cleanup; + } else { + if (!(dev_name = virSCSIDeviceGetDevName(hostdev->source.subsys.u.scsi.adapter, + hostdev->source.subsys.u.scsi.bus, + hostdev->source.subsys.u.scsi.target, + hostdev->source.subsys.u.scsi.unit))) + goto cleanup; + + if (virAsprintf(&dev_path, "/dev/%s", dev_name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (!(key = qemuGetSharedDeviceKey(dev_path))) + goto cleanup; + } if (!(entry = virHashLookup(driver->sharedDevices, key))) goto cleanup; @@ -1233,6 +1302,8 @@ qemuRemoveSharedDisk(virQEMUDriverPtr driver, ret = 0; cleanup: qemuDriverUnlock(driver); + VIR_FREE(dev_name); + VIR_FREE(dev_path); VIR_FREE(key); return ret; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d4a54a0..c004f7f 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -283,22 +283,22 @@ bool qemuSharedDeviceEntryDomainExists(qemuSharedDeviceEntryPtr entry, int *index) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int qemuAddSharedDisk(virQEMUDriverPtr driver, - virDomainDiskDefPtr disk, - const char *name) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); - -int qemuRemoveSharedDisk(virQEMUDriverPtr driver, - virDomainDiskDefPtr disk, - const char *name) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); - char * qemuGetSharedDeviceKey(const char *disk_path) ATTRIBUTE_NONNULL(1); void qemuSharedDeviceEntryFree(void *payload, const void *name) ATTRIBUTE_NONNULL(1); +int qemuAddSharedDevice(virQEMUDriverPtr driver, + virDomainDeviceDefPtr dev, + const char *name) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + +int qemuRemoveSharedDevice(virQEMUDriverPtr driver, + virDomainDeviceDefPtr dev, + const char *name) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); + int qemuDriverAllocateID(virQEMUDriverPtr driver); virDomainXMLOptionPtr virQEMUDriverCreateXMLConf(virQEMUDriverPtr driver); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4c2ab7b..991b68b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5648,7 +5648,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (qemuTranslateDiskSourcePool(conn, disk) < 0) goto end; - if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0) + if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) goto end; if (qemuSetUnprivSGIO(disk) < 0) @@ -5694,8 +5694,8 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, * if the operation is either ejecting or updating. */ if (ret == 0) - ignore_value(qemuRemoveSharedDisk(driver, dev_copy->data.disk, - vm->def->name)); + ignore_value(qemuRemoveSharedDevice(driver, dev_copy, + vm->def->name)); break; case VIR_DOMAIN_DISK_DEVICE_DISK: case VIR_DOMAIN_DISK_DEVICE_LUN: @@ -5732,7 +5732,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, end: if (ret != 0) - ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); + ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name)); virObjectUnref(caps); virDomainDeviceDefFree(dev_copy); return ret; @@ -5848,7 +5848,7 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver, } if (ret == 0) - ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); + ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name)); return ret; } @@ -5955,7 +5955,7 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, dev->data.disk = tmp; /* Add the new disk src into shared disk hash table */ - if (qemuAddSharedDisk(driver, dev->data.disk, vm->def->name) < 0) + if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) goto end; ret = qemuDomainChangeEjectableMedia(driver, vm, disk, orig_disk, force); @@ -5968,8 +5968,8 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn, */ if (ret == 0) { dev->data.disk = NULL; - ignore_value(qemuRemoveSharedDisk(driver, dev_copy->data.disk, - vm->def->name)); + ignore_value(qemuRemoveSharedDevice(driver, dev_copy, + vm->def->name)); } break; default: diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 70eadb6..117c669 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2982,10 +2982,14 @@ qemuProcessReconnect(void *opaque) * qemu_driver->sharedDevices. */ for (i = 0; i < obj->def->ndisks; i++) { + virDomainDeviceDef dev; + if (qemuTranslateDiskSourcePool(conn, obj->def->disks[i]) < 0) goto error; - if (qemuAddSharedDisk(driver, obj->def->disks[i], - obj->def->name) < 0) + + dev.type = VIR_DOMAIN_DEVICE_DISK; + dev.data.disk = obj->def->disks[i]; + if (qemuAddSharedDevice(driver, &dev, obj->def->name) < 0) goto error; } @@ -3663,6 +3667,7 @@ int qemuProcessStart(virConnectPtr conn, /* in case a certain disk is desirous of CAP_SYS_RAWIO, add this */ for (i = 0; i < vm->def->ndisks; i++) { + virDomainDeviceDef dev; virDomainDiskDefPtr disk = vm->def->disks[i]; if (vm->def->disks[i]->rawio == 1) @@ -3673,7 +3678,9 @@ int qemuProcessStart(virConnectPtr conn, _("Raw I/O is not supported on this platform")); #endif - if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0) + dev.type = VIR_DOMAIN_DEVICE_DISK; + dev.data.disk = disk; + if (qemuAddSharedDevice(driver, &dev, vm->def->name) < 0) goto cleanup; if (qemuSetUnprivSGIO(disk) < 0) @@ -4084,8 +4091,12 @@ void qemuProcessStop(virQEMUDriverPtr driver, virSecurityManagerReleaseLabel(driver->securityManager, vm->def); for (i = 0; i < vm->def->ndisks; i++) { + virDomainDeviceDef dev; virDomainDiskDefPtr disk = vm->def->disks[i]; - ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name)); + + dev.type = VIR_DOMAIN_DEVICE_DISK; + dev.data.disk = disk; + ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name)); } /* Clear out dynamically assigned labels */