[libvirt] [PATCHv2 0/2] qemu: fix the audit log is not correct for memory device
First review: http://www.redhat.com/archives/libvir-list/2015-July/msg00982.html Change in v2: - split it to two patches - fix some small mistake in hot-unplug part I change the code in qemuDomainAttachMemory like what we do in other attach deivce functions. And I have removed the jump in qemuDomainRemoveMemoryDevice, since i just see only one place need jump to audit, so i refactor the code (thanks John's advise). Luyao Huang (2): qemu: fix the audit log is not correct after hot-plug memory success qemu: fix the audit log is not correct after hot-unplug memory device src/qemu/qemu_hotplug.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) -- 1.8.3.1
https://bugzilla.redhat.com/show_bug.cgi?id=1226234#c3 After hot-plug a memory device success, the audit log show that memory update failed: type=VIRT_RESOURCE ... old-mem=1024000 new-mem=1548288 \ exe="/usr/sbin/libvirtd" hostname=? addr=? terminal=pts/2 res=failed This is because the ret is still -1 when we call audit function to help Also we need audit when hot-plugget failed in qemu side. And i notice we use virDomainDefGetMemoryActual to get the newmem , but when we failed to attach the memory device we the virDomainDefGetMemoryActual will still output the oldmem size, so the audit log will not right in that case. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_hotplug.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1ea397f..def3de8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1745,6 +1745,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned long long oldmem = virDomainDefGetMemoryActual(vm->def); + unsigned long long newmem = oldmem + mem->size; char *devstr = NULL; char *objalias = NULL; const char *backendType; @@ -1800,7 +1801,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (qemuDomainObjExitMonitor(driver, vm) < 0) { /* we shouldn't touch mem now, as the def might be freed */ mem = NULL; - goto cleanup; + goto audit; } event = virDomainEventDeviceAddedNewFromObj(vm, objalias); @@ -1811,9 +1812,6 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (fix_balloon) vm->def->mem.cur_balloon += mem->size; - virDomainAuditMemory(vm, oldmem, virDomainDefGetMemoryActual(vm->def), - "update", ret == 0); - /* mem is consumed by vm->def */ mem = NULL; @@ -1823,6 +1821,8 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, ret = 0; + audit: + virDomainAuditMemory(vm, oldmem, newmem, "update", ret == 0); cleanup: virObjectUnref(cfg); VIR_FREE(devstr); @@ -1833,7 +1833,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, removedef: if (qemuDomainObjExitMonitor(driver, vm) < 0) { mem = NULL; - goto cleanup; + goto audit; } if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0) @@ -1841,7 +1841,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, else mem = NULL; - goto cleanup; + goto audit; } -- 1.8.3.1
https://bugzilla.redhat.com/show_bug.cgi?id=1226234#c3 We use virDomainDefGetMemoryActual to get the newmem , but when we failed to remove the memory device, the virDomainDefGetMemoryActual will still output the oldmem size, so the audit log will not right in that case. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_hotplug.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index def3de8..f4b6178 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2904,11 +2904,11 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; unsigned long long oldmem = virDomainDefGetMemoryActual(vm->def); + unsigned long long newmem = oldmem - mem->size; virObjectEventPtr event; char *backendAlias = NULL; int rc; int idx; - int ret = -1; VIR_DEBUG("Removing memory device %s from domain %p %s", mem->info.alias, vm, vm->def->name); @@ -2917,12 +2917,18 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver, qemuDomainEventQueue(driver, event); if (virAsprintf(&backendAlias, "mem%s", mem->info.alias) < 0) - goto cleanup; + return -1; qemuDomainObjEnterMonitor(driver, vm); rc = qemuMonitorDelObject(priv->mon, backendAlias); - if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) - goto cleanup; + if (qemuDomainObjExitMonitor(driver, vm) < 0) + rc = -1; + + VIR_FREE(backendAlias); + + virDomainAuditMemory(vm, oldmem, newmem, "update", rc == 0); + if (rc < 0) + return -1; vm->def->mem.cur_balloon -= mem->size; @@ -2930,14 +2936,7 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver, virDomainMemoryRemove(vm->def, idx); virDomainMemoryDefFree(mem); - ret = 0; - - cleanup: - virDomainAuditMemory(vm, oldmem, virDomainDefGetMemoryActual(vm->def), - "update", ret == 0); - - VIR_FREE(backendAlias); - return ret; + return 0; } -- 1.8.3.1
On 08/13/2015 10:15 AM, Luyao Huang wrote:
First review: http://www.redhat.com/archives/libvir-list/2015-July/msg00982.html
Change in v2: - split it to two patches - fix some small mistake in hot-unplug part
I change the code in qemuDomainAttachMemory like what we do in other attach deivce functions. And I have removed the jump in qemuDomainRemoveMemoryDevice, since i just see only one place need jump to audit, so i refactor the code (thanks John's advise).
Luyao Huang (2): qemu: fix the audit log is not correct after hot-plug memory success qemu: fix the audit log is not correct after hot-unplug memory device
src/qemu/qemu_hotplug.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-)
These look much cleaner code wise - had to clean up the commit messages, but otherwise fine. I have pushed the two patches. John
On 08/27/2015 05:54 AM, John Ferlan wrote:
On 08/13/2015 10:15 AM, Luyao Huang wrote:
First review: http://www.redhat.com/archives/libvir-list/2015-July/msg00982.html
Change in v2: - split it to two patches - fix some small mistake in hot-unplug part
I change the code in qemuDomainAttachMemory like what we do in other attach deivce functions. And I have removed the jump in qemuDomainRemoveMemoryDevice, since i just see only one place need jump to audit, so i refactor the code (thanks John's advise).
Luyao Huang (2): qemu: fix the audit log is not correct after hot-plug memory success qemu: fix the audit log is not correct after hot-unplug memory device
src/qemu/qemu_hotplug.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-)
These look much cleaner code wise - had to clean up the commit messages, but otherwise fine. I have pushed the two patches.
Thanks a lot for your review and help.
John
Luyao
participants (3)
-
John Ferlan -
lhuang -
Luyao Huang