[libvirt] [PATCH 00/10] Implement shared memory device - Part 1
This part contains some small fix and Implement the hot-plug/hot-unplug and cold-plug/cold-unplug for Inter-VM Shared Memory PCI device. Luyao Huang (10): qemu: auto assign pci address for shared memory device qemu: always build id when generate shared memory device CLI qemu: Refactor creation of shared memory device commandline conf: use virDomainChrSourceDef to save the path conf:audit: introduce audit function for shared memory device conf: Add helpers to insert/remove/find shmem devices in domain def qemu: Implement shared memory device cold (un)plug qemu: Implement share memory device hot-plug qemu: Implement shared memory device hot-unplug qemu: report error when shmem have a invalid address docs/auditlog.html.in | 16 +++ src/conf/domain_audit.c | 16 +++ src/conf/domain_audit.h | 6 + src/conf/domain_conf.c | 69 ++++++++++- src/conf/domain_conf.h | 9 +- src/libvirt_private.syms | 5 + src/qemu/qemu_command.c | 82 +++++++------ src/qemu/qemu_command.h | 7 ++ src/qemu/qemu_driver.c | 35 +++++- src/qemu/qemu_hotplug.c | 156 ++++++++++++++++++++++++- src/qemu/qemu_hotplug.h | 6 + tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 +-- 12 files changed, 372 insertions(+), 51 deletions(-) -- 1.8.3.1
Shared memory device is base on PCI address, even we do not pass the pci address to qemu, qemu will auto assign a pci address for it. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 11 +++++++++++ tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89f775d..5ac43d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2562,6 +2562,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } + /* Further non-primary video cards which have to be qxl type */ for (i = 1; i < def->nvideos; i++) { if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { @@ -2575,6 +2576,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } + + /* Shared Memory */ + for (i = 0; i < def->nshmems; i++) { + if (def->shmems[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->shmems[i]->info, flags) < 0) + goto error; + } for (i = 0; i < def->ninputs; i++) { /* Nada - none are PCI based (yet) */ } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index a3d3cc8..d37879a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -1,16 +1,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ --device ivshmem,shm=shmem0 \ --device ivshmem,size=128m,shm=shmem1 \ --device ivshmem,size=256m,shm=shmem2 \ --device ivshmem,size=512m,chardev=charshmem3 \ +-device ivshmem,shm=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,size=128m,shm=shmem1,bus=pci.0,addr=0x4 \ +-device ivshmem,size=256m,shm=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ --device ivshmem,size=1024m,chardev=charshmem4 \ +-device ivshmem,size=1024m,chardev=charshmem4,bus=pci.0,addr=0x7 \ -chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ --device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off \ +-device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ -chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ --device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16 \ +-device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ -chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ --device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on \ +-device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \ -chardev socket,id=charshmem7,path=/tmp/shmem7-sock -- 1.8.3.1
On Wed, Jun 17, 2015 at 11:56:12AM +0800, Luyao Huang wrote:
Shared memory device is base on PCI address, even we do not pass the pci address to qemu, qemu will auto assign a pci address for it.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 11 +++++++++++ tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89f775d..5ac43d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2562,6 +2562,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } +
Spurious change, ACK without that.
/* Further non-primary video cards which have to be qxl type */ for (i = 1; i < def->nvideos; i++) { if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { @@ -2575,6 +2576,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } + + /* Shared Memory */ + for (i = 0; i < def->nshmems; i++) { + if (def->shmems[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->shmems[i]->info, flags) < 0) + goto error; + } for (i = 0; i < def->ninputs; i++) { /* Nada - none are PCI based (yet) */ } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index a3d3cc8..d37879a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -1,16 +1,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ --device ivshmem,shm=shmem0 \ --device ivshmem,size=128m,shm=shmem1 \ --device ivshmem,size=256m,shm=shmem2 \ --device ivshmem,size=512m,chardev=charshmem3 \ +-device ivshmem,shm=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,size=128m,shm=shmem1,bus=pci.0,addr=0x4 \ +-device ivshmem,size=256m,shm=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ --device ivshmem,size=1024m,chardev=charshmem4 \ +-device ivshmem,size=1024m,chardev=charshmem4,bus=pci.0,addr=0x7 \ -chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ --device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off \ +-device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ -chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ --device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16 \ +-device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ -chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ --device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on \ +-device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \ -chardev socket,id=charshmem7,path=/tmp/shmem7-sock -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/03/2015 08:39 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:12AM +0800, Luyao Huang wrote:
Shared memory device is base on PCI address, even we do not pass the pci address to qemu, qemu will auto assign a pci address for it.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 11 +++++++++++ tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89f775d..5ac43d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2562,6 +2562,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } +
Spurious change, ACK without that.
Thanks a lot for your review, i will remove it in next version. Luyao
/* Further non-primary video cards which have to be qxl type */ for (i = 1; i < def->nvideos; i++) { if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { @@ -2575,6 +2576,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } + + /* Shared Memory */ + for (i = 0; i < def->nshmems; i++) { + if (def->shmems[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->shmems[i]->info, flags) < 0) + goto error; + } for (i = 0; i < def->ninputs; i++) { /* Nada - none are PCI based (yet) */ } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index a3d3cc8..d37879a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -1,16 +1,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ --device ivshmem,shm=shmem0 \ --device ivshmem,size=128m,shm=shmem1 \ --device ivshmem,size=256m,shm=shmem2 \ --device ivshmem,size=512m,chardev=charshmem3 \ +-device ivshmem,shm=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,size=128m,shm=shmem1,bus=pci.0,addr=0x4 \ +-device ivshmem,size=256m,shm=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ --device ivshmem,size=1024m,chardev=charshmem4 \ +-device ivshmem,size=1024m,chardev=charshmem4,bus=pci.0,addr=0x7 \ -chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ --device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off \ +-device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ -chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ --device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16 \ +-device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ -chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ --device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on \ +-device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \ -chardev socket,id=charshmem7,path=/tmp/shmem7-sock -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jul 03, 2015 at 02:39:49PM +0200, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:12AM +0800, Luyao Huang wrote:
Shared memory device is base on PCI address, even we do not pass the pci address to qemu, qemu will auto assign a pci address for it.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 11 +++++++++++ tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89f775d..5ac43d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2562,6 +2562,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } +
Spurious change, ACK without that.
I also squashed in the following to make sure it works fine (which it does): diff --git i/tests/qemuxml2argvdata/qemuxml2argv-shmem.args w/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index 4c383db6985f..08cd5ac4588e 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ w/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -2,8 +2,8 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -device ivshmem,shm=shmem0,id=shmem0,bus=pci.0,addr=0x3 \ --device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x4 \ --device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x5 \ +-device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x4 \ -device ivshmem,size=512m,chardev=charshmem3,id=shmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ -device ivshmem,size=1024m,chardev=charshmem4,id=shmem4,bus=pci.0,addr=0x7 \ diff --git i/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml w/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml index fd79c89c1a43..d4b38f91b050 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml +++ w/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml @@ -23,6 +23,7 @@ </shmem> <shmem name='shmem2'> <size unit='M'>256</size> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </shmem> <shmem name='shmem3'> <size unit='M'>512</size>
/* Further non-primary video cards which have to be qxl type */ for (i = 1; i < def->nvideos; i++) { if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { @@ -2575,6 +2576,16 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } + + /* Shared Memory */ + for (i = 0; i < def->nshmems; i++) { + if (def->shmems[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + + if (virDomainPCIAddressReserveNextSlot(addrs, + &def->shmems[i]->info, flags) < 0) + goto error; + } for (i = 0; i < def->ninputs; i++) { /* Nada - none are PCI based (yet) */ } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index a3d3cc8..d37879a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -1,16 +1,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ --device ivshmem,shm=shmem0 \ --device ivshmem,size=128m,shm=shmem1 \ --device ivshmem,size=256m,shm=shmem2 \ --device ivshmem,size=512m,chardev=charshmem3 \ +-device ivshmem,shm=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,size=128m,shm=shmem1,bus=pci.0,addr=0x4 \ +-device ivshmem,size=256m,shm=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ --device ivshmem,size=1024m,chardev=charshmem4 \ +-device ivshmem,size=1024m,chardev=charshmem4,bus=pci.0,addr=0x7 \ -chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ --device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off \ +-device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ -chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ --device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16 \ +-device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ -chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ --device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on \ +-device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \ -chardev socket,id=charshmem7,path=/tmp/shmem7-sock -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/08/2015 05:37 PM, Martin Kletzander wrote:
On Fri, Jul 03, 2015 at 02:39:49PM +0200, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:12AM +0800, Luyao Huang wrote:
Shared memory device is base on PCI address, even we do not pass the pci address to qemu, qemu will auto assign a pci address for it.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 11 +++++++++++ tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89f775d..5ac43d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2562,6 +2562,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, flags) < 0) goto error; } +
Spurious change, ACK without that.
I also squashed in the following to make sure it works fine (which it does):
Good idea ! Thanks a lot for your help.
diff --git i/tests/qemuxml2argvdata/qemuxml2argv-shmem.args w/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index 4c383db6985f..08cd5ac4588e 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ w/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -2,8 +2,8 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -device ivshmem,shm=shmem0,id=shmem0,bus=pci.0,addr=0x3 \ --device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x4 \ --device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x5 \ +-device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x4 \ -device ivshmem,size=512m,chardev=charshmem3,id=shmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ -device ivshmem,size=1024m,chardev=charshmem4,id=shmem4,bus=pci.0,addr=0x7 \ diff --git i/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml w/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml index fd79c89c1a43..d4b38f91b050 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml +++ w/tests/qemuxml2argvdata/qemuxml2argv-shmem.xml @@ -23,6 +23,7 @@ </shmem> <shmem name='shmem2'> <size unit='M'>256</size> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </shmem> <shmem name='shmem3'> <size unit='M'>512</size>
Luyao
When hot-unplug the device, qmp command device_del require a device id. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5ac43d8..636e040 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8470,9 +8470,9 @@ qemuBuildShmemDevCmd(virCommandPtr cmd, } if (!shmem->server.enabled) { - virBufferAsprintf(&buf, ",shm=%s", shmem->name); + virBufferAsprintf(&buf, ",shm=%s,id=%s", shmem->name, shmem->info.alias); } else { - virBufferAsprintf(&buf, ",chardev=char%s", shmem->info.alias); + virBufferAsprintf(&buf, ",chardev=char%s,id=%s", shmem->info.alias, shmem->info.alias); if (shmem->msi.enabled) { virBufferAddLit(&buf, ",msi=on"); if (shmem->msi.vectors) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index d37879a..601167c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -1,16 +1,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ --device ivshmem,shm=shmem0,bus=pci.0,addr=0x3 \ --device ivshmem,size=128m,shm=shmem1,bus=pci.0,addr=0x4 \ --device ivshmem,size=256m,shm=shmem2,bus=pci.0,addr=0x5 \ --device ivshmem,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \ +-device ivshmem,shm=shmem0,id=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x4 \ +-device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=512m,chardev=charshmem3,id=shmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ --device ivshmem,size=1024m,chardev=charshmem4,bus=pci.0,addr=0x7 \ +-device ivshmem,size=1024m,chardev=charshmem4,id=shmem4,bus=pci.0,addr=0x7 \ -chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ --device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ +-device ivshmem,size=2048m,chardev=charshmem5,id=shmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ -chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ --device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ +-device ivshmem,size=4096m,chardev=charshmem6,id=shmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ -chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ --device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \ +-device ivshmem,size=8192m,chardev=charshmem7,id=shmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \ -chardev socket,id=charshmem7,path=/tmp/shmem7-sock -- 1.8.3.1
On Wed, Jun 17, 2015 at 11:56:13AM +0800, Luyao Huang wrote:
When hot-unplug the device, qmp command device_del require a device id.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5ac43d8..636e040 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8470,9 +8470,9 @@ qemuBuildShmemDevCmd(virCommandPtr cmd, }
if (!shmem->server.enabled) { - virBufferAsprintf(&buf, ",shm=%s", shmem->name); + virBufferAsprintf(&buf, ",shm=%s,id=%s", shmem->name, shmem->info.alias); } else { - virBufferAsprintf(&buf, ",chardev=char%s", shmem->info.alias); + virBufferAsprintf(&buf, ",chardev=char%s,id=%s", shmem->info.alias, shmem->info.alias); if (shmem->msi.enabled) { virBufferAddLit(&buf, ",msi=on"); if (shmem->msi.vectors) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index d37879a..601167c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -1,16 +1,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ --device ivshmem,shm=shmem0,bus=pci.0,addr=0x3 \ --device ivshmem,size=128m,shm=shmem1,bus=pci.0,addr=0x4 \ --device ivshmem,size=256m,shm=shmem2,bus=pci.0,addr=0x5 \ --device ivshmem,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \ +-device ivshmem,shm=shmem0,id=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x4 \ +-device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=512m,chardev=charshmem3,id=shmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ --device ivshmem,size=1024m,chardev=charshmem4,bus=pci.0,addr=0x7 \ +-device ivshmem,size=1024m,chardev=charshmem4,id=shmem4,bus=pci.0,addr=0x7 \ -chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ --device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ +-device ivshmem,size=2048m,chardev=charshmem5,id=shmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ -chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ --device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ +-device ivshmem,size=4096m,chardev=charshmem6,id=shmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ -chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ --device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \ +-device ivshmem,size=8192m,chardev=charshmem7,id=shmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \
make syntax-check complains about these, so they need to be split. No problem, though, ACK with that changed.
-chardev socket,id=charshmem7,path=/tmp/shmem7-sock -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/03/2015 08:41 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:13AM +0800, Luyao Huang wrote:
When hot-unplug the device, qmp command device_del require a device id.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5ac43d8..636e040 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8470,9 +8470,9 @@ qemuBuildShmemDevCmd(virCommandPtr cmd, }
if (!shmem->server.enabled) { - virBufferAsprintf(&buf, ",shm=%s", shmem->name); + virBufferAsprintf(&buf, ",shm=%s,id=%s", shmem->name, shmem->info.alias); } else { - virBufferAsprintf(&buf, ",chardev=char%s", shmem->info.alias); + virBufferAsprintf(&buf, ",chardev=char%s,id=%s", shmem->info.alias, shmem->info.alias); if (shmem->msi.enabled) { virBufferAddLit(&buf, ",msi=on"); if (shmem->msi.vectors) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index d37879a..601167c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -1,16 +1,16 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ --device ivshmem,shm=shmem0,bus=pci.0,addr=0x3 \ --device ivshmem,size=128m,shm=shmem1,bus=pci.0,addr=0x4 \ --device ivshmem,size=256m,shm=shmem2,bus=pci.0,addr=0x5 \ --device ivshmem,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \ +-device ivshmem,shm=shmem0,id=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x4 \ +-device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x5 \ +-device ivshmem,size=512m,chardev=charshmem3,id=shmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ --device ivshmem,size=1024m,chardev=charshmem4,bus=pci.0,addr=0x7 \ +-device ivshmem,size=1024m,chardev=charshmem4,id=shmem4,bus=pci.0,addr=0x7 \ -chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ --device ivshmem,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ +-device ivshmem,size=2048m,chardev=charshmem5,id=shmem5,msi=on,ioeventfd=off,bus=pci.0,addr=0x8 \ -chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ --device ivshmem,size=4096m,chardev=charshmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ +-device ivshmem,size=4096m,chardev=charshmem6,id=shmem6,msi=on,vectors=16,bus=pci.0,addr=0x9 \ -chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ --device ivshmem,size=8192m,chardev=charshmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \ +-device ivshmem,size=8192m,chardev=charshmem7,id=shmem7,msi=on,vectors=32,ioeventfd=on,bus=pci.0,addr=0xa \
make syntax-check complains about these, so they need to be split. No problem, though, ACK with that changed.
Got it, and i will split them to two lines in next version. Thanks a lot for your review. Luyao
-chardev socket,id=charshmem7,path=/tmp/shmem7-sock -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Rename qemuBuildShmemDevCmd to qemuBuildShmemDevStr and change the return type so that it can be reused in the device hotplug code later. And split the chardev creation part in a new function qemuBuildShmemBackendStr for reused in the device hotplug code later. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_command.h | 7 +++++ 2 files changed, 45 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 636e040..0414f77 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8433,9 +8433,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, return ret; } -static int -qemuBuildShmemDevCmd(virCommandPtr cmd, - virDomainDefPtr def, +char * +qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { @@ -8489,14 +8488,38 @@ qemuBuildShmemDevCmd(virCommandPtr cmd, if (virBufferCheckError(&buf) < 0) goto error; - virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &buf); - - return 0; + return virBufferContentAndReset(&buf); error: virBufferFreeAndReset(&buf); - return -1; + return NULL; +} + +char * +qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + char *devstr = NULL; + virDomainChrSourceDef source = { + .type = VIR_DOMAIN_CHR_TYPE_UNIX, + .data.nix = { + .path = shmem->server.path, + .listen = false, + } + }; + + if (!shmem->server.path && + virAsprintf(&source.data.nix.path, + "/var/lib/libvirt/shmem-%s-sock", + shmem->name) < 0) + return NULL; + + devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); + + if (!shmem->server.path) + VIR_FREE(source.data.nix.path); + + return devstr; } static int @@ -8505,35 +8528,18 @@ qemuBuildShmemCommandLine(virCommandPtr cmd, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { - if (qemuBuildShmemDevCmd(cmd, def, shmem, qemuCaps) < 0) + char *devstr = NULL; + + if (!(devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps))) return -1; + virCommandAddArgList(cmd, "-device", devstr, NULL); + VIR_FREE(devstr); if (shmem->server.enabled) { - char *devstr = NULL; - virDomainChrSourceDef source = { - .type = VIR_DOMAIN_CHR_TYPE_UNIX, - .data.nix = { - .path = shmem->server.path, - .listen = false, - } - }; - - if (!shmem->server.path && - virAsprintf(&source.data.nix.path, - "/var/lib/libvirt/shmem-%s-sock", - shmem->name) < 0) + if (!(devstr = qemuBuildShmemBackendStr(shmem, qemuCaps))) return -1; - devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); - - if (!shmem->server.path) - VIR_FREE(source.data.nix.path); - - if (!devstr) - return -1; - - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 0fc59a8..73f24dc 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -194,6 +194,13 @@ int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, const char **type, virJSONValuePtr *props); +char *qemuBuildShmemDevStr(virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); +char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); + + int qemuOpenPCIConfig(virDomainHostdevDefPtr dev); /* Legacy, pre device support */ -- 1.8.3.1
On Wed, Jun 17, 2015 at 11:56:14AM +0800, Luyao Huang wrote:
Rename qemuBuildShmemDevCmd to qemuBuildShmemDevStr and change the return type so that it can be reused in the device hotplug code later.
And split the chardev creation part in a new function qemuBuildShmemBackendStr for reused in the device hotplug code later.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_command.h | 7 +++++ 2 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 636e040..0414f77 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8433,9 +8433,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, return ret; }
-static int -qemuBuildShmemDevCmd(virCommandPtr cmd, - virDomainDefPtr def, +char * +qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { @@ -8489,14 +8488,38 @@ qemuBuildShmemDevCmd(virCommandPtr cmd, if (virBufferCheckError(&buf) < 0) goto error;
- virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &buf); - - return 0; + return virBufferContentAndReset(&buf);
You should be able to just unconditionally do return virBufferContentAndReset() here since it returns NULL if there's a buf->error. ACK with that changed.
error: virBufferFreeAndReset(&buf); - return -1; + return NULL; +} + +char * +qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + char *devstr = NULL; + virDomainChrSourceDef source = { + .type = VIR_DOMAIN_CHR_TYPE_UNIX, + .data.nix = { + .path = shmem->server.path, + .listen = false, + } + }; + + if (!shmem->server.path && + virAsprintf(&source.data.nix.path, + "/var/lib/libvirt/shmem-%s-sock", + shmem->name) < 0) + return NULL; + + devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); + + if (!shmem->server.path) + VIR_FREE(source.data.nix.path); + + return devstr; }
static int @@ -8505,35 +8528,18 @@ qemuBuildShmemCommandLine(virCommandPtr cmd, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { - if (qemuBuildShmemDevCmd(cmd, def, shmem, qemuCaps) < 0) + char *devstr = NULL; + + if (!(devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps))) return -1; + virCommandAddArgList(cmd, "-device", devstr, NULL); + VIR_FREE(devstr);
if (shmem->server.enabled) { - char *devstr = NULL; - virDomainChrSourceDef source = { - .type = VIR_DOMAIN_CHR_TYPE_UNIX, - .data.nix = { - .path = shmem->server.path, - .listen = false, - } - }; - - if (!shmem->server.path && - virAsprintf(&source.data.nix.path, - "/var/lib/libvirt/shmem-%s-sock", - shmem->name) < 0) + if (!(devstr = qemuBuildShmemBackendStr(shmem, qemuCaps))) return -1;
- devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); - - if (!shmem->server.path) - VIR_FREE(source.data.nix.path); - - if (!devstr) - return -1; - - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); }
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 0fc59a8..73f24dc 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -194,6 +194,13 @@ int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, const char **type, virJSONValuePtr *props);
+char *qemuBuildShmemDevStr(virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); +char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); + + int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
/* Legacy, pre device support */ -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/03/2015 08:56 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:14AM +0800, Luyao Huang wrote:
Rename qemuBuildShmemDevCmd to qemuBuildShmemDevStr and change the return type so that it can be reused in the device hotplug code later.
And split the chardev creation part in a new function qemuBuildShmemBackendStr for reused in the device hotplug code later.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_command.h | 7 +++++ 2 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 636e040..0414f77 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8433,9 +8433,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, return ret; }
-static int -qemuBuildShmemDevCmd(virCommandPtr cmd, - virDomainDefPtr def, +char * +qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { @@ -8489,14 +8488,38 @@ qemuBuildShmemDevCmd(virCommandPtr cmd, if (virBufferCheckError(&buf) < 0) goto error;
- virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &buf); - - return 0; + return virBufferContentAndReset(&buf);
You should be able to just unconditionally do return virBufferContentAndReset() here since it returns NULL if there's a buf->error.
ACK with that changed.
Right, i forgot that, thanks a lot for your review Luyao
error: virBufferFreeAndReset(&buf); - return -1; + return NULL; +} + +char * +qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + char *devstr = NULL; + virDomainChrSourceDef source = { + .type = VIR_DOMAIN_CHR_TYPE_UNIX, + .data.nix = { + .path = shmem->server.path, + .listen = false, + } + }; + + if (!shmem->server.path && + virAsprintf(&source.data.nix.path, + "/var/lib/libvirt/shmem-%s-sock", + shmem->name) < 0) + return NULL; + + devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); + + if (!shmem->server.path) + VIR_FREE(source.data.nix.path); + + return devstr; }
static int @@ -8505,35 +8528,18 @@ qemuBuildShmemCommandLine(virCommandPtr cmd, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { - if (qemuBuildShmemDevCmd(cmd, def, shmem, qemuCaps) < 0) + char *devstr = NULL; + + if (!(devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps))) return -1; + virCommandAddArgList(cmd, "-device", devstr, NULL); + VIR_FREE(devstr);
if (shmem->server.enabled) { - char *devstr = NULL; - virDomainChrSourceDef source = { - .type = VIR_DOMAIN_CHR_TYPE_UNIX, - .data.nix = { - .path = shmem->server.path, - .listen = false, - } - }; - - if (!shmem->server.path && - virAsprintf(&source.data.nix.path, - "/var/lib/libvirt/shmem-%s-sock", - shmem->name) < 0) + if (!(devstr = qemuBuildShmemBackendStr(shmem, qemuCaps))) return -1;
- devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); - - if (!shmem->server.path) - VIR_FREE(source.data.nix.path); - - if (!devstr) - return -1; - - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); }
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 0fc59a8..73f24dc 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -194,6 +194,13 @@ int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, const char **type, virJSONValuePtr *props);
+char *qemuBuildShmemDevStr(virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); +char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); + + int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
/* Legacy, pre device support */ -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 06, 2015 at 10:23:59AM +0800, lhuang wrote:
On 07/03/2015 08:56 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:14AM +0800, Luyao Huang wrote:
Rename qemuBuildShmemDevCmd to qemuBuildShmemDevStr and change the return type so that it can be reused in the device hotplug code later.
And split the chardev creation part in a new function qemuBuildShmemBackendStr for reused in the device hotplug code later.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_command.h | 7 +++++ 2 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 636e040..0414f77 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8433,9 +8433,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, return ret; }
-static int -qemuBuildShmemDevCmd(virCommandPtr cmd, - virDomainDefPtr def, +char * +qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { @@ -8489,14 +8488,38 @@ qemuBuildShmemDevCmd(virCommandPtr cmd, if (virBufferCheckError(&buf) < 0) goto error;
- virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &buf); - - return 0; + return virBufferContentAndReset(&buf);
You should be able to just unconditionally do return virBufferContentAndReset() here since it returns NULL if there's a buf->error.
ACK with that changed.
Right, i forgot that, thanks a lot for your review
Sorry, you cannot, there's a "goto error;" from some part of the code where there is no buf->error set, so no change to this, you were right. No need to resend these first three, I'll push whatever is OK to go in after I finish the review, and let you know what needs work. Thanks, Martin
Luyao
error: virBufferFreeAndReset(&buf); - return -1; + return NULL; +} + +char * +qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + char *devstr = NULL; + virDomainChrSourceDef source = { + .type = VIR_DOMAIN_CHR_TYPE_UNIX, + .data.nix = { + .path = shmem->server.path, + .listen = false, + } + }; + + if (!shmem->server.path && + virAsprintf(&source.data.nix.path, + "/var/lib/libvirt/shmem-%s-sock", + shmem->name) < 0) + return NULL; + + devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); + + if (!shmem->server.path) + VIR_FREE(source.data.nix.path); + + return devstr; }
static int @@ -8505,35 +8528,18 @@ qemuBuildShmemCommandLine(virCommandPtr cmd, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { - if (qemuBuildShmemDevCmd(cmd, def, shmem, qemuCaps) < 0) + char *devstr = NULL; + + if (!(devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps))) return -1; + virCommandAddArgList(cmd, "-device", devstr, NULL); + VIR_FREE(devstr);
if (shmem->server.enabled) { - char *devstr = NULL; - virDomainChrSourceDef source = { - .type = VIR_DOMAIN_CHR_TYPE_UNIX, - .data.nix = { - .path = shmem->server.path, - .listen = false, - } - }; - - if (!shmem->server.path && - virAsprintf(&source.data.nix.path, - "/var/lib/libvirt/shmem-%s-sock", - shmem->name) < 0) + if (!(devstr = qemuBuildShmemBackendStr(shmem, qemuCaps))) return -1;
- devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); - - if (!shmem->server.path) - VIR_FREE(source.data.nix.path); - - if (!devstr) - return -1; - - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); }
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 0fc59a8..73f24dc 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -194,6 +194,13 @@ int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, const char **type, virJSONValuePtr *props);
+char *qemuBuildShmemDevStr(virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); +char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); + + int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
/* Legacy, pre device support */ -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/06/2015 01:38 PM, Martin Kletzander wrote:
On Mon, Jul 06, 2015 at 10:23:59AM +0800, lhuang wrote:
On 07/03/2015 08:56 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:14AM +0800, Luyao Huang wrote:
Rename qemuBuildShmemDevCmd to qemuBuildShmemDevStr and change the return type so that it can be reused in the device hotplug code later.
And split the chardev creation part in a new function qemuBuildShmemBackendStr for reused in the device hotplug code later.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 70 +++++++++++++++++++++++++++---------------------- src/qemu/qemu_command.h | 7 +++++ 2 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 636e040..0414f77 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8433,9 +8433,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, return ret; }
-static int -qemuBuildShmemDevCmd(virCommandPtr cmd, - virDomainDefPtr def, +char * +qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { @@ -8489,14 +8488,38 @@ qemuBuildShmemDevCmd(virCommandPtr cmd, if (virBufferCheckError(&buf) < 0) goto error;
- virCommandAddArg(cmd, "-device"); - virCommandAddArgBuffer(cmd, &buf); - - return 0; + return virBufferContentAndReset(&buf);
You should be able to just unconditionally do return virBufferContentAndReset() here since it returns NULL if there's a buf->error.
ACK with that changed.
Right, i forgot that, thanks a lot for your review
Sorry, you cannot, there's a "goto error;" from some part of the code where there is no buf->error set, so no change to this, you were right.
No need to resend these first three, I'll push whatever is OK to go in after I finish the review, and let you know what needs work.
Okay, got it, thanks a lot for your helps. BR, Luyao
Thanks, Martin
Luyao
error: virBufferFreeAndReset(&buf); - return -1; + return NULL; +} + +char * +qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + char *devstr = NULL; + virDomainChrSourceDef source = { + .type = VIR_DOMAIN_CHR_TYPE_UNIX, + .data.nix = { + .path = shmem->server.path, + .listen = false, + } + }; + + if (!shmem->server.path && + virAsprintf(&source.data.nix.path, + "/var/lib/libvirt/shmem-%s-sock", + shmem->name) < 0) + return NULL; + + devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); + + if (!shmem->server.path) + VIR_FREE(source.data.nix.path); + + return devstr; }
static int @@ -8505,35 +8528,18 @@ qemuBuildShmemCommandLine(virCommandPtr cmd, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { - if (qemuBuildShmemDevCmd(cmd, def, shmem, qemuCaps) < 0) + char *devstr = NULL; + + if (!(devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps))) return -1; + virCommandAddArgList(cmd, "-device", devstr, NULL); + VIR_FREE(devstr);
if (shmem->server.enabled) { - char *devstr = NULL; - virDomainChrSourceDef source = { - .type = VIR_DOMAIN_CHR_TYPE_UNIX, - .data.nix = { - .path = shmem->server.path, - .listen = false, - } - }; - - if (!shmem->server.path && - virAsprintf(&source.data.nix.path, - "/var/lib/libvirt/shmem-%s-sock", - shmem->name) < 0) + if (!(devstr = qemuBuildShmemBackendStr(shmem, qemuCaps))) return -1;
- devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); - - if (!shmem->server.path) - VIR_FREE(source.data.nix.path); - - if (!devstr) - return -1; - - virCommandAddArg(cmd, "-chardev"); - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); }
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 0fc59a8..73f24dc 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -194,6 +194,13 @@ int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, const char **type, virJSONValuePtr *props);
+char *qemuBuildShmemDevStr(virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); +char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps); + + int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
/* Legacy, pre device support */ -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
As the backend of shmem server is a unix type chr device, save it in virDomainChrSourceDef can reuse the exist code for chr device. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 8 +++++--- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 16 +++------------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 60d4d07..306b718 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1945,7 +1945,7 @@ void virDomainShmemDefFree(virDomainShmemDefPtr def) return; virDomainDeviceInfoClear(&def->info); - VIR_FREE(def->server.path); + virDomainChrSourceDefClear(&def->server.chr); VIR_FREE(def->name); VIR_FREE(def); } @@ -11042,8 +11042,10 @@ virDomainShmemDefParseXML(xmlNodePtr node, if ((server = virXPathNode("./server[1]", ctxt))) { def->server.enabled = true; + def->server.chr.type = VIR_DOMAIN_CHR_TYPE_UNIX; + def->server.chr.data.nix.listen = false; if ((tmp = virXMLPropString(server, "path"))) - def->server.path = virFileSanitizePath(tmp); + def->server.chr.data.nix.path = virFileSanitizePath(tmp); VIR_FREE(tmp); } @@ -19997,7 +19999,7 @@ virDomainShmemDefFormat(virBufferPtr buf, if (def->server.enabled) { virBufferAddLit(buf, "<server"); - virBufferEscapeString(buf, " path='%s'", def->server.path); + virBufferEscapeString(buf, " path='%s'", def->server.chr.data.nix.path); virBufferAddLit(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ba17a8d..a4b1bf3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1599,7 +1599,7 @@ struct _virDomainShmemDef { unsigned long long size; struct { bool enabled; - char *path; + virDomainChrSourceDef chr; } server; struct { bool enabled; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0414f77..5d55794 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8500,24 +8500,14 @@ qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { char *devstr = NULL; - virDomainChrSourceDef source = { - .type = VIR_DOMAIN_CHR_TYPE_UNIX, - .data.nix = { - .path = shmem->server.path, - .listen = false, - } - }; - if (!shmem->server.path && - virAsprintf(&source.data.nix.path, + if (!shmem->server.chr.data.nix.path && + virAsprintf(&shmem->server.chr.data.nix.path, "/var/lib/libvirt/shmem-%s-sock", shmem->name) < 0) return NULL; - devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); - - if (!shmem->server.path) - VIR_FREE(source.data.nix.path); + devstr = qemuBuildChrChardevStr(&shmem->server.chr, shmem->info.alias, qemuCaps); return devstr; } -- 1.8.3.1
On Wed, Jun 17, 2015 at 11:56:15AM +0800, Luyao Huang wrote:
As the backend of shmem server is a unix type chr device, save it in virDomainChrSourceDef can reuse the exist code for chr device.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 8 +++++--- src/conf/domain_conf.h | 2 +- src/qemu/qemu_command.c | 16 +++------------- 3 files changed, 9 insertions(+), 17 deletions(-)
ACK
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 60d4d07..306b718 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1945,7 +1945,7 @@ void virDomainShmemDefFree(virDomainShmemDefPtr def) return;
virDomainDeviceInfoClear(&def->info); - VIR_FREE(def->server.path); + virDomainChrSourceDefClear(&def->server.chr); VIR_FREE(def->name); VIR_FREE(def); } @@ -11042,8 +11042,10 @@ virDomainShmemDefParseXML(xmlNodePtr node, if ((server = virXPathNode("./server[1]", ctxt))) { def->server.enabled = true;
+ def->server.chr.type = VIR_DOMAIN_CHR_TYPE_UNIX; + def->server.chr.data.nix.listen = false; if ((tmp = virXMLPropString(server, "path"))) - def->server.path = virFileSanitizePath(tmp); + def->server.chr.data.nix.path = virFileSanitizePath(tmp); VIR_FREE(tmp); }
@@ -19997,7 +19999,7 @@ virDomainShmemDefFormat(virBufferPtr buf,
if (def->server.enabled) { virBufferAddLit(buf, "<server"); - virBufferEscapeString(buf, " path='%s'", def->server.path); + virBufferEscapeString(buf, " path='%s'", def->server.chr.data.nix.path); virBufferAddLit(buf, "/>\n"); }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ba17a8d..a4b1bf3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1599,7 +1599,7 @@ struct _virDomainShmemDef { unsigned long long size; struct { bool enabled; - char *path; + virDomainChrSourceDef chr; } server; struct { bool enabled; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0414f77..5d55794 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8500,24 +8500,14 @@ qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) { char *devstr = NULL; - virDomainChrSourceDef source = { - .type = VIR_DOMAIN_CHR_TYPE_UNIX, - .data.nix = { - .path = shmem->server.path, - .listen = false, - } - };
- if (!shmem->server.path && - virAsprintf(&source.data.nix.path, + if (!shmem->server.chr.data.nix.path && + virAsprintf(&shmem->server.chr.data.nix.path, "/var/lib/libvirt/shmem-%s-sock", shmem->name) < 0) return NULL;
- devstr = qemuBuildChrChardevStr(&source, shmem->info.alias, qemuCaps); - - if (!shmem->server.path) - VIR_FREE(source.data.nix.path); + devstr = qemuBuildChrChardevStr(&shmem->server.chr, shmem->info.alias, qemuCaps);
return devstr; } -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- docs/auditlog.html.in | 16 ++++++++++++++++ src/conf/domain_audit.c | 16 ++++++++++++++++ src/conf/domain_audit.h | 6 ++++++ src/libvirt_private.syms | 1 + 4 files changed, 39 insertions(+) diff --git a/docs/auditlog.html.in b/docs/auditlog.html.in index 8a007ca..b168cbf 100644 --- a/docs/auditlog.html.in +++ b/docs/auditlog.html.in @@ -301,6 +301,22 @@ <dd>Updated path of the backing character device for given emulated device</dd> </dl> + <h4><a name="typeresourceivshmem">Shared memory device</a></h4> + <p> + The <code>msg</code> field will include the following sub-fields + </p> + + <dl> + <dt>reason</dt> + <dd>The reason which caused the resource to be assigned to happen</dd> + <dt>resrc</dt> + <dd>The type of resource assigned. Set to <code>shmem</code></dd> + <dt>old-shmem</dt> + <dd>Original memory size of share memory device in bytes, or 0</dd> + <dt>new-shmem</dt> + <dd>Updated memory size of share memory device in bytes</dd> + </dl> + <h4><a name="typeresourcesmartcard">smartcard</a></h4> <p> The <code>msg</code> field will include the following sub-fields diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 1900039..aa2b4b5 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -799,6 +799,19 @@ virDomainAuditIOThread(virDomainObjPtr vm, reason, success); } + +void +virDomainAuditShmem(virDomainObjPtr vm, + virDomainShmemDefPtr oldDef, virDomainShmemDefPtr newDef, + const char *reason, bool success) +{ + return virDomainAuditResource(vm, "shmem", + oldDef ? oldDef->size : 0, + newDef ? newDef->size : 0, + reason, success); +} + + static void virDomainAuditLifecycle(virDomainObjPtr vm, const char *op, const char *reason, bool success) @@ -880,6 +893,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) for (i = 0; i < vm->def->nrngs; i++) virDomainAuditRNG(vm, NULL, vm->def->rngs[i], "start", true); + for (i = 0; i < vm->def->nshmems; i++) + virDomainAuditShmem(vm, NULL, vm->def->shmems[i], "start", true); + if (vm->def->tpm) virDomainAuditTPM(vm, vm->def->tpm, "start", true); diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 97dadca..081cbb1 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -129,6 +129,12 @@ void virDomainAuditRNG(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditShmem(virDomainObjPtr vm, + virDomainShmemDefPtr oldDef, + virDomainShmemDefPtr newDef, + const char *reason, + bool success) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); #endif /* __VIR_DOMAIN_AUDIT_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc8a52d..3ceb4e3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -134,6 +134,7 @@ virDomainAuditNetDevice; virDomainAuditRedirdev; virDomainAuditRNG; virDomainAuditSecurityLabel; +virDomainAuditShmem; virDomainAuditStart; virDomainAuditStop; virDomainAuditVcpu; -- 1.8.3.1
On Wed, Jun 17, 2015 at 11:56:16AM +0800, Luyao Huang wrote:
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- docs/auditlog.html.in | 16 ++++++++++++++++ src/conf/domain_audit.c | 16 ++++++++++++++++ src/conf/domain_audit.h | 6 ++++++ src/libvirt_private.syms | 1 + 4 files changed, 39 insertions(+)
diff --git a/docs/auditlog.html.in b/docs/auditlog.html.in index 8a007ca..b168cbf 100644 --- a/docs/auditlog.html.in +++ b/docs/auditlog.html.in @@ -301,6 +301,22 @@ <dd>Updated path of the backing character device for given emulated device</dd> </dl>
+ <h4><a name="typeresourceivshmem">Shared memory device</a></h4> + <p> + The <code>msg</code> field will include the following sub-fields + </p> + + <dl> + <dt>reason</dt> + <dd>The reason which caused the resource to be assigned to happen</dd> + <dt>resrc</dt> + <dd>The type of resource assigned. Set to <code>shmem</code></dd> + <dt>old-shmem</dt> + <dd>Original memory size of share memory device in bytes, or 0</dd> + <dt>new-shmem</dt> + <dd>Updated memory size of share memory device in bytes</dd>
I don't think memory size is the thing audit cares about, it should be the name/path mostly. Even better if we could audit all of it (size, name, path).
+ </dl> + <h4><a name="typeresourcesmartcard">smartcard</a></h4> <p> The <code>msg</code> field will include the following sub-fields diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 1900039..aa2b4b5 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -799,6 +799,19 @@ virDomainAuditIOThread(virDomainObjPtr vm, reason, success); }
+ +void +virDomainAuditShmem(virDomainObjPtr vm, + virDomainShmemDefPtr oldDef, virDomainShmemDefPtr newDef, + const char *reason, bool success) +{ + return virDomainAuditResource(vm, "shmem", + oldDef ? oldDef->size : 0, + newDef ? newDef->size : 0, + reason, success); +} + + static void virDomainAuditLifecycle(virDomainObjPtr vm, const char *op, const char *reason, bool success) @@ -880,6 +893,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) for (i = 0; i < vm->def->nrngs; i++) virDomainAuditRNG(vm, NULL, vm->def->rngs[i], "start", true);
+ for (i = 0; i < vm->def->nshmems; i++) + virDomainAuditShmem(vm, NULL, vm->def->shmems[i], "start", true); + if (vm->def->tpm) virDomainAuditTPM(vm, vm->def->tpm, "start", true);
diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 97dadca..081cbb1 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -129,6 +129,12 @@ void virDomainAuditRNG(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditShmem(virDomainObjPtr vm, + virDomainShmemDefPtr oldDef, + virDomainShmemDefPtr newDef, + const char *reason, + bool success) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
#endif /* __VIR_DOMAIN_AUDIT_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc8a52d..3ceb4e3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -134,6 +134,7 @@ virDomainAuditNetDevice; virDomainAuditRedirdev; virDomainAuditRNG; virDomainAuditSecurityLabel; +virDomainAuditShmem; virDomainAuditStart; virDomainAuditStop; virDomainAuditVcpu; -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/08/2015 07:56 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:16AM +0800, Luyao Huang wrote:
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- docs/auditlog.html.in | 16 ++++++++++++++++ src/conf/domain_audit.c | 16 ++++++++++++++++ src/conf/domain_audit.h | 6 ++++++ src/libvirt_private.syms | 1 + 4 files changed, 39 insertions(+)
diff --git a/docs/auditlog.html.in b/docs/auditlog.html.in index 8a007ca..b168cbf 100644 --- a/docs/auditlog.html.in +++ b/docs/auditlog.html.in @@ -301,6 +301,22 @@ <dd>Updated path of the backing character device for given emulated device</dd> </dl>
+ <h4><a name="typeresourceivshmem">Shared memory device</a></h4> + <p> + The <code>msg</code> field will include the following sub-fields + </p> + + <dl> + <dt>reason</dt> + <dd>The reason which caused the resource to be assigned to happen</dd> + <dt>resrc</dt> + <dd>The type of resource assigned. Set to <code>shmem</code></dd> + <dt>old-shmem</dt> + <dd>Original memory size of share memory device in bytes, or 0</dd> + <dt>new-shmem</dt> + <dd>Updated memory size of share memory device in bytes</dd>
I don't think memory size is the thing audit cares about, it should be the name/path mostly. Even better if we could audit all of it (size, name, path).
Okay, i agreed with you, Thanks a lot for your review. Luyao
+ </dl> + <h4><a name="typeresourcesmartcard">smartcard</a></h4> <p> The <code>msg</code> field will include the following sub-fields diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 1900039..aa2b4b5 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -799,6 +799,19 @@ virDomainAuditIOThread(virDomainObjPtr vm, reason, success); }
+ +void +virDomainAuditShmem(virDomainObjPtr vm, + virDomainShmemDefPtr oldDef, virDomainShmemDefPtr newDef, + const char *reason, bool success) +{ + return virDomainAuditResource(vm, "shmem", + oldDef ? oldDef->size : 0, + newDef ? newDef->size : 0, + reason, success); +} + + static void virDomainAuditLifecycle(virDomainObjPtr vm, const char *op, const char *reason, bool success) @@ -880,6 +893,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) for (i = 0; i < vm->def->nrngs; i++) virDomainAuditRNG(vm, NULL, vm->def->rngs[i], "start", true);
+ for (i = 0; i < vm->def->nshmems; i++) + virDomainAuditShmem(vm, NULL, vm->def->shmems[i], "start", true); + if (vm->def->tpm) virDomainAuditTPM(vm, vm->def->tpm, "start", true);
diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 97dadca..081cbb1 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -129,6 +129,12 @@ void virDomainAuditRNG(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditShmem(virDomainObjPtr vm, + virDomainShmemDefPtr oldDef, + virDomainShmemDefPtr newDef, + const char *reason, + bool success) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
#endif /* __VIR_DOMAIN_AUDIT_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dc8a52d..3ceb4e3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -134,6 +134,7 @@ virDomainAuditNetDevice; virDomainAuditRedirdev; virDomainAuditRNG; virDomainAuditSecurityLabel; +virDomainAuditShmem; virDomainAuditStart; virDomainAuditStop; virDomainAuditVcpu; -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
The helpers will be useful when implementing hotplug and coldplug of shared memory devices. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++++ src/libvirt_private.syms | 3 +++ 3 files changed, 71 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 306b718..8a8e4f7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def, } +int +virDomainShmemInsert(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem); +} + + +ssize_t +virDomainShmemFind(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + size_t i; + + for (i = 0; i < def->nshmems; i++) { + virDomainShmemDefPtr tmpshmem = def->shmems[i]; + + if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name)) + continue; + + if (shmem->size != tmpshmem->size) + continue; + + if (shmem->server.enabled != tmpshmem->server.enabled || + (shmem->server.enabled && + STRNEQ_NULLABLE(shmem->server.chr.data.nix.path, + tmpshmem->server.chr.data.nix.path))) + continue; + + if (shmem->msi.enabled != tmpshmem->msi.enabled || + (shmem->msi.enabled && + (shmem->msi.vectors != tmpshmem->msi.vectors || + shmem->msi.ioeventfd != tmpshmem->msi.ioeventfd))) + continue; + + if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&shmem->info, &tmpshmem->info)) + continue; + + break; + } + + if (i < def->nshmems) + return i; + + return -1; +} + + +virDomainShmemDefPtr +virDomainShmemRemove(virDomainDefPtr def, + size_t idx) +{ + virDomainShmemDefPtr ret = def->shmems[idx]; + + VIR_DELETE_ELEMENT(def->shmems, idx, def->nshmems); + + return ret; +} + + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a4b1bf3..39bc928 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2943,6 +2943,13 @@ int virDomainMemoryFindInactiveByDef(virDomainDefPtr def, virDomainMemoryDefPtr mem) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +int virDomainShmemInsert(virDomainDefPtr def, virDomainShmemDefPtr shmem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +ssize_t virDomainShmemFind(virDomainDefPtr def, virDomainShmemDefPtr shmem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def, size_t idx) + ATTRIBUTE_NONNULL(1); + VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) VIR_ENUM_DECL(virDomainBoot) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ceb4e3..6127f51 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -439,6 +439,9 @@ virDomainSaveStatus; virDomainSaveXML; virDomainSeclabelTypeFromString; virDomainSeclabelTypeToString; +virDomainShmemFind; +virDomainShmemInsert; +virDomainShmemRemove; virDomainShutdownReasonTypeFromString; virDomainShutdownReasonTypeToString; virDomainShutoffReasonTypeFromString; -- 1.8.3.1
On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:
The helpers will be useful when implementing hotplug and coldplug of shared memory devices.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++++ src/libvirt_private.syms | 3 +++ 3 files changed, 71 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 306b718..8a8e4f7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def, }
+int +virDomainShmemInsert(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem); +} + + +ssize_t +virDomainShmemFind(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + size_t i; + + for (i = 0; i < def->nshmems; i++) { + virDomainShmemDefPtr tmpshmem = def->shmems[i]; + + if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name)) + continue; +
I think that you shouldn't be able to have two <shmem/> elements in the same domain, and since the name is mandatory, STREQ() should be enough to check whether you need to return current 'i'. Well, depending on whether you are unplugging it (you want everything that was specified to match) or plugging it in (if there's the same name, just reject it).
+ if (shmem->size != tmpshmem->size) + continue; + + if (shmem->server.enabled != tmpshmem->server.enabled || + (shmem->server.enabled && + STRNEQ_NULLABLE(shmem->server.chr.data.nix.path, + tmpshmem->server.chr.data.nix.path))) + continue; + + if (shmem->msi.enabled != tmpshmem->msi.enabled || + (shmem->msi.enabled && + (shmem->msi.vectors != tmpshmem->msi.vectors || + shmem->msi.ioeventfd != tmpshmem->msi.ioeventfd))) + continue; + + if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&shmem->info, &tmpshmem->info)) + continue; + + break; + } + + if (i < def->nshmems) + return i; + + return -1; +} + + +virDomainShmemDefPtr +virDomainShmemRemove(virDomainDefPtr def, + size_t idx) +{ + virDomainShmemDefPtr ret = def->shmems[idx]; + + VIR_DELETE_ELEMENT(def->shmems, idx, def->nshmems); + + return ret; +} + + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a4b1bf3..39bc928 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2943,6 +2943,13 @@ int virDomainMemoryFindInactiveByDef(virDomainDefPtr def, virDomainMemoryDefPtr mem) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virDomainShmemInsert(virDomainDefPtr def, virDomainShmemDefPtr shmem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +ssize_t virDomainShmemFind(virDomainDefPtr def, virDomainShmemDefPtr shmem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def, size_t idx) + ATTRIBUTE_NONNULL(1); + VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) VIR_ENUM_DECL(virDomainBoot) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ceb4e3..6127f51 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -439,6 +439,9 @@ virDomainSaveStatus; virDomainSaveXML; virDomainSeclabelTypeFromString; virDomainSeclabelTypeToString; +virDomainShmemFind; +virDomainShmemInsert; +virDomainShmemRemove; virDomainShutdownReasonTypeFromString; virDomainShutdownReasonTypeToString; virDomainShutoffReasonTypeFromString; -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/08/2015 08:14 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:
The helpers will be useful when implementing hotplug and coldplug of shared memory devices.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++++ src/libvirt_private.syms | 3 +++ 3 files changed, 71 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 306b718..8a8e4f7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def, }
+int +virDomainShmemInsert(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem); +} + + +ssize_t +virDomainShmemFind(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + size_t i; + + for (i = 0; i < def->nshmems; i++) { + virDomainShmemDefPtr tmpshmem = def->shmems[i]; + + if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name)) + continue; +
I think that you shouldn't be able to have two <shmem/> elements in the same domain, and since the name is mandatory, STREQ() should be enough to check whether you need to return current 'i'.
Okay, i agree use STREQ() instead of STRNEQ_NULLABLE. BTW I think one guest could have more than one <shmem/> element, maybe one is non-server shmem and another is server shmem, it depends on how to use it.
Well, depending on whether you are unplugging it (you want everything that was specified to match) or plugging it in (if there's the same name, just reject it).
Okay, we could just check the name if use the same shmem (both server and non-server) in the same guest is a invalid case. Thanks a lot for your review. Luyao
+ if (shmem->size != tmpshmem->size) + continue; + + if (shmem->server.enabled != tmpshmem->server.enabled || + (shmem->server.enabled && + STRNEQ_NULLABLE(shmem->server.chr.data.nix.path, + tmpshmem->server.chr.data.nix.path))) + continue; + + if (shmem->msi.enabled != tmpshmem->msi.enabled || + (shmem->msi.enabled && + (shmem->msi.vectors != tmpshmem->msi.vectors || + shmem->msi.ioeventfd != tmpshmem->msi.ioeventfd))) + continue; + + if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&shmem->info, &tmpshmem->info)) + continue; + + break; + } + + if (i < def->nshmems) + return i; + + return -1; +} + + +virDomainShmemDefPtr +virDomainShmemRemove(virDomainDefPtr def, + size_t idx) +{ + virDomainShmemDefPtr ret = def->shmems[idx]; + + VIR_DELETE_ELEMENT(def->shmems, idx, def->nshmems); + + return ret; +} + + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a4b1bf3..39bc928 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2943,6 +2943,13 @@ int virDomainMemoryFindInactiveByDef(virDomainDefPtr def, virDomainMemoryDefPtr mem) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virDomainShmemInsert(virDomainDefPtr def, virDomainShmemDefPtr shmem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +ssize_t virDomainShmemFind(virDomainDefPtr def, virDomainShmemDefPtr shmem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def, size_t idx) + ATTRIBUTE_NONNULL(1); + VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) VIR_ENUM_DECL(virDomainBoot) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ceb4e3..6127f51 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -439,6 +439,9 @@ virDomainSaveStatus; virDomainSaveXML; virDomainSeclabelTypeFromString; virDomainSeclabelTypeToString; +virDomainShmemFind; +virDomainShmemInsert; +virDomainShmemRemove; virDomainShutdownReasonTypeFromString; virDomainShutdownReasonTypeToString; virDomainShutoffReasonTypeFromString; -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jul 09, 2015 at 11:44:30AM +0800, lhuang wrote:
On 07/08/2015 08:14 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:
The helpers will be useful when implementing hotplug and coldplug of shared memory devices.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++++ src/libvirt_private.syms | 3 +++ 3 files changed, 71 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 306b718..8a8e4f7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def, }
+int +virDomainShmemInsert(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem); +} + + +ssize_t +virDomainShmemFind(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + size_t i; + + for (i = 0; i < def->nshmems; i++) { + virDomainShmemDefPtr tmpshmem = def->shmems[i]; + + if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name)) + continue; +
I think that you shouldn't be able to have two <shmem/> elements in the same domain, and since the name is mandatory, STREQ() should be enough to check whether you need to return current 'i'.
Okay, i agree use STREQ() instead of STRNEQ_NULLABLE. BTW I think one guest could have more than one <shmem/> element, maybe one is non-server shmem and another is server shmem, it depends on how to use it.
Maybe this function could have a bool parameter (something like a 'full_match') that would say whether everything must match or the name is enough. And it the bool is false, streq is enough, but if it's true, you could abstract the internals of this for body into virDomainShmemEquals() or something and that would be called instead.
Well, depending on whether you are unplugging it (you want everything that was specified to match) or plugging it in (if there's the same name, just reject it).
Okay, we could just check the name if use the same shmem (both server and non-server) in the same guest is a invalid case.
Thanks a lot for your review.
Luyao
+ if (shmem->size != tmpshmem->size) + continue; + + if (shmem->server.enabled != tmpshmem->server.enabled || + (shmem->server.enabled && + STRNEQ_NULLABLE(shmem->server.chr.data.nix.path, + tmpshmem->server.chr.data.nix.path))) + continue; + + if (shmem->msi.enabled != tmpshmem->msi.enabled || + (shmem->msi.enabled && + (shmem->msi.vectors != tmpshmem->msi.vectors || + shmem->msi.ioeventfd != tmpshmem->msi.ioeventfd))) + continue; + + if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&shmem->info, &tmpshmem->info)) + continue; + + break; + } + + if (i < def->nshmems) + return i; + + return -1; +} + + +virDomainShmemDefPtr +virDomainShmemRemove(virDomainDefPtr def, + size_t idx) +{ + virDomainShmemDefPtr ret = def->shmems[idx]; + + VIR_DELETE_ELEMENT(def->shmems, idx, def->nshmems); + + return ret; +} + + char * virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a4b1bf3..39bc928 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2943,6 +2943,13 @@ int virDomainMemoryFindInactiveByDef(virDomainDefPtr def, virDomainMemoryDefPtr mem) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virDomainShmemInsert(virDomainDefPtr def, virDomainShmemDefPtr shmem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +ssize_t virDomainShmemFind(virDomainDefPtr def, virDomainShmemDefPtr shmem) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +virDomainShmemDefPtr virDomainShmemRemove(virDomainDefPtr def, size_t idx) + ATTRIBUTE_NONNULL(1); + VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) VIR_ENUM_DECL(virDomainBoot) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ceb4e3..6127f51 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -439,6 +439,9 @@ virDomainSaveStatus; virDomainSaveXML; virDomainSeclabelTypeFromString; virDomainSeclabelTypeToString; +virDomainShmemFind; +virDomainShmemInsert; +virDomainShmemRemove; virDomainShutdownReasonTypeFromString; virDomainShutdownReasonTypeToString; virDomainShutoffReasonTypeFromString; -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/09/2015 02:09 PM, Martin Kletzander wrote:
On Thu, Jul 09, 2015 at 11:44:30AM +0800, lhuang wrote:
On 07/08/2015 08:14 PM, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:17AM +0800, Luyao Huang wrote:
The helpers will be useful when implementing hotplug and coldplug of shared memory devices.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 7 ++++++ src/libvirt_private.syms | 3 +++ 3 files changed, 71 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 306b718..8a8e4f7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13343,6 +13343,67 @@ virDomainMemoryRemove(virDomainDefPtr def, }
+int +virDomainShmemInsert(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + return VIR_APPEND_ELEMENT(def->shmems, def->nshmems, shmem); +} + + +ssize_t +virDomainShmemFind(virDomainDefPtr def, + virDomainShmemDefPtr shmem) +{ + size_t i; + + for (i = 0; i < def->nshmems; i++) { + virDomainShmemDefPtr tmpshmem = def->shmems[i]; + + if (STRNEQ_NULLABLE(shmem->name, tmpshmem->name)) + continue; +
I think that you shouldn't be able to have two <shmem/> elements in the same domain, and since the name is mandatory, STREQ() should be enough to check whether you need to return current 'i'.
Okay, i agree use STREQ() instead of STRNEQ_NULLABLE. BTW I think one guest could have more than one <shmem/> element, maybe one is non-server shmem and another is server shmem, it depends on how to use it.
Maybe this function could have a bool parameter (something like a 'full_match') that would say whether everything must match or the name is enough. And it the bool is false, streq is enough, but if it's true, you could abstract the internals of this for body into virDomainShmemEquals() or something and that would be called instead.
Good idea, this way is okay to me. Thanks a lot for your opinion. Luyao
Add support for using the attach/detach device APIs on the inactive configuration to add/del shared memory devices. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6127f51..aff4e83 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -439,6 +439,7 @@ virDomainSaveStatus; virDomainSaveXML; virDomainSeclabelTypeFromString; virDomainSeclabelTypeToString; +virDomainShmemDefFree; virDomainShmemFind; virDomainShmemInsert; virDomainShmemRemove; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6bb8549..932928f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8160,6 +8160,15 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, dev->data.memory = NULL; break; + case VIR_DOMAIN_DEVICE_SHMEM: + if (virDomainShmemInsert(vmdef, dev->data.shmem) < 0) + return -1; + dev->data.shmem = NULL; + + if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) + return -1; + break; + case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: @@ -8169,7 +8178,6 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, 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: @@ -8296,6 +8304,16 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainMemoryDefFree(virDomainMemoryRemove(vmdef, idx)); break; + case VIR_DOMAIN_DEVICE_SHMEM: + if ((idx = virDomainShmemFind(vmdef, dev->data.shmem)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("no matching share memory device was found")); + return -1; + } + + virDomainShmemDefFree(virDomainShmemRemove(vmdef, idx)); + break; + case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: @@ -8305,7 +8323,6 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, 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: -- 1.8.3.1
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 10 ++++++++- src/qemu/qemu_hotplug.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.h | 3 +++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 932928f..b5a0be0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7789,6 +7789,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm, dev->data.memory = NULL; break; + case VIR_DOMAIN_DEVICE_SHMEM: + ret = qemuDomainAttachShmemDevice(driver, vm, + dev->data.shmem); + if (!ret) { + alias = dev->data.shmem->info.alias; + dev->data.shmem = NULL; + } + break; + case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_FS: case VIR_DOMAIN_DEVICE_INPUT: @@ -7800,7 +7809,6 @@ qemuDomainAttachDeviceLive(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_TPM: case VIR_DOMAIN_DEVICE_PANIC: case VIR_DOMAIN_DEVICE_LAST: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cc86a3b..188807e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1832,6 +1832,66 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, } +int +qemuDomainAttachShmemDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainShmemDefPtr shmem) +{ + int ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; + char *devstr = NULL; + char *charAlias = NULL; + + if (virAsprintf(&shmem->info.alias, "shmem%zu", vm->def->nshmems) < 0) + return -1; + + if (VIR_REALLOC_N(vm->def->shmems, vm->def->nshmems + 1) < 0) + return -1; + + if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || + shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &shmem->info) < 0)) + return -1; + + if (!(devstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps))) + goto cleanup; + + if (virAsprintf(&charAlias, "char%s", shmem->info.alias) < 0) + goto cleanup; + + qemuDomainObjEnterMonitor(driver, vm); + + if (shmem->server.enabled && + qemuMonitorAttachCharDev(priv->mon, charAlias, + &shmem->server.chr) < 0) { + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto audit; + } + + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) { + if (shmem->server.enabled) + ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias)); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + goto audit; + } + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto audit; + + if (virDomainShmemInsert(vm->def, shmem) < 0) + goto audit; + + ret = 0; + audit: + virDomainAuditShmem(vm, NULL, shmem, "attach", ret == 0); + cleanup: + if (ret < 0) + qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL); + VIR_FREE(charAlias); + VIR_FREE(devstr); + return ret; +} + + static int qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index 4140da3..60137a6 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -109,6 +109,9 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainRNGDefPtr rng); +int qemuDomainAttachShmemDevice(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainShmemDefPtr shmem); int qemuDomainChrInsert(virDomainDefPtr vmdef, -- 1.8.3.1
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_driver.c | 6 ++-- src/qemu/qemu_hotplug.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_hotplug.h | 3 ++ 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b5a0be0..7c585a3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7887,6 +7887,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: @@ -7898,7 +7901,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: @@ -8315,7 +8317,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef, case VIR_DOMAIN_DEVICE_SHMEM: if ((idx = virDomainShmemFind(vmdef, dev->data.shmem)) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("no matching share memory device was found")); + _("no matching shared memory device was found")); return -1; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 188807e..0f996c3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3298,6 +3298,47 @@ 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; + } + + virDomainAuditShmem(vm, shmem, NULL, "detach", rc == 0); + + if (rc < 0) + return -1; + + if ((event = virDomainEventDeviceRemovedNewFromObj(vm, shmem->info.alias))) + qemuDomainEventQueue(driver, event); + + if ((idx = virDomainShmemFind(vm->def, shmem)) >= 0) + virDomainShmemRemove(vm->def, idx); + qemuDomainReleaseDeviceAddress(vm, &shmem->info, NULL); + virDomainShmemDefFree(shmem); + return 0; +} + + int qemuDomainRemoveDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3329,6 +3370,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: @@ -3342,7 +3387,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: @@ -4335,3 +4379,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; + } + + if ((idx = virDomainShmemFind(vm->def, shmem)) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("device not present in domain configuration")); + 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, -- 1.8.3.1
If user pass a invalid address shared memory device to qemu, qemu won't report the error, but will auto assign a pci address to the shared memory device. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5d55794..9c659fe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8482,6 +8482,13 @@ qemuBuildShmemDevStr(virDomainDefPtr def, } } + if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only 'pci' addresses are supported for the " + "shared memory device")); + goto error; + } + if (qemuBuildDeviceAddressStr(&buf, def, &shmem->info, qemuCaps) < 0) goto error; -- 1.8.3.1
On Wed, Jun 17, 2015 at 11:56:21AM +0800, Luyao Huang wrote:
If user pass a invalid address shared memory device to qemu, qemu won't report the error, but will auto assign a pci address to the shared memory device.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 7 +++++++ 1 file changed, 7 insertions(+)
ACK
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5d55794..9c659fe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8482,6 +8482,13 @@ qemuBuildShmemDevStr(virDomainDefPtr def, } }
+ if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only 'pci' addresses are supported for the " + "shared memory device")); + goto error; + } + if (qemuBuildDeviceAddressStr(&buf, def, &shmem->info, qemuCaps) < 0) goto error;
-- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 08, 2015 at 02:22:36PM +0200, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:21AM +0800, Luyao Huang wrote:
If user pass a invalid address shared memory device to qemu, qemu won't report the error, but will auto assign a pci address to the shared memory device.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 7 +++++++ 1 file changed, 7 insertions(+)
ACK
I also added a test case for this particular patch and pushed it along with the other ACK'd ones. The test case diff squashed in: diff --git c/tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml i/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-address.xml similarity index 95% copy from tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml copy to tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-address.xml index d70279c21faa..8a4e56d5926a 100644 --- c/tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml +++ i/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-address.xml @@ -18,7 +18,7 @@ <controller type='pci' index='0' model='pci-root'/> <memballoon model='none'/> <shmem name='shmem0'> - <msi/> + <address type='isa'/> </shmem> </devices> </domain> diff --git c/tests/qemuxml2argvtest.c i/tests/qemuxml2argvtest.c index bee66372767b..24c1f301e4b9 100644 --- c/tests/qemuxml2argvtest.c +++ i/tests/qemuxml2argvtest.c @@ -1614,6 +1614,8 @@ mymain(void) DO_TEST_FAILURE("shmem", NONE); DO_TEST_FAILURE("shmem-invalid-size", QEMU_CAPS_PCIDEVICE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); + DO_TEST_FAILURE("shmem-invalid-address", QEMU_CAPS_PCIDEVICE, + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST_FAILURE("shmem-small-size", QEMU_CAPS_PCIDEVICE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST_PARSE_ERROR("shmem-msi-only", NONE); --
On 07/08/2015 10:30 PM, Martin Kletzander wrote:
On Wed, Jul 08, 2015 at 02:22:36PM +0200, Martin Kletzander wrote:
On Wed, Jun 17, 2015 at 11:56:21AM +0800, Luyao Huang wrote:
If user pass a invalid address shared memory device to qemu, qemu won't report the error, but will auto assign a pci address to the shared memory device.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/qemu/qemu_command.c | 7 +++++++ 1 file changed, 7 insertions(+)
ACK
I also added a test case for this particular patch and pushed it along with the other ACK'd ones.
Thanks a lot for your review and help. Luyao
The test case diff squashed in:
diff --git c/tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml i/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-address.xml similarity index 95% copy from tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml copy to tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-address.xml index d70279c21faa..8a4e56d5926a 100644 --- c/tests/qemuxml2argvdata/qemuxml2argv-shmem-msi-only.xml +++ i/tests/qemuxml2argvdata/qemuxml2argv-shmem-invalid-address.xml @@ -18,7 +18,7 @@ <controller type='pci' index='0' model='pci-root'/> <memballoon model='none'/> <shmem name='shmem0'> - <msi/> + <address type='isa'/> </shmem> </devices> </domain> diff --git c/tests/qemuxml2argvtest.c i/tests/qemuxml2argvtest.c index bee66372767b..24c1f301e4b9 100644 --- c/tests/qemuxml2argvtest.c +++ i/tests/qemuxml2argvtest.c @@ -1614,6 +1614,8 @@ mymain(void) DO_TEST_FAILURE("shmem", NONE); DO_TEST_FAILURE("shmem-invalid-size", QEMU_CAPS_PCIDEVICE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); + DO_TEST_FAILURE("shmem-invalid-address", QEMU_CAPS_PCIDEVICE, + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST_FAILURE("shmem-small-size", QEMU_CAPS_PCIDEVICE, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); DO_TEST_PARSE_ERROR("shmem-msi-only", NONE); --
participants (3)
-
lhuang -
Luyao Huang
-
Martin Kletzander