7:13 a.m.
Fixes regression in media changing/disk hotplug as the ordering of the
alias allocation and disk preparation was bad.
v2:
- be more explicit about old and new definitions used in certain steps
- clean up legacy hotplug to not access old disk source definition
- also treat network disks as 'raw' if the format was not provided
Peter Krempa (12):
Revert "qemu: hotplug: Prepare disk source in
qemuDomainAttachDeviceDiskLive"
Revert "qemu: hotplug: consolidate media change code paths"
qemu: hotplug: Don't pretend that we support secrets for media change
qemu: domain: Assume 'raw' default storage format also for network
storage
qemu: hotplug: Remove code handling possible missing disk source
format
qemu: hotplug: Allow specifying explicit source for disk backend
hotplug code
qemu: hotplug: Be explicit about old/new sources when changing media
qemu: hotplug: Prepare disk source for media changing
qemu: hotplug: Add wrapper for disk hotplug code
qemu: conf: Export qemuAddSharedDisk
qemu: hotplug: Split out media change code from disk hotplug
qemu: hotplug: Refactor qemuDomainAttachDeviceDiskLiveInternal
src/qemu/qemu_conf.c | 2 +-
src/qemu/qemu_conf.h | 5 +
src/qemu/qemu_domain.c | 4 +-
src/qemu/qemu_driver.c | 7 +-
src/qemu/qemu_hotplug.c | 276 ++++++++++--------
src/qemu/qemu_hotplug.h | 9 +-
tests/qemuhotplugtest.c | 2 +-
.../disk-source-pool-mode.args | 6 +-
tests/qemuxml2argvdata/disk-source-pool.args | 4 +-
.../disk-source-pool-mode.xml | 6 +-
tests/qemuxml2xmloutdata/disk-source-pool.xml | 2 +-
11 files changed, 185 insertions(+), 138 deletions(-)
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 01/12] Revert "qemu: hotplug: Prepare disk source in qemuDomainAttachDeviceDiskLive"
Preparing the storage source prior to assigning the alias will not work
as the names of the certain objects depend on the alias for the legacy
hotplug case as we generate the object names for the secrets based on
the alias.
This reverts commit 192fdaa614e3800255048a8a70c1292ccf18397a.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4558a3c02d..ccf3336633 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -781,6 +781,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuHotplugDiskSourceDataPtr diskdata = NULL;
char *devstr = NULL;
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
if (qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, false) < 0)
goto cleanup;
@@ -788,6 +789,9 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
+ if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
+ goto error;
+
if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, priv->qemuCaps)))
goto error;
@@ -822,6 +826,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
qemuHotplugDiskSourceDataFree(diskdata);
qemuDomainSecretDiskDestroy(disk);
VIR_FREE(devstr);
+ virObjectUnref(cfg);
return ret;
exit_monitor:
@@ -1062,8 +1067,6 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
bool forceMediaChange)
{
size_t i;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
int ret = -1;
@@ -1080,9 +1083,6 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0)
goto cleanup;
- if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
- goto cleanup;
-
switch ((virDomainDiskDevice) disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
@@ -1153,7 +1153,6 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
cleanup:
if (ret != 0)
ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
- virObjectUnref(cfg);
return ret;
}
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 02/12] Revert "qemu: hotplug: consolidate media change code paths"
While the idea was good the implementation not so much as we need to
take into account the old disk data and the new source. The code will be
consolidated later in a different way.
This reverts commit 663b1d55de652201b19d875f0eff730dc28e689e.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 20 ++++++++++++++++++--
src/qemu/qemu_hotplug.c | 18 +++---------------
src/qemu/qemu_hotplug.h | 9 +++++++--
tests/qemuhotplugtest.c | 2 +-
4 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ef87a6ef05..91ac3640d3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7600,7 +7600,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
switch ((virDomainDeviceType)dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL);
- ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev, false);
+ ret = qemuDomainAttachDeviceDiskLive(driver, vm, dev);
if (!ret) {
alias = dev->data.disk->info.alias;
dev->data.disk = NULL;
@@ -7851,6 +7851,12 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
virDomainDeviceDef oldDev = { .type = dev->type };
int ret = -1;
+ if (virDomainDiskTranslateSourcePool(disk) < 0)
+ goto cleanup;
+
+ if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0)
+ goto cleanup;
+
if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
disk->bus, disk->dst))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -7879,8 +7885,18 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
goto cleanup;
}
- if (qemuDomainAttachDeviceDiskLive(driver, vm, dev, force) < 0)
+ /* Add the new disk src into shared disk hash table */
+ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
goto cleanup;
+
+ if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
+ dev->data.disk->src, force) < 0) {
+ ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
+ vm->def->name));
+ goto cleanup;
+ }
+
+ dev->data.disk->src = NULL;
}
orig_disk->startupPolicy = dev->data.disk->startupPolicy;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ccf3336633..86afda636e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -722,7 +722,7 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
*
* Returns 0 on success, -1 on error and reports libvirt error
*/
-static int
+int
qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
@@ -1049,22 +1049,10 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
}
-/**
- * qemuDomainAttachDeviceDiskLive:
- * @driver: qemu driver struct
- * @vm: domain object
- * @dev: device to attach (expected type is DISK)
- * @forceMediaChange: Forcibly open the drive if changing media
- *
- * Attach a new disk or in case of cdroms/floppies change the media in the drive.
- * This function handles all the necessary steps to attach a new storage source
- * to the VM. If @forceMediaChange is true the drive is opened forcibly.
- */
int
qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev,
- bool forceMediaChange)
+ virDomainDeviceDefPtr dev)
{
size_t i;
virDomainDiskDefPtr disk = dev->data.disk;
@@ -1098,7 +1086,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
}
if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
- disk->src, forceMediaChange) < 0)
+ disk->src, false) < 0)
goto cleanup;
disk->src = NULL;
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index c085c45082..0297e42a98 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -28,6 +28,12 @@
# include "qemu_domain.h"
# include "domain_conf.h"
+int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr newsrc,
+ bool force);
+
void qemuDomainDelTLSObjects(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob,
@@ -54,8 +60,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
virDomainControllerDefPtr controller);
int qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
virDomainObjPtr vm,
- virDomainDeviceDefPtr dev,
- bool forceMediaChange);
+ virDomainDeviceDefPtr dev);
int qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainNetDefPtr net);
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index e0e248556f..5b1e0db104 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -120,7 +120,7 @@ testQemuHotplugAttach(virDomainObjPtr vm,
/* conn in only used for storage pool and secrets lookup so as long
* as we don't use any of them, passing NULL should be safe
*/
- ret = qemuDomainAttachDeviceDiskLive(&driver, vm, dev, false);
+ ret = qemuDomainAttachDeviceDiskLive(&driver, vm, dev);
break;
case VIR_DOMAIN_DEVICE_CHR:
ret = qemuDomainAttachChrDevice(&driver, vm, dev->data.chr);
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 03/12] qemu: hotplug: Don't pretend that we support secrets for media change
Old media changing code does not bother setting up the secrets for new
media or actually removing/adding of the corresponding objects.
Additionally it uses secrets setup for the old image to be removed as
the secret for the new image which is wrong.
Remove the support for secrets while changing media for the legacy
approach. The only reasonable way to fix it is when using blockdev.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 86afda636e..ad7023c085 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -210,8 +210,6 @@ qemuDomainChangeMediaLegacy(virQEMUDriverPtr driver,
char *driveAlias = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
- qemuDomainStorageSourcePrivatePtr srcPriv =
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
- qemuDomainSecretInfoPtr secinfo = NULL;
const char *format = NULL;
char *sourcestr = NULL;
@@ -221,9 +219,6 @@ qemuDomainChangeMediaLegacy(virQEMUDriverPtr driver,
goto cleanup;
}
- if (srcPriv)
- secinfo = srcPriv->secinfo;
-
if (!(driveAlias = qemuAliasDiskDriveFromDisk(disk)))
goto cleanup;
@@ -252,7 +247,7 @@ qemuDomainChangeMediaLegacy(virQEMUDriverPtr driver,
}
if (!virStorageSourceIsEmpty(newsrc)) {
- if (qemuGetDriveSourceString(newsrc, secinfo, &sourcestr) < 0)
+ if (qemuGetDriveSourceString(newsrc, NULL, &sourcestr) < 0)
goto cleanup;
if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) {
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: domain: Assume 'raw' default storage format also for network storage
Post parse callback adds the 'raw' type only for local files. Remote
files can also have backing store (even local) so we should do this also
for network backed storage.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 4 +---
tests/qemuxml2argvdata/disk-source-pool-mode.args | 6 +++---
tests/qemuxml2argvdata/disk-source-pool.args | 4 ++--
tests/qemuxml2xmloutdata/disk-source-pool-mode.xml | 6 +++---
tests/qemuxml2xmloutdata/disk-source-pool.xml | 2 +-
5 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f00f1b3fdb..82737bcdee 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6364,9 +6364,7 @@ qemuDomainDeviceDiskDefPostParse(virDomainDiskDefPtr disk,
return -1;
/* default disk format for drives */
- if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE &&
- (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_FILE ||
- virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_BLOCK))
+ if (virDomainDiskGetFormat(disk) == VIR_STORAGE_FILE_NONE)
virDomainDiskSetFormat(disk, VIR_STORAGE_FILE_RAW);
/* default disk format for mirrored drive */
diff --git a/tests/qemuxml2argvdata/disk-source-pool-mode.args
b/tests/qemuxml2argvdata/disk-source-pool-mode.args
index e8d9aacd77..930d360d16 100644
--- a/tests/qemuxml2argvdata/disk-source-pool-mode.args
+++ b/tests/qemuxml2argvdata/disk-source-pool-mode.args
@@ -21,16 +21,16 @@ server,nowait \
-no-shutdown \
-no-acpi \
-usb \
--drive file=/some/block/device/unit:0:0:1,if=none,id=drive-ide0-0-1,\
+-drive file=/some/block/device/unit:0:0:1,format=raw,if=none,id=drive-ide0-0-1,\
media=cdrom,readonly=on \
-device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
--drive file=iscsi://iscsi.example.com:3260/demo-target/2,if=none,\
+-drive file=iscsi://iscsi.example.com:3260/demo-target/2,format=raw,if=none,\
id=drive-ide0-0-2,media=cdrom,readonly=on \
-device ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 \
-drive file=/tmp/idedisk.img,format=raw,if=none,id=drive-ide0-0-3 \
-device ide-drive,bus=ide.0,unit=3,drive=drive-ide0-0-3,id=ide0-0-3,\
bootindex=1 \
--drive file=iscsi://iscsi.example.com:3260/demo-target/3,if=none,\
+-drive file=iscsi://iscsi.example.com:3260/demo-target/3,format=raw,if=none,\
id=drive-ide0-0-4,media=cdrom,readonly=on \
-device ide-drive,bus=ide.0,unit=4,drive=drive-ide0-0-4,id=ide0-0-4 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/disk-source-pool.args
b/tests/qemuxml2argvdata/disk-source-pool.args
index 49dc853bcd..fe95aa2250 100644
--- a/tests/qemuxml2argvdata/disk-source-pool.args
+++ b/tests/qemuxml2argvdata/disk-source-pool.args
@@ -21,8 +21,8 @@ server,nowait \
-no-shutdown \
-no-acpi \
-usb \
--drive file=/some/block/device/cdrom,if=none,id=drive-ide0-0-1,media=cdrom,\
-readonly=on \
+-drive file=/some/block/device/cdrom,format=raw,if=none,id=drive-ide0-0-1,\
+media=cdrom,readonly=on \
-device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
-drive if=none,id=drive-ide0-1-0,media=cdrom,readonly=on \
-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
diff --git a/tests/qemuxml2xmloutdata/disk-source-pool-mode.xml
b/tests/qemuxml2xmloutdata/disk-source-pool-mode.xml
index 29706200db..4d039102fd 100644
--- a/tests/qemuxml2xmloutdata/disk-source-pool-mode.xml
+++ b/tests/qemuxml2xmloutdata/disk-source-pool-mode.xml
@@ -15,7 +15,7 @@
<devices>
<emulator>/usr/bin/qemu-system-i686</emulator>
<disk type='volume' device='cdrom'>
- <driver name='qemu'/>
+ <driver name='qemu' type='raw'/>
<source pool='pool-iscsi-auth' volume='unit:0:0:1'
mode='host'>
<seclabel model='selinux' relabel='yes'>
<label>system_u:system_r:public_content_t:s0</label>
@@ -26,7 +26,7 @@
<address type='drive' controller='0' bus='0'
target='0' unit='1'/>
</disk>
<disk type='volume' device='cdrom'>
- <driver name='qemu'/>
+ <driver name='qemu' type='raw'/>
<source pool='pool-iscsi' volume='unit:0:0:2'
mode='direct'>
<seclabel model='selinux' relabel='yes'>
<label>system_u:system_r:public_content_t:s0</label>
@@ -43,7 +43,7 @@
<address type='drive' controller='0' bus='0'
target='0' unit='3'/>
</disk>
<disk type='volume' device='cdrom'>
- <driver name='qemu'/>
+ <driver name='qemu' type='raw'/>
<auth username='myname'>
<secret type='iscsi' usage='mycluster_myname'/>
</auth>
diff --git a/tests/qemuxml2xmloutdata/disk-source-pool.xml
b/tests/qemuxml2xmloutdata/disk-source-pool.xml
index 567b22db84..51851b220a 100644
--- a/tests/qemuxml2xmloutdata/disk-source-pool.xml
+++ b/tests/qemuxml2xmloutdata/disk-source-pool.xml
@@ -15,7 +15,7 @@
<devices>
<emulator>/usr/bin/qemu-system-i686</emulator>
<disk type='volume' device='cdrom'>
- <driver name='qemu'/>
+ <driver name='qemu' type='raw'/>
<source pool='pool-disk' volume='block+cdrom'>
<seclabel model='selinux' relabel='yes'>
<label>system_u:system_r:public_content_t:s0</label>
--
2.17.1
6:10 a.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: domain: Assume 'raw' default storage format also for network storage
On 10/5/18 8:14 AM, Peter Krempa wrote:
Post parse callback adds the 'raw' type only for local files.
Remote
files can also have backing store (even local) so we should do this also
for network backed storage.
NIT: The .xml/.args in this patch are not all remote storage - they are
volume storage...
The disk-source-pool-mode.args gets the look/feel of remote from
virDomainDiskTranslateISCSIDirect
John
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_domain.c | 4 +---
tests/qemuxml2argvdata/disk-source-pool-mode.args | 6 +++---
tests/qemuxml2argvdata/disk-source-pool.args | 4 ++--
tests/qemuxml2xmloutdata/disk-source-pool-mode.xml | 6 +++---
tests/qemuxml2xmloutdata/disk-source-pool.xml | 2 +-
5 files changed, 10 insertions(+), 12 deletions(-)
[...]
6:16 a.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: domain: Assume 'raw' default storage format also for network storage
On Mon, Oct 08, 2018 at 07:10:28 -0400, John Ferlan wrote:
On 10/5/18 8:14 AM, Peter Krempa wrote:
> Post parse callback adds the 'raw' type only for local files. Remote
> files can also have backing store (even local) so we should do this also
> for network backed storage.
NIT: The .xml/.args in this patch are not all remote storage - they are
volume storage...
The disk-source-pool-mode.args gets the look/feel of remote from
virDomainDiskTranslateISCSIDirect
Oh! actually you are right. For disk type "VOLUME" we should not add any
default format in the parser since the function that fetches the volume
from the storage driver should do that.
I'll investigate slightly further and change the condition to only
exclude "VOLUME" disks rather than try to include a variety of the types
which require this.
7:14 a.m.
New subject: [libvirt] [PATCH v2 05/12] qemu: hotplug: Remove code handling possible missing disk source format
qemu media changing code tried to assume old media's format for the new
one if that was not specified. Since the format will always be present
it does not make sense to keep the code around.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ad7023c085..8123c12a26 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -250,14 +250,9 @@ qemuDomainChangeMediaLegacy(virQEMUDriverPtr driver,
if (qemuGetDriveSourceString(newsrc, NULL, &sourcestr) < 0)
goto cleanup;
- if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) {
- if (newsrc->format > 0) {
- format = virStorageFileFormatTypeToString(newsrc->format);
- } else {
- if (disk->src->format > 0)
- format = virStorageFileFormatTypeToString(disk->src->format);
- }
- }
+ if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR)
+ format = virStorageFileFormatTypeToString(newsrc->format);
+
qemuDomainObjEnterMonitor(driver, vm);
rc = qemuMonitorChangeMedia(priv->mon,
driveAlias,
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 06/12] qemu: hotplug: Allow specifying explicit source for disk backend hotplug code
Since the code is also used when changing media we need to allow
specifying explicit source for which we are going to prepare. With this
change callers don't have to replace disk->src with the new source
definition for generating these.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 31 +++++++++++++++++++++++--------
1 file changed, 23 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8123c12a26..f9a9778fb2 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -458,6 +458,7 @@ qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src,
static qemuHotplugDiskSourceDataPtr
qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
+ virStorageSourcePtr src,
virQEMUCapsPtr qemuCaps)
{
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
@@ -474,7 +475,7 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
if (VIR_STRDUP(data->corAlias, diskPriv->nodeCopyOnRead) < 0)
goto cleanup;
- for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
+ for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(n, NULL)))
goto cleanup;
@@ -485,7 +486,7 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
goto cleanup;
- if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(disk->src,
+ if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(src,
drivealias)))
goto cleanup;
@@ -505,6 +506,7 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
/**
* qemuHotplugDiskSourceAttachPrepare:
* @disk: disk to generate attachment data for
+ * @src: disk source to prepare attachment
* @qemuCaps: capabilities of the qemu process
*
* Prepares and returns qemuHotplugDiskSourceData structure filled with all data
@@ -512,11 +514,13 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
*/
static qemuHotplugDiskSourceDataPtr
qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk,
+ virStorageSourcePtr src,
virQEMUCapsPtr qemuCaps)
{
qemuBlockStorageSourceAttachDataPtr backend = NULL;
qemuHotplugDiskSourceDataPtr data;
qemuHotplugDiskSourceDataPtr ret = NULL;
+ virStorageSourcePtr savesrc = NULL;
virStorageSourcePtr n;
if (VIR_ALLOC(data) < 0)
@@ -527,7 +531,7 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk,
!(data->corProps = qemuBlockStorageGetCopyOnReadProps(disk)))
goto cleanup;
- for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
+ for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
if (!(backend = qemuBlockStorageSourceAttachPrepareBlockdev(n)))
goto cleanup;
@@ -538,10 +542,15 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk,
goto cleanup;
}
} else {
+ VIR_STEAL_PTR(savesrc, disk->src);
+ disk->src = src;
+
if (!(backend = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps)))
goto cleanup;
- if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, backend, qemuCaps)
< 0)
+ VIR_STEAL_PTR(disk->src, savesrc);
+
+ if (qemuBuildStorageSourceAttachPrepareCommon(src, backend, qemuCaps) < 0)
goto cleanup;
if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
@@ -551,6 +560,9 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk,
VIR_STEAL_PTR(ret, data);
cleanup:
+ if (savesrc)
+ VIR_STEAL_PTR(disk->src, savesrc);
+
qemuBlockStorageSourceAttachDataFree(backend);
qemuHotplugDiskSourceDataFree(data);
return ret;
@@ -640,12 +652,13 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
int ret = -1;
if (!virStorageSourceIsEmpty(disk->src) &&
- !(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk, priv->qemuCaps)))
+ !(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk, disk->src,
+ priv->qemuCaps)))
goto cleanup;
disk->src = newsrc;
if (!virStorageSourceIsEmpty(disk->src)) {
- if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk,
+ if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk, disk->src,
priv->qemuCaps)))
goto cleanup;
@@ -782,7 +795,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
goto error;
- if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, priv->qemuCaps)))
+ if (!(diskdata = qemuHotplugDiskSourceAttachPrepare(disk, disk->src,
+ priv->qemuCaps)))
goto error;
if (!(devstr = qemuBuildDiskDeviceStr(vm->def, disk, 0, priv->qemuCaps)))
@@ -4159,7 +4173,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
VIR_DEBUG("Removing disk %s from domain %p %s",
disk->info.alias, vm, vm->def->name);
- if (!(diskbackend = qemuHotplugDiskSourceRemovePrepare(disk, priv->qemuCaps)))
+ if (!(diskbackend = qemuHotplugDiskSourceRemovePrepare(disk, disk->src,
+ priv->qemuCaps)))
return -1;
for (i = 0; i < vm->def->ndisks; i++) {
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 07/12] qemu: hotplug: Be explicit about old/new sources when changing media
Some functions require us to replace disk->src with the new source for
them to work properly. To avoid confusion all places which allow
explicit virStorageSource should get the appropriate definition.
The legacy code fortunately does not need anything from the old source
so that does not require modifications.
Blockdev does require the old definition so we'll pass it explicitly.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f9a9778fb2..7ee8201ce0 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -625,6 +625,7 @@ qemuHotplugDiskSourceRemove(qemuMonitorPtr mon,
* @driver: qemu driver structure
* @vm: domain definition
* @disk: disk definition to change the source of
+ * @oldsrc: old source definition
* @newsrc: new disk source to change to
* @force: force the change of media
*
@@ -639,6 +640,7 @@ static int
qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
+ virStorageSourcePtr oldsrc,
virStorageSourcePtr newsrc,
bool force)
{
@@ -646,19 +648,17 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
qemuHotplugDiskSourceDataPtr newbackend = NULL;
qemuHotplugDiskSourceDataPtr oldbackend = NULL;
- virStorageSourcePtr oldsrc = disk->src;
char *nodename = NULL;
int rc;
int ret = -1;
- if (!virStorageSourceIsEmpty(disk->src) &&
- !(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk, disk->src,
+ if (!virStorageSourceIsEmpty(oldsrc) &&
+ !(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk, oldsrc,
priv->qemuCaps)))
goto cleanup;
- disk->src = newsrc;
- if (!virStorageSourceIsEmpty(disk->src)) {
- if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk, disk->src,
+ if (!virStorageSourceIsEmpty(newsrc)) {
+ if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk, newsrc,
priv->qemuCaps)))
goto cleanup;
@@ -704,8 +704,6 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
qemuHotplugDiskSourceDataFree(newbackend);
qemuHotplugDiskSourceDataFree(oldbackend);
VIR_FREE(nodename);
- /* caller handles correct exchange of sources */
- disk->src = oldsrc;
return ret;
}
@@ -743,7 +741,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
goto cleanup;
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
- rc = qemuDomainChangeMediaBlockdev(driver, vm, disk, newsrc, force);
+ rc = qemuDomainChangeMediaBlockdev(driver, vm, disk, disk->src, newsrc,
force);
else
rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 08/12] qemu: hotplug: Prepare disk source for media changing
The disk storage source needs to be prepared if we want to use -blockdev
or secrets for the new media image. It does not hurt to do the same for
the legacy hotplug code as well.
Unfortunately helpers like qemuDomainPrepareDiskSource take
virDomainDiskDef as an argument and it would be hard to fix them to take
an explicit source, so the function also temporarily replaces disk->src
for the new source in this function.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7ee8201ce0..d201266805 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -730,10 +730,17 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virStorageSourcePtr newsrc,
bool force)
{
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virStorageSourcePtr oldsrc = disk->src;
int ret = -1;
int rc;
+ disk->src = newsrc;
+
+ if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
+ goto cleanup;
+
if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0)
goto cleanup;
@@ -741,11 +748,11 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
goto cleanup;
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
- rc = qemuDomainChangeMediaBlockdev(driver, vm, disk, disk->src, newsrc,
force);
+ rc = qemuDomainChangeMediaBlockdev(driver, vm, disk, oldsrc, newsrc, force);
else
rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
- virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0);
+ virDomainAuditDisk(vm, oldsrc, newsrc, "update", rc >= 0);
if (rc < 0) {
ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true));
@@ -753,17 +760,24 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
}
/* remove the old source from shared device list */
+ disk->src = oldsrc;
ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
- ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true));
+ ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, oldsrc, true));
- virStorageSourceFree(disk->src);
- VIR_STEAL_PTR(disk->src, newsrc);
+ /* media was changed, so we can remove the old media definition now */
+ virStorageSourceFree(oldsrc);
+ oldsrc = NULL;
+ disk->src = newsrc;
ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE));
ret = 0;
cleanup:
+ if (oldsrc)
+ disk->src = oldsrc;
+
+ virObjectUnref(cfg);
return ret;
}
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 09/12] qemu: hotplug: Add wrapper for disk hotplug code
The disk hotplug code also overloads media change which is not ideal.
This will allow splitting out of the media change code.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d201266805..ed7076ea01 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1065,10 +1065,10 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
}
-int
-qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virDomainDeviceDefPtr dev)
+static int
+qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
{
size_t i;
virDomainDiskDefPtr disk = dev->data.disk;
@@ -1161,6 +1161,25 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
}
+/**
+ * qemuDomainAttachDeviceDiskLive:
+ * @driver: qemu driver struct
+ * @vm: domain object
+ * @dev: device to attach (expected type is DISK)
+ *
+ * Attach a new disk or in case of cdroms/floppies change the media in the drive.
+ * This function handles all the necessary steps to attach a new storage source
+ * to the VM.
+ */
+int
+qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ return qemuDomainAttachDeviceDiskLiveInternal(driver, vm, dev);
+}
+
+
static void
qemuDomainNetDeviceVportRemove(virDomainNetDefPtr net)
{
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 10/12] qemu: conf: Export qemuAddSharedDisk
In cases where we know the device is a disk we can avoid using the full
device definition.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 +-
src/qemu/qemu_conf.h | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index fc84186a7e..17b7e11e02 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1400,7 +1400,7 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver,
* records all the domains that use the shared device if the entry
* already exists, otherwise add a new entry.
*/
-static int
+int
qemuAddSharedDisk(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk,
const char *name)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index c227ac72cc..f876f9117c 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -339,6 +339,11 @@ char *qemuGetSharedDeviceKey(const char *disk_path)
void qemuSharedDeviceEntryFree(void *payload, const void *name);
+int qemuAddSharedDisk(virQEMUDriverPtr driver,
+ virDomainDiskDefPtr disk,
+ const char *name)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
int qemuAddSharedDevice(virQEMUDriverPtr driver,
virDomainDeviceDefPtr dev,
const char *name)
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 11/12] qemu: hotplug: Split out media change code from disk hotplug
Disk hotplug has slightly different semantics from media changing. Move
the media change code out and add proper initialization of the new
source object and proper cleanups if something fails.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 15 +-------
src/qemu/qemu_hotplug.c | 77 ++++++++++++++++++++++++++---------------
2 files changed, 51 insertions(+), 41 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 91ac3640d3..a52e2495d5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7851,12 +7851,6 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
virDomainDeviceDef oldDev = { .type = dev->type };
int ret = -1;
- if (virDomainDiskTranslateSourcePool(disk) < 0)
- goto cleanup;
-
- if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0)
- goto cleanup;
-
if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
disk->bus, disk->dst))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -7885,16 +7879,9 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
goto cleanup;
}
- /* Add the new disk src into shared disk hash table */
- if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
- goto cleanup;
-
if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
- dev->data.disk->src, force) < 0) {
- ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
- vm->def->name));
+ dev->data.disk->src, force) < 0)
goto cleanup;
- }
dev->data.disk->src = NULL;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ed7076ea01..62470b1a2f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -733,11 +733,23 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
virStorageSourcePtr oldsrc = disk->src;
+ bool sharedAdded = false;
int ret = -1;
int rc;
disk->src = newsrc;
+ if (virDomainDiskTranslateSourcePool(disk) < 0)
+ goto cleanup;
+
+ if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0)
+ goto cleanup;
+
+ sharedAdded = true;
+
+ if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0)
+ goto cleanup;
+
if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
goto cleanup;
@@ -754,10 +766,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virDomainAuditDisk(vm, oldsrc, newsrc, "update", rc >= 0);
- if (rc < 0) {
- ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true));
+ if (rc < 0)
goto cleanup;
- }
/* remove the old source from shared device list */
disk->src = oldsrc;
@@ -769,11 +779,21 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
oldsrc = NULL;
disk->src = newsrc;
- ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE));
-
ret = 0;
cleanup:
+ /* undo changes to the new disk */
+ if (ret < 0) {
+ if (sharedAdded)
+ ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
+
+ ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, true));
+ }
+
+ /* remove PR manager object if unneeded */
+ ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE));
+
+ /* revert old image do the disk definition */
if (oldsrc)
disk->src = oldsrc;
@@ -1072,9 +1092,15 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver,
{
size_t i;
virDomainDiskDefPtr disk = dev->data.disk;
- virDomainDiskDefPtr orig_disk = NULL;
int ret = -1;
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
+ disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cdrom/floppy device hotplug isn't supported"));
+ return -1;
+ }
+
if (virDomainDiskTranslateSourcePool(disk) < 0)
goto cleanup;
@@ -1088,27 +1114,6 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver,
goto cleanup;
switch ((virDomainDiskDevice) disk->device) {
- case VIR_DOMAIN_DISK_DEVICE_CDROM:
- case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
- if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
- disk->bus, disk->dst))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("No device with bus '%s' and target
'%s'. "
- "cdrom and floppy device hotplug isn't supported
"
- "by libvirt"),
- virDomainDiskBusTypeToString(disk->bus),
- disk->dst);
- goto cleanup;
- }
-
- if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
- disk->src, false) < 0)
- goto cleanup;
-
- disk->src = NULL;
- ret = 0;
- break;
-
case VIR_DOMAIN_DISK_DEVICE_DISK:
case VIR_DOMAIN_DISK_DEVICE_LUN:
for (i = 0; i < vm->def->ndisks; i++) {
@@ -1150,6 +1155,8 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver,
}
break;
+ case VIR_DOMAIN_DISK_DEVICE_CDROM:
+ case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
case VIR_DOMAIN_DISK_DEVICE_LAST:
break;
}
@@ -1176,6 +1183,22 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
+ virDomainDiskDefPtr disk = dev->data.disk;
+ virDomainDiskDefPtr orig_disk = NULL;
+
+ /* this API overloads media change semantics on disk hotplug
+ * for devices supporting media changes */
+ if ((disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
+ disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) &&
+ (orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus,
disk->dst))) {
+ if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
+ disk->src, false) < 0)
+ return -1;
+
+ disk->src = NULL;
+ return 0;
+ }
+
return qemuDomainAttachDeviceDiskLiveInternal(driver, vm, dev);
}
--
2.17.1
7:14 a.m.
New subject: [libvirt] [PATCH v2 12/12] qemu: hotplug: Refactor qemuDomainAttachDeviceDiskLiveInternal
We now explicitly handle media change elsewhere so we can drop the
switch statement. This will also make it more intuitive once CDROM
device hotplug might be supported.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 70 ++++++++++++++++++-----------------------
1 file changed, 30 insertions(+), 40 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 62470b1a2f..0a63741b9e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1113,52 +1113,42 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriverPtr driver,
if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0)
goto cleanup;
- switch ((virDomainDiskDevice) disk->device) {
- case VIR_DOMAIN_DISK_DEVICE_DISK:
- case VIR_DOMAIN_DISK_DEVICE_LUN:
- for (i = 0; i < vm->def->ndisks; i++) {
- if (virDomainDiskDefCheckDuplicateInfo(vm->def->disks[i], disk) <
0)
- goto cleanup;
- }
-
- switch ((virDomainDiskBus) disk->bus) {
- case VIR_DOMAIN_DISK_BUS_USB:
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("disk device='lun' is not supported for usb
bus"));
- break;
- }
- ret = qemuDomainAttachUSBMassStorageDevice(driver, vm, disk);
- break;
-
- case VIR_DOMAIN_DISK_BUS_VIRTIO:
- ret = qemuDomainAttachVirtioDiskDevice(driver, vm, disk);
- break;
+ for (i = 0; i < vm->def->ndisks; i++) {
+ if (virDomainDiskDefCheckDuplicateInfo(vm->def->disks[i], disk) < 0)
+ goto cleanup;
+ }
- case VIR_DOMAIN_DISK_BUS_SCSI:
- ret = qemuDomainAttachSCSIDisk(driver, vm, disk);
+ switch ((virDomainDiskBus) disk->bus) {
+ case VIR_DOMAIN_DISK_BUS_USB:
+ if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("disk device='lun' is not supported for usb
bus"));
break;
-
- case VIR_DOMAIN_DISK_BUS_IDE:
- case VIR_DOMAIN_DISK_BUS_FDC:
- case VIR_DOMAIN_DISK_BUS_XEN:
- case VIR_DOMAIN_DISK_BUS_UML:
- case VIR_DOMAIN_DISK_BUS_SATA:
- case VIR_DOMAIN_DISK_BUS_SD:
- /* Note that SD card hotplug support should be added only once
- * they support '-device' (don't require -drive only).
- * See also: qemuDiskBusNeedsDriveArg */
- case VIR_DOMAIN_DISK_BUS_LAST:
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
- _("disk bus '%s' cannot be hotplugged."),
- virDomainDiskBusTypeToString(disk->bus));
}
+ ret = qemuDomainAttachUSBMassStorageDevice(driver, vm, disk);
+ break;
+
+ case VIR_DOMAIN_DISK_BUS_VIRTIO:
+ ret = qemuDomainAttachVirtioDiskDevice(driver, vm, disk);
break;
- case VIR_DOMAIN_DISK_DEVICE_CDROM:
- case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
- case VIR_DOMAIN_DISK_DEVICE_LAST:
+ case VIR_DOMAIN_DISK_BUS_SCSI:
+ ret = qemuDomainAttachSCSIDisk(driver, vm, disk);
break;
+
+ case VIR_DOMAIN_DISK_BUS_IDE:
+ case VIR_DOMAIN_DISK_BUS_FDC:
+ case VIR_DOMAIN_DISK_BUS_XEN:
+ case VIR_DOMAIN_DISK_BUS_UML:
+ case VIR_DOMAIN_DISK_BUS_SATA:
+ case VIR_DOMAIN_DISK_BUS_SD:
+ /* Note that SD card hotplug support should be added only once
+ * they support '-device' (don't require -drive only).
+ * See also: qemuDiskBusNeedsDriveArg */
+ case VIR_DOMAIN_DISK_BUS_LAST:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("disk bus '%s' cannot be hotplugged."),
+ virDomainDiskBusTypeToString(disk->bus));
}
cleanup:
--
2.17.1
4:16 a.m.
On 10/05/2018 02:13 PM, Peter Krempa wrote:
Fixes regression in media changing/disk hotplug as the ordering of
the
alias allocation and disk preparation was bad.
v2:
- be more explicit about old and new definitions used in certain steps
- clean up legacy hotplug to not access old disk source definition
- also treat network disks as 'raw' if the format was not provided
Peter Krempa (12):
Revert "qemu: hotplug: Prepare disk source in
qemuDomainAttachDeviceDiskLive"
Revert "qemu: hotplug: consolidate media change code paths"
qemu: hotplug: Don't pretend that we support secrets for media change
qemu: domain: Assume 'raw' default storage format also for network
storage
qemu: hotplug: Remove code handling possible missing disk source
format
qemu: hotplug: Allow specifying explicit source for disk backend
hotplug code
qemu: hotplug: Be explicit about old/new sources when changing media
qemu: hotplug: Prepare disk source for media changing
qemu: hotplug: Add wrapper for disk hotplug code
qemu: conf: Export qemuAddSharedDisk
qemu: hotplug: Split out media change code from disk hotplug
qemu: hotplug: Refactor qemuDomainAttachDeviceDiskLiveInternal
src/qemu/qemu_conf.c | 2 +-
src/qemu/qemu_conf.h | 5 +
src/qemu/qemu_domain.c | 4 +-
src/qemu/qemu_driver.c | 7 +-
src/qemu/qemu_hotplug.c | 276 ++++++++++--------
src/qemu/qemu_hotplug.h | 9 +-
tests/qemuhotplugtest.c | 2 +-
.../disk-source-pool-mode.args | 6 +-
tests/qemuxml2argvdata/disk-source-pool.args | 4 +-
.../disk-source-pool-mode.xml | 6 +-
tests/qemuxml2xmloutdata/disk-source-pool.xml | 2 +-
11 files changed, 185 insertions(+), 138 deletions(-)
ACK series. I can also confirm that this passes my testing which caused
this in the first place.
Michal
2238
days inactive
2241
days old
15 comments
3 participants
participants (3)
-
John Ferlan -
Michal Privoznik -
Peter Krempa