[libvirt] [PATCH 0/2] qemu: Remove memory backing files on failed memory hotplug
see 2/2 Peter Krempa (2): REPRODUCER -- DO NOT PUSH qemu: hotplug: Clean up memory backing files after failed memory hotplug dom.xml | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ mem.xml | 6 +++ src/qemu/qemu_hotplug.c | 7 +++ 3 files changed, 123 insertions(+) create mode 100644 dom.xml create mode 100644 mem.xml -- 2.16.2
https://bugzilla.redhat.com/show_bug.cgi?id=1553085 This breaks hotplug so that we can see whether the file was left dangling. Reproduce by: virsh create dom.xml virsh attach-device memplug mem.xml ls /var/lib/libvirt/qemu/ram/libvirt/qemu/*-memplug After the failed hotplug, qemu should leave behind the file called 'dimm0' in the above path. The original bugreport was caused by failing in the 'device-add' phase. --- dom.xml | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ mem.xml | 6 +++ src/qemu/qemu_hotplug.c | 3 ++ 3 files changed, 119 insertions(+) create mode 100644 dom.xml create mode 100644 mem.xml diff --git a/dom.xml b/dom.xml new file mode 100644 index 0000000000..242615569e --- /dev/null +++ b/dom.xml @@ -0,0 +1,110 @@ +<domain type='kvm'> + <name>memplug</name> + <uuid>41277c77-9bb0-416c-aacc-4dcac4e03cea</uuid> + <maxMemory slots='16' unit='KiB'>152428800</maxMemory> + <memory unit='KiB'>1024000</memory> + <currentMemory unit='KiB'>1024000</currentMemory> + <memoryBacking> + <source type='file'/> + <access mode='shared'/> + <allocation mode='immediate'/> + </memoryBacking> + <vcpu placement='static' current='2'>8</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.9'>hvm</type> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + <apic/> + <vmport state='off'/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0,2,4,6' memory='512000' unit='KiB'/> + <cell id='1' cpus='1,3,5,7' memory='512000' unit='KiB'/> + </numa> + </cpu> + <clock offset='utc'> + <timer name='rtc' tickpolicy='catchup'/> + <timer name='pit' tickpolicy='delay'/> + <timer name='hpet' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='scsi' index='0' model='lsilogic'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <serial type='pty'> + <target type='isa-serial' port='0'> + <model name='isa-serial'/> + </target> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <channel type='unix'> + <target type='virtio' name='org.qemu.guest_agent.0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> + </channel> + <input type='tablet' bus='usb'> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' autoport='yes'> + <listen type='address'/> + <image compression='off'/> + </graphics> + <sound model='ich6'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </sound> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='2'/> + </redirdev> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='3'/> + </redirdev> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </memballoon> + </devices> +</domain> + diff --git a/mem.xml b/mem.xml new file mode 100644 index 0000000000..d536786b8b --- /dev/null +++ b/mem.xml @@ -0,0 +1,6 @@ +<memory model='dimm' access='private'> + <target> + <size unit='MiB'>128</size> + <node>0</node> + </target> + </memory> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e0a5300f08..cff40625f8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2162,6 +2162,9 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, goto exit_monitor; objAdded = true; + /* !!! REPRODUCER !!! */ + goto exit_monitor; + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) goto exit_monitor; -- 2.16.2
On 03/13/2018 06:36 PM, Peter Krempa wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1553085
This breaks hotplug so that we can see whether the file was left dangling.
Reproduce by:
virsh create dom.xml virsh attach-device memplug mem.xml ls /var/lib/libvirt/qemu/ram/libvirt/qemu/*-memplug
After the failed hotplug, qemu should leave behind the file called 'dimm0' in the above path.
The original bugreport was caused by failing in the 'device-add' phase. --- dom.xml | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ mem.xml | 6 +++ src/qemu/qemu_hotplug.c | 3 ++ 3 files changed, 119 insertions(+) create mode 100644 dom.xml create mode 100644 mem.xml
diff --git a/dom.xml b/dom.xml new file mode 100644 index 0000000000..242615569e --- /dev/null +++ b/dom.xml @@ -0,0 +1,110 @@ +<domain type='kvm'> + <name>memplug</name> + <uuid>41277c77-9bb0-416c-aacc-4dcac4e03cea</uuid> + <maxMemory slots='16' unit='KiB'>152428800</maxMemory> + <memory unit='KiB'>1024000</memory> + <currentMemory unit='KiB'>1024000</currentMemory> + <memoryBacking> + <source type='file'/> + <access mode='shared'/> + <allocation mode='immediate'/> + </memoryBacking> + <vcpu placement='static' current='2'>8</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.9'>hvm</type> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + <apic/> + <vmport state='off'/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0,2,4,6' memory='512000' unit='KiB'/> + <cell id='1' cpus='1,3,5,7' memory='512000' unit='KiB'/> + </numa> + </cpu> + <clock offset='utc'> + <timer name='rtc' tickpolicy='catchup'/> + <timer name='pit' tickpolicy='delay'/> + <timer name='hpet' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <controller type='virtio-serial' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='scsi' index='0' model='lsilogic'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/> + </controller> + <serial type='pty'> + <target type='isa-serial' port='0'> + <model name='isa-serial'/> + </target> + </serial> + <console type='pty'> + <target type='serial' port='0'/> + </console> + <channel type='unix'> + <target type='virtio' name='org.qemu.guest_agent.0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> + </channel> + <input type='tablet' bus='usb'> + <address type='usb' bus='0' port='1'/> + </input> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' autoport='yes'> + <listen type='address'/> + <image compression='off'/> + </graphics> + <sound model='ich6'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </sound> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='2'/> + </redirdev> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='3'/> + </redirdev> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </memballoon> + </devices> +</domain> +
Empty line at EOF :-P Michal
On Tue, Mar 13, 2018 at 06:36:32PM +0100, Peter Krempa wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1553085
This breaks hotplug so that we can see whether the file was left dangling.
NACK, this breaks hotplug.
Reproduce by:
virsh create dom.xml virsh attach-device memplug mem.xml ls /var/lib/libvirt/qemu/ram/libvirt/qemu/*-memplug
After the failed hotplug, qemu should leave behind the file called 'dimm0' in the above path.
The original bugreport was caused by failing in the 'device-add' phase. --- dom.xml | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ mem.xml | 6 +++ src/qemu/qemu_hotplug.c | 3 ++ 3 files changed, 119 insertions(+) create mode 100644 dom.xml create mode 100644 mem.xml
Also, 'make syntax-check' does not pass: dom.xml maint.mk: empty line(s) or no newline at EOF make: *** [maint.mk:930: sc_prohibit_empty_lines_at_EOF] Error 1 make: *** Waiting for unfinished jobs.... Jan
Libvirt provides full path to the backing file since commit fec8f9c49afb479f6. This made qemu create the backend object but did not delete it. This was fixed for unplug case in 4d83a6722f but not in case of failure to hotplug the frontend. We'd leave the files behind which would make memory unusable in case of hugepages. https://bugzilla.redhat.com/show_bug.cgi?id=1553085 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cff40625f8..70325a1246 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2219,6 +2219,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (!mem) goto audit; + /* we need to remove the memory backing file so that it does not hog memory */ + if (objAdded) + ignore_value(qemuProcessDestroyMemoryBackingPath(driver, vm, mem)); + removedef: if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0) mem = virDomainMemoryRemove(vm->def, id); -- 2.16.2
On 03/13/2018 06:36 PM, Peter Krempa wrote:
Libvirt provides full path to the backing file since commit fec8f9c49afb479f6. This made qemu create the backend object but did not delete it. This was fixed for unplug case in 4d83a6722f but not in case of failure to hotplug the frontend. We'd leave the files behind which would make memory unusable in case of hugepages.
https://bugzilla.redhat.com/show_bug.cgi?id=1553085
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 4 ++++ 1 file changed, 4 insertions(+)
ACK Michal
On Tue, Mar 13, 2018 at 06:36:33PM +0100, Peter Krempa wrote:
Libvirt provides full path to the backing file since commit fec8f9c49afb479f6. This made qemu create the backend object but did not delete it. This was fixed for unplug case in 4d83a6722f but not in case of failure to hotplug the frontend. We'd leave the files behind which would make memory unusable in case of hugepages.
https://bugzilla.redhat.com/show_bug.cgi?id=1553085
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 4 ++++ 1 file changed, 4 insertions(+)
ACK
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cff40625f8..70325a1246 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2219,6 +2219,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (!mem) goto audit;
+ /* we need to remove the memory backing file so that it does not hog memory */
This comment does not really help with understanding what's happening. I suggest deleting it since readers will have to read the code anyway. Jan
participants (3)
-
Ján Tomko -
Michal Privoznik -
Peter Krempa