[libvirt] [PATCHv2 4/4] qemu: Implement shared memory device hot-unplug
lhuang
lhuang at redhat.com
Tue Dec 15 10:07:07 UTC 2015
On 12/10/2015 09:43 AM, John Ferlan wrote:
>
> On 11/26/2015 04:07 AM, Luyao Huang wrote:
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 4 ++-
>> src/qemu/qemu_hotplug.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++-
>> src/qemu/qemu_hotplug.h | 3 ++
>> 3 files changed, 99 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 3c25c07..9e7429a 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -7881,6 +7881,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
>> case VIR_DOMAIN_DEVICE_MEMORY:
>> ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory);
>> break;
>> + case VIR_DOMAIN_DEVICE_SHMEM:
>> + ret = qemuDomainDetachShmemDevice(driver, vm, dev->data.shmem);
>> + break;
>>
>> case VIR_DOMAIN_DEVICE_FS:
>> case VIR_DOMAIN_DEVICE_INPUT:
>> @@ -7892,7 +7895,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
>> case VIR_DOMAIN_DEVICE_SMARTCARD:
>> case VIR_DOMAIN_DEVICE_MEMBALLOON:
>> case VIR_DOMAIN_DEVICE_NVRAM:
>> - case VIR_DOMAIN_DEVICE_SHMEM:
>> case VIR_DOMAIN_DEVICE_REDIRDEV:
>> case VIR_DOMAIN_DEVICE_NONE:
>> case VIR_DOMAIN_DEVICE_TPM:
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index c5e544d..4ab05f3 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -3347,6 +3347,45 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
>> }
>>
>>
>> +static int
>> +qemuDomainRemoveShmemDevice(virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + virDomainShmemDefPtr shmem)
>> +{
>> + virObjectEventPtr event;
>> + char *charAlias = NULL;
>> + qemuDomainObjPrivatePtr priv = vm->privateData;
>> + ssize_t idx;
>> + int rc = 0;
>> +
>> + VIR_DEBUG("Removing shared memory device %s from domain %p %s",
>> + shmem->info.alias, vm, vm->def->name);
>> +
>> + if (shmem->server.enabled) {
>> + if (virAsprintf(&charAlias, "char%s", shmem->info.alias) < 0)
>> + return -1;
>> +
>> + qemuDomainObjEnterMonitor(driver, vm);
>> + rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
>> + VIR_FREE(charAlias);
>> + if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> + return -1;
>> + }
>> +
> Auditing code?
I have removed them since i remember @Martin will finish the audit part.
>> + if (rc < 0)
>> + return -1;
>> +
> I know this is a copy of the RemoveRNGDevice; however, this code doesn't
> remove an 'obj'. In fact, if !shmem->server.enabled, then we don't enter
> the monitor at all.
>
> Thus the following event probably won't happen...
I am not sure what your mean is ... i guess your mean the device remove
event we get from qmp monitor won't happen ? we will get that event if
qemu remove shmem device success, it should always happen if qemu really
remove it and there is no bugs on qemu :)
>> + if ((event = virDomainEventDeviceRemovedNewFromObj(vm, shmem->info.alias)))
>> + qemuDomainEventQueue(driver, event);
>> +
>> + if ((idx = virDomainShmemFind(vm->def, shmem, true)) >= 0)
>> + virDomainShmemRemove(vm->def, idx);
>> + qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL);
>> + virDomainShmemDefFree(shmem);
>> + return 0;
>> +}
>> +
>> +
>> int
>> qemuDomainRemoveDevice(virQEMUDriverPtr driver,
>> virDomainObjPtr vm,
>> @@ -3378,6 +3417,10 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
>> ret = qemuDomainRemoveMemoryDevice(driver, vm, dev->data.memory);
>> break;
>>
>> + case VIR_DOMAIN_DEVICE_SHMEM:
>> + ret = qemuDomainRemoveShmemDevice(driver, vm, dev->data.shmem);
>> + break;
>> +
>> case VIR_DOMAIN_DEVICE_NONE:
>> case VIR_DOMAIN_DEVICE_LEASE:
>> case VIR_DOMAIN_DEVICE_FS:
>> @@ -3391,7 +3434,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
>> case VIR_DOMAIN_DEVICE_SMARTCARD:
>> 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_LAST:
>> @@ -4378,3 +4420,53 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
>> qemuDomainResetDeviceRemoval(vm);
>> return ret;
>> }
>> +
>> +
>> +int
>> +qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + virDomainShmemDefPtr shmem)
>> +{
>> + qemuDomainObjPrivatePtr priv = vm->privateData;
>> + ssize_t idx;
>> + virDomainShmemDefPtr tmpshmem;
>> + int rc;
>> + int ret = -1;
>> +
>> + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> + _("qemu does not support -device"));
>> + return -1;
>> + }
> Well it's here, but not in AttachShmemDevice...
I must forgot add it in the AttachShmemDevice, thanks for pointing out.
>> +
>> + if ((idx = virDomainShmemFind(vm->def, shmem, true)) < 0) {
> Again we could use a 'flags' argument instead... Of course there isn't
> a "false" argument to any of the added callers to date, so not sure of
> the need for it... Could still use/add flags and keep it as "unused"
> for now.
Okay
>> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> + _("device not present in domain configuration"));
> s/(("/(("shared memory /
I will fix it in next version,
Thanks a lot for your review
Luyao
>
> John
>> + return -1;
>> + }
>> +
>> + tmpshmem = vm->def->shmems[idx];
>> +
>> + if (!tmpshmem->info.alias) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> + _("alias not set for shared memory device"));
>> + return -1;
>> + }
>> +
>> + qemuDomainMarkDeviceForRemoval(vm, &tmpshmem->info);
>> +
>> + qemuDomainObjEnterMonitor(driver, vm);
>> + rc = qemuMonitorDelDevice(priv->mon, tmpshmem->info.alias);
>> + if (qemuDomainObjExitMonitor(driver, vm) || rc < 0)
>> + goto cleanup;
>> +
>> + rc = qemuDomainWaitForDeviceRemoval(vm);
>> + if (rc == 0 || rc == 1)
>> + ret = qemuDomainRemoveShmemDevice(driver, vm, tmpshmem);
>> + else
>> + ret = 0;
>> +
>> + cleanup:
>> + qemuDomainResetDeviceRemoval(vm);
>> + return ret;
>> +}
>> diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
>> index 60137a6..1b18e8a 100644
>> --- a/src/qemu/qemu_hotplug.h
>> +++ b/src/qemu/qemu_hotplug.h
>> @@ -112,6 +112,9 @@ int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
>> int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
>> virDomainObjPtr vm,
>> virDomainShmemDefPtr shmem);
>> +int qemuDomainDetachShmemDevice(virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + virDomainShmemDefPtr shmem);
>>
>> int
>> qemuDomainChrInsert(virDomainDefPtr vmdef,
>>
More information about the libvir-list
mailing list