[libvirt] [PATCH] storage_fs: Create directory with UID if needed
The code already exists there, it just modified different flags. I just noticed this when looking at the code. This patch is better to view with bigger context or '-W'. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/storage/storage_backend_fs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bcbbb3ae252a..5dc712925b27 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -807,7 +807,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, (needs_create_as_uid || !virFileExists(pool-def-target.path))) mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; if (needs_create_as_uid) -flags |= VIR_DIR_CREATE_AS_UID; +dir_create_flags |= VIR_DIR_CREATE_AS_UID; /* Now create the final dir in the path with the uid/gid/mode * requested in the config. If the dir already exists, just set -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage_fs: Create directory with UID if needed
On Wed, May 27, 2015 at 10:10:53AM +0200, Martin Kletzander wrote: The code already exists there, it just modified different flags. I just noticed this when looking at the code. This patch is better to view with bigger context or '-W'. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/storage/storage_backend_fs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bcbbb3ae252a..5dc712925b27 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or Please configure your editor to not generate noise. @@ -807,7 +807,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, (needs_create_as_uid || !virFileExists(pool-def-target.path))) mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; if (needs_create_as_uid) -flags |= VIR_DIR_CREATE_AS_UID; +dir_create_flags |= VIR_DIR_CREATE_AS_UID; /* Now create the final dir in the path with the uid/gid/mode * requested in the config. If the dir already exists, just set ACK to this hunk. Jan signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt now using Zanata for translation
On Wed, May 27, 2015 at 08:45:09AM +0800, zhang bo wrote: On 2015/3/6 21:20, Daniel P. Berrange wrote: As of current GIT master, libvirt is fully using Zanata for po file translation After some trouble with the zanata python client, I have now successfully refreshed pushed the current libvirt.pot file, and synchronized all the translation po files back down to GIT master. For ongoing refreshes of the libvirt.pot, it can be rebuild pushed to zanata and .po files resynchonized using # cd po # rm libvirt.pot # make libvirt.pot # zanata-cli push # zanata-cli pull Where 'zanata-cli' is the *JAVA* client, not the python client. I strongly recommend against use of the python client for the libvirt project as it doesn't handle the size of the libvirt.pot/.po files well resulting in unexplained data loss. Even the java client has problems pushing the .po files to the server due to poorly designed rate limiting in the zanata server. Fortunately this is not something we need do again, now we have imported the existing .po file. The java client has been troublefree wrt to pushing the .pot file and pulling .po files, which is all we need for ongoing work now. Currently myself Daniel Veillard are setup as admins of the libvirt project in Zanata we can add further people as required. Regards, Daniel Would you please add me as admin so that I could help to translate and review (in Chinese)? The libvirt translations are managed under the Fedora translations teams. So in fact you just need to request to join the Chinese Fedora team. If you go to this page: https://fedora.zanata.org/language/view/zh-CN There's a drop down menu 'v ...' which has a 'Request to join team' option in it. Once you've joined you'd be able to work on any Fedora related project, or just libvirt, as you so prefer. 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] lxc / docker containers gives option to inherit the namespaces. Example lxc-start has option [ --share-[net|ipc|uts] name|pid ] where --share-net name|pid means Inherit a network
On 21.05.2015 19:43, ik.nitk wrote: This patch tries to add the similar option to libvirt lxc. So to inherit namespace from name container c2. add this into xml. lxc:namespace sharenet type='name' value='c2'/ /lxc:namespace And to inherit namespace from a pid. add this into xml. lxc:namespace sharenet type='pid' value='10245'/ /lxc:namespace And to inherit namespace from a netns. add this into xml. lxc:namespace sharenet type='netns' value='red'/ /lxc:namespace Similar options for ipc/uts. shareipc/ , shareuts / The reasong lxc xml namespace is added because this feature is very specific to lxc. Therfore wanted to keep it seperated from actual libvirt xml domain. -imran --- src/Makefile.am | 5 +- src/lxc/lxc_conf.c | 2 +- src/lxc/lxc_conf.h | 23 + src/lxc/lxc_container.c | 191 ++-- src/lxc/lxc_domain.c| 254 +++- src/lxc/lxc_domain.h| 1 + 6 files changed, 463 insertions(+), 13 deletions(-) Unfortunately, we entered freeze and this patch slipped. Sorry. I'll review after the release. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] qemu: add a check for slot and base when build dimm address
When hot-plug a memory device, we don't check if there is a memory device have the same address with the memory device we want hot-pluged. Qemu forbid use/hot-plug 2 memory device with same slot or the same base(qemu side this elemnt named addr). Introduce a address check when build memory device qemu command line. Signed-off-by: Luyao Huang lhu...@redhat.com --- v2: move the check to qemuBuildMemoryDeviceStr() to check the dimm address when start/hot-plug a memory device. src/qemu/qemu_command.c | 77 - 1 file changed, 57 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d8ce511..0380a3b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4955,6 +4955,61 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr mem, } +static int +qemuBuildMemoryDeviceAddr(virBuffer *buf, + virDomainDefPtr def, + virDomainMemoryDefPtr mem) +{ +ssize_t i; + +if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) +return 0; + +if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(only 'dimm' addresses are supported for the + pc-dimm device)); +return -1; +} + +if (mem-info.addr.dimm.slot = def-mem.memory_slots) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(memory device slot '%u' exceeds slots count '%u'), + mem-info.addr.dimm.slot, def-mem.memory_slots); +return -1; +} + +for (i = 0; i def-nmems; i++) { + virDomainMemoryDefPtr tmp = def-mems[i]; + + if (tmp == mem || + tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) + continue; + + if (mem-info.addr.dimm.slot == tmp-info.addr.dimm.slot) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, +_(memory device slot '%u' already been + used by other memory device), +mem-info.addr.dimm.slot); + return -1; + } + + if (mem-info.addr.dimm.base != 0 tmp-info.addr.dimm.base != 0 + mem-info.addr.dimm.base == tmp-info.addr.dimm.base) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, +_(memory device base '0x%llx' already been + used by other memory device), +mem-info.addr.dimm.base); + return -1; + } +} + +virBufferAsprintf(buf, ,slot=%d, mem-info.addr.dimm.slot); +virBufferAsprintf(buf, ,addr=%llu, mem-info.addr.dimm.base); + +return 0; +} + char * qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, virDomainDefPtr def, @@ -4976,29 +5031,11 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, return NULL; } -if (mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM -mem-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(only 'dimm' addresses are supported for the - pc-dimm device)); -return NULL; -} - -if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM -mem-info.addr.dimm.slot = def-mem.memory_slots) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(memory device slot '%u' exceeds slots count '%u'), - mem-info.addr.dimm.slot, def-mem.memory_slots); -return NULL; -} - virBufferAsprintf(buf, pc-dimm,node=%d,memdev=mem%s,id=%s, mem-targetNode, mem-info.alias, mem-info.alias); -if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) { -virBufferAsprintf(buf, ,slot=%d, mem-info.addr.dimm.slot); -virBufferAsprintf(buf, ,addr=%llu, mem-info.addr.dimm.base); -} +if (qemuBuildMemoryDeviceAddr(buf, def, mem) 0) +return NULL; break; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] lxc: properly clean up qemu-nbd
Add the qemu-nbd tasks to the container cgroup to make sure those will be killed when the container is stopped. In order to reliably get the qemu-nbd tasks PIDs, we use /sys/devices/virtual/block/DEV/pid as qemu-nbd is daemonizing itself. --- src/libvirt_private.syms | 1 + src/lxc/lxc_controller.c | 56 src/util/virprocess.c| 45 ++ src/util/virprocess.h| 2 ++ 4 files changed, 104 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8c50ea2..409bb4f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1973,6 +1973,7 @@ virProcessAbort; virProcessExitWithStatus; virProcessGetAffinity; virProcessGetNamespaces; +virProcessGetPids; virProcessGetStartTime; virProcessKill; virProcessKillPainfully; diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index e144c2d..14d873e 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -107,6 +107,9 @@ struct _virLXCController { pid_t initpid; +size_t nnbdpids; +pid_t *nbdpids; + size_t nveths; char **veths; @@ -283,6 +286,8 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) virObjectUnref(ctrl-server); virLXCControllerFreeFuse(ctrl); +VIR_FREE(ctrl-nbdpids); + virCgroupFree(ctrl-cgroup); /* This must always be the last thing to be closed */ @@ -471,6 +476,9 @@ static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs) return -1; } +/* The NBD device will be cleaned up while the cgroup will end. + * For this we need to remember the qemu-nbd pid and add it to + * the cgroup*/ if (virFileNBDDeviceAssociate(fs-src, fs-format, fs-readonly, @@ -503,6 +511,9 @@ static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk) return -1; } +/* The NBD device will be cleaned up while the cgroup will end. + * For this we need to remember the qemu-nbd pid and add it to + * the cgroup*/ if (virFileNBDDeviceAssociate(src, format, disk-src-readonly, @@ -525,6 +536,38 @@ static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk) return 0; } +static int virLXCControllerAppendNBDPids(virLXCControllerPtr ctrl, + const char *dev) +{ +char *pidpath = NULL; +pid_t *pids; +size_t npids; +size_t i; +int ret = -1; +pid_t pid; + +if (!STRPREFIX(dev, /dev/) || +virAsprintf(pidpath, /sys/devices/virtual/block/%s/pid, dev + 5) 0) +goto cleanup; + +if (virPidFileReadPath(pidpath, pid) 0) +goto cleanup; + +if (virProcessGetPids(pid, npids, pids) 0) +goto cleanup; + +for (i = 0; i npids; i++) { +if (VIR_APPEND_ELEMENT(ctrl-nbdpids, ctrl-nnbdpids, pids[i]) 0) +goto cleanup; +} + +ret = 0; + + cleanup: +VIR_FREE(pids); +VIR_FREE(pidpath); +return ret; +} static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) { @@ -570,6 +613,9 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) } else if (fs-fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_NBD) { if (virLXCControllerSetupNBDDeviceFS(fs) 0) goto cleanup; + +if (virLXCControllerAppendNBDPids(ctrl, fs-src) 0) +goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(fs driver %s is not supported), @@ -629,6 +675,9 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) } if (virLXCControllerSetupNBDDeviceDisk(disk) 0) goto cleanup; + +if (virLXCControllerAppendNBDPids(ctrl, virDomainDiskGetSource(disk)) 0) +goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(disk driver %s is not supported), @@ -781,6 +830,7 @@ static int virLXCControllerSetupCgroupLimits(virLXCControllerPtr ctrl) virBitmapPtr auto_nodeset = NULL; int ret = -1; virBitmapPtr nodeset = NULL; +size_t i; VIR_DEBUG(Setting up cgroup resource limits); @@ -798,6 +848,12 @@ static int virLXCControllerSetupCgroupLimits(virLXCControllerPtr ctrl) if (virCgroupAddTask(ctrl-cgroup, getpid()) 0) goto cleanup; +/* Add all qemu-nbd tasks to the cgroup */ +for (i = 0; i ctrl-nnbdpids; i++) { +if (virCgroupAddTask(ctrl-cgroup, ctrl-nbdpids[i]) 0) +goto cleanup; +} + if (virLXCCgroupSetup(ctrl-def, ctrl-cgroup, nodeset) 0) goto cleanup; diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 7a79970..8b4b32f 100644 --- a/src/util/virprocess.c +++
Re: [libvirt] Libvirt now using Zanata for translation
On 2015/5/27 17:00, Daniel P. Berrange wrote: On Wed, May 27, 2015 at 08:45:09AM +0800, zhang bo wrote: On 2015/3/6 21:20, Daniel P. Berrange wrote: As of current GIT master, libvirt is fully using Zanata for po file translation After some trouble with the zanata python client, I have now successfully refreshed pushed the current libvirt.pot file, and synchronized all the translation po files back down to GIT master. For ongoing refreshes of the libvirt.pot, it can be rebuild pushed to zanata and .po files resynchonized using # cd po # rm libvirt.pot # make libvirt.pot # zanata-cli push # zanata-cli pull Where 'zanata-cli' is the *JAVA* client, not the python client. I strongly recommend against use of the python client for the libvirt project as it doesn't handle the size of the libvirt.pot/.po files well resulting in unexplained data loss. Even the java client has problems pushing the .po files to the server due to poorly designed rate limiting in the zanata server. Fortunately this is not something we need do again, now we have imported the existing .po file. The java client has been troublefree wrt to pushing the .pot file and pulling .po files, which is all we need for ongoing work now. Currently myself Daniel Veillard are setup as admins of the libvirt project in Zanata we can add further people as required. Regards, Daniel Would you please add me as admin so that I could help to translate and review (in Chinese)? The libvirt translations are managed under the Fedora translations teams. So in fact you just need to request to join the Chinese Fedora team. If you go to this page: https://fedora.zanata.org/language/view/zh-CN There's a drop down menu 'v ...' which has a 'Request to join team' option in it. Once you've joined you'd be able to work on any Fedora related project, or just libvirt, as you so prefer. Regards, Daniel Thanks! waiting for the coordinator to accept my application. -- Oscar oscar.zhan...@huawei.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Deal with panic device on pSeries.
On 05/15/2015 05:35 AM, Andrea Bolognani wrote: The guest firmware provides the same functionality as the pvpanic device, which is not available in QEMU on pSeries: make sure the XML reflects this fact by automatically adding a panic/ element when not already present. On the other hand, unlike the pvpanic device, the guest firmware can't be configured, so report an error if an address has been provided in the XML. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182388 --- src/qemu/qemu_command.c| 25 - src/qemu/qemu_domain.c | 14 ++ .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 1 + .../qemuxml2argv-pseries-nvram.xml | 1 + .../qemuxml2argv-pseries-panic-address.xml | 32 ++ .../qemuxml2argv-pseries-panic-missing.args| 7 + .../qemuxml2argv-pseries-panic-missing.xml | 29 .../qemuxml2argv-pseries-panic-no-address.args | 7 + .../qemuxml2argv-pseries-panic-no-address.xml | 30 tests/qemuxml2argvtest.c | 6 .../qemuxml2xmlout-pseries-panic-missing.xml | 30 tests/qemuxml2xmltest.c| 2 ++ 12 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-address.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-missing.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-panic-no-address.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-missing.xml This should be split into two patches The first one dealing with just the error/bug and the second dealing with adding a panic device by default for PPC* diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5d0a167..138a8b6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10737,13 +10737,28 @@ qemuBuildCommandLine(virConnectPtr conn, } if (def-panic) { -if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { +if (ARCH_IS_PPC64(def-os.arch) STRPREFIX(def-os.machine, pseries)) { +/* For pSeries guests, the firmware provides the same + * functionality of the pvpanic device. The address + * cannot be configured by the user */ +if (def-panic-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(setting the panic device address is not + supported for pSeries guests)); +goto error; +} Seems to be something that should documented in formatdomain.html.in - that is a panic device for PPC64 can not have an address +} else { +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(your QEMU is too old to support pvpanic)); Since you're moving it - use the opportunity to change the message to: This QEMU doesn't support the panic device Whether you go with pvpanic or panic just depends on how technically detailed you want to get - libvirt refers to it as panic, while QEMU refers to it as pvpanic... I'm ambivalent as to which to use since it points at the same root issue. +goto error; +} + if (def-panic-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) { virCommandAddArg(cmd, -device); virCommandAddArgFormat(cmd, pvpanic,ioport=%d, def-panic-info.addr.isa.iobase); -} else if (def-panic-info.type == - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { +} else if (def-panic-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { This change makes the line 80 characters - so it should be changed back to two lines. virCommandAddArgList(cmd, -device, pvpanic, NULL); } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, @@ -10751,10 +10766,6 @@ qemuBuildCommandLine(virConnectPtr conn, with ISA address type)); goto error; } -} else { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, - _(your QEMU is too old to support pvpanic)); -goto error; } } The above seems to be patch 1 (bug fix) while the rest would be patch 2 (feature change). Although I think swapping the order of
Re: [libvirt] [PATCH] zfs: fix storagepoolxml2xml test
Cole Robinson wrote: On 05/26/2015 12:30 AM, Roman Bogorodskiy wrote: Commit 7c2d65d dropped setting default mode. Update zfs tests accordingly. --- tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml | 3 --- tests/storagepoolxml2xmlout/pool-zfs.xml | 3 --- 2 files changed, 6 deletions(-) diff --git a/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml b/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml index 89dcdbf..8d9be73 100644 --- a/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml +++ b/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml @@ -10,8 +10,5 @@ /source target path/dev/zvol/testpool/path -permissions - mode0755/mode -/permissions /target /pool diff --git a/tests/storagepoolxml2xmlout/pool-zfs.xml b/tests/storagepoolxml2xmlout/pool-zfs.xml index c9e5df9..1a8de4f 100644 --- a/tests/storagepoolxml2xmlout/pool-zfs.xml +++ b/tests/storagepoolxml2xmlout/pool-zfs.xml @@ -9,8 +9,5 @@ /source target path/dev/zvol/testpool/path -permissions - mode0755/mode -/permissions /target /pool ACK, sorry about that Pushed, thanks! Roman Bogorodskiy -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] debug: assure NULLSTR() around all %s args in debug at top of public APIs
There are also a couple that were very uninformatively just logging the value of the pointer rather than the string itself: * the name arg to virNodeDeviceLookupByName() * wwnn and wwpn args to virNodeDeviceLookupSCSIHostByWWN() All char*'s that make sense should now have their contents logged rather than the pointer, and all %s args should now be inside NULLSTR(). --- src/libvirt-domain.c| 36 ++-- src/libvirt-host.c | 6 +++--- src/libvirt-interface.c | 6 +++--- src/libvirt-network.c | 6 +++--- src/libvirt-nodedev.c | 10 +- src/libvirt-nwfilter.c | 6 +++--- src/libvirt-secret.c| 4 ++-- src/libvirt-storage.c | 16 8 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 2edba1a..5907f58 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -167,7 +167,7 @@ virDomainPtr virDomainCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags) { -VIR_DEBUG(conn=%p, xmlDesc=%s, flags=%x, conn, xmlDesc, flags); +VIR_DEBUG(conn=%p, xmlDesc=%s, flags=%x, conn, NULLSTR(xmlDesc), flags); virResetLastError(); @@ -235,7 +235,7 @@ virDomainCreateXMLWithFiles(virConnectPtr conn, const char *xmlDesc, unsigned int flags) { VIR_DEBUG(conn=%p, xmlDesc=%s, nfiles=%u, files=%p, flags=%x, - conn, xmlDesc, nfiles, files, flags); + conn, NULLSTR(xmlDesc), nfiles, files, flags); virResetLastError(); @@ -413,7 +413,7 @@ virDomainLookupByUUIDString(virConnectPtr conn, const char *uuidstr) virDomainPtr virDomainLookupByName(virConnectPtr conn, const char *name) { -VIR_DEBUG(conn=%p, name=%s, conn, name); +VIR_DEBUG(conn=%p, name=%s, conn, NULLSTR(name)); virResetLastError(); @@ -955,7 +955,7 @@ virDomainSaveFlags(virDomainPtr domain, const char *to, int virDomainRestore(virConnectPtr conn, const char *from) { -VIR_DEBUG(conn=%p, from=%s, conn, from); +VIR_DEBUG(conn=%p, from=%s, conn, NULLSTR(from)); virResetLastError(); @@ -1025,7 +1025,7 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, unsigned int flags) { VIR_DEBUG(conn=%p, from=%s, dxml=%s, flags=%x, - conn, from, NULLSTR(dxml), flags); + conn, NULLSTR(from), NULLSTR(dxml), flags); virResetLastError(); @@ -1089,7 +1089,7 @@ virDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *file, unsigned int flags) { VIR_DEBUG(conn=%p, file=%s, flags=%x, - conn, file, flags); + conn, NULLSTR(file), flags); virResetLastError(); @@ -1162,7 +1162,7 @@ virDomainSaveImageDefineXML(virConnectPtr conn, const char *file, const char *dxml, unsigned int flags) { VIR_DEBUG(conn=%p, file=%s, dxml=%s, flags=%x, - conn, file, dxml, flags); + conn, NULLSTR(file), NULLSTR(dxml), flags); virResetLastError(); @@ -2623,7 +2623,7 @@ virConnectDomainXMLFromNative(virConnectPtr conn, unsigned int flags) { VIR_DEBUG(conn=%p, format=%s, config=%s, flags=%x, - conn, nativeFormat, nativeConfig, flags); + conn, NULLSTR(nativeFormat), NULLSTR(nativeConfig), flags); virResetLastError(); @@ -2673,7 +2673,7 @@ virConnectDomainXMLToNative(virConnectPtr conn, unsigned int flags) { VIR_DEBUG(conn=%p, format=%s, xml=%s, flags=%x, - conn, nativeFormat, domainXml, flags); + conn, NULLSTR(nativeFormat), NULLSTR(domainXml), flags); virResetLastError(); @@ -4597,7 +4597,7 @@ virDomainMigrateFinish(virConnectPtr dconn, { VIR_DEBUG(dconn=%p, dname=%s, cookie=%p, cookielen=%d, uri=%s, flags=%lx, dconn, NULLSTR(dname), cookie, cookielen, - uri, flags); + NULLSTR(uri), flags); virResetLastError(); @@ -4639,8 +4639,8 @@ virDomainMigratePrepare2(virConnectPtr dconn, { VIR_DEBUG(dconn=%p, cookie=%p, cookielen=%p, uri_in=%s, uri_out=%p, flags=%lx, dname=%s, bandwidth=%lu, dom_xml=%s, dconn, - cookie, cookielen, uri_in, uri_out, flags, NULLSTR(dname), - bandwidth, dom_xml); + cookie, cookielen, NULLSTR(uri_in), uri_out, flags, NULLSTR(dname), + bandwidth, NULLSTR(dom_xml)); virResetLastError(); @@ -4681,7 +4681,7 @@ virDomainMigrateFinish2(virConnectPtr dconn, { VIR_DEBUG(dconn=%p, dname=%s, cookie=%p, cookielen=%d, uri=%s, flags=%lx, retcode=%d, dconn, NULLSTR(dname), cookie, - cookielen, uri, flags, retcode); + cookielen, NULLSTR(uri), flags, retcode); virResetLastError(); @@ -4721,7 +4721,7 @@ virDomainMigratePrepareTunnel(virConnectPtr conn, {
[libvirt] [PATCH] node_device: more informative error log when device isn't found
In a couple of cases, the node device driver (and the test node device driver which likely copied it) was only logging Node device not found when it couldn't find the requested device. This patch changes those cases to log the name (and in the case when it's relevant, the wwnn and wwpn) as well. --- src/node_device/node_device_driver.c | 14 ++ src/test/test_driver.c | 8 ++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 34ba1fa..768db7f 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -248,7 +248,9 @@ nodeDeviceLookupByName(virConnectPtr conn, const char *name) nodeDeviceUnlock(); if (!obj) { -virReportError(VIR_ERR_NO_NODE_DEVICE, NULL); +virReportError(VIR_ERR_NO_NODE_DEVICE, + _(no node device with matching name '%s'), + name); goto cleanup; } @@ -597,8 +599,10 @@ nodeDeviceCreateXML(virConnectPtr conn, * we're returning what we get... */ if (dev == NULL) -virReportError(VIR_ERR_NO_NODE_DEVICE, NULL); - +virReportError(VIR_ERR_NO_NODE_DEVICE, + _(no node device for '%s' with matching + wwnn '%s' and wwpn '%s'), + def-name, wwnn, wwpn); cleanup: nodeDeviceUnlock(); virNodeDeviceDefFree(def); @@ -621,7 +625,9 @@ nodeDeviceDestroy(virNodeDevicePtr dev) nodeDeviceUnlock(); if (!obj) { -virReportError(VIR_ERR_NO_NODE_DEVICE, NULL); +virReportError(VIR_ERR_NO_NODE_DEVICE, + _(no node device with matching name '%s'), + dev-name); goto out; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 038b2b8..d1f0af3 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5673,7 +5673,9 @@ testNodeDeviceLookupByName(virConnectPtr conn, const char *name) testDriverUnlock(driver); if (!obj) { -virReportError(VIR_ERR_NO_NODE_DEVICE, NULL); +virReportError(VIR_ERR_NO_NODE_DEVICE, + _(no node device with matching name '%s'), + name); goto cleanup; } @@ -5893,7 +5895,9 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) testDriverUnlock(driver); if (!obj) { -virReportError(VIR_ERR_NO_NODE_DEVICE, NULL); +virReportError(VIR_ERR_NO_NODE_DEVICE, + _(no node device with matching name '%s'), + dev-name); goto out; } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] check if console PTY is null before attempting to open
Console devices have their pty devices assigned when the qemu is actually started. The libvirt spends considerable amount of time to start the qemu if the guest has multiple passthrough devices. If time is spent during the hostdev preparation, someone attempts to open the console, the libvirt crashes in virChrdevLockFilePath().The patch attempts to fix the crash by adding a check before attempting to open. Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1233d8f..fec928f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16278,6 +16278,12 @@ qemuDomainOpenConsole(virDomainPtr dom, goto cleanup; } +if (!(chr-source.data.file.path)) { +virReportError(VIR_ERR_OPERATION_FAILED, %s, + _(PTY device is not yet assigned)); +goto cleanup; +} + /* handle mutually exclusive access to console devices */ ret = virChrdevOpen(priv-devs, chr-source, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt] interface: don't error out if a bond has no interfaces
It's not a problem at all and causes virt-manager to break down. Note: netcf 0.2.8 generates invalid XML for a bond with no interfaces anyway, so this error is in fact not reached as we fail earlier. Fix submitted upstream. Signed-off-by: Lubomir Rintel lkund...@v3.sk --- src/conf/interface_conf.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index c2eb945..29769ac 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -553,19 +553,15 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def, nbItf = virXPathNodeSet(./interface, ctxt, interfaces); if (nbItf 0) { ret = -1; -goto error; +goto cleanup; } -if (nbItf == 0) { -virReportError(VIR_ERR_XML_ERROR, - %s, _(bond has no interfaces)); -ret = -1; -goto error; -} +if (nbItf == 0) +goto cleanup; if (VIR_ALLOC_N(def-data.bond.itf, nbItf) 0) { ret = -1; -goto error; +goto cleanup; } def-data.bond.nbItf = nbItf; @@ -575,12 +571,12 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def, if (itf == NULL) { ret = -1; def-data.bond.nbItf = i; -goto error; +goto cleanup; } def-data.bond.itf[i] = itf; } - error: + cleanup: VIR_FREE(interfaces); ctxt-node = bond; return ret; -- 2.4.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt] interface: don't error out if a bond has no interfaces
On 05/27/2015 01:30 PM, Lubomir Rintel wrote: It's not a problem at all and causes virt-manager to break down. Note: netcf 0.2.8 generates invalid XML for a bond with no interfaces anyway, so this error is in fact not reached as we fail earlier. Fix submitted upstream. ACK. This patch also makes bonds more consistent with bridges, which also require the bridge element, but allow it to be empty. Since this is a bugfix and straightforward, I'm pushing it now (after adding a small bit to the commit log, and changing ret initialization from 0 to -1, so that it's more in line with the rest of libvirt's code), and will be looking at the netcf patch momentarily. Thanks for taking the time to report this error with a patch :-) Signed-off-by: Lubomir Rintel lkund...@v3.sk --- src/conf/interface_conf.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index c2eb945..29769ac 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -553,19 +553,15 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def, nbItf = virXPathNodeSet(./interface, ctxt, interfaces); if (nbItf 0) { ret = -1; -goto error; +goto cleanup; } -if (nbItf == 0) { -virReportError(VIR_ERR_XML_ERROR, - %s, _(bond has no interfaces)); -ret = -1; -goto error; -} +if (nbItf == 0) +goto cleanup; if (VIR_ALLOC_N(def-data.bond.itf, nbItf) 0) { ret = -1; -goto error; +goto cleanup; } def-data.bond.nbItf = nbItf; @@ -575,12 +571,12 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def, if (itf == NULL) { ret = -1; def-data.bond.nbItf = i; -goto error; +goto cleanup; } def-data.bond.itf[i] = itf; } - error: + cleanup: VIR_FREE(interfaces); ctxt-node = bond; return ret; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Plans for next release
On 05/23/2015 07:22 AM, Daniel Veillard wrote: Hi everybody, if we want to get it by next month, we should probably freeze on Tuesday for an 1.2.16 on June 1st, we 'only' have 137 commits since 1.2.15 but sticking to the monthly release is important. Hi Daniel, I have a few old patches on the list that would be nice to get into 1.2.16. First is the libxl config file fixes in the spec file that we postponed during the 1.2.15 release https://www.redhat.com/archives/libvir-list/2015-May/msg4.html Next is a patch to fix generation of AUTHORS file in the python bindings https://www.redhat.com/archives/libvir-list/2015-May/msg00319.html Finally, since the domXML - xl.cfg conversions for SPICE where committed a few weeks back, it would be nice to provide support for SPICE and qxl config in the libxl driver too https://www.redhat.com/archives/libvir-list/2015-May/msg00256.html It would be unfortunate to convert xl.cfg SPICE config to domXML but then be unable to use that config in the libxl driver. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Simplify allocation check in storageVolResize
On 05/27/2015 11:08 AM, Ján Tomko wrote: The volume cannot be shrinked below existing allocation, thus a successful resize with VOL_RESIZE_ALLOCATE will never increase the pool's available value. Since shrinking a volume below existing allocation is not allowed, it is not possible for a successful resize with VOL_RESIZE_ALLOCATE to increase the pool's available value. Even with the SHRINK flag it is possible to extend the current allocation or even the capacity. Remove the overflow when computing delta with this flag and do the check even if the flag was specified. https://bugzilla.redhat.com/show_bug.cgi?id=1073305 Editorial comment: This bz should go back to POST... --- src/storage/storage_driver.c | 21 + 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ac4a74a..fbb8050 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2292,7 +2292,7 @@ storageVolResize(virStorageVolPtr obj, virStorageBackendPtr backend; virStoragePoolObjPtr pool = NULL; virStorageVolDefPtr vol = NULL; -unsigned long long abs_capacity, delta; +unsigned long long abs_capacity, delta = 0; int ret = -1; virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE | @@ -2341,18 +2341,14 @@ storageVolResize(virStorageVolPtr obj, goto cleanup; } -if (flags VIR_STORAGE_VOL_RESIZE_SHRINK) -delta = vol-target.allocation - abs_capacity; -else +if (flags VIR_STORAGE_VOL_RESIZE_ALLOCATE) delta = abs_capacity - vol-target.allocation; /* If the operation is going to increase the allocation value and not * just the capacity value, then let's make sure there's enough space * in the pool in order to perform that operation */ The comment won't make sense any more as well. ACK I would think safe for freeze John -if (flags VIR_STORAGE_VOL_RESIZE_ALLOCATE -!(flags VIR_STORAGE_VOL_RESIZE_SHRINK) -delta pool-def-available) { +if (delta pool-def-available) { virReportError(VIR_ERR_OPERATION_FAILED, %s, _(Not enough space left in storage pool)); goto cleanup; @@ -2375,15 +2371,8 @@ storageVolResize(virStorageVolPtr obj, */ if (flags VIR_STORAGE_VOL_RESIZE_ALLOCATE) { vol-target.allocation = abs_capacity; - -/* Update pool metadata */ -if (flags VIR_STORAGE_VOL_RESIZE_SHRINK) { - pool-def-allocation -= delta; - pool-def-available += delta; -} else { - pool-def-allocation += delta; - pool-def-available -= delta; -} +pool-def-allocation += delta; +pool-def-available -= delta; } ret = 0; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Fix shrinking volumes with the delta flag
On 05/27/2015 11:08 AM, Ján Tomko wrote: This never worked. In 0.9.10 when this API was introduced, it was intended that the SHRINK flag combined with DELTA would shrink the volume by the specified capacity (to avoid passing negative numbers). See commit 055bbf4. When the SHRINK flag was finally implemented for the first backend in 1.2.13 (commit aa9aa6a), it was only implemented for the absolute values and with the delta flag the volume is always extended, regardless of the SHRINK flag. Treat the SHRINK flag as a minus sign when used together with DELTA, to allow shrinking volumes as was documented in the API since 0.9.10. https://bugzilla.redhat.com/show_bug.cgi?id=1220213 --- src/storage/storage_driver.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) ACK (seemingly safe too) John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] virsh: make negative values with vol-resize more convenient
On 05/27/2015 11:08 AM, Ján Tomko wrote: When shrinking a volume by a certain size, instead of typing vol-resize volume 1G --delta --shrink we allow the convience of specifying a negative value: vol-resize volume -1G --delta --shrink getting the same results with one more character. A negative value only makes sense as a delta. Imply the --delta parameter if the value is negative. Still require --shrink, because the operation is potentially destructive. --- tools/virsh-volume.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) Should virsh.pod be updated as well to indicate that --shrink with negative number implies --delta (not required, but the text in virsh.pod around this is lacking... ACK (seemingly safe too) John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libxl: load on FreeBSD
On Sun, May 24, 2015 at 06:45:02PM +0300, Roman Bogorodskiy wrote: The libxl tries to check if it's running in dom0 by parsing /proc/xen/capabilities and if that fails it doesn't load. There's no procfs interface in Xen on FreeBSD, so this check always fails. In addition to checking procfs, check if /dev/xen/xenstored, that's enough to check if we're running in dom0 in FreeBSD case. --- src/libxl/libxl_driver.c | 42 ++ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 12be816..fddafa1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -74,6 +74,7 @@ VIR_LOG_INIT(libxl.libxl_driver); #define LIBXL_CONFIG_FORMAT_SEXPR xen-sxpr #define HYPERVISOR_CAPABILITIES /proc/xen/capabilities +#define HYPERVISOR_XENSTORED /dev/xen/xenstored /* Number of Xen scheduler parameters */ #define XEN_SCHED_CREDIT_NPARAM 2 @@ -427,8 +428,6 @@ static bool libxlDriverShouldLoad(bool privileged) { bool ret = false; -int status; -char *output = NULL; /* Don't load if non-root */ if (!privileged) { @@ -436,24 +435,27 @@ libxlDriverShouldLoad(bool privileged) return ret; } -if (!virFileExists(HYPERVISOR_CAPABILITIES)) { -VIR_INFO(Disabling driver as HYPERVISOR_CAPABILITIES - does not exist); -return ret; -} -/* - * Don't load if not running on a Xen control domain (dom0). It is not - * sufficient to check for the file to exist as any guest can mount - * xenfs to /proc/xen. - */ -status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, output); -if (status = 0) -status = strncmp(output, control_d, 9); -VIR_FREE(output); -if (status) { -VIR_INFO(No Xen capabilities detected, probably not running - in a Xen Dom0. Disabling libxenlight driver); - +if (virFileExists(HYPERVISOR_CAPABILITIES)) { +int status; +char *output = NULL; +/* + * Don't load if not running on a Xen control domain (dom0). It is not + * sufficient to check for the file to exist as any guest can mount + * xenfs to /proc/xen. + */ +status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, output); +if (status = 0) +status = strncmp(output, control_d, 9); +VIR_FREE(output); +if (status) { +VIR_INFO(No Xen capabilities detected, probably not running + in a Xen Dom0. Disabling libxenlight driver); + +return ret; +} +} else if (!virFileExists(HYPERVISOR_XENSTORED)) { +VIR_INFO(Disabling driver as neither HYPERVISOR_CAPABILITIES + nor HYPERVISOR_CAPABILITIES exist); s/HYPERVISOR_CAPABILITIES/HYPERVISOR_XENSTORED/ ACK with that changed. return ret; } -- 2.3.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] virsh: Pass vshControl to all vshCommandOpt*() calls.
On Wed, 2015-05-27 at 16:47 +0200, Michal Privoznik wrote: -bool config = vshCommandOptBool(cmd, config); +bool config = vshCommandOptBool(ctl, cmd, config); I don't think this is needed. vshCommandOptBool should never return an error. Well, it's returning just if a flag was specified or not. I added it in to keep all vshCommandOpt*() calls consistent, but since both you and Peter feel like it shouldn't be there I'll get rid of it. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team $ python -c print('a'.join(['', 'bologn', '@redh', 't.com'])) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage_fs: Create directory with UID if needed
On 05/27/2015 04:10 AM, Martin Kletzander wrote: The code already exists there, it just modified different flags. I just noticed this when looking at the code. This patch is better to view with bigger context or '-W'. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/storage/storage_backend_fs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bcbbb3ae252a..5dc712925b27 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -807,7 +807,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, (needs_create_as_uid || !virFileExists(pool-def-target.path))) mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; if (needs_create_as_uid) -flags |= VIR_DIR_CREATE_AS_UID; +dir_create_flags |= VIR_DIR_CREATE_AS_UID; /* Now create the final dir in the path with the uid/gid/mode * requested in the config. If the dir already exists, just set ACK, thanks for catching that - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] util: make it more robust to calculate timeout value
On Mon, May 25, 2015 at 02:22:42PM +0800, Wang Yufei wrote: From: Zhang Bo oscar.zhan...@huawei.com When we change system clock to years ago, a certain CPU may use up 100% cputime. The reason is that in function virEventPollCalculateTimeout(), we assign the unsigned long long result to an INT variable, *timeout = then - now; // timeout is INT, and then/now are long long if (*timeout 0) *timeout = 0; there's a chance that variable @then minus variable @now may be a very large number that overflows INT value expression, then *timeout will be negative and be assigned to 0. Next the 'poll' in function virEventPollRunOnce() will get into an 'endless' while loop there. thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%. Although as we discussed before in https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html it should be prohibited to set-time while other applications are running, but it does seems to have no harm to make the codes more robust. Signed-off-by: Wang Yufei james.wangyu...@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/util/vireventpoll.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c index ffda206..4e4f407 100644 --- a/src/util/vireventpoll.c +++ b/src/util/vireventpoll.c @@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout) return -1; EVENT_DEBUG(Schedule timeout then=%llu now=%llu, then, now); -*timeout = then - now; -if (*timeout 0) +if (then = now) *timeout = 0; +else +*timeout = ((then-now) INT_MAX) ? INT_MAX : (then-now); This will work, although the following would be nicer to read since there were such problems with the calculation in earlier versions: if (then = now) { *timeout = 0; } else { long long tmp = then - now; if (tmp INT_MAX) *timeout = INT_MAX else *timeout = tmp; } But that, of course, has the same meaning as your version, which is fine. ACK to that, I'll push it in a while. } else { *timeout = -1; } -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virsh: Move error messages inside vshCommandOpt*() functions.
On 22.05.2015 10:59, Andrea Bolognani wrote: --- tests/vcpupin| 4 +- tools/virsh-domain-monitor.c | 9 +-- tools/virsh-domain.c | 134 +++ tools/virsh-host.c | 57 +++--- tools/virsh-interface.c | 6 +- tools/virsh-network.c| 6 +- tools/virsh-volume.c | 24 ++-- tools/virsh.c| 111 +-- 8 files changed, 104 insertions(+), 247 deletions(-) Nice cleanup. diff --git a/tools/virsh.c b/tools/virsh.c index 9f44793..db9354c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1517,23 +1517,27 @@ vshCommandOpt(const vshCmd *cmd, * 0 in all other cases (@value untouched) */ Does it makes sense to note in comments that this function (and others that you're fixing) is reporting an error? int -vshCommandOptInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptInt(vshControl *ctl, const vshCmd *cmd, const char *name, int *value) { vshCmdOpt *arg; int ret; -ret = vshCommandOpt(cmd, name, arg, true); -if (ret = 0) +if ((ret = vshCommandOpt(cmd, name, arg, true)) = 0) return ret; -if (virStrToLong_i(arg-data, NULL, 10, value) 0) -return -1; -return 1; +if ((ret = virStrToLong_i(arg-data, NULL, 10, value)) 0) +vshError(ctl, + _(Numeric value '%s' for %s option is malformed or out of range), + arg-data, name); +else +ret = 1; + +return ret; } While reworking this, you've missed vshCommandOptTimeoutToMs() which in case of something like following '--timeout blah' will report error twice: from both OptInt() and OptTimeoutToMs(): error: Numeric value 'blah' for timeout option is malformed or out of range error: invalid timeout I think this should be squashed in: @@ -1906,11 +1907,13 @@ vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, { int rv = vshCommandOptInt(ctl, cmd, timeout, timeout); -if (rv 0 || (rv 0 *timeout 1)) { -vshError(ctl, %s, _(invalid timeout)); +if (rv 0) return -1; -} if (rv 0) { +if (*timeout 1) { +vshError(ctl, %s, _(invalid timeout)); +return -1; +} /* Ensure that we can multiply by 1000 without overflowing. */ if (*timeout INT_MAX / 1000) { vshError(ctl, %s, _(timeout is too big)); static int -vshCommandOptULongLongInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptULongLongInternal(vshControl *ctl, const vshCmd *cmd, const char *name, unsigned long long *value, bool wrap) { @@ -1754,15 +1767,18 @@ vshCommandOptULongLongInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *c if ((ret = vshCommandOpt(cmd, name, arg, true)) = 0) return ret; -if (wrap) { -if (virStrToLong_ull(arg-data, NULL, 10, value) 0) -return -1; -} else { -if (virStrToLong_ullp(arg-data, NULL, 10, value) 0) -return -1; -} +if (wrap) +ret = virStrToLong_ull(arg-data, NULL, 10, value); +else +ret = virStrToLong_ullp(arg-data, NULL, 10, value); +if (ret 0) +vshError(ctl, + _(Numeric value '%s' for %s option is malformed or out of range), + arg-data, name); +else +ret = 1; -return 1; +return ret; } You've missed one ocurrance of vshCommandOptULongLong in cmdAllocpages(). /** @@ -1812,7 +1828,7 @@ vshCommandOptULongLongWrap(vshControl *ctl, const vshCmd *cmd, * See vshCommandOptInt() */ int -vshCommandOptScaledInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptScaledInt(vshControl *ctl, const vshCmd *cmd, const char *name, unsigned long long *value, int scale, unsigned long long max) { @@ -1825,9 +1841,16 @@ vshCommandOptScaledInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, if (virStrToLong_ull(arg-data, end, 10, value) 0 || virScaleInteger(value, end, scale, max) 0) -return -1; +{ +vshError(ctl, + _(Numeric value '%s' for %s option is malformed or out of range), + arg-data, name); +ret = -1; +} else { +ret = 1; +} -return 1; +return ret; } Interestingly vshCommandOptString() is missing. So far, the only way for the function to fail is if one uses --option and VSH_OFLAG_EMPTY_OK is not specified. So if we come up with good error message for that case, we are okay to drop all the other error messages. Otherwise looking good. Michal -- libvir-list mailing list libvir-list@redhat.com
[libvirt] [PATCH 1/3] Simplify allocation check in storageVolResize
The volume cannot be shrinked below existing allocation, thus a successful resize with VOL_RESIZE_ALLOCATE will never increase the pool's available value. Even with the SHRINK flag it is possible to extend the current allocation or even the capacity. Remove the overflow when computing delta with this flag and do the check even if the flag was specified. https://bugzilla.redhat.com/show_bug.cgi?id=1073305 --- src/storage/storage_driver.c | 21 + 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ac4a74a..fbb8050 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2292,7 +2292,7 @@ storageVolResize(virStorageVolPtr obj, virStorageBackendPtr backend; virStoragePoolObjPtr pool = NULL; virStorageVolDefPtr vol = NULL; -unsigned long long abs_capacity, delta; +unsigned long long abs_capacity, delta = 0; int ret = -1; virCheckFlags(VIR_STORAGE_VOL_RESIZE_ALLOCATE | @@ -2341,18 +2341,14 @@ storageVolResize(virStorageVolPtr obj, goto cleanup; } -if (flags VIR_STORAGE_VOL_RESIZE_SHRINK) -delta = vol-target.allocation - abs_capacity; -else +if (flags VIR_STORAGE_VOL_RESIZE_ALLOCATE) delta = abs_capacity - vol-target.allocation; /* If the operation is going to increase the allocation value and not * just the capacity value, then let's make sure there's enough space * in the pool in order to perform that operation */ -if (flags VIR_STORAGE_VOL_RESIZE_ALLOCATE -!(flags VIR_STORAGE_VOL_RESIZE_SHRINK) -delta pool-def-available) { +if (delta pool-def-available) { virReportError(VIR_ERR_OPERATION_FAILED, %s, _(Not enough space left in storage pool)); goto cleanup; @@ -2375,15 +2371,8 @@ storageVolResize(virStorageVolPtr obj, */ if (flags VIR_STORAGE_VOL_RESIZE_ALLOCATE) { vol-target.allocation = abs_capacity; - -/* Update pool metadata */ -if (flags VIR_STORAGE_VOL_RESIZE_SHRINK) { - pool-def-allocation -= delta; - pool-def-available += delta; -} else { - pool-def-allocation += delta; - pool-def-available -= delta; -} +pool-def-allocation += delta; +pool-def-available -= delta; } ret = 0; -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] volume resize fixes
First two patches fix bugs and are applicable for the freeze. Ján Tomko (3): Simplify allocation check in storageVolResize Fix shrinking volumes with the delta flag virsh: make negative values with vol-resize more convenient src/storage/storage_driver.c | 26 +- tools/virsh-volume.c | 15 --- 2 files changed, 17 insertions(+), 24 deletions(-) -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] virsh: make negative values with vol-resize more convenient
When shrinking a volume by a certain size, instead of typing vol-resize volume 1G --delta --shrink we allow the convience of specifying a negative value: vol-resize volume -1G --delta --shrink getting the same results with one more character. A negative value only makes sense as a delta. Imply the --delta parameter if the value is negative. Still require --shrink, because the operation is potentially destructive. --- tools/virsh-volume.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 5d6cc6a..989ce87 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1124,14 +1124,10 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) unsigned long long capacity = 0; unsigned int flags = 0; bool ret = false; -bool delta = false; +bool delta = vshCommandOptBool(cmd, delta); if (vshCommandOptBool(cmd, allocate)) flags |= VIR_STORAGE_VOL_RESIZE_ALLOCATE; -if (vshCommandOptBool(cmd, delta)) { -delta = true; -flags |= VIR_STORAGE_VOL_RESIZE_DELTA; -} if (vshCommandOptBool(cmd, shrink)) flags |= VIR_STORAGE_VOL_RESIZE_SHRINK; @@ -1144,14 +1140,19 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd) if (*capacityStr == '-') { /* The API always requires a positive value; but we allow a * negative value for convenience. */ -if (delta vshCommandOptBool(cmd, shrink)) { +if (vshCommandOptBool(cmd, shrink)) { capacityStr++; +delta = true; } else { vshError(ctl, %s, - _(negative size requires --delta and --shrink)); + _(negative size requires --shrink)); goto cleanup; } } + +if (delta) +flags |= VIR_STORAGE_VOL_RESIZE_DELTA; + if (vshVolSize(capacityStr, capacity) 0) { vshError(ctl, _(Malformed size %s), capacityStr); goto cleanup; -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Fix shrinking volumes with the delta flag
This never worked. In 0.9.10 when this API was introduced, it was intended that the SHRINK flag combined with DELTA would shrink the volume by the specified capacity (to avoid passing negative numbers). See commit 055bbf4. When the SHRINK flag was finally implemented for the first backend in 1.2.13 (commit aa9aa6a), it was only implemented for the absolute values and with the delta flag the volume is always extended, regardless of the SHRINK flag. Treat the SHRINK flag as a minus sign when used together with DELTA, to allow shrinking volumes as was documented in the API since 0.9.10. https://bugzilla.redhat.com/show_bug.cgi?id=1220213 --- src/storage/storage_driver.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index fbb8050..1ba6828 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2320,7 +2320,10 @@ storageVolResize(virStorageVolPtr obj, } if (flags VIR_STORAGE_VOL_RESIZE_DELTA) { -abs_capacity = vol-target.capacity + capacity; +if (flags VIR_STORAGE_VOL_RESIZE_SHRINK) +abs_capacity = vol-target.capacity - MIN(capacity, vol-target.capacity); +else +abs_capacity = vol-target.capacity + capacity; flags = ~VIR_STORAGE_VOL_RESIZE_DELTA; } else { abs_capacity = capacity; -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Limit rtc-reset-reinjection requirement to x86 only.
On Tue, May 26, 2015 at 06:13:40PM +0200, Andrea Bolognani wrote: The QMP command, like the interrupt reinjection logic it's connected to, is only implemented in QEMU when TARGET_I386 is defined, so checking for its availability on any other architecture is pointless. On the other hand, when we're on x86, we shouldn still make sure that rtc-reset-reinjection is available and refuse to set the time otherwise. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1211938 --- src/qemu/qemu_driver.c | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) ACK, will push in a while. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index aa0acde..db72bad 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18945,7 +18945,12 @@ qemuDomainSetTime(virDomainPtr dom, goto endjob; } -if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) { +/* On x86, the rtc-reset-reinjection QMP command must be called after + * setting the time to avoid trouble down the line. If the command is + * not available, don't set the time at all and report an error */ +if (ARCH_IS_X86(vm-def-os.arch) +!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) +{ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s, _(cannot set time: qemu doesn't support rtc-reset-reinjection command)); @@ -18968,13 +18973,16 @@ qemuDomainSetTime(virDomainPtr dom, goto endjob; } -qemuDomainObjEnterMonitor(driver, vm); -rv = qemuMonitorRTCResetReinjection(priv-mon); -if (qemuDomainObjExitMonitor(driver, vm) 0) -goto endjob; +/* Don't try to call rtc-reset-reinjection if it's not available */ +if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) { +qemuDomainObjEnterMonitor(driver, vm); +rv = qemuMonitorRTCResetReinjection(priv-mon); +if (qemuDomainObjExitMonitor(driver, vm) 0) +goto endjob; -if (rv 0) -goto endjob; +if (rv 0) +goto endjob; +} ret = 0; -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3] util: make it more robust to calculate timeout value
On Wed, May 27, 2015 at 04:58:22PM +0200, Martin Kletzander wrote: On Mon, May 25, 2015 at 02:22:42PM +0800, Wang Yufei wrote: From: Zhang Bo oscar.zhan...@huawei.com When we change system clock to years ago, a certain CPU may use up 100% cputime. The reason is that in function virEventPollCalculateTimeout(), we assign the unsigned long long result to an INT variable, *timeout = then - now; // timeout is INT, and then/now are long long if (*timeout 0) *timeout = 0; there's a chance that variable @then minus variable @now may be a very large number that overflows INT value expression, then *timeout will be negative and be assigned to 0. Next the 'poll' in function virEventPollRunOnce() will get into an 'endless' while loop there. thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%. Although as we discussed before in https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html it should be prohibited to set-time while other applications are running, but it does seems to have no harm to make the codes more robust. Signed-off-by: Wang Yufei james.wangyu...@huawei.com Signed-off-by: Zhang Bo oscar.zhan...@huawei.com --- src/util/vireventpoll.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c index ffda206..4e4f407 100644 --- a/src/util/vireventpoll.c +++ b/src/util/vireventpoll.c @@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout) return -1; EVENT_DEBUG(Schedule timeout then=%llu now=%llu, then, now); -*timeout = then - now; -if (*timeout 0) +if (then = now) *timeout = 0; +else +*timeout = ((then-now) INT_MAX) ? INT_MAX : (then-now); I added spaces around the minus sign. This will work, although the following would be nicer to read since there were such problems with the calculation in earlier versions: if (then = now) { *timeout = 0; } else { long long tmp = then - now; And of course this would be unsigned. if (tmp INT_MAX) *timeout = INT_MAX else *timeout = tmp; } But that, of course, has the same meaning as your version, which is fine. ACK to that, I'll push it in a while. } else { *timeout = -1; } -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] maint: document use of zanata for translations
Based on recent list questions on how to contribute a translation fix. Signed-off-by: Eric Blake ebl...@redhat.com --- Should be safe for freeze, but as I have never contributed a translation fix, I'll wait for review. HACKING | 19 --- docs/hacking.html.in | 7 +++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/HACKING b/HACKING index fbe838b..e308568 100644 --- a/HACKING +++ b/HACKING @@ -18,7 +18,12 @@ listen to feedback. and is browsable along with other libvirt-related repositories (e.g. libvirt-python) online http://libvirt.org/git/. -(3) Post patches in unified diff format, with git rename detection enabled. You +(3) Patches to translations are maintained via the zanata project +https://fedora.zanata.org/. If you want to fix a translation in a .po file, +join the appropriate language team. The libvirt release process automatically +pulls the latest version of each translation file from zanata. + +(4) Post patches in unified diff format, with git rename detection enabled. You need a one-time setup of: git config diff.renames true @@ -70,7 +75,7 @@ the correct version if needed though). -(4) In your commit message, make the summary line reasonably short (60 characters +(5) In your commit message, make the summary line reasonably short (60 characters is typical), followed by a blank line, followed by any longer description of why your patch makes sense. If the patch fixes a regression, and you know what commit introduced the problem, mentioning that is useful. If the patch @@ -82,7 +87,7 @@ is up to you if you want to include or omit them in the commit message. -(5) Split large changes into a series of smaller patches, self-contained if +(6) Split large changes into a series of smaller patches, self-contained if possible, with an explanation of each patch and an explanation of how the sequence of patches fits together. Moreover, please keep in mind that it's required to be able to compile cleanly (*including* make check and make @@ -93,10 +98,10 @@ things). -(6) Make sure your patches apply against libvirt GIT. Developers only follow GIT +(7) Make sure your patches apply against libvirt GIT. Developers only follow GIT and don't care much about released versions. -(7) Run the automated tests on your code before submitting any changes. In +(8) Run the automated tests on your code before submitting any changes. In particular, configure with compile warnings set to -Werror. This is done automatically for a git checkout; from a tarball, use: @@ -149,7 +154,7 @@ various tests under gdb or Valgrind. -(8) The Valgrind test should produce similar output to make check. If the output +(9) The Valgrind test should produce similar output to make check. If the output has traces within libvirt API's, then investigation is required in order to determine the cause of the issue. Output such as the following indicates some sort of leak: @@ -225,7 +230,7 @@ to tests/.valgrind.supp in order to suppress the warning: -(9) Update tests and/or documentation, particularly if you are adding a new +(10) Update tests and/or documentation, particularly if you are adding a new feature or changing the output of a program. diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 408ea50..5cd23a2 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -16,6 +16,13 @@ along with other libvirt-related repositories (e.g. libvirt-python) a href=http://libvirt.org/git/;online/a./li + liPatches to translations are maintained via +the a href=https://fedora.zanata.org/;zanata project/a. +If you want to fix a translation in a .po file, join the +appropriate language team. The libvirt release process +automatically pulls the latest version of each translation +file from zanata./li + lipPost patches in unified diff format, with git rename detection enabled. You need a one-time setup of:/p pre -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] virsh: Make vshCommandOptScaledInt() use vshCommandOpt().
On 22.05.2015 10:59, Andrea Bolognani wrote: This aligns it to the other vshCommandOpt*() functions. --- tools/virsh.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 4425774..11c2c30 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1804,16 +1804,17 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char *name, unsigned long long *value, int scale, unsigned long long max) { -const char *str; -int ret; +vshCmdOpt *arg; char *end; +int ret; -ret = vshCommandOptString(cmd, name, str); -if (ret = 0) +if ((ret = vshCommandOpt(cmd, name, arg, true)) = 0) This cancels check of arg-def-flags VSH_OFLAG_EMPTY_OK; but since this flag makes sense only for string arguments, it's okay. return ret; -if (virStrToLong_ull(str, end, 10, value) 0 || + +if (virStrToLong_ull(arg-data, end, 10, value) 0 || virScaleInteger(value, end, scale, max) 0) return -1; + return 1; } ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] virsh: Pass vshControl to all vshCommandOpt*() calls.
On 22.05.2015 10:59, Andrea Bolognani wrote: This will allow us to use vshError() to report errors from inside vshCommandOpt*(), instead of replicating the same logic and error messages all over the place. We also have more context inside the vshCommandOpt*() functions, for example the actual value used on the command line, which means we can produce more detailed error messages. --- tools/virsh-domain-monitor.c | 90 +++ tools/virsh-domain.c | 598 +-- tools/virsh-host.c | 46 ++-- tools/virsh-interface.c | 14 +- tools/virsh-network.c| 34 +-- tools/virsh-nodedev.c| 6 +- tools/virsh-pool.c | 26 +- tools/virsh-secret.c | 8 +- tools/virsh-snapshot.c | 88 +++ tools/virsh-volume.c | 34 +-- tools/virsh.c| 107 tools/virsh.h| 77 +++--- 12 files changed, 574 insertions(+), 554 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index a42c150..db7ef8b 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -317,9 +317,9 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) bool ret = false; int rv = 0; int period = -1; -bool config = vshCommandOptBool(cmd, config); -bool live = vshCommandOptBool(cmd, live); -bool current = vshCommandOptBool(cmd, current); +bool config = vshCommandOptBool(ctl, cmd, config); I don't think this is needed. vshCommandOptBool should never return an error. Well, it's returning just if a flag was specified or not. +bool live = vshCommandOptBool(ctl, cmd, live); +bool current = vshCommandOptBool(ctl, cmd, current); unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; VSH_EXCLUSIVE_OPTIONS_VAR(current, live); @@ -340,7 +340,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd) /* Providing a period will adjust the balloon driver collection period. * This is not really an unsigned long, but it */ -if ((rv = vshCommandOptInt(cmd, period, period)) 0) { +if ((rv = vshCommandOptInt(ctl, cmd, period, period)) 0) { This is of course different, specified value may not be a number in which case we want an error to be reported. vshError(ctl, _(Numeric value for %s option is malformed or out of range), period); ACK modulo the OptBool() change. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2] libxl: support domainReset
Currently, libxl does not provide a reset function, but domainReset can be implemented in the libxl driver by forcibly destroying the domain and starting it again. Signed-off-by: Jim Fehlig jfeh...@suse.com --- This is essentially a V2 of a patch submitted quite some time ago https://www.redhat.com/archives/libvir-list/2014-August/msg00077.html The idea of implmenting domainReset in the libxl driver by forcibly destroying the domain and starting it again was ACK'ed in principle by Ian Campbell https://www.redhat.com/archives/libvir-list/2014-August/msg00109.html I never pushed the patch since it was not ACK'ed by a libvirt maintainer and stumbled across it while cleaning up some of my old branches. The only change here is rebase against current libvirt.git master. src/libxl/libxl_driver.c | 59 1 file changed, 59 insertions(+) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 12be816..671d336 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1237,6 +1237,64 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) } static int +libxlDomainReset(virDomainPtr dom, unsigned int flags) +{ +libxlDriverPrivatePtr driver = dom-conn-privateData; +libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); +virDomainObjPtr vm; +int ret = -1; + +virCheckFlags(0, -1); + +if (!(vm = libxlDomObjFromDomain(dom))) +goto cleanup; + +if (virDomainResetEnsureACL(dom-conn, vm-def) 0) +goto cleanup; + +if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + %s, _(Domain is not running)); +goto endjob; +} + +/* + * The semantics of reset can be achieved by forcibly destroying + * the domain and starting it again. + */ +if (libxl_domain_destroy(cfg-ctx, vm-def-id, NULL) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Failed to destroy domain '%d' before reset), + vm-def-id); +goto endjob; +} + +libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED); + +if (libxlDomainStart(driver, vm, false, -1) 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(Failed to start domain '%d' after reset), + vm-def-id); +goto endjob; +} + +ret = 0; + + endjob: +if (!libxlDomainObjEndJob(driver, vm)) +vm = NULL; + + cleanup: +if (vm) +virObjectUnlock(vm); +virObjectUnref(cfg); +return ret; +} + +static int libxlDomainDestroyFlags(virDomainPtr dom, unsigned int flags) { @@ -5066,6 +5124,7 @@ static virHypervisorDriver libxlHypervisorDriver = { .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ +.domainReset = libxlDomainReset, /* 1.2.8 */ .domainDestroy = libxlDomainDestroy, /* 0.9.0 */ .domainDestroyFlags = libxlDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = libxlDomainGetOSType, /* 0.9.0 */ -- 2.3.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libxl: load on FreeBSD
On 05/27/2015 09:06 AM, Martin Kletzander wrote: On Sun, May 24, 2015 at 06:45:02PM +0300, Roman Bogorodskiy wrote: The libxl tries to check if it's running in dom0 by parsing /proc/xen/capabilities and if that fails it doesn't load. There's no procfs interface in Xen on FreeBSD, so this check always fails. In addition to checking procfs, check if /dev/xen/xenstored, that's enough to check if we're running in dom0 in FreeBSD case. --- src/libxl/libxl_driver.c | 42 ++ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 12be816..fddafa1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -74,6 +74,7 @@ VIR_LOG_INIT(libxl.libxl_driver); #define LIBXL_CONFIG_FORMAT_SEXPR xen-sxpr #define HYPERVISOR_CAPABILITIES /proc/xen/capabilities +#define HYPERVISOR_XENSTORED /dev/xen/xenstored /* Number of Xen scheduler parameters */ #define XEN_SCHED_CREDIT_NPARAM 2 @@ -427,8 +428,6 @@ static bool libxlDriverShouldLoad(bool privileged) { bool ret = false; -int status; -char *output = NULL; /* Don't load if non-root */ if (!privileged) { @@ -436,24 +435,27 @@ libxlDriverShouldLoad(bool privileged) return ret; } -if (!virFileExists(HYPERVISOR_CAPABILITIES)) { -VIR_INFO(Disabling driver as HYPERVISOR_CAPABILITIES - does not exist); -return ret; -} -/* - * Don't load if not running on a Xen control domain (dom0). It is not - * sufficient to check for the file to exist as any guest can mount - * xenfs to /proc/xen. - */ -status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, output); -if (status = 0) -status = strncmp(output, control_d, 9); -VIR_FREE(output); -if (status) { -VIR_INFO(No Xen capabilities detected, probably not running - in a Xen Dom0. Disabling libxenlight driver); - +if (virFileExists(HYPERVISOR_CAPABILITIES)) { +int status; +char *output = NULL; +/* + * Don't load if not running on a Xen control domain (dom0). It is not + * sufficient to check for the file to exist as any guest can mount + * xenfs to /proc/xen. + */ +status = virFileReadAll(HYPERVISOR_CAPABILITIES, 10, output); +if (status = 0) +status = strncmp(output, control_d, 9); +VIR_FREE(output); +if (status) { +VIR_INFO(No Xen capabilities detected, probably not running + in a Xen Dom0. Disabling libxenlight driver); + +return ret; +} +} else if (!virFileExists(HYPERVISOR_XENSTORED)) { +VIR_INFO(Disabling driver as neither HYPERVISOR_CAPABILITIES + nor HYPERVISOR_CAPABILITIES exist); s/HYPERVISOR_CAPABILITIES/HYPERVISOR_XENSTORED/ ACK with that changed. For the record, I tested this on Linux. Looks good with Martin's comment addressed. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Entering freeze for libvirt 1.2.16
I'm one day late but I have tagged the release candidate 1 in git and pushed signed tarballs and rpms to the usual place: ftp://libvirt.org/libvirt/ we have a bit less than 160 commits so far since 1.2.15. I tried it with my usual limited testing and it looks fine, https://ci.centos.org/ seems fine when it comes to libvirt, but please give it a try so we can get potential bugs or portability issues out. I will probably make an rc2 on Friday and push the final release next Monday or Tuesday, thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: improve the address check for dimm type
On Wed, May 27, 2015 at 14:39:58 +0800, Luyao Huang wrote: When hot-plug/cold-plug a memory device, we use memcmp() function to check if there is a memory device have the same address with the memory device we want hot-pluged. But qemu forbid use/hot-plug 2 memory device with same slot *or* the same base(qemu side this elemnt named addr). Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e57425..413f839 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3089,7 +3089,10 @@ virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a, break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM: -if (memcmp(a-addr.dimm, b-addr.dimm, sizeof(a-addr.dimm))) +if (a-addr.dimm.slot != b-addr.dimm.slot +(a-addr.dimm.base == 0 || + b-addr.dimm.base == 0 || + a-addr.dimm.base != b-addr.dimm.base)) return false; This function is designed to check if the address is equal not if it is not conflicting for a particular hypervisor. If you are going to enforce that both the address and base are different, this function is not the right place. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: improve the address check for dimm type
When hot-plug/cold-plug a memory device, we use memcmp() function to check if there is a memory device have the same address with the memory device we want hot-pluged. But qemu forbid use/hot-plug 2 memory device with same slot *or* the same base(qemu side this elemnt named addr). Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e57425..413f839 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3089,7 +3089,10 @@ virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a, break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM: -if (memcmp(a-addr.dimm, b-addr.dimm, sizeof(a-addr.dimm))) +if (a-addr.dimm.slot != b-addr.dimm.slot +(a-addr.dimm.base == 0 || + b-addr.dimm.base == 0 || + a-addr.dimm.base != b-addr.dimm.base)) return false; break; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: improve the address check for dimm type
On 05/27/2015 03:03 PM, Peter Krempa wrote: On Wed, May 27, 2015 at 14:39:58 +0800, Luyao Huang wrote: When hot-plug/cold-plug a memory device, we use memcmp() function to check if there is a memory device have the same address with the memory device we want hot-pluged. But qemu forbid use/hot-plug 2 memory device with same slot *or* the same base(qemu side this elemnt named addr). Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e57425..413f839 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3089,7 +3089,10 @@ virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a, break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM: -if (memcmp(a-addr.dimm, b-addr.dimm, sizeof(a-addr.dimm))) +if (a-addr.dimm.slot != b-addr.dimm.slot +(a-addr.dimm.base == 0 || + b-addr.dimm.base == 0 || + a-addr.dimm.base != b-addr.dimm.base)) return false; This function is designed to check if the address is equal not if it is not conflicting for a particular hypervisor. If you are going to enforce that both the address and base are different, this function is not the right place. Okay, reasonable to me, i will found a place for the dimm address check in src/qemu/* Thanks you advise and quick review. Peter Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Fix compilation error when enum variable size differs from 'int'
On Tue, May 26, 2015 at 09:04:09 -0600, Eric Blake wrote: On 05/26/2015 02:31 AM, Peter Krempa wrote: Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value via a pointer rather than in the return value. The change triggered problems with platforms where the compiler decides to use a data type of size different than integer at the point where we typecast it. Work around the issue by using an intermediate variable of the correct type that gets casted back by the default typecasting rules. --- Version 2: - removed block from the case statement - added ignore value section to silence coverity and initialized the temp variable src/qemu/qemu_driver.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) ACK Pushed; Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: improve the sysinfo element XML format
On 05/27/2015 07:59 AM, John Ferlan wrote: On 05/22/2015 05:26 AM, Luyao Huang wrote: When set sysinfo element without sub-elements, libvirt will format it as: sysinfo type='smbios' /sysinfo After improve the format: sysinfo type='smbios'/ Signed-off-by: Luyao Huang lhu...@redhat.com --- src/util/virsysinfo.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) ACK - I'll slightly adjust commit message before pushing Thanks for your quick review John Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: do not format redirfilter element if it do not have sub-element
On 05/27/2015 08:00 AM, John Ferlan wrote: On 05/22/2015 05:26 AM, Luyao Huang wrote: When set a redirfilter element without sub-element, libvirt will format it like this: redirfilter /redirfilter Just drop this element if it do not have any sub-element. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 4 1 file changed, 4 insertions(+) ACK - I'll slightly adjust commit message before pushing Thank you John :) John Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list