Re: [libvirt] [PATCHv2 0/2] doc: Improve snapshot/blockjob docs
On 07/11/14 18:27, Eric Blake wrote: On 07/11/2014 03:01 AM, Peter Krempa wrote: Peter Krempa (2): doc: Document that snapshot name of block-backed disk isnt autogenerated doc: Be more specific about semantics of _REUSE_EXT flag ACK series; you picked up on all my suggestions on v1. Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] OVMF exposure in libvirt
Dear list, there's been a lot of development in QEMU on this part. And I think it's settled down enough long so I can start looking at it. So I'd like to hear you opinion what's the best way to expose this in libvirt. OVMF can bee looked at as a UEFI enablement in guest. Standard UEFI consists of two parts: a) the firmware binary image (RO) b) UEFI variables flash (RW) IIUC both of these are to be passed to qemu on the command line as: -drive file=img_1,if=pflash,format=raw,readonly \ -drive file=img_2,if=pflash,format=raw Subsequently, -bios parameter should be dropped. The idea of splitting the UEFI into two files allows distros to update the UEFI firmware (FW for short) without modifying guest written UEFI variables file (the variables should have unified name so they should be transferable between two versions of UEFI FW). So my question is: how to expose this in the domain XML? We have the os/ element which handles the booting arguments. It can have loader/ (which would be great for the FW, wouldn't it?). But then we need to invent a different element (say vars/) which would contain path the the UEFI vars file. Moreover, the element would exclude other elements like boot/, bios/ or smbios/. So my proposal is: os typehvm/type loader/path/to/uefi.fw/loader vars/path/to/uefi.nvvarstore/vars /os Does this make any sense or am I just blabbing? Michal BTW OVMF stands for Open Virtual Machine Firmware. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virSecurityLabelDef: use enum type for @type
On 11.07.2014 18:59, Eric Blake wrote: On 07/11/2014 03:32 AM, Michal Privoznik wrote: There's this trend in libvirt of using enum types wherever possible. Now that I'm at virSecurityLabelDef let's rework @type item of the structure so we don't have to typecast it elsewhere. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 6 -- src/security/security_dac.c | 2 +- src/util/virseclabel.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) I'm not quite as sure about this one. This solves the issue of how to detect errors when parsing strings to enum, but required the use of an intermediate variable which in turn made the patch a net gain in lines of code. If someone forgets to use the intermediate variable for parsing, this backfires. On the other hand, parsing string to enum should be done in just one location, and that's the location touched by this patch. I'm 50-50 on whether to take this, so I'd like someone else to chime in with an opinion. I hear you. This patch is just a refactor. It does not add anything useful nor solve any issue. It's okay if dropped. But the more I think about our vir*TypeFromString() the more I feel we should do something about it. How about making it follow our typical function return pattern: int func() { virMyFavourite x; const char *string; if (virMyFavouriteTypeFromString(string, x) 0) { virReportError(unknown value: %s, string); goto error; } That is, we need this diff: diff --git a/src/util/virutil.c b/src/util/virutil.c index 95d1ff9..40075e9 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -407,15 +407,20 @@ virParseVersionString(const char *str, unsigned long *version, int virEnumFromString(const char *const*types, unsigned int ntypes, - const char *type) + const char *type, + int *val) { size_t i; if (!type) return -1; -for (i = 0; i ntypes; i++) -if (STREQ(types[i], type)) -return i; +for (i = 0; i ntypes; i++) { +if (STREQ(types[i], type)) { +if (val) +*val = i; +return 0; +} +} return -1; } diff --git a/src/util/virutil.h b/src/util/virutil.h index 2bb74e2..670b2e1 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -73,7 +73,8 @@ char *virIndexToDiskName(int idx, const char *prefix); int virEnumFromString(const char *const*types, unsigned int ntypes, - const char *type); + const char *type, + int *val); const char *virEnumToString(const char *const*types, unsigned int ntypes, @@ -87,10 +88,10 @@ const char *virEnumToString(const char *const*types, ARRAY_CARDINALITY(name ## TypeList), \ type); \ } \ -int name ## TypeFromString(const char *type) { \ +int name ## TypeFromString(const char *type, name *val) { \ return virEnumFromString(name ## TypeList, \ ARRAY_CARDINALITY(name ## TypeList), \ - type); \ + type, (int *) val);\ } # define VIR_ENUM_DECL(name) \ And then a tons of follow up patches. Or even make virEnumString() report the error (that could save a lot of virReportError() calls). Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] OVMF exposure in libvirt
On Mon, Jul 14, 2014 at 09:32:34AM +0200, Michal Privoznik wrote: Dear list, there's been a lot of development in QEMU on this part. And I think it's settled down enough long so I can start looking at it. So I'd like to hear you opinion what's the best way to expose this in libvirt. OVMF can bee looked at as a UEFI enablement in guest. Standard UEFI consists of two parts: a) the firmware binary image (RO) b) UEFI variables flash (RW) IIUC both of these are to be passed to qemu on the command line as: -drive file=img_1,if=pflash,format=raw,readonly \ -drive file=img_2,if=pflash,format=raw Subsequently, -bios parameter should be dropped. The idea of splitting the UEFI into two files allows distros to update the UEFI firmware (FW for short) without modifying guest written UEFI variables file (the variables should have unified name so they should be transferable between two versions of UEFI FW). So my question is: how to expose this in the domain XML? We have the os/ element which handles the booting arguments. It can have loader/ (which would be great for the FW, wouldn't it?). But then we need to invent a different element (say vars/) which would contain path the the UEFI vars file. Moreover, the element would exclude other elements like boot/, bios/ or smbios/. So my proposal is: os typehvm/type loader/path/to/uefi.fw/loader vars/path/to/uefi.nvvarstore/vars /os Does this make any sense or am I just blabbing? We already use loader/ for specifying alternative BIOS blobs for the QEMU -bios arg. Since you say this obsoletes the -bios arg, I think it makes sense to use loader/ for the read-only firmware image. For the variable storage, I'd probably suggest nvram/ as the element name, since IIUC that's a fairly commonly used term for this concept. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 1/3] Add gvir_config_capabilities_cpu_get_model()
On Tue, Jul 08, 2014 at 07:41:55PM +0100, Zeeshan Ali (Khattak) wrote: On Mon, Jul 7, 2014 at 12:09 PM, Christophe Fergeau cferg...@redhat.com wrote: This is returning a char * capabilities host cpu modelxxx/model /cpu /host /capabilities while the next patch exposes the model from the /domain/cpu/model node as an actual object, why the difference? Because /domain/cpu/model node has at least one attribute and hence a simple char * won't do there. My question was the opposite one, why return a char * here instead of an actual object since you have to add it anyway? Christophe pgp7EDfIJt_dd.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] OVMF exposure in libvirt
On 07/14/14 11:27, Daniel P. Berrange wrote: On Mon, Jul 14, 2014 at 09:32:34AM +0200, Michal Privoznik wrote: Dear list, there's been a lot of development in QEMU on this part. And I think it's settled down enough long so I can start looking at it. So I'd like to hear you opinion what's the best way to expose this in libvirt. OVMF can bee looked at as a UEFI enablement in guest. Standard UEFI consists of two parts: a) the firmware binary image (RO) b) UEFI variables flash (RW) IIUC both of these are to be passed to qemu on the command line as: -drive file=img_1,if=pflash,format=raw,readonly \ -drive file=img_2,if=pflash,format=raw Subsequently, -bios parameter should be dropped. The idea of splitting the UEFI into two files allows distros to update the UEFI firmware (FW for short) without modifying guest written UEFI variables file (the variables should have unified name so they should be transferable between two versions of UEFI FW). So my question is: how to expose this in the domain XML? We have the os/ element which handles the booting arguments. It can have loader/ (which would be great for the FW, wouldn't it?). But then we need to invent a different element (say vars/) which would contain path the the UEFI vars file. Moreover, the element would exclude other elements like boot/, bios/ or smbios/. So my proposal is: os typehvm/type loader/path/to/uefi.fw/loader vars/path/to/uefi.nvvarstore/vars /os Does this make any sense or am I just blabbing? We already use loader/ for specifying alternative BIOS blobs for the QEMU -bios arg. Since you say this obsoletes the -bios arg, I think it makes sense to use loader/ for the read-only firmware image. For the variable storage, I'd probably suggest nvram/ as the element name, since IIUC that's a fairly commonly used term for this concept. I prefer nvram too. Additionally it would be great if we'd be able to generate an empty nvram for a guest if the user doesn't specify it. The only problem here is that an empty variable store isn't in fact a zero filled image, but has some structure. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] OVMF exposure in libvirt
On Mon, Jul 14, 2014 at 11:55:52AM +0200, Peter Krempa wrote: On 07/14/14 11:27, Daniel P. Berrange wrote: On Mon, Jul 14, 2014 at 09:32:34AM +0200, Michal Privoznik wrote: Dear list, there's been a lot of development in QEMU on this part. And I think it's settled down enough long so I can start looking at it. So I'd like to hear you opinion what's the best way to expose this in libvirt. OVMF can bee looked at as a UEFI enablement in guest. Standard UEFI consists of two parts: a) the firmware binary image (RO) b) UEFI variables flash (RW) IIUC both of these are to be passed to qemu on the command line as: -drive file=img_1,if=pflash,format=raw,readonly \ -drive file=img_2,if=pflash,format=raw Subsequently, -bios parameter should be dropped. The idea of splitting the UEFI into two files allows distros to update the UEFI firmware (FW for short) without modifying guest written UEFI variables file (the variables should have unified name so they should be transferable between two versions of UEFI FW). So my question is: how to expose this in the domain XML? We have the os/ element which handles the booting arguments. It can have loader/ (which would be great for the FW, wouldn't it?). But then we need to invent a different element (say vars/) which would contain path the the UEFI vars file. Moreover, the element would exclude other elements like boot/, bios/ or smbios/. So my proposal is: os typehvm/type loader/path/to/uefi.fw/loader vars/path/to/uefi.nvvarstore/vars /os Does this make any sense or am I just blabbing? We already use loader/ for specifying alternative BIOS blobs for the QEMU -bios arg. Since you say this obsoletes the -bios arg, I think it makes sense to use loader/ for the read-only firmware image. For the variable storage, I'd probably suggest nvram/ as the element name, since IIUC that's a fairly commonly used term for this concept. I prefer nvram too. Additionally it would be great if we'd be able to generate an empty nvram for a guest if the user doesn't specify it. The only problem here is that an empty variable store isn't in fact a zero filled image, but has some structure. BTW, we I believe some non-x86 architectures would already like to have some nvram capability in libvirt, but I can't remember the details right now. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] LXC: create a bind mount for sysfs when enable userns but disable netns
kernel commit 7dc5dbc879bd0779924b5132a48b731a0bc04a1e forbid us doing a fresh mount for sysfs when enable userns but disable netns. This patch will create a bind mount in this senario. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- src/lxc/lxc_container.c | 44 +--- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 4d89677..8a27215 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -815,10 +815,13 @@ static int lxcContainerSetReadOnly(void) } -static int lxcContainerMountBasicFS(bool userns_enabled) +static int lxcContainerMountBasicFS(bool userns_enabled, +bool netns_disabled) { size_t i; int rc = -1; +char* mnt_src = NULL; +int mnt_mflags; VIR_DEBUG(Mounting basic filesystems); @@ -826,8 +829,25 @@ static int lxcContainerMountBasicFS(bool userns_enabled) bool bindOverReadonly; virLXCBasicMountInfo const *mnt = lxcBasicMounts[i]; +/* When enable userns but disable netns, kernel will + * forbid us doing a new fresh mount for sysfs. + * So we had to do a bind mount for sysfs instead. + */ +if (userns_enabled netns_disabled +STREQ(mnt-src, sysfs)) { +if (VIR_STRDUP(mnt_src, /sys) 0) { +goto cleanup; +} +mnt_mflags = MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY|MS_BIND; +} else { +if (VIR_STRDUP(mnt_src, mnt-src) 0) { +goto cleanup; +} +mnt_mflags = mnt-mflags; +} + VIR_DEBUG(Processing %s - %s, - mnt-src, mnt-dst); + mnt_src, mnt-dst); if (mnt-skipUnmounted) { char *hostdir; @@ -856,7 +876,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled) if (virFileMakePath(mnt-dst) 0) { virReportSystemError(errno, _(Failed to mkdir %s), - mnt-src); + mnt_src); goto cleanup; } @@ -867,24 +887,24 @@ static int lxcContainerMountBasicFS(bool userns_enabled) * we mount the filesystem in read-write mode initially, and then do a * separate read-only bind mount on top of that. */ -bindOverReadonly = !!(mnt-mflags MS_RDONLY); +bindOverReadonly = !!(mnt_mflags MS_RDONLY); VIR_DEBUG(Mount %s on %s type=%s flags=%x, - mnt-src, mnt-dst, mnt-type, mnt-mflags ~MS_RDONLY); -if (mount(mnt-src, mnt-dst, mnt-type, mnt-mflags ~MS_RDONLY, NULL) 0) { + mnt_src, mnt-dst, mnt-type, mnt_mflags ~MS_RDONLY); +if (mount(mnt_src, mnt-dst, mnt-type, mnt_mflags ~MS_RDONLY, NULL) 0) { virReportSystemError(errno, _(Failed to mount %s on %s type %s flags=%x), - mnt-src, mnt-dst, NULLSTR(mnt-type), - mnt-mflags ~MS_RDONLY); + mnt_src, mnt-dst, NULLSTR(mnt-type), + mnt_mflags ~MS_RDONLY); goto cleanup; } if (bindOverReadonly -mount(mnt-src, mnt-dst, NULL, +mount(mnt_src, mnt-dst, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY, NULL) 0) { virReportSystemError(errno, _(Failed to re-mount %s on %s flags=%x), - mnt-src, mnt-dst, + mnt_src, mnt-dst, MS_BIND|MS_REMOUNT|MS_RDONLY); goto cleanup; } @@ -893,6 +913,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled) rc = 0; cleanup: +VIR_FREE(mnt_src); VIR_DEBUG(rc=%d, rc); return rc; } @@ -1643,7 +1664,8 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, goto cleanup; /* Mounts the core /proc, /sys, etc filesystems */ -if (lxcContainerMountBasicFS(vmDef-idmap.nuidmap) 0) +if (lxcContainerMountBasicFS(vmDef-idmap.nuidmap, + !vmDef-nnets) 0) goto cleanup; /* Ensure entire root filesystem (except /.oldroot) is readonly */ -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2]util:openvswitch:Delete port if it is exist when add port
On 12.07.2014 06:47, Lichunhe wrote: If the ovs service stop abnormal, or host cold reboot, vm is destroyed after ovs service stop. The ovs port which connect to interface of vm will not be clear. When the ovs service restart, recover configuration from db, but the interface is no exist, port recovery failed, and then vm restart on the same host, libvirt add port again, but the port configuration is same as before, ovs will not connect the interface, only store the configuration in db. Below will trigger this problem, We like the commit messages wrapped at 80 characters per line. Moreover, I find it somehow hard to parse. virsh start vm service openvswitch-switch stop virsh destroy vm service openvswitch-switch start virsh start vm Signed-off-by: Chunhe Li lichu...@huawei.com --- src/util/virnetdevopenvswitch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) I had some difficulties applying this patch. Did you hand edit it before sending? diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 9bcbfb1..2c414ad 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -84,8 +84,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, cmd = virCommandNew(OVSVSCTL); -virCommandAddArgList(cmd, --timeout=5, --, --may-exist, add-port, -brname, ifname, NULL); +virCommandAddArgList(cmd, --timeout=5, --, --if-exists, del-port, Trailing whitespace. +ifname, --, add-port, brname, ifname, NULL); if (virtVlan virtVlan-nTags 0) { -- 1.9.2.msysgit.0 Strange git version string :) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Well, I've pointed out enough problems to request v3, but since this practically oneliner, I rather fix all the nits and push. ACKed and pushed. Congratulations on your first libvirt contribution! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 5/6] tests: Add test cases for secondary-vga
From: Zeng Junliang zengjunli...@huawei.com While adding support for secondary-vga, we should add test cases for it and test its basic functions. Signed-off-by: Zeng Junliang zengjunli...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- qemuxml2argv-graphics-vnc-secondary-vga.xml| 39 ++ .../qemuxml2argv-graphics-vnc-secondary-vga.args | 7 .../qemuxml2argv-graphics-vnc-secondary-vga.xml| 39 ++ tests/qemuxml2argvtest.c | 10 +++--- tests/qemuxml2xmltest.c| 1 + 5 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 qemuxml2argv-graphics-vnc-secondary-vga.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.xml diff --git a/qemuxml2argv-graphics-vnc-secondary-vga.xml b/qemuxml2argv-graphics-vnc-secondary-vga.xml new file mode 100644 index 000..d43cf36 --- /dev/null +++ b/qemuxml2argv-graphics-vnc-secondary-vga.xml @@ -0,0 +1,39 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + driver name='qemu' type='raw'/ + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='pci' index='0' model='pci-root'/ +input type='mouse' bus='ps2'/ +input type='keyboard' bus='ps2'/ +graphics type='vnc' port='5903' autoport='no' listen='2001:1:2:3:4:5:1234:1234' + listen type='address' address='2001:1:2:3:4:5:1234:1234'/ +/graphics +video + model type='secondary' vgamem='16384' heads='1'/ +/video +video + model type='secondary' vgamem='16384' heads='1'/ +/video +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.args new file mode 100644 index 000..305c76a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.args @@ -0,0 +1,7 @@ +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 -nodefaults -monitor unix:/tmp/test-monitor,\ +server,nowait -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 \ +-vnc '[2001:1:2:3:4:5:1234:1234]:3' \ +-device secondary-vga,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2 \ +-device secondary-vga,id=video1,vgamem_mb=16,bus=pci.0,addr=0x4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.xml new file mode 100644 index 000..d43cf36 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.xml @@ -0,0 +1,39 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + driver name='qemu' type='raw'/ + source dev='/dev/HostVG/QEMUGuest1'/ + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='pci' index='0' model='pci-root'/ +input type='mouse' bus='ps2'/ +input type='keyboard' bus='ps2'/ +graphics type='vnc' port='5903' autoport='no' listen='2001:1:2:3:4:5:1234:1234' + listen type='address' address='2001:1:2:3:4:5:1234:1234'/ +/graphics +video + model type='secondary' vgamem='16384' heads='1'/ +/video +video + model type='secondary' vgamem='16384' heads='1'/ +/video +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a841adb..5e75e8f 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -291,7 +291,6 @@ static int testCompareXMLToArgvFiles(const char *xml, goto ok; goto out; } - if
[libvirt] [PATCH V2 1/6] qemu: Remove vram attribute in some cases
From: Zeng Junliang zengjunli...@huawei.com The vram attribute is invalid for cirrus and stdvga device, and default vram value would make us confused. It would be better to remove it. Signed-off-by: Zeng Junliang zengjunli...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- src/conf/domain_conf.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b91ccf7..63d97ec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9306,16 +9306,13 @@ virDomainVideoDefaultRAM(const virDomainDef *def, int type) { switch (type) { -/* Weird, QEMU defaults to 9 MB ??! */ -case VIR_DOMAIN_VIDEO_TYPE_VGA: -case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: case VIR_DOMAIN_VIDEO_TYPE_VMVGA: if (def-virtType == VIR_DOMAIN_VIRT_VBOX) return 8 * 1024; else if (def-virtType == VIR_DOMAIN_VIRT_VMWARE) return 4 * 1024; else -return 9 * 1024; +return 0; break; case VIR_DOMAIN_VIDEO_TYPE_XEN: @@ -9474,9 +9471,19 @@ virDomainVideoDefParseXML(xmlNodePtr node, } if (vram) { +/* For type of kvm, vram attribute seems to be invalid + * for VIR_DOMAIN_VIDEO_TYPE_VMVGA. Shall we also need + * to add judge here? Will it affect other drivers? */ +if (def-type == VIR_DOMAIN_VIDEO_TYPE_VGA || +def-type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(vram attribute is not supported + for type of vga and cirrus)); +goto error; +} if (virStrToLong_ui(vram, NULL, 10, def-vram) 0) { virReportError(VIR_ERR_XML_ERROR, - _(cannot parse video ram '%s'), vram); + _(cannot parse video vram '%s'), vram); goto error; } } else { -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 0/6] Some improvements for video model.
From: Zeng Junliang zengjunli...@huawei.com https://www.redhat.com/archives/libvir-list/2014-June/msg00569.html diff to v1: - Rmoving the confusing vram attribute for cirrus and stdvga. - leave out cirrus for the new vgamem attribute. - add secondary-vga supports and some other docs and tests. Zeng Junliang (6): qemu: Remove vram attribute in some cases qemu: Introduce vgamem attribute for video model qemu: Add support for secondary-vga tests: modify test case related to vgamem attribute tests: Add test cases for secondary-vga docs: add description for vgamem attribute and secondary-vga docs/formatdomain.html.in | 39 --- docs/schemas/domaincommon.rng | 5 + qemuxml2argv-graphics-vnc-secondary-vga.xml| 39 +++ src/conf/domain_conf.c | 71 ++-- src/conf/domain_conf.h | 4 +- src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 125 ++--- ...qemuhotplug-console-compat-2+console-virtio.xml | 2 +- .../qemuxml2argv-console-compat-2.xml | 2 +- .../qemuxml2argv-controller-order.xml | 2 +- .../qemuxml2argv-graphics-listen-network.xml | 2 +- .../qemuxml2argv-graphics-listen-network2.xml | 2 +- .../qemuxml2argv-graphics-sdl-fullscreen.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +- ...emuxml2argv-graphics-spice-agent-file-xfer.args | 5 +- ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 4 +- .../qemuxml2argv-graphics-spice-agentmouse.xml | 2 +- .../qemuxml2argv-graphics-spice-compression.args | 4 +- .../qemuxml2argv-graphics-spice-compression.xml| 4 +- .../qemuxml2argv-graphics-spice-listen-network.xml | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.args | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.xml| 4 +- .../qemuxml2argv-graphics-spice-sasl.args | 3 +- .../qemuxml2argv-graphics-spice-sasl.xml | 2 +- .../qemuxml2argv-graphics-spice-timeout.xml| 2 +- .../qemuxml2argv-graphics-spice.args | 5 +- .../qemuxml2argv-graphics-spice.xml| 4 +- .../qemuxml2argv-graphics-vnc-policy.xml | 2 +- .../qemuxml2argv-graphics-vnc-sasl.xml | 2 +- .../qemuxml2argv-graphics-vnc-secondary-vga.args | 7 ++ .../qemuxml2argv-graphics-vnc-secondary-vga.xml| 39 +++ .../qemuxml2argv-graphics-vnc-socket.xml | 2 +- .../qemuxml2argv-graphics-vnc-tls.xml | 2 +- .../qemuxml2argv-graphics-vnc-websocket.xml| 2 +- .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 2 +- .../qemuxml2argv-net-bandwidth.xml | 2 +- .../qemuxml2argv-pci-autoadd-addr.xml | 2 +- .../qemuxml2argv-pci-autoadd-idx.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 2 +- .../qemuxml2argv-pcihole64-q35.args| 3 +- .../qemuxml2argv-pcihole64-q35.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-q35.xml| 2 +- .../qemuxml2argv-serial-spiceport.args | 4 +- .../qemuxml2argv-serial-spiceport.xml | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 9 +- .../qemuxml2argv-video-device-pciaddr-default.xml | 6 +- tests/qemuxml2argvtest.c | 10 +- .../qemuxml2xmlout-graphics-listen-network2.xml| 2 +- .../qemuxml2xmlout-graphics-spice-timeout.xml | 2 +- .../qemuxml2xmlout-pci-autoadd-addr.xml| 2 +- .../qemuxml2xmlout-pci-autoadd-idx.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml| 2 +- tests/qemuxml2xmltest.c| 1 + 57 files changed, 345 insertions(+), 123 deletions(-) create mode 100644 qemuxml2argv-graphics-vnc-secondary-vga.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.xml -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 3/6] qemu: Add support for secondary-vga
From: Zeng Junliang zengjunli...@huawei.com Secondary-vga is supported by QEMU in currently master. Add it supported in libvirt as qemu commandline shows: '-device secondary-vga'. Signed-off-by: Zeng Junliang zengjunli...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- src/conf/domain_conf.c | 16 -- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 50 ++-- 5 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d5a65c3..216d4e1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -478,7 +478,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, vmvga, xen, vbox, - qxl) + qxl, + secondary) VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST, mouse, @@ -9335,7 +9336,8 @@ virDomainVideoDefaultVgamem(int type) case VIR_DOMAIN_VIDEO_TYPE_VGA: case VIR_DOMAIN_VIDEO_TYPE_VMVGA: case VIR_DOMAIN_VIDEO_TYPE_QXL: -/* QEMU use 16M as default value for vga/vmvga/qxl device*/ +case VIR_DOMAIN_VIDEO_TYPE_SECONDARY: +/* QEMU use 16M as default value for vga/vmvga/qxl/secondary device*/ return 16 * 1024; default: @@ -9491,10 +9493,11 @@ virDomainVideoDefParseXML(xmlNodePtr node, * for VIR_DOMAIN_VIDEO_TYPE_VMVGA. Shall we also need * to add judge here? Will it affect other drivers? */ if (def-type == VIR_DOMAIN_VIDEO_TYPE_VGA || -def-type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS) { +def-type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS || +def-type == VIR_DOMAIN_VIDEO_TYPE_SECONDARY) { virReportError(VIR_ERR_XML_ERROR, %s, _(vram attribute is not supported - for type of vga and cirrus)); + for type of vga, cirrus, qxl and secondary)); goto error; } if (virStrToLong_ui(vram, NULL, 10, def-vram) 0) { @@ -9509,10 +9512,11 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (vgamem) { if (def-type != VIR_DOMAIN_VIDEO_TYPE_VGA def-type != VIR_DOMAIN_VIDEO_TYPE_VMVGA -def-type != VIR_DOMAIN_VIDEO_TYPE_QXL) { +def-type != VIR_DOMAIN_VIDEO_TYPE_QXL +def-type != VIR_DOMAIN_VIDEO_TYPE_SECONDARY) { virReportError(VIR_ERR_XML_ERROR, %s, _(vgamem attribute only supported - for type of vga, vmvga and qxl)); + for type of vga, vmvga, qxl and secondary)); goto error; } if (virStrToLong_ui(vgamem, NULL, 10, def-vgamem) 0) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a63ec84..bf1bd55 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1199,6 +1199,7 @@ typedef enum { VIR_DOMAIN_VIDEO_TYPE_XEN, VIR_DOMAIN_VIDEO_TYPE_VBOX, VIR_DOMAIN_VIDEO_TYPE_QXL, +VIR_DOMAIN_VIDEO_TYPE_SECONDARY, VIR_DOMAIN_VIDEO_TYPE_LAST } virDomainVideoType; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c665e2b..68b86dc 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -259,6 +259,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, msg-timestamp, active-commit, change-backing-file, + secondary-vga, ); @@ -1451,6 +1452,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { spicevmc, QEMU_CAPS_DEVICE_SPICEVMC }, { qxl-vga, QEMU_CAPS_DEVICE_QXL_VGA }, { qxl, QEMU_CAPS_DEVICE_QXL }, +{ secondary-vga, QEMU_CAPS_DEVICE_SECONDARY_VGA }, { sga, QEMU_CAPS_SGA }, { scsi-block, QEMU_CAPS_SCSI_BLOCK }, { scsi-cd, QEMU_CAPS_SCSI_CD }, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 99cf9ed..73c45d7 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -209,6 +209,7 @@ typedef enum { QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */ QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */ QEMU_CAPS_CHANGE_BACKING_FILE = 169, /* change name of backing file in metadata */ +QEMU_CAPS_DEVICE_SECONDARY_VGA = 170, /* -device secondary-vga */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cb6d6e2..3a6762c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -109,7 +109,9 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, vmware, , /* no arg needed for xen */ , /* don't support vbox */ -
[libvirt] [PATCH V2 4/6] tests: modify test case related to vgamem attribute
From: Zeng Junliang zengjunli...@huawei.com While removing the useless vram attribute for cirrus and stdvga device, we should update the corresponding test cases. Adding test cases for the new vgamem attribute is also needed. Signed-off-by: Zeng Junliang zengjunli...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- .../qemuhotplug-console-compat-2+console-virtio.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network2.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +- .../qemuxml2argv-graphics-spice-agent-file-xfer.args | 5 +++-- .../qemuxml2argv-graphics-spice-agent-file-xfer.xml | 4 ++-- .../qemuxml2argvdata/qemuxml2argv-graphics-spice-agentmouse.xml | 2 +- .../qemuxml2argv-graphics-spice-compression.args | 4 +++- .../qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml | 4 ++-- .../qemuxml2argv-graphics-spice-listen-network.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args | 4 +++- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args | 5 +++-- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-policy.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-websocket.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-net-bandwidth.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-addr.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.xml| 2 +- tests/qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 3 ++- tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.args| 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-serial-spiceport.xml | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 9 ++--- .../qemuxml2argv-video-device-pciaddr-default.xml| 6 +++--- .../qemuxml2xmlout-graphics-listen-network2.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autoadd-addr.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autoadd-idx.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 2 +- 44 files changed, 68 insertions(+), 56 deletions(-) diff --git a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml index ec1c6e8..6ee2b58 100644 --- a/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml +++ b/tests/qemuhotplugtestdata/qemuhotplug-console-compat-2+console-virtio.xml @@ -91,7 +91,7 @@ address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0'/ /sound video - model type='cirrus' vram='9216' heads='1'/ + model type='cirrus' heads='1'/ address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/ /video memballoon model='virtio' diff --git a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml index 4d4ac47..bae4ff2a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-console-compat-2.xml @@ -88,7 +88,7 @@ address type='pci' domain='0x' bus='0x00' slot='0x04' function='0x0'/ /sound video - model type='cirrus' vram='9216' heads='1'/ + model type='cirrus' heads='1'/ address type='pci' domain='0x' bus='0x00' slot='0x02' function='0x0'/ /video memballoon model='virtio' diff --git
[libvirt] [PATCH V2 6/6] docs: add description for vgamem attribute and secondary-vga
From: Zeng Junliang zengjunli...@huawei.com Enhance schema. Add description for vgamem attribute and secondary-vga. Signed-off-by: Zeng Junliang zengjunli...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- docs/formatdomain.html.in | 39 ++- docs/schemas/domaincommon.rng | 5 + 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b69da4c..98dca4c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4272,7 +4272,7 @@ qemu-kvm -net nic,model=? /dev/null ... lt;devicesgt; lt;videogt; - lt;model type='vga' vram='8192' heads='1'gt; + lt;model type='vga' vgamem='16384' heads='1'gt; lt;acceleration accel3d='yes' accel2d='yes'/gt; lt;/modelgt; lt;/videogt; @@ -4287,28 +4287,41 @@ qemu-kvm -net nic,model=? /dev/null is set but there is a codegraphics/code in domain xml, then libvirt will add a default codevideo/code according to the guest type. For a guest of type kvm, the default codevideo/code for it is: -codetype/code with value cirrus, codevram/code with value -9216, and codeheads/code with value 1. By default, the first -video device in domain xml is the primary one, but the optional -attribute codeprimary/code (span class=sincesince 1.0.2/span) -with value 'yes' can be used to mark the primary in cases of multiple -video device. The non-primary must be type of qxl. The optional +codetype/code with value cirrus, codeheads/code with +value 1. By default, the first video device in domain xml is the +primary one, but the optional attribute codeprimary/code +(span class=sincesince 1.0.2/span) with value 'yes' can be +used to mark the primary in cases of multiple video device. The +non-primary must be type of qxl or secondary. The optional attribute coderam/code (span class=sincesince 1.0.2/span) is allowed for qxl type only and specifies the size of the primary bar, while codevram/code specifies the -secondary bar size. If ram or vram are not supplied a default -value is used. +secondary bar size. If ram, vram or vgamem are not supplied +a default value is used. /dd dtcodemodel/code/dt dd The codemodel/code element has a mandatory codetype/code attribute which takes the value vga, cirrus, vmvga, xen, -vbox, or qxl (span class=sincesince 0.8.6/span) +vbox, qxl (span class=sincesince 0.8.6/span), or + secondary (span class=sincesince 1.2.6/span) depending on the hypervisor features available. -You can also provide the amount of video memory in kibibytes -(blocks of 1024 bytes) using -codevram/code and the number of screen with codeheads/code. +p +codevram/code attribute specifies the amount of video memory +in kibibytes (blocks of 1024 bytes). It is invalid for type of + cirrus or vga. +/p +p +codevgamem/code attribute span class=sincesince 1.2.6, +QEMU and KVM only/span specifies the size of the framebuffer +portion of the ram region. +And it is only valid for type of vga, vmvga, qxl, +and secondary. +/p +p +codeheads/code attribute specifies the number of screen. +/p /dd dtcodeacceleration/code/dt diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7be028d..cb272ca 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2708,6 +2708,11 @@ /attribute /optional optional +attribute name=vgamem + ref name=unsignedInt/ +/attribute + /optional + optional attribute name=heads ref name=unsignedInt/ /attribute -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 2/6] qemu: Introduce vgamem attribute for video model
From: Zeng Junliang zengjunli...@huawei.com This patch introduces vgamem attribute for video model, and sets its default value as qemu used. Parse it in two ways accroding to qemu startup parameters supported: -device or -vga. Signed-off-by: Zeng Junliang zengjunli...@huawei.com Signed-off-by: Wang Rui moon.wang...@huawei.com --- src/conf/domain_conf.c | 48 ++- src/conf/domain_conf.h | 3 +- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 75 4 files changed, 101 insertions(+), 26 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 63d97ec..d5a65c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9328,6 +9328,20 @@ virDomainVideoDefaultRAM(const virDomainDef *def, } } +int +virDomainVideoDefaultVgamem(int type) +{ +switch (type) { +case VIR_DOMAIN_VIDEO_TYPE_VGA: +case VIR_DOMAIN_VIDEO_TYPE_VMVGA: +case VIR_DOMAIN_VIDEO_TYPE_QXL: +/* QEMU use 16M as default value for vga/vmvga/qxl device*/ +return 16 * 1024; + +default: +return 0; +} +} int virDomainVideoDefaultType(const virDomainDef *def) @@ -9413,6 +9427,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *type = NULL; char *heads = NULL; char *vram = NULL; +char *vgamem = NULL; char *ram = NULL; char *primary = NULL; @@ -9422,11 +9437,12 @@ virDomainVideoDefParseXML(xmlNodePtr node, cur = node-children; while (cur != NULL) { if (cur-type == XML_ELEMENT_NODE) { -if (!type !vram !ram !heads +if (!type !vram !ram !heads !vgamem xmlStrEqual(cur-name, BAD_CAST model)) { type = virXMLPropString(cur, type); ram = virXMLPropString(cur, ram); vram = virXMLPropString(cur, vram); +vgamem = virXMLPropString(cur, vgamem); heads = virXMLPropString(cur, heads); if ((primary = virXMLPropString(cur, primary)) != NULL) { @@ -9490,6 +9506,24 @@ virDomainVideoDefParseXML(xmlNodePtr node, def-vram = virDomainVideoDefaultRAM(dom, def-type); } +if (vgamem) { +if (def-type != VIR_DOMAIN_VIDEO_TYPE_VGA +def-type != VIR_DOMAIN_VIDEO_TYPE_VMVGA +def-type != VIR_DOMAIN_VIDEO_TYPE_QXL) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(vgamem attribute only supported + for type of vga, vmvga and qxl)); +goto error; +} +if (virStrToLong_ui(vgamem, NULL, 10, def-vgamem) 0) { +virReportError(VIR_ERR_XML_ERROR, + _(cannot parse video vgamem '%s'), vgamem); +goto error; +} +} else { +def-vgamem = virDomainVideoDefaultVgamem(def-type); +} + if (heads) { if (virStrToLong_ui(heads, NULL, 10, def-heads) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -9506,6 +9540,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); +VIR_FREE(vgamem); VIR_FREE(heads); return def; @@ -9515,6 +9550,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); +VIR_FREE(vgamem); VIR_FREE(heads); return NULL; } @@ -12636,6 +12672,7 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(video); goto error; } +video-vgamem = virDomainVideoDefaultVgamem(video-type); video-vram = virDomainVideoDefaultRAM(def, video-type); video-heads = 1; if (VIR_ALLOC_N(def-videos, 1) 0) { @@ -13558,6 +13595,13 @@ virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, return false; } +if (src-vgamem!= dst-vgamem) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Target video card vgamem %u does not match source %u), + dst-vgamem, src-vgamem); +return false; +} + if (src-heads != dst-heads) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(Target video card heads %u does not match source %u), @@ -16532,6 +16576,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, ram='%u', def-ram); if (def-vram) virBufferAsprintf(buf, vram='%u', def-vram); +if (def-vgamem) +virBufferAsprintf(buf, vgamem='%u', def-vgamem); if (def-heads) virBufferAsprintf(buf, heads='%u', def-heads); if (def-primary) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 32674e0..a63ec84 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1216,6 +1216,7 @@ struct _virDomainVideoDef { int type; unsigned int ram; /* kibibytes (multiples of 1024) */ unsigned int vram; /*
Re: [libvirt] [PATCH 0/2] Implement interface stats for BSD
On 06.07.2014 18:28, Roman Bogorodskiy wrote: This series implements support for querying network interface stats on (Free)BSD. It's more of an RFC, because I'm uncertain about few things: - It feels a little strange to have a source file that implements only a single function like this. I am wondering if it would be better to just move it to something like util/virnetdev.c? - FreeBSD stores interface data in the if_data struct and a number of outgoing packet drops is stored in a field 'ifi_oqdrops'. This field was added in -CURRENT and later merged back to 10-STABLE. In order not to break the ABI, it's available only if _IFI_OQDROPS is defined. I've added a configure.ac check which adds -D_IFI_OQDROPS before checking this field and resetting it back if it is not present. This way, this flag will present when the field is available even if the flag is not needed (e.g. on -CURRENT). Is there a better way of doing it? I was thinking about trying to check this field without the flag and if it fails check one more time with the flag, but it looks a little messy. - Did I get it right that the stats reported are from the guest POV, e.g. when downloading a large file from guest, it should look like: vnet0 rx_bytes 731603341 vnet0 rx_packets 518354 vnet0 rx_errs 0 vnet0 rx_drop 0 vnet0 tx_bytes 17577834 vnet0 tx_packets 264226 vnet0 tx_errs 0 vnet0 tx_drop 0 Roman Bogorodskiy (2): util: virstatslinux: make more generic Implement interface stats for BSD configure.ac | 13 - po/POTFILES.in | 2 +- src/Makefile.am | 2 +- src/libvirt_linux.syms | 3 -- src/libvirt_private.syms | 2 + src/lxc/lxc_driver.c | 2 +- src/openvz/openvz_driver.c | 2 +- src/qemu/qemu_driver.c | 16 +- src/uml/uml_driver.c | 2 +- src/util/{virstatslinux.c = virstats.c} | 93 +--- src/util/{virstatslinux.h = virstats.h} | 12 ++--- src/xen/xen_hypervisor.c | 2 +- tests/statstest.c| 2 +- 13 files changed, 102 insertions(+), 51 deletions(-) rename src/util/{virstatslinux.c = virstats.c} (61%) rename src/util/{virstatslinux.h = virstats.h} (77%) ACK to both patches. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Rework lxc apparmor profile
Quoting Cédric Bosdonnat (cbosdon...@suse.com): diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc index d404328..4bfb503 100644 --- a/examples/apparmor/libvirt-lxc +++ b/examples/apparmor/libvirt-lxc @@ -2,16 +2,115 @@ Hi, this being a verbatim copy from lxc's policy, is there any plan for keeping the policy uptodate as the lxc policy is updated? Does lxc-enter-namespace --cmd /bin/bash still work? (I would expect so) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 1e2a38b..778d233 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -351,26 +351,36 @@ AppArmorSetSecuritySCSILabel(virSCSIDevicePtr dev ATTRIBUTE_UNUSED, static int AppArmorSecurityManagerProbe(const char *virtDriver ATTRIBUTE_UNUSED) { -char *template = NULL; +char *template_qemu = NULL; +char *template_lxc = NULL; int rc = SECURITY_DRIVER_DISABLE; if (use_apparmor() 0) return rc; /* see if template file exists */ -if (virAsprintf(template, %s/TEMPLATE, +if (virAsprintf(template_qemu, %s/TEMPLATE.qemu, APPARMOR_DIR /libvirt) == -1) return rc; -if (!virFileExists(template)) { +if (virAsprintf(template_lxc, %s/TEMPLATE.lxc, + APPARMOR_DIR /libvirt) == -1) (This remains a bug, a 'goto cleanup' is needed here) + +if (!virFileExists(template_qemu)) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(template \'%s\' does not exist), template_qemu); +goto cleanup; +} +if (!virFileExists(template_lxc)) { virReportError(VIR_ERR_INTERNAL_ERROR, - _(template \'%s\' does not exist), template); + _(template \'%s\' does not exist), template_lxc); goto cleanup; } rc = SECURITY_DRIVER_ENABLE; cleanup: -VIR_FREE(template); +VIR_FREE(template_qemu); +VIR_FREE(template_lxc); return rc; } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index d563b98..2a09145 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -336,24 +336,20 @@ create_profile(const char *profile, const char *profile_name, char *pcontent = NULL; char *replace_name = NULL; char *replace_files = NULL; -char *replace_driver = NULL; const char *template_name = \nprofile LIBVIRT_TEMPLATE; const char *template_end = \n}; -const char *template_driver = libvirt-driver; int tlen, plen; int fd; int rc = -1; -const char *driver_name = qemu; - -if (virtType == VIR_DOMAIN_VIRT_LXC) -driver_name = lxc; if (virFileExists(profile)) { vah_error(NULL, 0, _(profile exists)); goto end; } -if (virAsprintfQuiet(template, %s/TEMPLATE, APPARMOR_DIR /libvirt) 0) { + +if (virAsprintfQuiet(template, %s/TEMPLATE.%s, APPARMOR_DIR /libvirt, + virDomainVirtTypeToString(virtType)) 0) { vah_error(NULL, 0, _(template name exceeds maximum length)); goto end; } @@ -378,11 +374,6 @@ create_profile(const char *profile, const char *profile_name, goto clean_tcontent; } -if (strstr(tcontent, template_driver) == NULL) { -vah_error(NULL, 0, _(no replacement string in template)); -goto clean_tcontent; -} - /* '\nprofile profile_name\0' */ if (virAsprintfQuiet(replace_name, \nprofile %s, profile_name) == -1) { vah_error(NULL, 0, _(could not allocate memory for profile name)); @@ -397,15 +388,7 @@ create_profile(const char *profile, const char *profile_name, goto clean_tcontent; } -/* 'libvirt-driver_name\0' */ -if (virAsprintfQuiet(replace_driver, libvirt-%s, driver_name) == -1) { -vah_error(NULL, 0, _(could not allocate memory for profile driver)); -VIR_FREE(replace_driver); -goto clean_tcontent; -} - -plen = tlen + strlen(replace_name) - strlen(template_name) + - strlen(replace_driver) - strlen(template_driver) + 1; +plen = tlen + strlen(replace_name) - strlen(template_name) + 1; if (virtType != VIR_DOMAIN_VIRT_LXC) plen += strlen(replace_files) - strlen(template_end); @@ -422,9 +405,6 @@ create_profile(const char *profile, const char *profile_name, pcontent[0] = '\0'; strcpy(pcontent, tcontent); -if (replace_string(pcontent, plen, template_driver, replace_driver) 0) -goto clean_all; - if (replace_string(pcontent, plen, template_name, replace_name) 0) goto clean_all; @@ -455,7 +435,6 @@ create_profile(const char *profile, const char *profile_name, clean_replace: VIR_FREE(replace_name); VIR_FREE(replace_files); -
Re: [libvirt] [libvirt-glib 1/3] Add gvir_config_capabilities_cpu_get_model()
On Mon, Jul 14, 2014 at 10:44 AM, Christophe Fergeau cferg...@redhat.com wrote: On Tue, Jul 08, 2014 at 07:41:55PM +0100, Zeeshan Ali (Khattak) wrote: On Mon, Jul 7, 2014 at 12:09 PM, Christophe Fergeau cferg...@redhat.com wrote: This is returning a char * capabilities host cpu modelxxx/model /cpu /host /capabilities while the next patch exposes the model from the /domain/cpu/model node as an actual object, why the difference? Because /domain/cpu/model node has at least one attribute and hence a simple char * won't do there. My question was the opposite one, why return a char * here instead of an actual object since you have to add it anyway? Ah so you mean that I use the same object to represent both? I'm not sure that makes sense since they are part of different XMLs and therefore different entities. I'm not sure here. -- Regards, Zeeshan Ali (Khattak) Befriend GNOME: http://www.gnome.org/friends/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] docs: mention more about older capability feature bits
Our documentation for features was rather sparse; this fleshes out more of the details for other existing capabilities (and cost me some time trawling git history). * docs/formatcaps.html.in: Document it feature bits. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/formatcaps.html.in | 42 +- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index 137af25..4245d8a 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -95,7 +95,47 @@ dtcodefeatures/code/dt ddThis optional element encases possible features that can be used -with a guest of described type./dd + with a guest of described type. Possible subelements are: + dl +dtpae/dtddIf present, 32-bit guests can use PAE + address space extensions, span class=sincesince + 0.4.1/span/dd +dtnonpae/dtddIf present, 32-bit guests can be run + without requiring PAE, span class=sincesince + 0.4.1/span/dd +dtia64_be/dtddIf present, IA64 guests can be run in + big-endian mode, span class=sincesince 0.4.1/span/dd +dtacpi/dtddIf this element is present, + the codedefault/code attribute describes whether the + hypervisor exposes ACPI to the guest by default, and + the codetoggle/code attribute describes whether the + user can override this + default. span class=sinceSince 0.4.1/span/dd +dtapic/dtddIf this element is present, + the codedefault/code attribute describes whether the + hypervisor exposes APIC to the guest by default, and + the codetoggle/code attribute describes whether the + user can override this + default. span class=sinceSince 0.4.1/span/dd +dtcpuselection/dtddIf this element is present, the + hypervisor supports the codelt;cpugt;/code element + within a domain definition for fine-grained control over + the CPU presented to the + guest. span class=sinceSince 0.7.5/span/dd +dtdeviceboot/dtddIf this element is present, + the codelt;boot order='...'/gt;/code element can + be used inside devices, rather than the older boot + specification by category. span class=sinceSince + 0.8.8/span/dd +dtdisksnapshot/dtddIf this element is present, + the codedefault/code attribute describes whether + external disk snapshots are supported. If absent, + external snapshots may still be supported, but it + requires attempting the API and checking for an error to + find out for sure. span class=sinceSince + 1.2.3/span/dd + /dl +/dd /dl h3a name=elementExamplesExamples/a/h3 -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] capabilities cleanups
I'm still trying to figure out how best to expose capabilities in the new virConnectGetDomainCapabilities for things such as active commit - we may want to tweak that XML before it becomes baked in as part of the 1.2.7 release. But meanwhile, these two patches should be fairly non-controversial. Eric Blake (2): docs: mention more about older capability feature bits capabilities: use bool instead of int docs/formatcaps.html.in | 42 +++- src/bhyve/bhyve_capabilities.c | 2 +- src/conf/capabilities.c | 18 - src/conf/capabilities.h | 18 - src/esx/esx_driver.c | 4 ++-- src/libxl/libxl_conf.c | 4 ++-- src/lxc/lxc_conf.c | 4 ++-- src/openvz/openvz_conf.c | 2 +- src/parallels/parallels_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_capabilities.c | 17 src/security/virt-aa-helper.c| 2 +- src/test/test_driver.c | 6 +++--- src/uml/uml_conf.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/vmware/vmware_conf.c | 2 +- src/xen/xen_hypervisor.c | 31 ++--- src/xenapi/xenapi_driver.c | 2 +- tests/qemucaps2xmltest.c | 2 +- tests/testutils.c| 2 +- tests/testutilslxc.c | 2 +- tests/testutilsqemu.c| 6 +++--- tests/testutilsxen.c | 2 +- tests/vircaps2xmltest.c | 2 +- tests/vircapstest.c | 2 +- tests/vmx2xmltest.c | 2 +- tests/xml2vmxtest.c | 2 +- 27 files changed, 112 insertions(+), 72 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] capabilities: use bool instead of int
While preparing to add a capability for active commit, I noticed that the existing code was abusing int for boolean values. * src/conf/capabilities.h (_virCapsGuestFeature, _virCapsHost) (virCapabilitiesNew, virCapabilitiesAddGuestFeature): Improve types. * src/conf/capabilities.c (virCapabilitiesNew) (virCapabilitiesAddGuestFeature): Adjust signature. * src/bhyve/bhyve_capabilities.c (virBhyveCapsBuild): Update clients. * src/esx/esx_driver.c (esxCapsInit): Likewise. * src/libxl/libxl_conf.c (libxlMakeCapabilities): Likewise. * src/lxc/lxc_conf.c (virLXCDriverCapsInit): Likewise. * src/openvz/openvz_conf.c (openvzCapsInit): Likewise. * src/parallels/parallels_driver.c (parallelsBuildCapabilities): Likewise. * src/phyp/phyp_driver.c (phypCapsInit): Likewise. * src/qemu/qemu_capabilities.c (virQEMUCapsInit) (virQEMUCapsInitGuestFromBinary): Likewise. * src/security/virt-aa-helper.c (get_definition): Likewise. * src/test/test_driver.c (testBuildCapabilities): Likewise. * src/uml/uml_conf.c (umlCapsInit): Likewise. * src/vbox/vbox_tmpl.c (vboxCapsInit): Likewise. * src/vmware/vmware_conf.c (vmwareCapsInit): Likewise. * src/xen/xen_hypervisor.c (xenHypervisorBuildCapabilities): Likewise. * src/xenapi/xenapi_driver.c (getCapsObject): Likewise. * tests/qemucaps2xmltest.c (testGetCaps): Likewise. * tests/testutils.c (virTestGenericCapsInit): Likewise. * tests/testutilslxc.c (testLXCCapsInit): Likewise. * tests/testutilsqemu.c (testQemuCapsInit): Likewise. * tests/testutilsxen.c (testXenCapsInit): Likewise. * tests/vircaps2xmltest.c (buildVirCapabilities): Likewise. * tests/vircapstest.c (buildNUMATopology): Likewise. * tests/vmx2xmltest.c (testCapsInit): Likewise. * tests/xml2vmxtest.c (testCapsInit): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/bhyve/bhyve_capabilities.c | 2 +- src/conf/capabilities.c | 18 +- src/conf/capabilities.h | 18 +- src/esx/esx_driver.c | 4 ++-- src/libxl/libxl_conf.c | 4 ++-- src/lxc/lxc_conf.c | 4 ++-- src/openvz/openvz_conf.c | 2 +- src/parallels/parallels_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_capabilities.c | 17 + src/security/virt-aa-helper.c| 2 +- src/test/test_driver.c | 6 +++--- src/uml/uml_conf.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/vmware/vmware_conf.c | 2 +- src/xen/xen_hypervisor.c | 31 +++ src/xenapi/xenapi_driver.c | 2 +- tests/qemucaps2xmltest.c | 2 +- tests/testutils.c| 2 +- tests/testutilslxc.c | 2 +- tests/testutilsqemu.c| 6 +++--- tests/testutilsxen.c | 2 +- tests/vircaps2xmltest.c | 2 +- tests/vircapstest.c | 2 +- tests/vmx2xmltest.c | 2 +- tests/xml2vmxtest.c | 2 +- 26 files changed, 71 insertions(+), 71 deletions(-) diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index bbca924..132ce91 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -82,7 +82,7 @@ virBhyveCapsBuild(void) virCapsGuestPtr guest; if ((caps = virCapabilitiesNew(virArchFromHost(), - 0, 0)) == NULL) + false, false)) == NULL) return NULL; if ((guest = virCapabilitiesAddGuest(caps, hvm, diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 8504d34..dc2c01a 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -58,15 +58,15 @@ VIR_ONCE_GLOBAL_INIT(virCapabilities) /** * virCapabilitiesNew: * @hostarch: host machine architecture - * @offlineMigrate: non-zero if offline migration is available - * @liveMigrate: non-zero if live migration is available + * @offlineMigrate: true if offline migration is available + * @liveMigrate: true if live migration is available * * Allocate a new capabilities object */ virCapsPtr virCapabilitiesNew(virArch hostarch, - int offlineMigrate, - int liveMigrate) + bool offlineMigrate, + bool liveMigrate) { virCapsPtr caps; @@ -502,16 +502,16 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest, * virCapabilitiesAddGuestFeature: * @guest: guest to associate feature with * @name: name of feature ('pae', 'acpi', 'apic') - * @defaultOn: non-zero if it defaults to on - * @toggle: non-zero if its state can be toggled + * @defaultOn: true if it defaults to on + * @toggle: true if its state can be toggled * - * Registers a feature for a guest domain + * Registers a feature for a guest domain. */ virCapsGuestFeaturePtr virCapabilitiesAddGuestFeature(virCapsGuestPtr guest, const char *name, - int defaultOn, -
Re: [libvirt] OVMF exposure in libvirt
Il 14/07/2014 11:27, Daniel P. Berrange ha scritto: -drive file=img_1,if=pflash,format=raw,readonly \ -drive file=img_2,if=pflash,format=raw It's safer to add ,unit=0 and ,unit=1 too. We already use loader/ for specifying alternative BIOS blobs for the QEMU -bios arg. Since you say this obsoletes the -bios arg, I think it makes sense to use loader/ for the read-only firmware image. It obsoletes the -bios argument, but it is not the same thing: 1) on some machines you can use -drive if=pflash for the nvram, but not for the firmware 2) on some older versions of QEMU, -drive if=pflash will work only on TCG or will not work at all so you cannot blindly replace it. What about: loader readonly='on|off' type='rom|flash'.../loader nvram.../nvram where the mapping from nvram to -drive if=flash is partly machine-dependent. On x86, for example, nvram adds the ,unit=1 sub-option and fails if the loader element is absent; it also fails if loader has type='rom'. For the variable storage, I'd probably suggest nvram/ as the element name, since IIUC that's a fairly commonly used term for this concept. I like this. Additionally it would be great if we'd be able to generate an empty nvram for a guest if the user doesn't specify it. Laszlo has OVMF patches to auto-format an all-zero nvram file. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] OVMF exposure in libvirt
On Mon, Jul 14, 2014 at 04:12:14PM +0200, Paolo Bonzini wrote: Il 14/07/2014 11:27, Daniel P. Berrange ha scritto: -drive file=img_1,if=pflash,format=raw,readonly \ -drive file=img_2,if=pflash,format=raw It's safer to add ,unit=0 and ,unit=1 too. Is there any compelling reason to make the unit numbers user configurable. Are they guest ABI sensitive for example ? If so, we're kind of screwed with loader because we can't add a address sub-element to it as we do for ABI addressing elsewhere. We already use loader/ for specifying alternative BIOS blobs for the QEMU -bios arg. Since you say this obsoletes the -bios arg, I think it makes sense to use loader/ for the read-only firmware image. It obsoletes the -bios argument, but it is not the same thing: 1) on some machines you can use -drive if=pflash for the nvram, but not for the firmware 2) on some older versions of QEMU, -drive if=pflash will work only on TCG or will not work at all so you cannot blindly replace it. What about: loader readonly='on|off' type='rom|flash'.../loader nvram.../nvram where the mapping from nvram to -drive if=flash is partly machine-dependent. On x86, for example, nvram adds the ,unit=1 sub-option and fails if the loader element is absent; it also fails if loader has type='rom'. That sounds reasonable to me. For the variable storage, I'd probably suggest nvram/ as the element name, since IIUC that's a fairly commonly used term for this concept. I like this. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Tri-state bool enum cleanups
We have been using a few different enum types with the same values: DEFAULT ENABLED = on / yes DISABLED = off / no Replace these with just two enums with rather unimaginative names: virDomainYesNo and virDomainOnOff Ján Tomko (2): Introduce virDomainYesNo enum type Introduce virDomainOnOff enum src/conf/domain_conf.c | 194 +-- src/conf/domain_conf.h | 127 +-- src/libvirt_private.syms | 26 +-- src/libxl/libxl_conf.c | 6 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c | 2 +- src/qemu/qemu_command.c | 80 +-- src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_process.c | 2 +- src/vbox/vbox_tmpl.c | 22 +++--- src/xenxs/xen_sxpr.c | 20 ++--- src/xenxs/xen_xm.c | 20 ++--- 12 files changed, 174 insertions(+), 333 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Introduce virDomainOnOff enum
Replace virDomainFeatureState virDomainIoEventFd virDomainVirtioEventIdx virDomainDiskCopyOnRead virDomainMemDump virDomainPCIRombarMode virDomainGraphicsSpicePlaybackCompression --- src/conf/domain_conf.c | 140 ++- src/conf/domain_conf.h | 82 +-- src/libvirt_private.syms | 18 ++ src/libxl/libxl_conf.c | 6 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c | 2 +- src/qemu/qemu_command.c | 58 ++-- src/qemu/qemu_process.c | 2 +- src/vbox/vbox_tmpl.c | 22 src/xenxs/xen_sxpr.c | 20 +++ src/xenxs/xen_xm.c | 20 +++ 11 files changed, 139 insertions(+), 235 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 113bd10..438b533 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -149,7 +149,7 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, hyperv, pvspinlock) -VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST, +VIR_ENUM_IMPL(virDomainOnOff, VIR_DOMAIN_ON_OFF_LAST, default, on, off) @@ -262,21 +262,6 @@ VIR_ENUM_IMPL(virDomainDeviceSGIO, VIR_DOMAIN_DEVICE_SGIO_LAST, filtered, unfiltered) -VIR_ENUM_IMPL(virDomainIoEventFd, VIR_DOMAIN_IO_EVENT_FD_LAST, - default, - on, - off) - -VIR_ENUM_IMPL(virDomainVirtioEventIdx, VIR_DOMAIN_VIRTIO_EVENT_IDX_LAST, - default, - on, - off) - -VIR_ENUM_IMPL(virDomainDiskCopyOnRead, VIR_DOMAIN_DISK_COPY_ON_READ_LAST, - default, - on, - off) - VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, ide, fdc, @@ -439,11 +424,6 @@ VIR_ENUM_IMPL(virDomainSoundModel, VIR_DOMAIN_SOUND_MODEL_LAST, ich6, ich9) -VIR_ENUM_IMPL(virDomainMemDump, VIR_DOMAIN_MEM_DUMP_LAST, - default, - on, - off) - VIR_ENUM_IMPL(virDomainMemballoonModel, VIR_DOMAIN_MEMBALLOON_MODEL_LAST, virtio, xen, @@ -552,12 +532,6 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceZlibCompression, never, always); -VIR_ENUM_IMPL(virDomainGraphicsSpicePlaybackCompression, - VIR_DOMAIN_GRAPHICS_SPICE_PLAYBACK_COMPRESSION_LAST, - default, - on, - off); - VIR_ENUM_IMPL(virDomainGraphicsSpiceMouseMode, VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_LAST, default, @@ -592,12 +566,6 @@ VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST, misc, net) -VIR_ENUM_IMPL(virDomainPCIRombarMode, - VIR_DOMAIN_PCI_ROMBAR_LAST, - default, - on, - off) - VIR_ENUM_IMPL(virDomainHub, VIR_DOMAIN_HUB_TYPE_LAST, usb) @@ -2542,7 +2510,7 @@ virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags) return true; if (info-mastertype != VIR_DOMAIN_CONTROLLER_MASTER_NONE) return true; -if ((info-rombar != VIR_DOMAIN_PCI_ROMBAR_DEFAULT) || +if ((info-rombar != VIR_DOMAIN_ON_OFF_DEFAULT) || info-romfile) return true; if (info-bootIndex) @@ -3149,7 +3117,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, virBufferAddLit(buf, rom); if (info-rombar) { -const char *rombar = virDomainPCIRombarModeTypeToString(info-rombar); +const char *rombar = virDomainOnOffTypeToString(info-rombar); if (!rombar) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -3666,7 +3634,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, if (rom) { char *rombar = virXMLPropString(rom, bar); if (rombar -((info-rombar = virDomainPCIRombarModeTypeFromString(rombar)) = 0)) { +((info-rombar = virDomainOnOffTypeFromString(rombar)) = 0)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(unknown rom bar value '%s'), rombar); VIR_FREE(rombar); @@ -5733,7 +5701,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } if (removable) { -if ((def-removable = virDomainFeatureStateTypeFromString(removable)) 0) { +if ((def-removable = virDomainOnOffTypeFromString(removable)) 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(unknown disk removable status '%s'), removable); goto error; @@ -5746,7 +5714,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } } else { if (def-bus == VIR_DOMAIN_DISK_BUS_USB) { -def-removable = VIR_DOMAIN_FEATURE_STATE_DEFAULT; +def-removable = VIR_DOMAIN_ON_OFF_DEFAULT; } } @@ -5806,7
Re: [libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type
On Mon, Jul 14, 2014 at 04:47:16PM +0200, Ján Tomko wrote: @@ -10901,9 +10884,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, for useserial)); goto cleanup; } -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED; } else { -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED; } VIR_FREE(tmp); } @@ -16943,10 +16926,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceMouseModeTypeToString(def-data.spice.mousemode)); if (def-data.spice.copypaste) virBufferAsprintf(buf, clipboard copypaste='%s'/\n, - virDomainGraphicsSpiceClipboardCopypasteTypeToString(def-data.spice.copypaste)); + virDomainYesNoTypeToString(def-data.spice.copypaste)); if (def-data.spice.filetransfer) virBufferAsprintf(buf, filetransfer enable='%s'/\n, - virDomainGraphicsSpiceAgentFileTransferTypeToString(def-data.spice.filetransfer)); + virDomainYesNoTypeToString(def-data.spice.filetransfer)); } I'm not really a fan of this cleanup, as IMHO the result is less clear harder to follow than the original code. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] capabilities cleanups
On 07/14/2014 10:08 AM, Eric Blake wrote: I'm still trying to figure out how best to expose capabilities in the new virConnectGetDomainCapabilities for things such as active commit - we may want to tweak that XML before it becomes baked in as part of the 1.2.7 release. But meanwhile, these two patches should be fairly non-controversial. Eric Blake (2): docs: mention more about older capability feature bits capabilities: use bool instead of int docs/formatcaps.html.in | 42 +++- src/bhyve/bhyve_capabilities.c | 2 +- src/conf/capabilities.c | 18 - src/conf/capabilities.h | 18 - src/esx/esx_driver.c | 4 ++-- src/libxl/libxl_conf.c | 4 ++-- src/lxc/lxc_conf.c | 4 ++-- src/openvz/openvz_conf.c | 2 +- src/parallels/parallels_driver.c | 2 +- src/phyp/phyp_driver.c | 2 +- src/qemu/qemu_capabilities.c | 17 src/security/virt-aa-helper.c| 2 +- src/test/test_driver.c | 6 +++--- src/uml/uml_conf.c | 2 +- src/vbox/vbox_tmpl.c | 2 +- src/vmware/vmware_conf.c | 2 +- src/xen/xen_hypervisor.c | 31 ++--- src/xenapi/xenapi_driver.c | 2 +- tests/qemucaps2xmltest.c | 2 +- tests/testutils.c| 2 +- tests/testutilslxc.c | 2 +- tests/testutilsqemu.c| 6 +++--- tests/testutilsxen.c | 2 +- tests/vircaps2xmltest.c | 2 +- tests/vircapstest.c | 2 +- tests/vmx2xmltest.c | 2 +- tests/xml2vmxtest.c | 2 +- 27 files changed, 112 insertions(+), 72 deletions(-) ACK to both John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type
Replace all three-state (default/yes/no) enums with it: virDomainBootMenu virDomainPMState virDomainGraphicsSpiceClipboardCopypaste virDomainGraphicsSpiceAgentFileTransfer --- src/conf/domain_conf.c | 54 ++-- src/conf/domain_conf.h | 45 ++-- src/libvirt_private.syms | 8 --- src/qemu/qemu_command.c | 22 ++-- src/qemu/qemu_driver.c | 4 ++-- 5 files changed, 35 insertions(+), 98 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1d83f13..113bd10 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -134,7 +134,7 @@ VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST, hd, network) -VIR_ENUM_IMPL(virDomainBootMenu, VIR_DOMAIN_BOOT_MENU_LAST, +VIR_ENUM_IMPL(virDomainYesNo, VIR_DOMAIN_YES_NO_LAST, default, yes, no) @@ -180,11 +180,6 @@ VIR_ENUM_IMPL(virDomainLockFailure, VIR_DOMAIN_LOCK_FAILURE_LAST, pause, ignore) -VIR_ENUM_IMPL(virDomainPMState, VIR_DOMAIN_PM_STATE_LAST, - default, - yes, - no) - VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, none, disk, @@ -576,18 +571,6 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceStreamingMode, all, off); -VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste, - VIR_DOMAIN_GRAPHICS_SPICE_CLIPBOARD_COPYPASTE_LAST, - default, - yes, - no); - -VIR_ENUM_IMPL(virDomainGraphicsSpiceAgentFileTransfer, - VIR_DOMAIN_GRAPHICS_SPICE_AGENT_FILE_TRANSFER_LAST, - default, - yes, - no); - VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST, subsystem, capabilities) @@ -8789,7 +8772,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, } if ((copypasteVal = - virDomainGraphicsSpiceClipboardCopypasteTypeFromString(copypaste)) = 0) { + virDomainYesNoTypeFromString(copypaste)) = 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(unknown copypaste value '%s'), copypaste); VIR_FREE(copypaste); @@ -8809,7 +8792,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, } if ((enableVal = - virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) = 0) { + virDomainYesNoTypeFromString(enable)) = 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(unknown enable value '%s'), enable); VIR_FREE(enable); @@ -9916,7 +9899,7 @@ virDomainPMStateParseXML(xmlXPathContextPtr ctxt, int ret = -1; char *tmp = virXPathString(xpath, ctxt); if (tmp) { -*val = virDomainPMStateTypeFromString(tmp); +*val = virDomainYesNoTypeFromString(tmp); if (*val 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(unknown PM state value %s), tmp); @@ -10879,14 +10862,14 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, tmp = virXPathString(string(./os/bootmenu[1]/@enable), ctxt); if (tmp) { -def-os.bootmenu = virDomainBootMenuTypeFromString(tmp); +def-os.bootmenu = virDomainYesNoTypeFromString(tmp); if (def-os.bootmenu = 0) { /* In order not to break misconfigured machines, this * should not emit an error, but rather set the bootmenu * to disabled */ VIR_WARN(disabling bootmenu due to unknown option '%s', tmp); -def-os.bootmenu = VIR_DOMAIN_BOOT_MENU_DISABLED; +def-os.bootmenu = VIR_DOMAIN_YES_NO_DISABLED; } VIR_FREE(tmp); } @@ -10901,9 +10884,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, for useserial)); goto cleanup; } -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED; } else { -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED; } VIR_FREE(tmp); } @@ -16943,10 +16926,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceMouseModeTypeToString(def-data.spice.mousemode)); if (def-data.spice.copypaste) virBufferAsprintf(buf, clipboard copypaste='%s'/\n, - virDomainGraphicsSpiceClipboardCopypasteTypeToString(def-data.spice.copypaste)); +
Re: [libvirt] OVMF exposure in libvirt
Il 14/07/2014 16:43, Daniel P. Berrange ha scritto: -drive file=img_1,if=pflash,format=raw,readonly \ -drive file=img_2,if=pflash,format=raw It's safer to add ,unit=0 and ,unit=1 too. Is there any compelling reason to make the unit numbers user configurable. Are they guest ABI sensitive for example ? unit=0 goes just up to the 4G limit (e.g. if it's 256k it occupies the 256K from fffc to ). unit=1 goes just below that. So yes they affect the guest ABI. However, unit=0 contains the reset vector, so the loader must be unit=0 and nvram must be unit=1. Exchanging them makes little sense. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] blockjob: wait for pivot to complete
https://bugzilla.redhat.com/show_bug.cgi?id=1119173 documents that commit eaba79d was flawed in the implementation of the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag when it comes to completing a blockcopy. Basically, the qemu pivot action is async (the QMP command returns immediately, but the user must wait for the BLOCK_JOB_COMPLETE event to know that all I/O related to the job has finally been flushed), but the libvirt command was documented as synchronous by default. As active block commit will also be using this code, it is worth fixing now. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Don't skip wait loop after pivot. Signed-off-by: Eric Blake ebl...@redhat.com --- Shown here with extra context, to hopefully aid the review. src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..300e49e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14973,45 +14973,45 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, if (!device) goto endjob; disk = vm-def-disks[idx]; if (mode == BLOCK_JOB_PULL disk-mirror) { virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, _(disk '%s' already in active block job), disk-dst); goto endjob; } if (mode == BLOCK_JOB_ABORT (flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) !(async disk-mirror)) { virReportError(VIR_ERR_OPERATION_INVALID, _(pivot of disk '%s' requires an active copy job), disk-dst); goto endjob; } if (disk-mirror mode == BLOCK_JOB_ABORT (flags VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT)) { ret = qemuDomainBlockPivot(conn, driver, vm, device, disk); -goto endjob; +goto waitjob; } if (base (virStorageFileParseChainIndex(disk-dst, base, baseIndex) 0 || !(baseSource = virStorageFileChainLookup(disk-src, disk-src, base, baseIndex, NULL goto endjob; if (baseSource) { if (qemuGetDriveSourceString(baseSource, NULL, basePath) 0) goto endjob; if (flags VIR_DOMAIN_BLOCK_REBASE_RELATIVE) { if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(this QEMU binary doesn't support relative block pull/rebase)); goto endjob; } if (virStorageFileGetRelativeBackingPath(disk-src-backingStore, baseSource, @@ -15030,44 +15030,45 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath, bandwidth, info, mode, async); qemuDomainObjExitMonitor(driver, vm); if (ret 0) goto endjob; /* Snoop block copy operations, so future cancel operations can * avoid checking if pivot is safe. */ if (mode == BLOCK_JOB_INFO ret == 1 disk-mirror info-cur == info-end info-type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) disk-mirroring = true; /* A successful block job cancelation stops any mirroring. */ if (mode == BLOCK_JOB_ABORT disk-mirror) { /* XXX We should also revoke security labels and disk lease on * the mirror, and audit that fact, before dropping things. */ virStorageSourceFree(disk-mirror); disk-mirror = NULL; disk-mirroring = false; } + waitjob: /* With synchronous block cancel, we must synthesize an event, and * we silently ignore the ABORT_ASYNC flag. With asynchronous * block cancel, the event will come from qemu, but without the * ABORT_ASYNC flag, we must block to guarantee synchronous * operation. We do the waiting while still holding the VM job, * to prevent newly scheduled block jobs from confusing us. */ if (mode == BLOCK_JOB_ABORT) { if (!async) { /* Older qemu that lacked async reporting also lacked * active commit, so we can hardcode the event to pull. * We have to generate two variants of the event. */ int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; int status = VIR_DOMAIN_BLOCK_JOB_CANCELED; event = virDomainEventBlockJobNewFromObj(vm, disk-src-path, type, status); event2 = virDomainEventBlockJob2NewFromObj(vm, disk-dst, type, status); } else if (!(flags VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)) { while (1) { /* Poll every 50ms */
[libvirt] [PATCH] GetBlockInfo: Use the correct path to qemuOpenFile
Commit id '3ea661de' refactored the code to use the 'disk-src-path' instead of getting the path from virDomainDiskGetSource(). The one call to qemuOpenFile() didn't use the disk source path, rather it used the path as passed from the caller (in this case 'vda') - this caused a failure with the virt-test/tp-libvirt as follows: $ virsh domblkinfo virt-tests-vm1 vda error: cannot stat file '/home/virt-test/shared/data/images/jeos-20-64.qcow2': Bad file descriptor $ Signed-off-by: John Ferlan jfer...@redhat.com --- NOTE: Luckily this hasn't crept into a release... src/qemu/qemu_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ecccf6c..8d40bc9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10370,7 +10370,8 @@ qemuDomainGetBlockInfo(virDomainPtr dom, goto cleanup; } -if ((fd = qemuOpenFile(driver, vm, path, O_RDONLY, NULL, NULL)) == -1) +if ((fd = qemuOpenFile(driver, vm, disk-src-path, O_RDONLY, + NULL, NULL)) == -1) goto cleanup; if (fstat(fd, sb) 0) { -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] capabilities cleanups
On 07/14/2014 08:58 AM, John Ferlan wrote: On 07/14/2014 10:08 AM, Eric Blake wrote: I'm still trying to figure out how best to expose capabilities in the new virConnectGetDomainCapabilities for things such as active commit - we may want to tweak that XML before it becomes baked in as part of the 1.2.7 release. But meanwhile, these two patches should be fairly non-controversial. Eric Blake (2): docs: mention more about older capability feature bits capabilities: use bool instead of int ACK to both Thanks; pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type
On 07/14/2014 08:56 AM, Daniel P. Berrange wrote: On Mon, Jul 14, 2014 at 04:47:16PM +0200, Ján Tomko wrote: @@ -10901,9 +10884,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, for useserial)); goto cleanup; } -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED; } else { -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED; } VIR_FREE(tmp); } @@ -16943,10 +16926,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceMouseModeTypeToString(def-data.spice.mousemode)); if (def-data.spice.copypaste) virBufferAsprintf(buf, clipboard copypaste='%s'/\n, - virDomainGraphicsSpiceClipboardCopypasteTypeToString(def-data.spice.copypaste)); + virDomainYesNoTypeToString(def-data.spice.copypaste)); if (def-data.spice.filetransfer) virBufferAsprintf(buf, filetransfer enable='%s'/\n, - virDomainGraphicsSpiceAgentFileTransferTypeToString(def-data.spice.filetransfer)); + virDomainYesNoTypeToString(def-data.spice.filetransfer)); } I'm not really a fan of this cleanup, as IMHO the result is less clear harder to follow than the original code. How so? The original code was very repetitive, with multiple enums (all with long names) copying the same few enum elements. We're not painting ourselves into a corner - if any of the replaced enums ever grows a third value (such as on, hybrid, off), then we just break that one enum back into a named list rather than using the generic on/off enum. I'm actually in favor of this cleanup. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type
On Mon, Jul 14, 2014 at 10:38:01AM -0600, Eric Blake wrote: On 07/14/2014 08:56 AM, Daniel P. Berrange wrote: On Mon, Jul 14, 2014 at 04:47:16PM +0200, Ján Tomko wrote: @@ -10901,9 +10884,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, for useserial)); goto cleanup; } -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED; } else { -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED; } VIR_FREE(tmp); } @@ -16943,10 +16926,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceMouseModeTypeToString(def-data.spice.mousemode)); if (def-data.spice.copypaste) virBufferAsprintf(buf, clipboard copypaste='%s'/\n, - virDomainGraphicsSpiceClipboardCopypasteTypeToString(def-data.spice.copypaste)); + virDomainYesNoTypeToString(def-data.spice.copypaste)); if (def-data.spice.filetransfer) virBufferAsprintf(buf, filetransfer enable='%s'/\n, - virDomainGraphicsSpiceAgentFileTransferTypeToString(def-data.spice.filetransfer)); + virDomainYesNoTypeToString(def-data.spice.filetransfer)); } I'm not really a fan of this cleanup, as IMHO the result is less clear harder to follow than the original code. How so? The original code was very repetitive, with multiple enums (all with long names) copying the same few enum elements. We're not painting ourselves into a corner - if any of the replaced enums ever grows a third value (such as on, hybrid, off), then we just break that one enum back into a named list rather than using the generic on/off enum. I'm actually in favor of this cleanup. Specifically a enum constant name like YES_NO_DISABLED is just awful IMHO compared to the original desriptive name. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virSecurityLabelDef: use enum type for @type
On 07/14/2014 03:09 AM, Michal Privoznik wrote: On 11.07.2014 18:59, Eric Blake wrote: On 07/11/2014 03:32 AM, Michal Privoznik wrote: There's this trend in libvirt of using enum types wherever possible. Now that I'm at virSecurityLabelDef let's rework @type item of the structure so we don't have to typecast it elsewhere. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/conf/domain_conf.c | 6 -- src/security/security_dac.c | 2 +- src/util/virseclabel.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) I'm not quite as sure about this one. This solves the issue of how to detect errors when parsing strings to enum, but required the use of an intermediate variable which in turn made the patch a net gain in lines of code. If someone forgets to use the intermediate variable for parsing, this backfires. On the other hand, parsing string to enum should be done in just one location, and that's the location touched by this patch. I'm 50-50 on whether to take this, so I'd like someone else to chime in with an opinion. I hear you. This patch is just a refactor. It does not add anything useful nor solve any issue. It's okay if dropped. But the more I think about our vir*TypeFromString() the more I feel we should do something about it. How about making it follow our typical function return pattern: int func() { virMyFavourite x; const char *string; if (virMyFavouriteTypeFromString(string, x) 0) { virReportError(unknown value: %s, string); goto error; } Yeah, making the API failsafe by separating error return from value setting is what we've done elsewhere (such as virStrToLong_*). It's a bigger, more invasive change, but may be worth it. And if we do it, we can then start sticking enum values inside non-public structs. That is, we need this diff: diff --git a/src/util/virutil.c b/src/util/virutil.c index 95d1ff9..40075e9 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -407,15 +407,20 @@ virParseVersionString(const char *str, unsigned long *version, int virEnumFromString(const char *const*types, unsigned int ntypes, - const char *type) + const char *type, + int *val) } \ -int name ## TypeFromString(const char *type) { \ +int name ## TypeFromString(const char *type, name *val) { \ return virEnumFromString(name ## TypeList, \ ARRAY_CARDINALITY(name ## TypeList), \ - type); \ + type, (int *) val);\ } # define VIR_ENUM_DECL(name) \ And then a tons of follow up patches. Or even make virEnumString() report the error (that could save a lot of virReportError() calls). Or even make the error reporting an optional bool parameter. It's also a huge patch; I wonder if we can come up with an alternative name (VIR_ENUM_DECL2 expands to name ## FromString, leaving VIR_ENUM_DECL expanding to name ## TypeFromString - also nice that it's a shorter name since it's usage will be longer), where we can then focus on just a few conversions per patch, until all patches are using the new name and finally delete the old name, rather than redefining VIR_ENUM_DECL up front and having to do all the refactoring in a single patch. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] GetBlockInfo: Use the correct path to qemuOpenFile
On 07/14/2014 10:30 AM, John Ferlan wrote: Commit id '3ea661de' refactored the code to use the 'disk-src-path' instead of getting the path from virDomainDiskGetSource(). The one call to qemuOpenFile() didn't use the disk source path, rather it used the path as passed from the caller (in this case 'vda') - this caused a failure with the virt-test/tp-libvirt as follows: $ virsh domblkinfo virt-tests-vm1 vda error: cannot stat file '/home/virt-test/shared/data/images/jeos-20-64.qcow2': Bad file descriptor $ Signed-off-by: John Ferlan jfer...@redhat.com --- NOTE: Luckily this hasn't crept into a release... ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Introduce virDomainYesNo enum type
On 07/14/2014 10:40 AM, Daniel P. Berrange wrote: } -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_YES; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_ENABLED; } else { -def-os.bios.useserial = VIR_DOMAIN_BIOS_USESERIAL_NO; +def-os.bios.useserial = VIR_DOMAIN_YES_NO_DISABLED; } if (def-data.spice.filetransfer) virBufferAsprintf(buf, filetransfer enable='%s'/\n, - virDomainGraphicsSpiceAgentFileTransferTypeToString(def-data.spice.filetransfer)); + virDomainYesNoTypeToString(def-data.spice.filetransfer)); } I'm not really a fan of this cleanup, as IMHO the result is less clear harder to follow than the original code. How so? The original code was very repetitive, with multiple enums (all with long names) copying the same few enum elements. We're not painting ourselves into a corner - if any of the replaced enums ever grows a third value (such as on, hybrid, off), then we just break that one enum back into a named list rather than using the generic on/off enum. I'm actually in favor of this cleanup. Specifically a enum constant name like YES_NO_DISABLED is just awful IMHO compared to the original desriptive name. Is it just a matter of coming up with a better name? Maybe: VIR_TRISTATE_ABSENT = 0, VIR_TRISTATE_NO, VIR_TRISTATE_YES, def-os.bios.useserial = VIR_TRISTATE_NO; if (def-data.spice.filetransfer) { virBufferAsprintf(buf, filetransfer enable='%s'/\n, virTristateToString(def-data.spice.filetransfer)); -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] LVM Volume Creation
Hi All, I'm having issue with creating LVM volume via libvirt. XML of LVM StoragePool is: VG01 /dev/VG01 (existing vg) XML of create volume in lvm pool is: ub_test01.img 0 1 When I create the volume with above XML defn, lvs command to list logical volume shows a value in the origin column indicating it was created as snapshot.. the strange thing is that the origin volume does not exist..! something strange going on. You can see the lvs output here See http://pastebin.com/prk6VqwD [1] I'd appreciate if you anyone can help me understand what is going on and/or describe how to make libvirt create logical volumes not as a snapshot. We are running libvirtd 1.2 with KVM. Regards, Ravi - Message sent via Atmail Open - http://atmail.org/ Links: -- [1] http://pastebin.com/prk6VqwD -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 1/3] Add gvir_config_capabilities_cpu_get_model()
On Mon, Jul 14, 2014 at 02:56:18PM +0100, Zeeshan Ali (Khattak) wrote: Ah so you mean that I use the same object to represent both? I'm not sure that makes sense since they are part of different XMLs and therefore different entities. I'm not sure here. We already have GVirConfigDomainCpu which inherits from GVirConfigCapabilitiesCpu and this is also used by GVirConfigCapabilitiesHost, I think this is a similar situation? Christophe pgp_pwftWHI_0.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-glib 1/3] Add gvir_config_capabilities_cpu_get_model()
On Mon, Jul 14, 2014 at 6:31 PM, Christophe Fergeau cferg...@redhat.com wrote: On Mon, Jul 14, 2014 at 02:56:18PM +0100, Zeeshan Ali (Khattak) wrote: Ah so you mean that I use the same object to represent both? I'm not sure that makes sense since they are part of different XMLs and therefore different entities. I'm not sure here. We already have GVirConfigDomainCpu which inherits from GVirConfigCapabilitiesCpu and this is also used by GVirConfigCapabilitiesHost, I think this is a similar situation? Hmm... yeah I guess so. -- Regards, Zeeshan Ali (Khattak) Befriend GNOME: http://www.gnome.org/friends/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] python bindings links?
According to http://libvirt.org/bindings.html, python bindings are still listed as part of libvirt.git. But now that they are a separate git repository, we probably ought to have a more official page for these bindings, including an easy-to-find link to the repository viewer. Right now, libvirt.org/python doesn't have any hits; seems like the obvious place to put such information, given that we already have libvirt.org/php. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] LVM Volume Creation
On 07/14/2014 07:26 AM, Ravi Samji wrote: Hi All, I'm having issue with creating LVM volume via libvirt. XML of LVM StoragePool is: VG01 /dev/VG01 (existing vg) XML of create volume in lvm pool is: ub_test01.img 0 1 Umm, that's not XML. It appears you sent your mail as html mail, but in a manner that renders poorly when html is stripped (most people read this list in plain text form). Can you please resend as plain text to begin with? When I create the volume with above XML defn, lvs command to list logical volume shows a value in the origin column indicating it was created as snapshot.. the strange thing is that the origin volume does not exist..! something strange going on. You can see the lvs output here See http://pastebin.com/prk6VqwD [1] Also, pastebin content tends to be transient; if the output is short, it would be better to paste it into the body of your email, so that it will survive in the archives alongside your email instead of suddenly disappearing. I'd appreciate if you anyone can help me understand what is going on and/or describe how to make libvirt create logical volumes not as a snapshot. We are running libvirtd 1.2 with KVM. Does the problem still exist with the most recent release of libvirt 1.2.6? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] python bindings links?
On Tue, Jul 15, 2014 at 1:25 AM, Eric Blake ebl...@redhat.com wrote: According to http://libvirt.org/bindings.html, python bindings are still listed as part of libvirt.git. But now that they are a separate git repository, we probably ought to have a more official page for these bindings, including an easy-to-find link to the repository viewer. Right now, libvirt.org/python doesn't have any hits; seems like the obvious place to put such information, given that we already have libvirt.org/php. The page http://libvirt.org/bindings.html also refers to http://libvirt.org/python.html, which is a html page. We should convert it to php, probably? So that http://libvirt.org/python doesn't give a 404 ? or some kind of rewrite rule in .htaccess to redirect http://libvirt.org/python to http://libvirt.org/python.html ? -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PREPOST 08/17] src/xenxs:Refactor graphics config parsing code
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com introduce function xenParseXMVfb(.); which parses the graphics config signed-off-by: David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 247 - 1 file changed, 131 insertions(+), 116 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 80a7941..89d5739 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -872,125 +872,14 @@ static int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def, return 0; } -virDomainDefPtr -xenParseXM(virConfPtr conf, int xendConfigVersion, - virCapsPtr caps) +static int xenParseXMVfb(virConfPtr conf, virDomainDefPtr def, + int xendConfigVersion) { -const char *str; -int hvm = 0; +char *listenAddr; int val; -virConfValuePtr list; -virDomainDefPtr def = NULL; -virDomainDiskDefPtr disk = NULL; -virDomainNetDefPtr net = NULL; +virConfValuePtr list = NULL; virDomainGraphicsDefPtr graphics = NULL; -size_t i; -char *script = NULL; -char *listenAddr = NULL; - -if (VIR_ALLOC(def) 0) -return NULL; - -def-virtType = VIR_DOMAIN_VIRT_XEN; -def-id = -1; -if (xenParseXMGeneral(conf, def, caps) 0) -goto cleanup; -hvm = (STREQ(def-os.type, hvm)); -if (hvm) { -const char *boot; -if (xenXMConfigCopyString(conf, kernel, def-os.loader) 0) -goto cleanup; - -if (xenXMConfigGetString(conf, boot, boot, c) 0) -goto cleanup; - -for (i = 0; i VIR_DOMAIN_BOOT_LAST boot[i]; i++) { -switch (*boot) { -case 'a': -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY; -break; -case 'd': -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM; -break; -case 'n': -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_NET; -break; -case 'c': -default: -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK; -break; -} -def-os.nBootDevs++; -} -} else { -const char *extra, *root; - -if (xenXMConfigCopyStringOpt(conf, bootloader, def-os.bootloader) 0) -goto cleanup; -if (xenXMConfigCopyStringOpt(conf, bootargs, def-os.bootloaderArgs) 0) -goto cleanup; - -if (xenXMConfigCopyStringOpt(conf, kernel, def-os.kernel) 0) -goto cleanup; -if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd) 0) -goto cleanup; -if (xenXMConfigGetString(conf, extra, extra, NULL) 0) -goto cleanup; -if (xenXMConfigGetString(conf, root, root, NULL) 0) -goto cleanup; - -if (root) { -if (virAsprintf(def-os.cmdline, root=%s %s, root, extra) 0) -goto cleanup; -} else { -if (VIR_STRDUP(def-os.cmdline, extra) 0) -goto cleanup; -} -} -if (xenParseXMMem(conf, def) 0) -goto cleanup; -if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator) 0) -goto cleanup; - if (xenParseXMVif(conf, def) 0) - goto cleanup; - - if (xenParseXMTimeOffset(conf, def, xendConfigVersion) 0) - goto cleanup; - if (xenParseXMEventsActions(conf, def) 0) - goto cleanup; - if (xenParseXMPCI(conf, def) 0) - goto cleanup; - if (xenParseXMCPUFeatures(conf, def) 0) - goto cleanup; - if (xenParseXMDisk(conf, def, xendConfigVersion) 0) - goto cleanup; - - if (hvm) { -if (xenXMConfigGetString(conf, usbdevice, str, NULL) 0) -goto cleanup; -if (str -(STREQ(str, tablet) || - STREQ(str, mouse) || - STREQ(str, keyboard))) { -virDomainInputDefPtr input; -if (VIR_ALLOC(input) 0) -goto cleanup; -input-bus = VIR_DOMAIN_INPUT_BUS_USB; -if (STREQ(str, mouse)) -input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE; -else if (STREQ(str, tablet)) -input-type = VIR_DOMAIN_INPUT_TYPE_TABLET; -else if (STREQ(str, keyboard)) -input-type = VIR_DOMAIN_INPUT_TYPE_KBD; -if (VIR_ALLOC_N(def-inputs, 1) 0) { -virDomainInputDefFree(input); -goto cleanup; -} -def-inputs[0] = input; -def-ninputs = 1; -} -} - +int hvm = STREQ(def-os.type, hvm); The usual whitespace comments: blank lines between functions, indentation of the function params, and blank line after local variables. /* HVM guests, or old PV guests use this config format */
Re: [libvirt] [PREPOST 09/17] src/xenxs:Refactor OS and Miscellaneous deivices config
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce functions xenParseXMOS(.) and xenParseXMMisc(.) to parse the OS and Miscellaneous devices configs signed-off-by: David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 146 ++--- 1 file changed, 82 insertions(+), 64 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 89d5739..312d890 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1020,36 +1020,21 @@ static int xenParseXMVfb(virConfPtr conf, virDomainDefPtr def, virDomainGraphicsDefFree(graphics); return -1; } -virDomainDefPtr -xenParseXM(virConfPtr conf, int xendConfigVersion, - virCapsPtr caps) +static int xenParseXMOS(virConfPtr conf, virDomainDefPtr def) { -const char *str; -int hvm = 0; -virConfValuePtr list; -virDomainDefPtr def = NULL; -virDomainDiskDefPtr disk = NULL; -virDomainNetDefPtr net = NULL; -virDomainGraphicsDefPtr graphics = NULL; size_t i; -char *script = NULL; -char *listenAddr = NULL; - -if (VIR_ALLOC(def) 0) -return NULL; -def-virtType = VIR_DOMAIN_VIRT_XEN; -def-id = -1; -if (xenParseXMGeneral(conf, def, caps) 0) -goto cleanup; -hvm = (STREQ(def-os.type, hvm)); -if (hvm) { +if (STREQ(def-os.type, hvm)) { const char *boot; + +if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator) 0) +return -1; + if (xenXMConfigCopyString(conf, kernel, def-os.loader) 0) -goto cleanup; +return -1; if (xenXMConfigGetString(conf, boot, boot, c) 0) -goto cleanup; +return -1; for (i = 0; i VIR_DOMAIN_BOOT_LAST boot[i]; i++) { switch (*boot) { @@ -1073,55 +1058,52 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, const char *extra, *root; if (xenXMConfigCopyStringOpt(conf, bootloader, def-os.bootloader) 0) -goto cleanup; +return -1; if (xenXMConfigCopyStringOpt(conf, bootargs, def-os.bootloaderArgs) 0) -goto cleanup; +return -1; if (xenXMConfigCopyStringOpt(conf, kernel, def-os.kernel) 0) -goto cleanup; +return -1; if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd) 0) -goto cleanup; +return -1; if (xenXMConfigGetString(conf, extra, extra, NULL) 0) -goto cleanup; +return -1; if (xenXMConfigGetString(conf, root, root, NULL) 0) -goto cleanup; +return -1; if (root) { if (virAsprintf(def-os.cmdline, root=%s %s, root, extra) 0) -goto cleanup; +return -1; } else { if (VIR_STRDUP(def-os.cmdline, extra) 0) -goto cleanup; +return -1; } } -if (xenParseXMMem(conf, def) 0) -goto cleanup; -if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator) 0) -goto cleanup; - if (xenParseXMVif(conf, def) 0) - goto cleanup; - - if (xenParseXMTimeOffset(conf, def, xendConfigVersion) 0) - goto cleanup; - if (xenParseXMEventsActions(conf, def) 0) - goto cleanup; - if (xenParseXMPCI(conf, def) 0) - goto cleanup; - if (xenParseXMCPUFeatures(conf, def) 0) - goto cleanup; - if (xenParseXMDisk(conf, def, xendConfigVersion) 0) - goto cleanup; - - if (hvm) { + +return 0; +} +static int xenParseXMMisc(virConfPtr conf, virDomainDefPtr def) Hrm, not sure I'm fond of this name since it could become a dumping ground for any miscellaneous stuff in the future. Perhaps xenParseXMInputDevs? Regards, Jim +{ +const char *str; + +if (STREQ(def-os.type, hvm)) { +if (xenXMConfigGetString(conf, soundhw, str, NULL) 0) +return -1; + +if (str +xenParseSxprSound(def, str) 0) +return -1; + if (xenXMConfigGetString(conf, usbdevice, str, NULL) 0) -goto cleanup; +return -1; + if (str (STREQ(str, tablet) || STREQ(str, mouse) || STREQ(str, keyboard))) { virDomainInputDefPtr input; if (VIR_ALLOC(input) 0) -goto cleanup; +return -1; input-bus = VIR_DOMAIN_INPUT_BUS_USB; if (STREQ(str, mouse)) input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE; @@ -1131,15 +1113,61 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, input-type = VIR_DOMAIN_INPUT_TYPE_KBD; if (VIR_ALLOC_N(def-inputs, 1) 0) {
Re: [libvirt] [PREPOST 10/17] src/xenxs:Refactor xenParseXM( )
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com A couple of miscellaneous fixed and wrap code common code into xenParseConfigCommon(...).I will drop some of the functions later though signed-off-by: David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 134 - src/xenxs/xen_xm.h | 3 +- 2 files changed, 73 insertions(+), 64 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 312d890..657dd1c 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -458,7 +458,7 @@ static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def) } return 0; -cleanup: + cleanup: This should be in 3/17 to fix 'make syntax-check'. VIR_FREE(script); return -1; } @@ -1026,7 +1026,8 @@ static int xenParseXMOS(virConfPtr conf, virDomainDefPtr def) if (STREQ(def-os.type, hvm)) { const char *boot; - +if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator) 0) +return -1; This replicates the next lines of code. Not needed IMO. if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator) 0) return -1; @@ -1122,54 +1123,13 @@ static int xenParseXMMisc(virConfPtr conf, virDomainDefPtr def) return 0; } -virDomainDefPtr -xenParseXM(virConfPtr conf, int xendConfigVersion, - virCapsPtr caps) +static int xenParseXMCharDev(virConfPtr conf, virDomainDefPtr def) { const char *str; -int hvm = 0; -virConfValuePtr list; -virDomainDefPtr def = NULL; -virDomainDiskDefPtr disk = NULL; -virDomainNetDefPtr net = NULL; -virDomainGraphicsDefPtr graphics = NULL; -char *script = NULL; -char *listenAddr = NULL; - -if (VIR_ALLOC(def) 0) -return NULL; - -def-virtType = VIR_DOMAIN_VIRT_XEN; -def-id = -1; -if (xenParseXMGeneral(conf, def, caps) 0) -goto cleanup; - -if (xenParseXMOS(conf, def) 0) -goto cleanup; +virConfValuePtr value = NULL; +virDomainChrDefPtr chr = NULL; -hvm = (STREQ(def-os.type, hvm)); -if (xenParseXMMem(conf, def) 0) -goto cleanup; -if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator) 0) -goto cleanup; -if (xenParseXMVif(conf, def) 0) -goto cleanup; -if (xenParseXMTimeOffset(conf, def, xendConfigVersion) 0) -goto cleanup; -if (xenParseXMEventsActions(conf, def) 0) -goto cleanup; -if (xenParseXMPCI(conf, def) 0) -goto cleanup; -if (xenParseXMCPUFeatures(conf, def) 0) -goto cleanup; -if (xenParseXMDisk(conf, def, xendConfigVersion) 0) -goto cleanup; -if (xenParseXMVfb(conf, def, xendConfigVersion) 0) -goto cleanup; -if (xenParseXMMisc(conf, def) 0) -goto cleanup; -if (hvm) { -virDomainChrDefPtr chr = NULL; +if (STREQ(def-os.type, hvm)) { if (xenXMConfigGetString(conf, parallel, str, NULL) 0) goto cleanup; @@ -1179,7 +1139,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, if (chr) { if (VIR_ALLOC_N(def-parallels, 1) 0) { -virDomainChrDefFree(chr); goto cleanup; } chr-deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL; @@ -1190,21 +1149,21 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, } /* Try to get the list of values to support multiple serial ports */ -list = virConfGetValue(conf, serial); -if (list list-type == VIR_CONF_LIST) { +value = virConfGetValue(conf, serial); +if (value value-type == VIR_CONF_LIST) { int portnum = -1; -list = list-list; -while (list) { +value = value-list; +while (value) { char *port = NULL; -if ((list-type != VIR_CONF_STRING) || (list-str == NULL)) +if ((value-type != VIR_CONF_STRING) || (value-str == NULL)) goto cleanup; -port = list-str; +port = value-str; portnum++; if (STREQ(port, none)) { -list = list-next; +value = value-next; continue; } @@ -1215,11 +1174,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, chr-target.port = portnum; if (VIR_APPEND_ELEMENT(def-serials, def-nserials, chr) 0) { -virDomainChrDefFree(chr); goto cleanup; } -list = list-next; +value = value-next; } } else { /* If domain is not using multiple serial ports we parse data old way */
Re: [libvirt] [PREPOST 11/17] src/xenxs:Refactor code formating general information
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com refactor code parsing uuid, memory and name options signed-off-by:David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 40 +++- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 657dd1c..a907252 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1609,6 +1609,31 @@ xenFormatXMPCI(virConfPtr conf, either 32, or 64 on a platform where long is big enough. */ verify(MAX_VIRT_CPUS = sizeof(1UL) * CHAR_BIT); +static int xenFormatXMGeneral(virConfPtr conf, virDomainDefPtr def) Similar comment here as in 1/17 regarding the name xenFormatXMGeneral. I'd suggest something like xenFormatXMGeneralMeta. +{ +char uuid[VIR_UUID_STRING_BUFLEN]; + +if (xenXMConfigSetString(conf, name, def-name) 0) +return -1; + +virUUIDFormat(def-uuid, uuid); +if (xenXMConfigSetString(conf, uuid, uuid) 0) +return -1; + +return 0; +} +static int xenFormatXMMem(virConfPtr conf, virDomainDefPtr def) +{ +if (xenXMConfigSetInt(conf, maxmem, + VIR_DIV_UP(def-mem.max_balloon, 1024)) 0) +return -1; + +if (xenXMConfigSetInt(conf, memory, + VIR_DIV_UP(def-mem.cur_balloon, 1024)) 0) +return -1; + +return 0; +} You split the general and mem functions in separate patches on the parse side. I think that is a good approach on the format side too. Regards, Jim virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def, int xendConfigVersion) @@ -1618,27 +1643,16 @@ virConfPtr xenFormatXM(virConnectPtr conn, size_t i; char *cpus = NULL; const char *lifecycle; -char uuid[VIR_UUID_STRING_BUFLEN]; virConfValuePtr diskVal = NULL; virConfValuePtr netVal = NULL; if (!(conf = virConfNew())) goto cleanup; - -if (xenXMConfigSetString(conf, name, def-name) 0) -goto cleanup; - -virUUIDFormat(def-uuid, uuid); -if (xenXMConfigSetString(conf, uuid, uuid) 0) +if (xenFormatXMGeneral(conf, def) 0) goto cleanup; -if (xenXMConfigSetInt(conf, maxmem, - VIR_DIV_UP(def-mem.max_balloon, 1024)) 0) -goto cleanup; - -if (xenXMConfigSetInt(conf, memory, - VIR_DIV_UP(def-mem.cur_balloon, 1024)) 0) +if (xenFormatXMMem(conf, def) 0) goto cleanup; if (xenXMConfigSetInt(conf, vcpus, def-maxvcpus) 0) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PREPOST 12/17] src/xenxs:Refactor virtual time config options formating code
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce function xenFormatXMTimeOffset(..) which formats virtual time config Ah, on the format side you split TimeOffset and EventActions as I requested on the parse side. Looks good with the exception of the usual whitespace comments. Regards, Jim signed-off-by:David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 149 - 1 file changed, 78 insertions(+), 71 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index a907252..c91fa73 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1634,12 +1634,88 @@ static int xenFormatXMMem(virConfPtr conf, virDomainDefPtr def) return 0; } +static int xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr def, + int xendConfigVersion) +{ +int vmlocaltime; +if (xendConfigVersion XEND_CONFIG_VERSION_3_1_0) { +/* 3.1: UTC and LOCALTIME */ +switch (def-clock.offset) { +case VIR_DOMAIN_CLOCK_OFFSET_UTC: +vmlocaltime = 0; +break; +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: +vmlocaltime = 1; +break; +default: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unsupported clock offset='%s'), + virDomainClockOffsetTypeToString(def-clock.offset)); +return -1; +} +} else { +if (STREQ(def-os.type, hvm)) { +/* =3.1 HV: VARIABLE */ +int rtc_timeoffset; +switch (def-clock.offset) { +case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: +vmlocaltime = (int)def-clock.data.variable.basis; +rtc_timeoffset = def-clock.data.variable.adjustment; +break; +case VIR_DOMAIN_CLOCK_OFFSET_UTC: +if (def-clock.data.utc_reset) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(unsupported clock adjustment='reset')); +return -1; +} +vmlocaltime = 0; +rtc_timeoffset = 0; +break; +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: +if (def-clock.data.utc_reset) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(unsupported clock adjustment='reset')); +return -1; +} +vmlocaltime = 1; +rtc_timeoffset = 0; +break; +default: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unsupported clock offset='%s'), + virDomainClockOffsetTypeToString(def-clock.offset)); +return -1; +} +if (xenXMConfigSetInt(conf, rtc_timeoffset, rtc_timeoffset) 0) +return -1; +} else { +/* =3.1 PV: UTC and LOCALTIME */ +switch (def-clock.offset) { +case VIR_DOMAIN_CLOCK_OFFSET_UTC: +vmlocaltime = 0; +break; +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: +vmlocaltime = 1; +break; +default: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unsupported clock offset='%s'), + virDomainClockOffsetTypeToString(def-clock.offset)); +return -1; +} +} /* !hvm */ +} +if (xenXMConfigSetInt(conf, localtime, vmlocaltime) 0) +return -1; + +return 0; +} virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def, int xendConfigVersion) { virConfPtr conf = NULL; -int hvm = 0, vmlocaltime = 0; +int hvm = 0; size_t i; char *cpus = NULL; const char *lifecycle; @@ -1778,78 +1854,9 @@ virConfPtr xenFormatXM(virConnectPtr conn, goto cleanup; } /* !hvm */ - -if (xendConfigVersion XEND_CONFIG_VERSION_3_1_0) { -/* 3.1: UTC and LOCALTIME */ -switch (def-clock.offset) { -case VIR_DOMAIN_CLOCK_OFFSET_UTC: -vmlocaltime = 0; -break; -case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: -vmlocaltime = 1; -break; -default: -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(unsupported clock offset='%s'), - virDomainClockOffsetTypeToString(def-clock.offset)); -goto cleanup; -} -} else { -if (hvm) { -/* =3.1 HV: VARIABLE */ -int
Re: [libvirt] [PREPOST 13/17] src/xenxs:Refactor code formating event actions config
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce function xenFormatXMEventActions(.) which formats actions following certain events Looks good with exception of my tiring whitespace comments :-). Regards, Jim signed-off-by:David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 60 ++ 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index c91fa73..367a8cd 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1710,6 +1710,38 @@ static int xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr def, return 0; } +static int xenFormatXMEventActions(virConfPtr conf, virDomainDefPtr def) +{ +const char *lifecycle = NULL; + +if (!(lifecycle = virDomainLifecycleTypeToString(def-onPoweroff))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(unexpected lifecycle action %d), def-onPoweroff); +return -1; +} +if (xenXMConfigSetString(conf, on_poweroff, lifecycle) 0) +return -1; + + +if (!(lifecycle = virDomainLifecycleTypeToString(def-onReboot))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(unexpected lifecycle action %d), def-onReboot); +return -1; +} +if (xenXMConfigSetString(conf, on_reboot, lifecycle) 0) +return -1; + + +if (!(lifecycle = virDomainLifecycleCrashTypeToString(def-onCrash))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(unexpected lifecycle action %d), def-onCrash); +return -1; +} +if (xenXMConfigSetString(conf, on_crash, lifecycle) 0) +return -1; + +return 0; +} virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def, int xendConfigVersion) @@ -1718,7 +1750,6 @@ virConfPtr xenFormatXM(virConnectPtr conn, int hvm = 0; size_t i; char *cpus = NULL; -const char *lifecycle; virConfValuePtr diskVal = NULL; virConfValuePtr netVal = NULL; @@ -1857,33 +1888,8 @@ virConfPtr xenFormatXM(virConnectPtr conn, if (xenFormatXMTimeOffset(conf, def, xendConfigVersion) 0) goto cleanup; -if (!(lifecycle = virDomainLifecycleTypeToString(def-onPoweroff))) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(unexpected lifecycle action %d), def-onPoweroff); -goto cleanup; -} -if (xenXMConfigSetString(conf, on_poweroff, lifecycle) 0) -goto cleanup; - - -if (!(lifecycle = virDomainLifecycleTypeToString(def-onReboot))) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(unexpected lifecycle action %d), def-onReboot); -goto cleanup; -} -if (xenXMConfigSetString(conf, on_reboot, lifecycle) 0) -goto cleanup; - - -if (!(lifecycle = virDomainLifecycleCrashTypeToString(def-onCrash))) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _(unexpected lifecycle action %d), def-onCrash); +if (xenFormatXMEventActions(conf, def) 0) goto cleanup; -} -if (xenXMConfigSetString(conf, on_crash, lifecycle) 0) -goto cleanup; - - if (hvm) { if (def-emulator -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PREPOST 14/17] src/xenxs:Refactor code formating char devices config
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce function xenFormatXMCharDev(); which formats char devices config signed-off-by:David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 155 - 1 file changed, 82 insertions(+), 73 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index 367a8cd..ee5dc19 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1742,6 +1742,85 @@ static int xenFormatXMEventActions(virConfPtr conf, virDomainDefPtr def) return 0; } +static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr def) +{ +size_t i; +if (STREQ(def-os.type, hvm)) { +if (def-nparallels) { +virBuffer buf = VIR_BUFFER_INITIALIZER; +char *str; +int ret; + +ret = xenFormatSxprChr(def-parallels[0], buf); +str = virBufferContentAndReset(buf); +if (ret == 0) +ret = xenXMConfigSetString(conf, parallel, str); +VIR_FREE(str); +if (ret 0) +goto cleanup; +} else { +if (xenXMConfigSetString(conf, parallel, none) 0) +goto cleanup; +} + +if (def-nserials) { +if ((def-nserials == 1) (def-serials[0]-target.port == 0)) { +virBuffer buf = VIR_BUFFER_INITIALIZER; +char *str; +int ret; + +ret = xenFormatSxprChr(def-serials[0], buf); +str = virBufferContentAndReset(buf); +if (ret == 0) +ret = xenXMConfigSetString(conf, serial, str); +VIR_FREE(str); +if (ret 0) +goto cleanup; +} else { +size_t j = 0; +int maxport = -1, port; +virConfValuePtr serialVal = NULL; + +if (VIR_ALLOC(serialVal) 0) +goto cleanup; +serialVal-type = VIR_CONF_LIST; +serialVal-list = NULL; + +for (i = 0; i def-nserials; i++) +if (def-serials[i]-target.port maxport) +maxport = def-serials[i]-target.port; + +for (port = 0; port = maxport; port++) { +virDomainChrDefPtr chr = NULL; +for (j = 0; j def-nserials; j++) { +if (def-serials[j]-target.port == port) { +chr = def-serials[j]; +break; +} +} +if (xenFormatXMSerial(serialVal, chr) 0) { +virConfFreeValue(serialVal); +goto cleanup; +} +} + +if (serialVal-list != NULL) { +int ret = virConfSetValue(conf, serial, serialVal); +serialVal = NULL; +if (ret 0) +goto cleanup; +} +VIR_FREE(serialVal); +} +} else { +if (xenXMConfigSetString(conf, serial, none) 0) +goto cleanup; +} +} +return 0; +cleanup: +return -1; +} virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def, int xendConfigVersion) @@ -2067,79 +2146,10 @@ virConfPtr xenFormatXM(virConnectPtr conn, if (xenFormatXMPCI(conf, def) 0) goto cleanup; -if (hvm) { -if (def-nparallels) { -virBuffer buf = VIR_BUFFER_INITIALIZER; -char *str; -int ret; - -ret = xenFormatSxprChr(def-parallels[0], buf); -str = virBufferContentAndReset(buf); -if (ret == 0) -ret = xenXMConfigSetString(conf, parallel, str); -VIR_FREE(str); -if (ret 0) -goto cleanup; -} else { -if (xenXMConfigSetString(conf, parallel, none) 0) -goto cleanup; -} - -if (def-nserials) { -if ((def-nserials == 1) (def-serials[0]-target.port == 0)) { -virBuffer buf = VIR_BUFFER_INITIALIZER; -char *str; -int ret; - -ret = xenFormatSxprChr(def-serials[0], buf); -str = virBufferContentAndReset(buf); -if (ret == 0) -ret = xenXMConfigSetString(conf, serial, str); -VIR_FREE(str); -if (ret 0) -goto cleanup; -} else { -size_t j = 0; -int maxport = -1, port; -virConfValuePtr serialVal = NULL; - -
Re: [libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code
David Kiarie wrote: From: Kiarie Kahurani davidkiar...@gmail.com Introduce the function xenFormatXMDomainNet(..) On the parsing side, you called this xenParseXMVif. To be consistent, this should be xenFormatXMVif. I think this could be done in a cleaner way signed-of-by: David Kiariedavidkiar...@gmail.com --- src/xenxs/xen_xm.c | 49 +++-- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index ee5dc19..8dd2823 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -1821,6 +1821,36 @@ static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr def) cleanup: return -1; } +static int xenFormatXMDomainNet(virConfPtr conf, virConnectPtr conn, + virDomainDefPtr def, int xendConfigVersion) +{ + virConfValuePtr netVal = NULL; + size_t i; + + int hvm = STREQ(def-os.type, hvm); + + if (VIR_ALLOC(netVal) 0) +goto cleanup; +netVal-type = VIR_CONF_LIST; +netVal-list = NULL; + +for (i = 0; i def-nnets; i++) { +if (xenFormatXMNet(conn, netVal, def-nets[i], + hvm, xendConfigVersion) 0) Ah, I see xenFormatXMNet already exists. So maybe xenFormatXMVifs for the list of VIFs and xenFormatXMVif for each individual VIF is clearer. Regards, Jim + return -1; +} +if (netVal-list != NULL) { +int ret = virConfSetValue(conf, vif, netVal); +netVal = NULL; +if (ret 0) +return -1; +} +VIR_FREE(netVal); +return 0; + cleanup: +VIR_FREE(netVal); +return -1; +} virConfPtr xenFormatXM(virConnectPtr conn, virDomainDefPtr def, int xendConfigVersion) @@ -2124,25 +2154,8 @@ virConfPtr xenFormatXM(virConnectPtr conn, goto cleanup; } VIR_FREE(diskVal); - -if (VIR_ALLOC(netVal) 0) +if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion) 0) goto cleanup; -netVal-type = VIR_CONF_LIST; -netVal-list = NULL; - -for (i = 0; i def-nnets; i++) { -if (xenFormatXMNet(conn, netVal, def-nets[i], - hvm, xendConfigVersion) 0) -goto cleanup; -} -if (netVal-list != NULL) { -int ret = virConfSetValue(conf, vif, netVal); -netVal = NULL; -if (ret 0) -goto cleanup; -} -VIR_FREE(netVal); - if (xenFormatXMPCI(conf, def) 0) goto cleanup; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 1/2] virsh: Reject negative numbers in vshCommandOptUInt
Bumping again - I found a related bz in my backlog... On 05/29/2014 01:15 PM, Eric Blake wrote: On 05/29/2014 05:54 AM, Peter Krempa wrote: Use virStrToLong_uip instead of virStrToLong_ui to reject negative numbers in the helper. None of the callers expects the wraparound feature for negative numbers. I had to audit all callers, and found the following (fortunately the list is fairly small): snip vol-upload, vol-download [offset, length]: Can't a negative offset be treated as offset from the end of the file? And doesn't -1 as implying unlimited length make sense? Here, allowing -1 might make sense BZ - https://bugzilla.redhat.com/show_bug.cgi?id=1087104 has a different take on whether negative values should be allowed. In particular: Additional Info: In manual page: vol-download [--pool pool-or-uuid] [--offset bytes] [--length bytes] --offset is the position in the storage volume at which to start reading the data. --length is an upper bound of the amount of data to be downloaded. It is said that offset is the position in the storage volume at which to start reading the data, so I think the value of offset should be no smaller than 0, option length as well. So - do we adjust the man page to indicate that using a -1 is OK and what it would do? Probably similar type action for the changes made (commit id's 0e2d73051 c62125395)? Or does a negative really make sense for offset? Sure -1 makes sense and works because 'lseek()' allows it, but other negative numbers I just tried get an error: virsh vol-download --pool default qcow3-vol2 /home/vm-images/raw --offset -2 --length -1000 error: cannot download from volume qcow3-vol2 error: Unable to seek /home/vm-images/qcow3-vol2 to 18446744073709551614: Invalid argument Not even sure what a negative length file is... Is that the definition of a sparse file? Is the suggestion that we'd be downloading (or uploading) from end of file backwards some number of bytes? Not quite sure that's what's happening as the negative value is turned positive and it seems means everything. So while, yes, -1 for both makes sense as a sort of pseudonym for maximum - other values don't, but how does one go about distinguishing that? (eg, that a -1 was supplied and it's OK, although other negative values are not). John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] libxl: support hotplug of interface device
This patch series is to add support for attach/detaching an interface device. At the same time, add two fixes (1/3 and 3/3) Chunyan Liu (3): libxl: add HOSTDEV type in libxlDomainDetachDeviceConfig libxl: support hotplug of interface libxl: fix return value error Attach|DetachDeviceFlags .gnulib | 2 +- src/libxl/libxl_domain.c | 12 ++- src/libxl/libxl_driver.c | 193 +-- 3 files changed, 180 insertions(+), 27 deletions(-) -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] libxl: add HOSTDEV type in libxlDomainDetachDeviceConfig
Missing HOSTDEV type in libxlDomainDetachDeviceConfig. Add it. Signed-off-by: Chunyan Liu cy...@suse.com --- src/libxl/libxl_driver.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b27581e..5e08bba 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2985,7 +2985,8 @@ static int libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk, detach; -int ret = -1; +virDomainHostdevDefPtr hostdev, det_hostdev; +int idx; switch (dev-type) { case VIR_DOMAIN_DEVICE_DISK: @@ -2993,18 +2994,30 @@ libxlDomainDetachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) if (!(detach = virDomainDiskRemoveByName(vmdef, disk-dst))) { virReportError(VIR_ERR_INVALID_ARG, _(no target device %s), disk-dst); -break; +return -1; } virDomainDiskDefFree(detach); -ret = 0; break; + +case VIR_DOMAIN_DEVICE_HOSTDEV: { +hostdev = dev-data.hostdev; +if ((idx = virDomainHostdevFind(vmdef, hostdev, det_hostdev)) 0) { +virReportError(VIR_ERR_INVALID_ARG, %s, + _(device not present in domain configuration)); +return -1; +} +virDomainHostdevRemove(vmdef, idx); +virDomainHostdevDefFree(det_hostdev); +break; +} + default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(persistent detach of device is not supported)); -break; +return -1; } -return ret; +return 0; } static int -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] libxl: support hotplug of interface
Add code to support attach/detaching a network device. Signed-off-by: Chunyan Liu cy...@suse.com --- src/libxl/libxl_domain.c | 12 +++- src/libxl/libxl_driver.c | 146 --- 2 files changed, 149 insertions(+), 9 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0c86601..6780e34 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -482,8 +482,16 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, STRNEQ(def-os.type, hvm)) dev-data.chr-targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; -if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) { -virDomainHostdevDefPtr hostdev = dev-data.hostdev; +if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV || +(dev-type == VIR_DOMAIN_DEVICE_NET + dev-data.net-type == VIR_DOMAIN_NET_TYPE_HOSTDEV)) { + +virDomainHostdevDefPtr hostdev; + +if (dev-type == VIR_DOMAIN_DEVICE_NET) +hostdev = (dev-data.net)-data.hostdev.def; +else +hostdev = dev-data.hostdev; /* forbid capabilities mode hostdev in this kind of hypervisor */ if (hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) { diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 5e08bba..9cd56b5 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -55,6 +55,7 @@ #include viraccessapicheck.h #include viratomic.h #include virhostdev.h +#include network/bridge_driver.h #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -2674,10 +2675,8 @@ static int libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver, libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, -virDomainDeviceDefPtr dev) +virDomainHostdevDefPtr hostdev) { -virDomainHostdevDefPtr hostdev = dev-data.hostdev; - if (hostdev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(hostdev mode '%s' not supported), @@ -2756,6 +2755,60 @@ libxlDomainDetachDeviceDiskLive(libxlDomainObjPrivatePtr priv, } static int +libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, + libxlDomainObjPrivatePtr priv, + virDomainObjPtr vm, + virDomainNetDefPtr net) +{ +int actualType; +libxl_device_nic nic; +int ret = -1; + +/* preallocate new slot for device */ +if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets + 1) 0) +return -1; + +/* If appropriate, grab a physical device from the configured + * network's pool of devices, or resolve bridge device name + * to the one defined in the network definition. + */ +if (networkAllocateActualDevice(vm-def, net) 0) +return -1; + +actualType = virDomainNetGetActualType(net); + +if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { +/* This is really a smart hostdev, so it should be attached + * as a hostdev (the hostdev code will reach over into the + * netdev-specific code as appropriate), then also added to + * the nets list (see out:) if successful. + */ +ret = libxlDomainAttachHostDevice(driver, priv, vm, + virDomainNetGetActualHostdev(net)); +goto out; +} + +libxl_device_nic_init(nic); +if (libxlMakeNic(vm-def, net, nic) 0) +goto cleanup; + +if (libxl_device_nic_add(priv-ctx, vm-def-id, nic, 0)) { +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(libxenlight failed to attach network device)); +goto cleanup; +} + +ret = 0; + + cleanup: +libxl_device_nic_dispose(nic); + out: +if (!ret) +vm-def-nets[vm-def-nnets++] = net; +return ret; +} + +static int libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver, libxlDomainObjPrivatePtr priv, virDomainObjPtr vm, @@ -2770,8 +2823,16 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver, dev-data.disk = NULL; break; +case VIR_DOMAIN_DEVICE_NET: +ret = libxlDomainAttachNetDevice(driver, priv, vm, + dev-data.net); +if (!ret) +dev-data.net = NULL; +break; + case VIR_DOMAIN_DEVICE_HOSTDEV: -ret = libxlDomainAttachHostDevice(driver, priv, vm, dev); +ret = libxlDomainAttachHostDevice(driver, priv, vm, + dev-data.hostdev); if (!ret) dev-data.hostdev = NULL; break; @@ -2790,6 +2851,7 @@ static int libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev) { virDomainDiskDefPtr disk; +virDomainNetDefPtr net;
[libvirt] [PATCH 3/3] libxl: fix return value error Attach|DetachDeviceFlags
Code logic in libxlDomainAttachDeviceFlags and libxlDomainDetachDeviceFlags is wrong with return value in error cases. 'ret' was being set to 0 if 'flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG' was false. Then if something like virDomainDeviceDefParse() failed in the VIR_DOMAIN_DEVICE_MODIFY_LIVE logic, the error would be reported but the function would return success. Signed-off-by: Chunyan Liu cy...@suse.com --- src/libxl/libxl_driver.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 9cd56b5..5fbff1c 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3285,10 +3285,8 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, driver-xmlopt))) goto endjob; -if ((ret = libxlDomainAttachDeviceConfig(vmdef, dev)) 0) +if (libxlDomainAttachDeviceConfig(vmdef, dev) 0) goto endjob; -} else { -ret = 0; } if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -3299,7 +3297,7 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto endjob; -if ((ret = libxlDomainAttachDeviceLive(driver, priv, vm, dev)) 0) +if (libxlDomainAttachDeviceLive(driver, priv, vm, dev) 0) goto endjob; /* @@ -3307,11 +3305,13 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, * changed even if we attach the device failed. */ if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0) -ret = -1; +goto endjob; } +ret = 0; + /* Finally, if no error until here, we can save config. */ -if (!ret (flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) { +if (flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { ret = virDomainSaveConfig(cfg-configDir, vmdef); if (!ret) { virDomainObjAssignDef(vm, vmdef, false, NULL); @@ -3396,10 +3396,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, driver-xmlopt))) goto endjob; -if ((ret = libxlDomainDetachDeviceConfig(vmdef, dev)) 0) +if (libxlDomainDetachDeviceConfig(vmdef, dev) 0) goto endjob; -} else { -ret = 0; } if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -3410,7 +3408,7 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, VIR_DOMAIN_XML_INACTIVE))) goto endjob; -if ((ret = libxlDomainDetachDeviceLive(driver, priv, vm, dev)) 0) +if (libxlDomainDetachDeviceLive(driver, priv, vm, dev) 0) goto endjob; /* @@ -3418,11 +3416,13 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, * changed even if we attach the device failed. */ if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0) -ret = -1; +goto endjob; } +ret = 0; + /* Finally, if no error until here, we can save config. */ -if (!ret (flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) { +if (flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { ret = virDomainSaveConfig(cfg-configDir, vmdef); if (!ret) { virDomainObjAssignDef(vm, vmdef, false, NULL); -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list