[libvirt] [PATCH v2 00/11] qemu: Fix <shareable/> disks for new qemu
Introduction of the disk image locking in qemu created a regression where disk shared access with <shareable/> would not work. Since the disk locking is a desired feature, allow libvirt to configure qemu in such way that <shareable/> disks are exempt from write locks. First few patches refactor some stuff so that it's less ugly. This series also mandates that sharing is used only with 'raw' disks since other formats could corrupt metadata. v2: - better capability detection (all the drive types) - fixed bug when share-rw would be used only with virtio disks - improved error message with automatic format probing - rebased to current master (new capability) - added BZ links into few cover letters Peter Krempa (11): qemu: Move snapshot disk validation functions into one qemu: domain: Despaghetify qemuDomainDeviceDefValidate qemu: domain: Move hostdev validation into separate function qemu: domain: Move video device validation into separate function qemu: domain: Refactor domain device validation function qemu: block: Add function to check if storage source allows concurrent access qemu: domain: Reject shared disk access if backing format does not support it qemu: snapshot: Disallow snapshot of unsupported shared disks qemu: Disallow pivot of shared disks to unsupported storage qemu: caps: Add capability for 'share-rw' disk option qemu: command: Mark <shared/> disks as such in qemu src/qemu/qemu_block.c | 15 ++ src/qemu/qemu_block.h | 3 + src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 4 + src/qemu/qemu_domain.c | 299 +++++++++++++-------- src/qemu/qemu_driver.c | 121 +++++---- .../caps_2.10.0-gicv2.aarch64.xml | 1 + .../caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + .../qemuxml2argv-disk-drive-shared-locking.args | 32 +++ .../qemuxml2argv-disk-drive-shared-locking.xml | 42 +++ .../qemuxml2argv-disk-drive-shared-qcow.xml | 28 ++ .../qemuxml2argv-disk-drive-shared.args | 2 +- .../qemuxml2argv-disk-drive-shared.xml | 2 +- tests/qemuxml2argvtest.c | 3 + 21 files changed, 401 insertions(+), 164 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml -- 2.14.3
Move the code so that both the new image and old image can be verified in the same function. --- src/qemu/qemu_driver.c | 91 ++++++++++++++++++++------------------------------ 1 file changed, 36 insertions(+), 55 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3a0e3b6cec..f830384a72 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13945,17 +13945,19 @@ qemuDomainSnapshotCreateActiveInternal(virConnectPtr conn, static int -qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) +qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdisk, + virDomainDiskDefPtr domdisk) { - int actualType = virStorageSourceGetActualType(disk->src); + int domDiskType = virStorageSourceGetActualType(domdisk->src); + int snapDiskType = virStorageSourceGetActualType(snapdisk->src); - switch ((virStorageType) actualType) { + switch ((virStorageType) domDiskType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: - return 0; + break; case VIR_STORAGE_TYPE_NETWORK: - switch ((virStorageNetProtocol) disk->src->protocol) { + switch ((virStorageNetProtocol) domdisk->src->protocol) { case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_RBD: @@ -13973,7 +13975,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) virReportError(VIR_ERR_INTERNAL_ERROR, _("external inactive snapshots are not supported on " "'network' disks using '%s' protocol"), - virStorageNetProtocolTypeToString(disk->src->protocol)); + virStorageNetProtocolTypeToString(domdisk->src->protocol)); return -1; } break; @@ -13984,7 +13986,23 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("external inactive snapshots are not supported on " - "'%s' disks"), virStorageTypeToString(actualType)); + "'%s' disks"), virStorageTypeToString(domDiskType)); + return -1; + } + + switch ((virStorageType) snapDiskType) { + case VIR_STORAGE_TYPE_BLOCK: + case VIR_STORAGE_TYPE_FILE: + break; + + case VIR_STORAGE_TYPE_NETWORK: + case VIR_STORAGE_TYPE_DIR: + case VIR_STORAGE_TYPE_VOLUME: + case VIR_STORAGE_TYPE_NONE: + case VIR_STORAGE_TYPE_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("external inactive snapshots are not supported on " + "'%s' disks"), virStorageTypeToString(snapDiskType)); return -1; } @@ -13993,33 +14011,27 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) static int -qemuDomainSnapshotPrepareDiskExternalBackingActive(virDomainDiskDefPtr disk) +qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk, + virDomainDiskDefPtr domdisk) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + int actualType = virStorageSourceGetActualType(snapdisk->src); + + if (domdisk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("external active snapshots are not supported on scsi " "passthrough devices")); return -1; } - return 0; -} - - -static int -qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr disk) -{ - int actualType = virStorageSourceGetActualType(disk->src); - switch ((virStorageType) actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: - return 0; + break; case VIR_STORAGE_TYPE_NETWORK: - switch ((virStorageNetProtocol) disk->src->protocol) { + switch ((virStorageNetProtocol) snapdisk->src->protocol) { case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - return 0; + break; case VIR_STORAGE_NET_PROTOCOL_NONE: case VIR_STORAGE_NET_PROTOCOL_NBD: @@ -14037,7 +14049,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d virReportError(VIR_ERR_INTERNAL_ERROR, _("external active snapshots are not supported on " "'network' disks using '%s' protocol"), - virStorageNetProtocolTypeToString(disk->src->protocol)); + virStorageNetProtocolTypeToString(snapdisk->src->protocol)); return -1; } @@ -14057,31 +14069,6 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d } -static int -qemuDomainSnapshotPrepareDiskExternalOverlayInactive(virDomainSnapshotDiskDefPtr disk) -{ - int actualType = virStorageSourceGetActualType(disk->src); - - switch ((virStorageType) actualType) { - case VIR_STORAGE_TYPE_BLOCK: - case VIR_STORAGE_TYPE_FILE: - return 0; - - case VIR_STORAGE_TYPE_NETWORK: - case VIR_STORAGE_TYPE_DIR: - case VIR_STORAGE_TYPE_VOLUME: - case VIR_STORAGE_TYPE_NONE: - case VIR_STORAGE_TYPE_LAST: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("external inactive snapshots are not supported on " - "'%s' disks"), virStorageTypeToString(actualType)); - return -1; - } - - return 0; -} - - static int qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, virDomainDiskDefPtr disk, @@ -14099,16 +14086,10 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, if (virStorageTranslateDiskSourcePool(conn, disk) < 0) return -1; - if (qemuDomainSnapshotPrepareDiskExternalBackingInactive(disk) < 0) - return -1; - - if (qemuDomainSnapshotPrepareDiskExternalOverlayInactive(snapdisk) < 0) + if (qemuDomainSnapshotPrepareDiskExternalInactive(snapdisk, disk) < 0) return -1; } else { - if (qemuDomainSnapshotPrepareDiskExternalBackingActive(disk) < 0) - return -1; - - if (qemuDomainSnapshotPrepareDiskExternalOverlayActive(snapdisk) < 0) + if (qemuDomainSnapshotPrepareDiskExternalActive(snapdisk, disk) < 0) return -1; } -- 2.14.3
Move network device validation into a separate function. --- src/qemu/qemu_domain.c | 158 ++++++++++++++++++++++++++----------------------- 1 file changed, 84 insertions(+), 74 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cc7596bad1..3755a23907 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3589,101 +3589,111 @@ qemuDomainWatchdogDefValidate(const virDomainWatchdogDef *dev, static int -qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, - const virDomainDef *def, - void *opaque ATTRIBUTE_UNUSED) +qemuDomainDeviceDefValidateNetwork(const virDomainNetDef *net) { - int ret = -1; + bool hasIPv4 = false; + bool hasIPv6 = false; size_t i; - if (dev->type == VIR_DOMAIN_DEVICE_NET) { - const virDomainNetDef *net = dev->data.net; - bool hasIPv4 = false, hasIPv6 = false; + if (net->type == VIR_DOMAIN_NET_TYPE_USER) { + if (net->guestIP.nroutes) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Invalid attempt to set network interface " + "guest-side IP route, not supported by QEMU")); + return -1; + } - if (net->type == VIR_DOMAIN_NET_TYPE_USER) { - if (net->guestIP.nroutes) { + for (i = 0; i < net->guestIP.nips; i++) { + const virNetDevIPAddr *ip = net->guestIP.ips[i]; + + if (VIR_SOCKET_ADDR_VALID(&net->guestIP.ips[i]->peer)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid attempt to set network interface " - "guest-side IP route, not supported by QEMU")); - goto cleanup; + _("Invalid attempt to set peer IP for guest")); + return -1; } - for (i = 0; i < net->guestIP.nips; i++) { - const virNetDevIPAddr *ip = net->guestIP.ips[i]; - - if (VIR_SOCKET_ADDR_VALID(&net->guestIP.ips[i]->peer)) { + if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) { + if (hasIPv4) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid attempt to set peer IP for guest")); - goto cleanup; + _("Only one IPv4 address per " + "interface is allowed")); + return -1; } + hasIPv4 = true; - if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET)) { - if (hasIPv4) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only one IPv4 address per " - "interface is allowed")); - goto cleanup; - } - hasIPv4 = true; - - if (ip->prefix > 27) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("prefix too long")); - goto cleanup; - } + if (ip->prefix > 27) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("prefix too long")); + return -1; } + } - if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) { - if (hasIPv6) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Only one IPv6 address per " - "interface is allowed")); - goto cleanup; - } - hasIPv6 = true; + if (VIR_SOCKET_ADDR_IS_FAMILY(&ip->address, AF_INET6)) { + if (hasIPv6) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Only one IPv6 address per " + "interface is allowed")); + return -1; + } + hasIPv6 = true; - if (ip->prefix > 120) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("prefix too long")); - goto cleanup; - } + if (ip->prefix > 120) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("prefix too long")); + return -1; } } - } else if (net->guestIP.nroutes || net->guestIP.nips) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Invalid attempt to set network interface " - "guest-side IP route and/or address info, " - "not supported by QEMU")); - goto cleanup; } + } else if (net->guestIP.nroutes || net->guestIP.nips) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Invalid attempt to set network interface " + "guest-side IP route and/or address info, " + "not supported by QEMU")); + return -1; + } - if (STREQ_NULLABLE(net->model, "virtio")) { - if (net->driver.virtio.rx_queue_size & (net->driver.virtio.rx_queue_size - 1)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("rx_queue_size has to be a power of two")); - goto cleanup; - } - if (net->driver.virtio.tx_queue_size & (net->driver.virtio.tx_queue_size - 1)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("tx_queue_size has to be a power of two")); - goto cleanup; - } + if (STREQ_NULLABLE(net->model, "virtio")) { + if (net->driver.virtio.rx_queue_size & (net->driver.virtio.rx_queue_size - 1)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("rx_queue_size has to be a power of two")); + return -1; } - - if (net->mtu && - !qemuDomainNetSupportsMTU(net->type)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("setting MTU on interface type %s is not supported yet"), - virDomainNetTypeToString(net->type)); - goto cleanup; + if (net->driver.virtio.tx_queue_size & (net->driver.virtio.tx_queue_size - 1)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("tx_queue_size has to be a power of two")); + return -1; } + } - if (net->coalesce && !qemuDomainNetSupportsCoalesce(net->type)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("coalesce settings on interface type %s are not supported"), - virDomainNetTypeToString(net->type)); + if (net->mtu && + !qemuDomainNetSupportsMTU(net->type)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("setting MTU on interface type %s is not supported yet"), + virDomainNetTypeToString(net->type)); + return -1; + } + + if (net->coalesce && !qemuDomainNetSupportsCoalesce(net->type)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("coalesce settings on interface type %s are not supported"), + virDomainNetTypeToString(net->type)); + return -1; + } + + return 0; +} + + +static int +qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, + const virDomainDef *def, + void *opaque ATTRIBUTE_UNUSED) +{ + int ret = -1; + + if (dev->type == VIR_DOMAIN_DEVICE_NET) { + if (qemuDomainDeviceDefValidateNetwork(dev->data.net) < 0) goto cleanup; - } } else if (dev->type == VIR_DOMAIN_DEVICE_CHR) { if (qemuDomainChrDefValidate(dev->data.chr, def) < 0) goto cleanup; -- 2.14.3
s/eti/etti/ in the summary Jan On Wed, Nov 22, 2017 at 11:56:45AM +0100, Peter Krempa wrote:
Move network device validation into a separate function. --- src/qemu/qemu_domain.c | 158 ++++++++++++++++++++++++++----------------------- 1 file changed, 84 insertions(+), 74 deletions(-)
--- src/qemu/qemu_domain.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3755a23907..7a369969d8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3684,6 +3684,23 @@ qemuDomainDeviceDefValidateNetwork(const virDomainNetDef *net) } +static int +qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev, + const virDomainDef *def) +{ + /* forbid capabilities mode hostdev in this kind of hypervisor */ + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("hostdev mode 'capabilities' is not " + "supported in %s"), + virDomainVirtTypeToString(def->virtType)); + return -1; + } + + return 0; +} + + static int qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, @@ -3709,16 +3726,9 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, } else if (dev->type == VIR_DOMAIN_DEVICE_WATCHDOG) { if (qemuDomainWatchdogDefValidate(dev->data.watchdog, def) < 0) goto cleanup; - } - - /* forbid capabilities mode hostdev in this kind of hypervisor */ - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && - dev->data.hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("hostdev mode 'capabilities' is not " - "supported in %s"), - virDomainVirtTypeToString(def->virtType)); - goto cleanup; + } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { + if (qemuDomainDeviceDefValidateHostdev(dev->data.hostdev, def) < 0) + goto cleanup; } if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) { -- 2.14.3
--- src/qemu/qemu_domain.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7a369969d8..acda06b913 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3701,6 +3701,29 @@ qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev, } +static int +qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) +{ + if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && + video->vgamem) { + if (video->vgamem < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("value for 'vgamem' must be at least 1 MiB " + "(1024 KiB)")); + return -1; + } + + if (video->vgamem != VIR_ROUND_UP_POWER_OF_TWO(video->vgamem)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("value for 'vgamem' must be power of two")); + return -1; + } + } + + return 0; +} + + static int qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, @@ -3729,23 +3752,9 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { if (qemuDomainDeviceDefValidateHostdev(dev->data.hostdev, def) < 0) goto cleanup; - } - - if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) { - if (dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - dev->data.video->vgamem) { - if (dev->data.video->vgamem < 1024) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("value for 'vgamem' must be at least 1 MiB " - "(1024 KiB)")); - goto cleanup; - } - if (dev->data.video->vgamem != VIR_ROUND_UP_POWER_OF_TWO(dev->data.video->vgamem)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("value for 'vgamem' must be power of two")); - goto cleanup; - } - } + } else if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) { + if (qemuDomainDeviceDefValidateVideo(dev->data.video) < 0) + goto cleanup; } ret = 0; -- 2.14.3
Use a style that will discourage from adding inline checks. --- src/qemu/qemu_domain.c | 79 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index acda06b913..29fdb49d14 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3729,36 +3729,61 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, void *opaque ATTRIBUTE_UNUSED) { - int ret = -1; + int ret = 0; - if (dev->type == VIR_DOMAIN_DEVICE_NET) { - if (qemuDomainDeviceDefValidateNetwork(dev->data.net) < 0) - goto cleanup; - } else if (dev->type == VIR_DOMAIN_DEVICE_CHR) { - if (qemuDomainChrDefValidate(dev->data.chr, def) < 0) - goto cleanup; - } else if (dev->type == VIR_DOMAIN_DEVICE_SMARTCARD) { - if (qemuDomainSmartcardDefValidate(dev->data.smartcard) < 0) - goto cleanup; - } else if (dev->type == VIR_DOMAIN_DEVICE_RNG) { - if (qemuDomainRNGDefValidate(dev->data.rng) < 0) - goto cleanup; - } else if (dev->type == VIR_DOMAIN_DEVICE_REDIRDEV) { - if (qemuDomainRedirdevDefValidate(dev->data.redirdev) < 0) - goto cleanup; - } else if (dev->type == VIR_DOMAIN_DEVICE_WATCHDOG) { - if (qemuDomainWatchdogDefValidate(dev->data.watchdog, def) < 0) - goto cleanup; - } else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV) { - if (qemuDomainDeviceDefValidateHostdev(dev->data.hostdev, def) < 0) - goto cleanup; - } else if (dev->type == VIR_DOMAIN_DEVICE_VIDEO) { - if (qemuDomainDeviceDefValidateVideo(dev->data.video) < 0) - goto cleanup; + switch ((virDomainDeviceType) dev->type) { + case VIR_DOMAIN_DEVICE_NET: + ret = qemuDomainDeviceDefValidateNetwork(dev->data.net); + break; + + case VIR_DOMAIN_DEVICE_CHR: + ret = qemuDomainChrDefValidate(dev->data.chr, def); + break; + + case VIR_DOMAIN_DEVICE_SMARTCARD: + ret = qemuDomainSmartcardDefValidate(dev->data.smartcard); + break; + + case VIR_DOMAIN_DEVICE_RNG: + ret = qemuDomainRNGDefValidate(dev->data.rng); + break; + + case VIR_DOMAIN_DEVICE_REDIRDEV: + ret = qemuDomainRedirdevDefValidate(dev->data.redirdev); + break; + + case VIR_DOMAIN_DEVICE_WATCHDOG: + ret = qemuDomainWatchdogDefValidate(dev->data.watchdog, def); + break; + + case VIR_DOMAIN_DEVICE_HOSTDEV: + ret = qemuDomainDeviceDefValidateHostdev(dev->data.hostdev, def); + break; + + case VIR_DOMAIN_DEVICE_VIDEO: + ret = qemuDomainDeviceDefValidateVideo(dev->data.video); + break; + + case VIR_DOMAIN_DEVICE_DISK: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_LAST: + break; } - ret = 0; - cleanup: return ret; } -- 2.14.3
Storage source format backing a shared device (e.g. running a cluster filesystem) needs to support the sharing so that metadata are not corrupted. Add a central function for checking this. --- src/qemu/qemu_block.c | 15 +++++++++++++++ src/qemu/qemu_block.h | 3 +++ 2 files changed, 18 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8b23df8227..29a341f149 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -390,6 +390,21 @@ qemuBlockGetNodeData(virJSONValuePtr data) } +/** + * qemuBlockStorageSourceSupportsConcurrentAccess: + * @src: disk storage source + * + * Returns true if the given storage format supports concurrent access from two + * separate processes. + */ +bool +qemuBlockStorageSourceSupportsConcurrentAccess(virStorageSourcePtr src) +{ + /* no need to check in backing chain since only RAW storage supports this */ + return src->format == VIR_STORAGE_FILE_RAW; +} + + /** * qemuBlockStorageSourceGetURI: * @src: disk storage source diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index b9ee97f488..45485733fc 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -54,6 +54,9 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, virHashTablePtr qemuBlockGetNodeData(virJSONValuePtr data); +bool +qemuBlockStorageSourceSupportsConcurrentAccess(virStorageSourcePtr src); + virJSONValuePtr qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src); -- 2.14.3
Disk sharing between two VMs may corrupt the images if the format driver does not support it. Check tha the user declared use of a supported storage format when they want to share the disk. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480 --- src/qemu/qemu_domain.c | 27 +++++++++++++++++++++ .../qemuxml2argv-disk-drive-shared-qcow.xml | 28 ++++++++++++++++++++++ .../qemuxml2argv-disk-drive-shared.args | 2 +- .../qemuxml2argv-disk-drive-shared.xml | 2 +- tests/qemuxml2argvtest.c | 1 + 5 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 29fdb49d14..3bdff770b4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -25,6 +25,7 @@ #include "qemu_domain.h" #include "qemu_alias.h" +#include "qemu_block.h" #include "qemu_cgroup.h" #include "qemu_command.h" #include "qemu_process.h" @@ -3724,6 +3725,29 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) } +static int +qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk) +{ + if (disk->src->shared && !disk->src->readonly) { + if (disk->src->format <= VIR_STORAGE_FILE_AUTO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("shared access for disk '%s' requires use of " + "explicitly specified disk format"), disk->dst); + return -1; + } + + if (!qemuBlockStorageSourceSupportsConcurrentAccess(disk->src)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("shared access for disk '%s' requires use of " + "supported storage format"), disk->dst); + return -1; + } + } + + return 0; +} + + static int qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, const virDomainDef *def, @@ -3765,6 +3789,9 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_DISK: + ret = qemuDomainDeviceDefValidateDisk(dev->data.disk); + break; + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml new file mode 100644 index 0000000000..ca88a944b3 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='qcow2'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <shareable/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args index 502157bf8c..326fde1b36 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args @@ -19,7 +19,7 @@ server,nowait \ -no-acpi \ -boot c \ -usb \ --drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0,\ serial=XYZXYZXYZYXXYZYZYXYZY,cache=none \ -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ -drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,media=cdrom,\ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml index 9f74723783..677c2b0b7d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml @@ -15,7 +15,7 @@ <devices> <emulator>/usr/bin/qemu-system-i686</emulator> <disk type='block' device='disk'> - <driver name='qemu' type='qcow2'/> + <driver name='qemu' type='raw'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <shareable/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 781c649bff..fdfc3c0b5e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -908,6 +908,7 @@ mymain(void) QEMU_CAPS_DRIVE_BOOT); DO_TEST("disk-drive-shared", QEMU_CAPS_DRIVE_SERIAL); + DO_TEST_PARSE_ERROR("disk-drive-shared-qcow", NONE); DO_TEST("disk-drive-error-policy-stop", QEMU_CAPS_MONITOR_JSON); DO_TEST("disk-drive-error-policy-enospace", -- 2.14.3
On Wed, Nov 22, 2017 at 11:56:50AM +0100, Peter Krempa wrote:
Disk sharing between two VMs may corrupt the images if the format driver does not support it. Check tha the user declared use of a supported
s/tha/that/g
storage format when they want to share the disk.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480 --- src/qemu/qemu_domain.c | 27 +++++++++++++++++++++ .../qemuxml2argv-disk-drive-shared-qcow.xml | 28 ++++++++++++++++++++++ .../qemuxml2argv-disk-drive-shared.args | 2 +- .../qemuxml2argv-disk-drive-shared.xml | 2 +- tests/qemuxml2argvtest.c | 1 + 5 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-qcow.xml
Creating a snapshot would introduce a possibly unsupported member for sharing into the backing chain. Add a check to prevent that from happening. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480 --- src/qemu/qemu_driver.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f830384a72..aa1f1e36d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -13944,6 +13944,24 @@ qemuDomainSnapshotCreateActiveInternal(virConnectPtr conn, } +static int +qemuDomainSnapshotPrepareDiskShared(virDomainSnapshotDiskDefPtr snapdisk, + virDomainDiskDefPtr domdisk) +{ + if (!domdisk->src->shared || domdisk->src->readonly) + return 0; + + if (!qemuBlockStorageSourceSupportsConcurrentAccess(snapdisk->src)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("shared access for disk '%s' requires use of " + "supported storage format"), domdisk->dst); + return -1; + } + + return 0; +} + + static int qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdisk, virDomainDiskDefPtr domdisk) @@ -14006,6 +14024,9 @@ qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdi return -1; } + if (qemuDomainSnapshotPrepareDiskShared(snapdisk, domdisk) < 0) + return -1; + return 0; } @@ -14065,6 +14086,9 @@ qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk return -1; } + if (qemuDomainSnapshotPrepareDiskShared(snapdisk, domdisk) < 0) + return -1; + return 0; } -- 2.14.3
Pivoting to a unsupported storage type might break the assumption that shared disks will not corrupt metadata. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1511480 --- src/qemu/qemu_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aa1f1e36d2..180f96ad5c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16501,6 +16501,16 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, goto cleanup; } + /* When pivoting to a shareable disk we need to make sure that the disk can + * be safely shared, since block copy might have changed the format. */ + if (disk->src->shared && !disk->src->readonly && + !qemuBlockStorageSourceSupportsConcurrentAccess(disk->mirror)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("can't pivot a shared disk to a storage volume not " + "supporting sharing")); + goto cleanup; + } + /* For active commit, the mirror is part of the already labeled * chain. For blockcopy, we previously labeled only the top-level * image; but if the user is reusing an external image that -- 2.14.3
'share-rw' for the disk device configures qemu to allow concurrent access to the backing storage. The capability is checked in various supported disk frontend buses since it does not make sense to partially backport it. --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 10 files changed, 14 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9c1eeacada..2b537433a5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -449,6 +449,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 275 */ "sclplmconsole", + "disk-share-rw", ); @@ -1693,6 +1694,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { { "scsi", QEMU_CAPS_VIRTIO_BLK_SCSI }, { "logical_block_size", QEMU_CAPS_BLOCKIO }, { "num-queues", QEMU_CAPS_VIRTIO_BLK_NUM_QUEUES }, + { "share-rw", QEMU_CAPS_DISK_SHARE_RW }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioNet[] = { @@ -1723,10 +1725,12 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVfioPCI[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsSCSIDisk[] = { { "channel", QEMU_CAPS_SCSI_DISK_CHANNEL }, { "wwn", QEMU_CAPS_SCSI_DISK_WWN }, + { "share-rw", QEMU_CAPS_DISK_SHARE_RW }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsIDEDrive[] = { { "wwn", QEMU_CAPS_IDE_DRIVE_WWN }, + { "share-rw", QEMU_CAPS_DISK_SHARE_RW }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsPiix4PM[] = { @@ -1757,6 +1761,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQ35PCIHost[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsUSBStorage[] = { { "removable", QEMU_CAPS_USB_STORAGE_REMOVABLE }, + { "share-rw", QEMU_CAPS_DISK_SHARE_RW }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsKVMPit[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 241764824c..8c3fd2789a 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -435,6 +435,7 @@ typedef enum { /* 275 */ QEMU_CAPS_DEVICE_SCLPLMCONSOLE, /* -device sclplmconsole */ + QEMU_CAPS_DISK_SHARE_RW, /* share-rw=on for concurrent disk access */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml index 9f9dceb684..a23e3a155f 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0-gicv2.aarch64.xml @@ -180,6 +180,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='vxhs'/> <flag name='virtio-blk.num-queues'/> + <flag name='disk-share-rw'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <package> (v2.10.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml index 3c2d2eed66..d481122f58 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0-gicv3.aarch64.xml @@ -180,6 +180,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='vxhs'/> <flag name='virtio-blk.num-queues'/> + <flag name='disk-share-rw'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <package> (v2.10.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml index b7b80799c0..c95025b41f 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml @@ -179,6 +179,7 @@ <flag name='virtio-blk.num-queues'/> <flag name='machine.pseries.resize-hpt'/> <flag name='spapr-vty'/> + <flag name='disk-share-rw'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <package> (v2.10.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index dee468252c..463c30c77a 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -142,6 +142,7 @@ <flag name='vxhs'/> <flag name='virtio-blk.num-queues'/> <flag name='sclplmconsole'/> + <flag name='disk-share-rw'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml index ddbd8c32fa..33db3e6674 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml @@ -224,6 +224,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='vxhs'/> <flag name='virtio-blk.num-queues'/> + <flag name='disk-share-rw'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <package> (v2.10.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml index e1b0074c9f..3b70b1a407 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64.xml @@ -174,6 +174,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='virtio-blk.num-queues'/> <flag name='spapr-vty'/> + <flag name='disk-share-rw'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index 6f965997ec..5c4a02c8b1 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -139,6 +139,7 @@ <flag name='virtio-gpu.max_outputs'/> <flag name='virtio-blk.num-queues'/> <flag name='sclplmconsole'/> + <flag name='disk-share-rw'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 05f9dc0308..0ea9a2fce3 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -221,6 +221,7 @@ <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> <flag name='virtio-blk.num-queues'/> + <flag name='disk-share-rw'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> -- 2.14.3
Qemu has now an internal mechanism for locking images to fix specific cases of disk corruption. This requires libvirt to mark the image as shared so that qemu lifts certain restrictions. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378242 --- src/qemu/qemu_command.c | 4 +++ .../qemuxml2argv-disk-drive-shared-locking.args | 32 +++++++++++++++++ .../qemuxml2argv-disk-drive-shared-locking.xml | 42 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 4 files changed, 80 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 216a4bdfe0..f009b28254 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2126,6 +2126,10 @@ qemuBuildDriveDevStr(const virDomainDef *def, goto error; } + if (disk->src->shared && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_SHARE_RW)) + virBufferAddLit(&opt, ",share-rw=on"); + if (!(drivealias = qemuAliasFromDisk(disk))) goto error; virBufferAsprintf(&opt, ",drive=%s,id=%s", drivealias, disk->info.alias); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args new file mode 100644 index 0000000000..cdf17f26d1 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.args @@ -0,0 +1,32 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-usb \ +-drive file=/dev/ide,format=raw,if=none,id=drive-ide0-0-0,cache=none \ +-device ide-drive,bus=ide.0,unit=0,share-rw=on,drive=drive-ide0-0-0,\ +id=ide0-0-0 \ +-drive file=/dev/scsi,format=raw,if=none,id=drive-scsi0-0-0-0,cache=none \ +-device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,share-rw=on,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-drive file=/dev/virtio,format=raw,if=none,id=drive-virtio-disk0,cache=none \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,share-rw=on,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml new file mode 100644 index 0000000000..dd48857a30 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared-locking.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</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-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/ide'/> + <target dev='hda' bus='ide'/> + <shareable/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/scsi'/> + <target dev='sda' bus='scsi'/> + <shareable/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/virtio'/> + <target dev='vda' bus='virtio'/> + <shareable/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='scsi' index='0' model='virtio-scsi'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index fdfc3c0b5e..fc6f2f10e1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -909,6 +909,8 @@ mymain(void) DO_TEST("disk-drive-shared", QEMU_CAPS_DRIVE_SERIAL); DO_TEST_PARSE_ERROR("disk-drive-shared-qcow", NONE); + DO_TEST("disk-drive-shared-locking", + QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DISK_SHARE_RW); DO_TEST("disk-drive-error-policy-stop", QEMU_CAPS_MONITOR_JSON); DO_TEST("disk-drive-error-policy-enospace", -- 2.14.3
On Wed, Nov 22, 2017 at 11:56:43AM +0100, Peter Krempa wrote:
Introduction of the disk image locking in qemu created a regression where disk shared access with <shareable/> would not work.
Since the disk locking is a desired feature, allow libvirt to configure qemu in such way that <shareable/> disks are exempt from write locks.
First few patches refactor some stuff so that it's less ugly.
Thank you.
This series also mandates that sharing is used only with 'raw' disks since other formats could corrupt metadata.
v2: - better capability detection (all the drive types) - fixed bug when share-rw would be used only with virtio disks - improved error message with automatic format probing - rebased to current master (new capability) - added BZ links into few cover letters
Peter Krempa (11): qemu: Move snapshot disk validation functions into one qemu: domain: Despaghetify qemuDomainDeviceDefValidate qemu: domain: Move hostdev validation into separate function qemu: domain: Move video device validation into separate function qemu: domain: Refactor domain device validation function qemu: block: Add function to check if storage source allows concurrent access qemu: domain: Reject shared disk access if backing format does not support it qemu: snapshot: Disallow snapshot of unsupported shared disks qemu: Disallow pivot of shared disks to unsupported storage qemu: caps: Add capability for 'share-rw' disk option qemu: command: Mark <shared/> disks as such in qemu
ACK series Jan
participants (2)
-
Ján Tomko -
Peter Krempa