10:44 a.m.
We need to update private data to handle it properly.
Peter Krempa (3):
qemuProcessRefreshDisks: Properly compare tray status
qemuProcessRefreshDisks: Extract update of a single disk
qemu: hotplug: Update disk private data after hotplug
src/qemu/qemu_hotplug.c | 21 ++++++++++++----
src/qemu/qemu_process.c | 55 ++++++++++++++++++++++++-----------------
src/qemu/qemu_process.h | 3 +++
tests/qemuhotplugtest.c | 40 ++++++++++++++++++++----------
4 files changed, 79 insertions(+), 40 deletions(-)
--
2.39.2
10:44 a.m.
New subject: [PATCH 1/3] qemuProcessRefreshDisks: Properly compare tray status
The code compares the 'tray_open' boolean from 'struct
qemuDomainDiskInfo' directly against 'disk->tray_status' which is
declared as virDomainDiskTray (enum). Now the logic works correctly
because the _OPEN enum has value '1'.
Separate the event emission code from the update code and remember the
old tray state in a separate variable rather than having the sneaky
logic we have today.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_process.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b9e9a7d320..0dc49b87d8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8695,6 +8695,7 @@ qemuProcessRefreshDisks(virDomainObj *vm,
qemuDomainDiskPrivate *diskpriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
struct qemuDomainDiskInfo *info;
const char *entryname = disk->info.alias;
+ virDomainDiskTray old_tray_status = disk->tray_status;
if (diskpriv->qomName)
entryname = diskpriv->qomName;
@@ -8703,31 +8704,32 @@ qemuProcessRefreshDisks(virDomainObj *vm,
continue;
if (info->removable) {
- bool emitEvent = info->tray_open != disk->tray_status;
- int reason;
-
if (info->empty)
virDomainDiskEmptySource(disk);
if (info->tray) {
- if (info->tray_open) {
- reason = VIR_DOMAIN_EVENT_TRAY_CHANGE_OPEN;
+ if (info->tray_open)
disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN;
- } else {
- reason = VIR_DOMAIN_EVENT_TRAY_CHANGE_CLOSE;
+ else
disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED;
- }
-
- if (emitEvent) {
- virObjectEvent *event = virDomainEventTrayChangeNewFromObj(vm,
disk->info.alias, reason);
- virObjectEventStateQueue(driver->domainEventState, event);
- }
}
}
/* fill in additional data */
diskpriv->removable = info->removable;
diskpriv->tray = info->tray;
+
+ if (diskpriv->tray &&
+ old_tray_status != disk->tray_status) {
+ virDomainEventTrayChangeReason reason = VIR_DOMAIN_EVENT_TRAY_CHANGE_OPEN;
+ virObjectEvent *event;
+
+ if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_CLOSED)
+ reason = VIR_DOMAIN_EVENT_TRAY_CHANGE_CLOSE;
+
+ event = virDomainEventTrayChangeNewFromObj(vm, disk->info.alias, reason);
+ virObjectEventStateQueue(driver->domainEventState, event);
+ }
}
return 0;
--
2.39.2
10:44 a.m.
New subject: [PATCH 2/3] qemuProcessRefreshDisks: Extract update of a single disk
Extract the logic to update one single disk (without emitting any
events) so that it can be reused when updating the state after a disk
hotplug.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_process.c | 39 ++++++++++++++++++++++++---------------
src/qemu/qemu_process.h | 3 +++
2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0dc49b87d8..7e7178ecec 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8673,6 +8673,29 @@ qemuProcessAutoDestroy(virDomainObj *dom,
}
+void
+qemuProcessRefreshDiskProps(virDomainDiskDef *disk,
+ struct qemuDomainDiskInfo *info)
+{
+ qemuDomainDiskPrivate *diskpriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+ if (info->removable) {
+ if (info->empty)
+ virDomainDiskEmptySource(disk);
+
+ if (info->tray) {
+ if (info->tray_open)
+ disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN;
+ else
+ disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED;
+ }
+ }
+
+ diskpriv->removable = info->removable;
+ diskpriv->tray = info->tray;
+}
+
+
int
qemuProcessRefreshDisks(virDomainObj *vm,
virDomainAsyncJob asyncJob)
@@ -8703,21 +8726,7 @@ qemuProcessRefreshDisks(virDomainObj *vm,
if (!(info = virHashLookup(table, entryname)))
continue;
- if (info->removable) {
- if (info->empty)
- virDomainDiskEmptySource(disk);
-
- if (info->tray) {
- if (info->tray_open)
- disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN;
- else
- disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED;
- }
- }
-
- /* fill in additional data */
- diskpriv->removable = info->removable;
- diskpriv->tray = info->tray;
+ qemuProcessRefreshDiskProps(disk, info);
if (diskpriv->tray &&
old_tray_status != disk->tray_status) {
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index b171f0464c..f5c06c2915 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -235,3 +235,6 @@ bool qemuProcessRebootAllowed(const virDomainDef *def);
void qemuProcessCleanupMigrationJob(virQEMUDriver *driver,
virDomainObj *vm);
+
+void qemuProcessRefreshDiskProps(virDomainDiskDef *disk,
+ struct qemuDomainDiskInfo *info);
--
2.39.2
10:44 a.m.
New subject: [PATCH 3/3] qemu: hotplug: Update disk private data after hotplug
The disk private data contain information about the tray and
removability of the disk. Until recently we didn't support hotplug of
removable disks thus it wasn't a problem but now when you can hotplug a
CDROM you would not be able to open its tray.
Fix it by updating the hotplugged disk the same way we do at startup.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2160435
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 21 ++++++++++++++++-----
tests/qemuhotplugtest.c | 40 +++++++++++++++++++++++++++-------------
2 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3f45a48393..611fd97eb8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -724,12 +724,23 @@ qemuDomainAttachDiskGeneric(virDomainObj *vm,
* As there isn't anything sane to do if this fails, let's just return
* success.
*/
- if (rc == 0 &&
- qemuDiskConfigBlkdeviotuneEnabled(disk)) {
+ if (rc == 0) {
qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
- if (qemuMonitorSetBlockIoThrottle(priv->mon, NULL, diskPriv->qomName,
- &disk->blkdeviotune) < 0)
- VIR_WARN("failed to set blkdeviotune for '%s' of
'%s'", disk->dst, vm->def->name);
+ g_autoptr(GHashTable) blockinfo = NULL;
+
+ if (qemuDiskConfigBlkdeviotuneEnabled(disk)) {
+ if (qemuMonitorSetBlockIoThrottle(priv->mon, NULL, diskPriv->qomName,
+ &disk->blkdeviotune) < 0)
+ VIR_WARN("failed to set blkdeviotune for '%s' of
'%s'", disk->dst, vm->def->name);
+ }
+
+ if ((blockinfo = qemuMonitorGetBlockInfo(priv->mon))) {
+ struct qemuDomainDiskInfo *diskinfo;
+
+ if ((diskinfo = virHashLookup(blockinfo, diskPriv->qomName))) {
+ qemuProcessRefreshDiskProps(disk, diskinfo);
+ }
+ }
}
qemuDomainObjExitMonitor(vm);
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 63a6833cfd..4a8a4c1538 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -709,6 +709,7 @@ mymain(void)
#define QMP_OK "{\"return\": {}}"
+#define QMP_EMPTY_ARRAY "{\"return\": []}"
#define QMP_DEVICE_DELETED(dev) \
"{" \
@@ -747,7 +748,8 @@ mymain(void)
DO_TEST_ATTACH("x86_64", "base-live", "disk-virtio",
false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("x86_64", "base-live", "disk-virtio",
true, true,
"device_del", QMP_OK);
DO_TEST_DETACH("x86_64", "base-live", "disk-virtio",
false, false,
@@ -758,7 +760,8 @@ mymain(void)
DO_TEST_ATTACH("x86_64", "base-live", "disk-usb",
false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("x86_64", "base-live", "disk-usb", true,
true,
"device_del", QMP_OK);
DO_TEST_DETACH("x86_64", "base-live", "disk-usb",
false, false,
@@ -769,7 +772,8 @@ mymain(void)
DO_TEST_ATTACH("x86_64", "base-live", "disk-scsi",
false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("x86_64", "base-live", "disk-scsi",
true, true,
"device_del", QMP_OK);
DO_TEST_DETACH("x86_64", "base-live", "disk-scsi",
false, false,
@@ -785,7 +789,8 @@ mymain(void)
"device_add", QMP_OK,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("x86_64", "base-with-scsi-controller-live",
"disk-scsi-2", true, true,
"device_del", QMP_OK);
DO_TEST_DETACH("x86_64", "base-with-scsi-controller-live",
"disk-scsi-2", false, false,
@@ -797,7 +802,8 @@ mymain(void)
"object-add", QMP_OK,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("x86_64", "base-live",
"disk-scsi-multipath", true, true,
"device_del", QMP_OK);
DO_TEST_DETACH("x86_64", "base-live",
"disk-scsi-multipath", false, false,
@@ -817,7 +823,8 @@ mymain(void)
DO_TEST_ATTACH("s390x", "base-ccw-live", "ccw-virtio",
false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("s390x", "base-ccw-live", "ccw-virtio",
false, false,
"device_del", QMP_DEVICE_DELETED("virtio-disk4")
QMP_OK,
"blockdev-del", QMP_OK,
@@ -826,7 +833,8 @@ mymain(void)
DO_TEST_ATTACH("s390x", "base-ccw-live-with-ccw-virtio",
"ccw-virtio-2", false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("s390x", "base-ccw-live-with-ccw-virtio",
"ccw-virtio-2", false, false,
"device_del", QMP_DEVICE_DELETED("virtio-disk0")
QMP_OK,
@@ -836,7 +844,8 @@ mymain(void)
DO_TEST_ATTACH("s390x", "base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("s390x", "base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, false,
"device_del", QMP_DEVICE_DELETED("virtio-disk0")
QMP_OK,
@@ -847,7 +856,8 @@ mymain(void)
DO_TEST_ATTACH("s390x", "base-ccw-live-with-ccw-virtio",
"ccw-virtio-2-explicit", false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("s390x", "base-ccw-live-with-2-ccw-virtio",
"ccw-virtio-1-explicit", false, true,
"device_del", QMP_DEVICE_DELETED("virtio-disk4")
QMP_OK);
@@ -855,7 +865,8 @@ mymain(void)
DO_TEST_ATTACH("s390x", "base-ccw-live-with-2-ccw-virtio",
"ccw-virtio-1-reverse", false, false,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_ATTACH("x86_64", "base-live", "ivshmem-plain",
false, true,
"object-add", QMP_OK,
@@ -873,7 +884,8 @@ mymain(void)
"disk-scsi-duplicate-wwn", false, false,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_ATTACH("x86_64", "base-live", "hostdev-pci",
false, true,
"device_add", QMP_OK);
@@ -918,7 +930,8 @@ mymain(void)
DO_TEST_ATTACH("x86_64", "base-live", "cdrom-usb",
false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("x86_64", "base-live", "cdrom-usb",
true, true,
"device_del", QMP_OK);
DO_TEST_DETACH("x86_64", "base-live", "cdrom-usb",
false, false,
@@ -929,7 +942,8 @@ mymain(void)
DO_TEST_ATTACH("x86_64", "base-live", "cdrom-scsi",
false, true,
"blockdev-add", QMP_OK,
"blockdev-add", QMP_OK,
- "device_add", QMP_OK);
+ "device_add", QMP_OK,
+ "query-block", QMP_EMPTY_ARRAY);
DO_TEST_DETACH("x86_64", "base-live", "cdrom-scsi",
true, true,
"device_del", QMP_OK);
DO_TEST_DETACH("x86_64", "base-live", "cdrom-scsi",
false, false,
--
2.39.2
10:56 a.m.
On a Wednesday in 2023, Peter Krempa wrote:
We need to update private data to handle it properly.
Peter Krempa (3):
qemuProcessRefreshDisks: Properly compare tray status
qemuProcessRefreshDisks: Extract update of a single disk
qemu: hotplug: Update disk private data after hotplug
src/qemu/qemu_hotplug.c | 21 ++++++++++++----
src/qemu/qemu_process.c | 55 ++++++++++++++++++++++++-----------------
src/qemu/qemu_process.h | 3 +++
tests/qemuhotplugtest.c | 40 ++++++++++++++++++++----------
4 files changed, 79 insertions(+), 40 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
582
days inactive
582
days old
4 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa