Re: [libvirt] [PATCH] qemu: Fix crash when changing media of cdrom or floppy disk

The disk def could be free'ed by qemuDomainChangeEjectableMedia for cdrom or floppy disk. This moves the setup/checking before the attaching takes place. --- src/qemu/qemu_driver.c | 29 ++++++++++++++--------------- src/qemu/qemu_process.c | 16 ++++++++++++---- src/qemu/qemu_process.h | 3 ++- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 906501b..3509880 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5829,8 +5829,20 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK && disk->shared && - (qemuCheckSharedDisk(driver->sharedDisks, disk) < 0)) - goto end; + disk->src) { + if (qemuCheckSharedDisk(driver->sharedDisks, disk, false) < 0) + goto end; + + if (qemuAddSharedDisk(driver->sharedDisks, disk->src) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to add disk '%s' to shared disk table"), + disk->src); + goto end; + } + } + + if (qemuSetUnprivSGIO(disk) < 0) + goto end;

@@ -5883,19 +5895,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, NULLSTR(disk->src)); } - if (ret == 0) { - if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK && - disk->shared && - disk->src) { - if (qemuAddSharedDisk(driver->sharedDisks, disk->src) < 0) - VIR_WARN("Failed to add disk '%s' to shared disk table", - disk->src); - } - - if (qemuSetUnprivSGIO(disk) < 0) - VIR_WARN("Failed to set unpriv_sgio of disk '%s'", disk->src); - } -

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 98ed552..ee5a10c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3442,7 +3442,14 @@ qemuSetUnprivSGIO(virDomainDiskDefPtr disk) return 0; } -/* Check if a shared disk's setting conflicts with the conf +/* qemuCheckSharedDisk: + * @sharedDisks: Shared disk hash table + * @disk: Disk def + * @after_adding: Whether this function is involked after + * adding the disk to the hash table by + * qemuAddSharedDisk. + * + * Check if a shared disk's setting conflicts with the conf * used by other domain(s). Currently only checks the sgio * setting. Note that this should only be called for disk with * block source. @@ -3451,7 +3458,8 @@ qemuSetUnprivSGIO(virDomainDiskDefPtr disk) */ int qemuCheckSharedDisk(virHashTablePtr sharedDisks, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + bool after_adding)

{ int val; size_t *ref = NULL; @@ -3470,7 +3478,7 @@ qemuCheckSharedDisk(virHashTablePtr sharedDisks, if (!(ref = virHashLookup(sharedDisks, key))) goto cleanup; - if (ref == (void *)0x1) + if (after_adding && ref == (void *)0x1) goto cleanup; if (virGetDeviceUnprivSGIO(disk->src, NULL, &val) < 0) { @@ -3846,7 +3854,7 @@ int qemuProcessStart(virConnectPtr conn, if (qemuAddSharedDisk(driver->sharedDisks, disk->src) < 0) goto cleanup; - if (qemuCheckSharedDisk(driver->sharedDisks, disk) < 0) + if (qemuCheckSharedDisk(driver->sharedDisks, disk, true) < 0) goto cleanup; } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 313fa39..ff294e7 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -101,6 +101,7 @@ virBitmapPtr qemuPrepareCpumap(virQEMUDriverPtr driver, int qemuSetUnprivSGIO(virDomainDiskDefPtr disk); int qemuCheckSharedDisk(virHashTablePtr sharedDisks, - virDomainDiskDefPtr disk); + virDomainDiskDefPtr disk, + bool after_adding); #endif /* __QEMU_PROCESS_H__ */