[libvirt] [PATCH 0/7] qemu: Fix removable devices without a tray
Peter Krempa (7): qemu: Move struct qemuDomainDiskInfo to qemu_domain.h qemu: Extract more information about qemu drives qemu: Move and rename qemuDomainCheckEjectableMedia to qemuProcessRefreshDisks qemu: process: Fix and improve disk data extraction qemu: hotplug: Extract code for waiting for tray eject qemu: hotplug: Fix error reported when cdrom tray is locked qemu: hotplug: wait for the tray to eject only for drives with a tray src/qemu/qemu_conf.h | 7 --- src/qemu/qemu_domain.h | 13 ++++ src/qemu/qemu_hotplug.c | 141 ++++++++++++++++--------------------------- src/qemu/qemu_hotplug.h | 3 - src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 18 ------ src/qemu/qemu_monitor.h | 3 - src/qemu/qemu_monitor_json.c | 12 ++-- src/qemu/qemu_process.c | 58 +++++++++++++++++- src/qemu/qemu_process.h | 5 ++ tests/qemumonitorjsontest.c | 25 +++++++- 11 files changed, 157 insertions(+), 130 deletions(-) -- 2.8.2
--- src/qemu/qemu_conf.h | 7 ------- src/qemu/qemu_domain.h | 7 +++++++ tests/qemumonitorjsontest.c | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index a714b84..1fdef70 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -287,13 +287,6 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver); virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr driver, bool refresh); -struct qemuDomainDiskInfo { - bool removable; - bool locked; - bool tray_open; - int io_status; -}; - typedef struct _qemuSharedDeviceEntry qemuSharedDeviceEntry; typedef qemuSharedDeviceEntry *qemuSharedDeviceEntryPtr; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f99f0bb..e29df07 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -304,6 +304,13 @@ struct _qemuDomainDiskPrivate { # define QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev) \ ((qemuDomainHostdevPrivatePtr) (hostdev)->privateData) +struct qemuDomainDiskInfo { + bool removable; + bool locked; + bool tray_open; + int io_status; +}; + typedef struct _qemuDomainHostdevPrivate qemuDomainHostdevPrivate; typedef qemuDomainHostdevPrivate *qemuDomainHostdevPrivatePtr; struct _qemuDomainHostdevPrivate { diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 829de90..229ccac 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -22,7 +22,7 @@ #include "testutils.h" #include "testutilsqemu.h" #include "qemumonitortestutils.h" -#include "qemu/qemu_conf.h" +#include "qemu/qemu_domain.h" #include "qemu/qemu_monitor_json.h" #include "virthread.h" #include "virerror.h" -- 2.8.2
Extract whether a given drive has a tray and whether there is no image inserted. Negative logic for the image insertion is chosen so that the flag is set only if we are certain of the fact. --- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_monitor_json.c | 12 +++++++----- tests/qemumonitorjsontest.c | 23 +++++++++++++++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e29df07..825036c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -307,7 +307,9 @@ struct _qemuDomainDiskPrivate { struct qemuDomainDiskInfo { bool removable; bool locked; + bool tray; bool tray_open; + bool empty; int io_status; }; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 64a8765..0508fe6 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1832,11 +1832,13 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, goto cleanup; } - /* Don't check for success here, because 'tray_open' is presented iff - * medium is ejected. - */ - ignore_value(virJSONValueObjectGetBoolean(dev, "tray_open", - &info->tray_open)); + /* 'tray_open' is present only if the device has a tray */ + if (virJSONValueObjectGetBoolean(dev, "tray_open", &info->tray_open) == 0) + info->tray = true; + + /* presence of 'inserted' notifies that a medium is in the device */ + if (!virJSONValueObjectGetObject(dev, "inserted")) + info->empty = true; /* Missing io-status indicates no error */ if ((status = virJSONValueObjectGetString(dev, "io-status"))) { diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 229ccac..87b1a8f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -114,6 +114,14 @@ const char *queryBlockReply = " }," " \"tray_open\": false," " \"type\": \"unknown\"" +" }," +" {" +" \"io-status\": \"ok\"," +" \"device\": \"drive-ide0-1-1\"," +" \"locked\": false," +" \"removable\": true," +" \"tray_open\": false," +" \"type\": \"unknown\"" " }" " ]," " \"id\": \"libvirt-10\"" @@ -1404,12 +1412,27 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data) info->locked = true; info->removable = true; + info->tray = true; + if (virHashAddEntry(expectedBlockDevices, "ide0-1-0", info) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Unable to create expectedBlockDevices hash table"); goto cleanup; } + if (VIR_ALLOC(info) < 0) + goto cleanup; + + info->removable = true; + info->tray = true; + info->empty = true; + + if (virHashAddEntry(expectedBlockDevices, "ide0-1-1", info) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "Unable to create expectedBlockDevices hash table"); + goto cleanup; + } + if (qemuMonitorTestAddItem(test, "query-block", queryBlockReply) < 0) goto cleanup; -- 2.8.2
Move it to a more sane place since it's refreshing data about disks. --- src/qemu/qemu_hotplug.c | 48 ---------------------------------------- src/qemu/qemu_hotplug.h | 3 --- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_process.c | 56 ++++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_process.h | 5 +++++ 5 files changed, 59 insertions(+), 55 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5f34a76..286a4a4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -283,54 +283,6 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto cleanup; } -int -qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainAsyncJob asyncJob) -{ - qemuDomainObjPrivatePtr priv = vm->privateData; - virHashTablePtr table = NULL; - int ret = -1; - size_t i; - - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { - table = qemuMonitorGetBlockInfo(priv->mon); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto cleanup; - } - - if (!table) - goto cleanup; - - for (i = 0; i < vm->def->ndisks; i++) { - virDomainDiskDefPtr disk = vm->def->disks[i]; - struct qemuDomainDiskInfo *info; - - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK || - disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - continue; - } - - info = qemuMonitorBlockInfoLookup(table, disk->info.alias); - if (!info) - goto cleanup; - - if (info->tray_open) { - if (virDomainDiskGetSource(disk)) - ignore_value(virDomainDiskSetSource(disk, NULL)); - - disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN; - } else { - disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED; - } - } - - ret = 0; - - cleanup: - virHashFree(table); - return ret; -} static int qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 868b4cf..165d345 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -33,9 +33,6 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, virStorageSourcePtr newsrc, bool force); -int qemuDomainCheckEjectableMedia(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainAsyncJob asyncJob); int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainControllerDefPtr controller); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 25093ac..c5b2963 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3263,7 +3263,7 @@ qemuMigrationBegin(virConnectPtr conn, * We don't want to require them on the destination. */ if (!(flags & VIR_MIGRATE_OFFLINE) && - qemuDomainCheckEjectableMedia(driver, vm, asyncJob) < 0) + qemuProcessRefreshDisks(driver, vm, asyncJob) < 0) goto endjob; if (!(xml = qemuMigrationBeginPhase(driver, vm, xmlin, dname, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b2669c0..f2e284f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3306,7 +3306,7 @@ qemuProcessReconnect(void *opaque) if (qemuProcessFiltersInstantiate(obj->def)) goto error; - if (qemuDomainCheckEjectableMedia(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) + if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error; if (qemuRefreshVirtioChannelState(driver, obj) < 0) @@ -5316,8 +5316,8 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessUpdateVideoRamSize(driver, vm, asyncJob) < 0) goto cleanup; - VIR_DEBUG("Updating ejectable media status"); - if (qemuDomainCheckEjectableMedia(driver, vm, asyncJob) < 0) + VIR_DEBUG("Updating disk data"); + if (qemuProcessRefreshDisks(driver, vm, asyncJob) < 0) goto cleanup; if (flags & VIR_QEMU_PROCESS_START_AUTODESTROY && @@ -6231,3 +6231,53 @@ bool qemuProcessAutoDestroyActive(virQEMUDriverPtr driver, cb = virCloseCallbacksGet(driver->closeCallbacks, vm, NULL); return cb == qemuProcessAutoDestroy; } + + +int +qemuProcessRefreshDisks(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virHashTablePtr table = NULL; + int ret = -1; + size_t i; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { + table = qemuMonitorGetBlockInfo(priv->mon); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + } + + if (!table) + goto cleanup; + + for (i = 0; i < vm->def->ndisks; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + struct qemuDomainDiskInfo *info; + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK || + disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { + continue; + } + + info = qemuMonitorBlockInfoLookup(table, disk->info.alias); + if (!info) + goto cleanup; + + if (info->tray_open) { + if (virDomainDiskGetSource(disk)) + ignore_value(virDomainDiskSetSource(disk, NULL)); + + disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN; + } else { + disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED; + } + } + + ret = 0; + + cleanup: + virHashFree(table); + return ret; +} diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 623e1e7..61d9f56 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -192,4 +192,9 @@ int qemuRefreshVirtioChannelState(virQEMUDriverPtr driver, int qemuProcessRefreshBalloonState(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob); + +int qemuProcessRefreshDisks(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob); + #endif /* __QEMU_PROCESS_H__ */ -- 2.8.2
Extract information for all disks and update tray state and source only for removable drives. Additionally store whether a drive is removable and whether it has a tray. --- src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_monitor.c | 18 ------------------ src/qemu/qemu_monitor.h | 3 --- src/qemu/qemu_process.c | 28 +++++++++++++++------------- 4 files changed, 19 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 825036c..9ac0209 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -299,6 +299,10 @@ struct _qemuDomainDiskPrivate { /* for storage devices using auth/secret * NB: *not* to be written to qemu domain object XML */ qemuDomainSecretInfoPtr secinfo; + + /* information about the device */ + bool tray; /* device has tray */ + bool removable; /* device media can be removed/changed */ }; # define QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev) \ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7d1ca91..a0060cc 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1826,24 +1826,6 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon) } -struct qemuDomainDiskInfo * -qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, - const char *dev) -{ - struct qemuDomainDiskInfo *info; - - VIR_DEBUG("blockInfo=%p dev=%s", blockInfo, NULLSTR(dev)); - - if (!(info = virHashLookup(blockInfo, dev))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find info for device '%s'"), - NULLSTR(dev)); - } - - return info; -} - - /** * qemuMonitorGetAllBlockStatsInfo: * @mon: monitor object diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6359314..a1cbc94 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -404,9 +404,6 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, int qemuMonitorBlockIOStatusToError(const char *status); virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon); -struct qemuDomainDiskInfo * -qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, - const char *dev_name); typedef struct _qemuBlockStats qemuBlockStats; typedef qemuBlockStats *qemuBlockStatsPtr; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f2e284f..106ffcd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6254,25 +6254,27 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskpriv = QEMU_DOMAIN_DISK_PRIVATE(disk); struct qemuDomainDiskInfo *info; - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK || - disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - continue; - } - - info = qemuMonitorBlockInfoLookup(table, disk->info.alias); - if (!info) - goto cleanup; + if (!(info = virHashLookup(table, disk->info.alias))) + continue; - if (info->tray_open) { - if (virDomainDiskGetSource(disk)) + if (info->removable) { + if (info->empty) ignore_value(virDomainDiskSetSource(disk, NULL)); - disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN; - } else { - disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED; + 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; } ret = 0; -- 2.8.2
On 24.05.2016 15:17, Peter Krempa wrote:
Extract information for all disks and update tray state and source only for removable drives. Additionally store whether a drive is removable and whether it has a tray. --- src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_monitor.c | 18 ------------------ src/qemu/qemu_monitor.h | 3 --- src/qemu/qemu_process.c | 28 +++++++++++++++------------- 4 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 825036c..9ac0209 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -299,6 +299,10 @@ struct _qemuDomainDiskPrivate { /* for storage devices using auth/secret * NB: *not* to be written to qemu domain object XML */ qemuDomainSecretInfoPtr secinfo; + + /* information about the device */ + bool tray; /* device has tray */ + bool removable; /* device media can be removed/changed */
I wonder whether we should drop these one day and replace them with struct qemuDomainDiskInfo which lives right below this struct (not to be seen in the context though). But that can be done later, not necessarily now.
};
# define QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev) \ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7d1ca91..a0060cc 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1826,24 +1826,6 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon) }
-struct qemuDomainDiskInfo * -qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, - const char *dev) -{ - struct qemuDomainDiskInfo *info; - - VIR_DEBUG("blockInfo=%p dev=%s", blockInfo, NULLSTR(dev)); - - if (!(info = virHashLookup(blockInfo, dev))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find info for device '%s'"), - NULLSTR(dev)); - } - - return info; -} - - /** * qemuMonitorGetAllBlockStatsInfo: * @mon: monitor object diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6359314..a1cbc94 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -404,9 +404,6 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
int qemuMonitorBlockIOStatusToError(const char *status); virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon); -struct qemuDomainDiskInfo * -qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, - const char *dev_name);
typedef struct _qemuBlockStats qemuBlockStats; typedef qemuBlockStats *qemuBlockStatsPtr; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f2e284f..106ffcd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6254,25 +6254,27 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver,
for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; + qemuDomainDiskPrivatePtr diskpriv = QEMU_DOMAIN_DISK_PRIVATE(disk); struct qemuDomainDiskInfo *info;
- if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK || - disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - continue; - } - - info = qemuMonitorBlockInfoLookup(table, disk->info.alias); - if (!info) - goto cleanup; + if (!(info = virHashLookup(table, disk->info.alias))) + continue;
- if (info->tray_open) { - if (virDomainDiskGetSource(disk)) + if (info->removable) { + if (info->empty) ignore_value(virDomainDiskSetSource(disk, NULL));
- disk->tray_status = VIR_DOMAIN_DISK_TRAY_OPEN; - } else { - disk->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED; + 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;
If we go with my suggestion, this code wouldn't be needed.
}
ret = 0;
Michal
On Wed, May 25, 2016 at 09:47:59 +0200, Michal Privoznik wrote:
On 24.05.2016 15:17, Peter Krempa wrote:
Extract information for all disks and update tray state and source only for removable drives. Additionally store whether a drive is removable and whether it has a tray. --- src/qemu/qemu_domain.h | 4 ++++ src/qemu/qemu_monitor.c | 18 ------------------ src/qemu/qemu_monitor.h | 3 --- src/qemu/qemu_process.c | 28 +++++++++++++++------------- 4 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 825036c..9ac0209 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -299,6 +299,10 @@ struct _qemuDomainDiskPrivate { /* for storage devices using auth/secret * NB: *not* to be written to qemu domain object XML */ qemuDomainSecretInfoPtr secinfo; + + /* information about the device */ + bool tray; /* device has tray */ + bool removable; /* device media can be removed/changed */
I wonder whether we should drop these one day and replace them with struct qemuDomainDiskInfo which lives right below this struct (not to
My plan is to actually remove qemuDomainDisk info in the future. The reason why I didn't do so is that some of the data in qemuDomainDiskInfo are just temporary or are represented differently in the disk definition. These two are true for the lifetime of the disk and won't change so therefore I've copied it and discarded the rest. Also one of my plans is to unify all 3 monitor APIs that call 'query-block' into one data structure which will contain all the data. Peter
The code grew rather convoluted. Extract it to a separate function. --- src/qemu/qemu_hotplug.c | 85 ++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 286a4a4..4c3f7da 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -145,6 +145,40 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, } +static int +qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + const char *driveAlias, + bool force) +{ + unsigned long long now; + int rc; + + if (virTimeMillisNow(&now) < 0) + return -1; + + while (disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) { + if ((rc = virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT)) < 0) + return -1; + + if (rc > 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timed out waiting for disk tray status update")); + return -1; + } + } + + /* re-issue ejection command to pop out the media */ + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorEjectMedia(qemuDomainGetMonitor(vm), driveAlias, force); + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; + + return 0; +} + + /** * qemuDomainChangeEjectableMedia: * @driver: qemu driver structure @@ -172,8 +206,6 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; const char *format = NULL; char *sourcestr = NULL; - bool ejectRetry = false; - unsigned long long now; if (!disk->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -195,45 +227,20 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (!(driveAlias = qemuDeviceDriveHostAlias(disk))) goto error; - do { - qemuDomainObjEnterMonitor(driver, vm); - rc = qemuMonitorEjectMedia(priv->mon, driveAlias, force); - if (qemuDomainObjExitMonitor(driver, vm) < 0) - goto cleanup; - - /* skip all retrying if qemu doesn't notify us on tray change */ - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_TRAY_MOVED)) { - if (rc == 0) - break; - - if (rc < 0) - goto error; - } - - if (rc < 0) { - /* we've already tried, error out */ - if (ejectRetry) - goto error; - - ejectRetry = true; - VIR_DEBUG("tray may be locked, wait for the guest to unlock " - "the tray and try to eject it again"); - } + qemuDomainObjEnterMonitor(driver, vm); + rc = qemuMonitorEjectMedia(priv->mon, driveAlias, force); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; - if (virTimeMillisNow(&now) < 0) + /* If the tray change event is supported wait for it to open. */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_TRAY_MOVED)) { + if (qemuHotplugWaitForTrayEject(driver, vm, disk, driveAlias, force) < 0) goto error; - - while (disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) { - int wait_rc = virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT); - if (wait_rc > 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("timed out waiting for " - "disk tray status update")); - } - if (wait_rc != 0) - goto error; - } - } while (rc < 0); + } else { + /* otherwise report possible errors from the attempt to eject the media*/ + if (rc < 0) + goto error; + } if (!virStorageSourceIsEmpty(newsrc)) { qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); -- 2.8.2
Commit 1fad65d49aae364576bd91352a001249510f8d4e used a really big hammer and overwrote the error message that might be reported by qemu if the tray is locked. Fix it by reporting the error only if no error is currently set. Error after commit mentioned above: error: internal error: timed out waiting for disk tray status update New error: error: internal error: unable to execute QEMU command 'eject': Tray of device 'drive-ide0-0-0' is not open --- src/qemu/qemu_hotplug.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4c3f7da..cb566d4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -163,8 +163,12 @@ qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver, return -1; if (rc > 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("timed out waiting for disk tray status update")); + /* the caller called qemuMonitorEjectMedia which usually reports an + * error. Report the failure in an off-chance that it didn't. */ + if (!virGetLastError()) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timed out waiting for disk tray status update")); + } return -1; } } -- 2.8.2
Use the detected tray presence flag to trigger the tray waiting code only if the given storage device in qemu reports to have a tray. This is necessary as the floppy device lost it's tray as of qemu commit: commit abb3e55b5b718d6392441f56ba0729a62105ac56 Author: Max Reitz <mreitz@redhat.com> Date: Fri Jan 29 20:49:12 2016 +0100 Revert "hw/block/fdc: Implement tray status" --- src/qemu/qemu_hotplug.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cb566d4..807aaf8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -208,6 +208,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, int ret = -1, rc; char *driveAlias = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); const char *format = NULL; char *sourcestr = NULL; @@ -236,8 +237,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; - /* If the tray change event is supported wait for it to open. */ - if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_TRAY_MOVED)) { + /* If the tray is present and tray change event is supported wait for it to open. */ + if (diskPriv->tray && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_TRAY_MOVED)) { if (qemuHotplugWaitForTrayEject(driver, vm, disk, driveAlias, force) < 0) goto error; } else { @@ -247,8 +249,6 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } if (!virStorageSourceIsEmpty(newsrc)) { - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - if (qemuGetDriveSourceString(newsrc, diskPriv->secinfo, &sourcestr) < 0) goto error; -- 2.8.2
On 24.05.2016 15:17, Peter Krempa wrote:
Peter Krempa (7): qemu: Move struct qemuDomainDiskInfo to qemu_domain.h qemu: Extract more information about qemu drives qemu: Move and rename qemuDomainCheckEjectableMedia to qemuProcessRefreshDisks qemu: process: Fix and improve disk data extraction qemu: hotplug: Extract code for waiting for tray eject qemu: hotplug: Fix error reported when cdrom tray is locked qemu: hotplug: wait for the tray to eject only for drives with a tray
src/qemu/qemu_conf.h | 7 --- src/qemu/qemu_domain.h | 13 ++++ src/qemu/qemu_hotplug.c | 141 ++++++++++++++++--------------------------- src/qemu/qemu_hotplug.h | 3 - src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_monitor.c | 18 ------ src/qemu/qemu_monitor.h | 3 - src/qemu/qemu_monitor_json.c | 12 ++-- src/qemu/qemu_process.c | 58 +++++++++++++++++- src/qemu/qemu_process.h | 5 ++ tests/qemumonitorjsontest.c | 25 +++++++- 11 files changed, 157 insertions(+), 130 deletions(-)
ACK series. Michal
participants (2)
-
Michal Privoznik -
Peter Krempa