[libvirt] [PATCH 18/25] qemu: Refactor the helpers to track shared scsi host device
John Ferlan
jferlan at redhat.com
Tue May 7 18:57:28 UTC 2013
On 05/03/2013 02:07 PM, Osier Yang wrote:
> 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))
And this is the controller that's being OR'd so I think this is right,
but making sure :-)
Over time HPVM was asked to allow a whole controller to be shareable
because the guest would control/manage the luns...
> + 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;
Same in reverse...
ACK on the remainder
John
> + } 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 */
>
More information about the libvir-list
mailing list