October 2014 - Devel - libvirt List Archives

[libvirt] [PATCH] remote: fix jump depends on uninitialised value
by Jincheng Miao 29 Oct '14

29 Oct '14

Currently remote driver only initializes partial fields of remote_connect_get_all_domain_stats_args. But xdr_array() will check the uninitialised field 'doms_val'. For safty reason, memset all fields of args is better. Fix the following error from valgrind, like: ==30515== 1 errors in context 1 of 3: ==30515== Conditional jump or move depends on uninitialised value(s) ==30515== at 0x85E9402: xdr_array (xdr_array.c:88) ==30515== by 0x4FD8FC9: xdr_remote_connect_get_all_domain_stats_args (remote_protocol.c:6473) ==30515== by 0x4FE72F2: virNetMessageEncodePayload (virnetmessage.c:350) ==30515== by 0x4FDD21C: virNetClientProgramCall (virnetclientprogram.c:326) ==30515== by 0x4FB4D01: callFull.isra.2 (remote_driver.c:6667) ==30515== by 0x4FCBD45: call (remote_driver.c:6689) ==30515== by 0x4FCBD45: remoteConnectGetAllDomainStats (remote_driver.c:7793) ==30515== by 0x4FA0E75: virConnectGetAllDomainStats (libvirt.c:21678) ==30515== by 0x147FD1: cmdDomstats (virsh-domain-monitor.c:2148) ==30515== by 0x13006B: vshCommandRun (virsh.c:1915) ==30515== by 0x12A9E1: main (virsh.c:3699) Signed-off-by: Jincheng Miao <jmiao(a)redhat.com> --- src/remote/remote_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 067f2d0..b8d90ed 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7779,6 +7779,8 @@ remoteConnectGetAllDomainStats(virConnectPtr conn, virDomainStatsRecordPtr elem = NULL; virDomainStatsRecordPtr *tmpret = NULL; + memset(&args, 0, sizeof(args)); + if (ndoms) { if (VIR_ALLOC_N(args.doms.doms_val, ndoms) < 0) goto cleanup; -- 1.7.1

2 1

[libvirt] [PATCH] virsh: Improve the job type reported of virsh cmd blockcommit
by Shanzhi Yu 29 Oct '14

29 Oct '14

When begin a blockcommit job(active) with virsh, it will report "Block Commit started"(it really donesn't matter much), but for more preciese it should report "Active Block Commit started" Signed-off-by: Shanzhi Yu <shyu(a)redhat.com> --- tools/virsh-domain.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 94ae3d3..3416e31 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1723,9 +1723,15 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (!blocking) { - vshPrint(ctl, "%s", _("Block Commit started")); - ret = true; - goto cleanup; + if (active) { + vshPrint(ctl, "%s", _("Active Block Commit started")); + ret = true; + goto cleanup; + } else { + vshPrint(ctl, "%s", _("Block Commit started")); + ret = true; + goto cleanup; + } } while (blocking) { -- 1.9.3

2 1

[libvirt] [PATCH 0/2] v1.1 bugfix: support dhcp network interfaces
by Gene Czarcinski 29 Oct '14

29 Oct '14

v1.1 adds some documentation changes. Support for a network such as -N dhcp,source=default was not working in that dhclient was not being started. Although I am not sure what the real problem is, the solution is to use g_spawn_sync() instead of g_spawn_async() to start /sbin/dhclient. The second patch addes "-v" to the dhclient arguments to improve debugging info. The dhclient into will be in /var/log/messages the Secure Contrainer host system and not in the container itself. Gene Czarcinski (2): v1.1 for dhclient use g_spawn_sync() v1.1 add -v to dhclient parameter arguments libvirt-sandbox/libvirt-sandbox-init-common.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) -- 1.9.3

3 5

[libvirt] [PATCH] Renamed internal __mon_yday into __vir_mon_yday to avoid conflicts
by Cédric Bosdonnat 29 Oct '14

29 Oct '14

libc has another constant with the same name, which leads to redefinition error when building against static libvirt --- src/util/virtime.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virtime.c b/src/util/virtime.c index acbec41..7b3ec44 100644 --- a/src/util/virtime.c +++ b/src/util/virtime.c @@ -105,7 +105,7 @@ int virTimeFieldsNowRaw(struct tm *fields) #define DIV(a, b) ((a) / (b) - ((a) % (b) < 0)) #define LEAPS_THRU_END_OF(y) (DIV (y, 4) - DIV (y, 100) + DIV (y, 400)) -const unsigned short int __mon_yday[2][13] = { +const unsigned short int __vir_mon_yday[2][13] = { /* Normal years. */ { 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 }, /* Leap years. */ @@ -160,7 +160,7 @@ void virTimeFieldsThen(unsigned long long when, struct tm *fields) fields->tm_year = y - 1900; fields->tm_yday = days; - ip = __mon_yday[is_leap_year(y)]; + ip = __vir_mon_yday[is_leap_year(y)]; for (y = 11; days < (long int) ip[y]; --y) continue; days -= ip[y]; -- 1.8.4.5

3 2

[libvirt] [PATCH] qemu_agent: Produce more readable error messages
by Michal Privoznik 29 Oct '14

29 Oct '14

Not every error message from qemu-ga has to have the 'class' field filled out. For instance, I've seen this error message lately: qemuAgentCheckError:1047 : unable to execute QEMU agent command \ {"execute":"guest-set-time"}: \ {"error":{"desc":"Invalid parameter type, expected: integer"}} However, this got translated into rather generic error message: internal error: unable to execute QEMU agent command 'guest-set-time': unknown QEMU command error So we've dropped better error message in favor of a generic one. This is due to our code which expects 'class' which is not present here. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/qemu/qemu_agent.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index fe38f6d..9f673bf 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1009,14 +1009,12 @@ static const char * qemuAgentStringifyError(virJSONValuePtr error) { const char *klass = virJSONValueObjectGetString(error, "class"); - const char *detail = NULL; + const char *detail = virJSONValueObjectGetString(error, "desc"); /* The QMP 'desc' field is usually sufficient for our generic - * error reporting needs. + * error reporting needs. However, if not present, translate + * the class into something readable. */ - if (klass) - detail = virJSONValueObjectGetString(error, "desc"); - if (!detail) detail = qemuAgentStringifyErrorClass(klass); -- 2.0.4

2 2

[libvirt] [PATCH python v2] fix libvirt headers list
by Dmitry Guryanov 29 Oct '14

29 Oct '14

Since libvirt.h has been split out, generator.py should be fixed accordingly. So add full list of header files. Signed-off-by: Dmitry Guryanov <dguryanov(a)parallels.com> --- Changes in v2: * fix list format generator.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/generator.py b/generator.py index 01ab441..c66c6d4 100755 --- a/generator.py +++ b/generator.py @@ -29,6 +29,21 @@ import xml.sax debug = 0 onlyOverrides = False +libvirt_headers = [ + "libvirt", + "libvirt-domain", + "libvirt-domain-snapshot", + "libvirt-event", + "libvirt-host", + "libvirt-interface", + "libvirt-network", + "libvirt-nodedev", + "libvirt-nwfilter", + "libvirt-secret", + "libvirt-storage", + "libvirt-stream", +] + def getparser(): # Attach parser to an unmarshalling object. return both objects. target = docParser() @@ -112,9 +127,7 @@ class docParser(xml.sax.handler.ContentHandler): self.function_return_field = attrs['field'] elif tag == 'enum': # enums come from header files, hence virterror.h - if (attrs['file'] == "libvirt" or - attrs['file'] == "virterror" or - attrs['file'] == "virerror"): + if attrs['file'] in libvirt_headers + ["virerror", "virterror"]: enum(attrs['type'],attrs['name'],attrs['value']) elif attrs['file'] == "libvirt-lxc": lxc_enum(attrs['type'],attrs['name'],attrs['value']) @@ -127,11 +140,8 @@ class docParser(xml.sax.handler.ContentHandler): if tag == 'function': # fuctions come from source files, hence 'virerror.c' if self.function is not None: - if (self.function_module == "libvirt" or - self.function_module == "event" or - self.function_module == "virevent" or - self.function_module == "virerror" or - self.function_module == "virterror"): + if self.function_module in libvirt_headers + \ + ["event", "virevent", "virerror", "virterror"]: function(self.function, self.function_descr, self.function_return, self.function_args, self.function_file, self.function_module, -- 1.9.3

2 1

[libvirt] [PATCH python] fix libvirt headers list
by Dmitry Guryanov 29 Oct '14

29 Oct '14

Since libvirt.h has been splitted out, generator.py should be fixed accordingly. So add full list of header files. Signed-off-by: Dmitry Guryanov <dguryanov(a)parallels.com> --- generator.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/generator.py b/generator.py index 01ab441..5ecbed5 100755 --- a/generator.py +++ b/generator.py @@ -29,6 +29,19 @@ import xml.sax debug = 0 onlyOverrides = False +libvirt_headers = ["libvirt", + "libvirt-domain", + "libvirt-domain-snapshot", + "libvirt-event", + "libvirt-host", + "libvirt-interface", + "libvirt-network", + "libvirt-nodedev", + "libvirt-nwfilter", + "libvirt-secret", + "libvirt-storage", + "libvirt-stream",] + def getparser(): # Attach parser to an unmarshalling object. return both objects. target = docParser() @@ -112,9 +125,7 @@ class docParser(xml.sax.handler.ContentHandler): self.function_return_field = attrs['field'] elif tag == 'enum': # enums come from header files, hence virterror.h - if (attrs['file'] == "libvirt" or - attrs['file'] == "virterror" or - attrs['file'] == "virerror"): + if attrs['file'] in libvirt_headers + ["virerror", "virterror"]: enum(attrs['type'],attrs['name'],attrs['value']) elif attrs['file'] == "libvirt-lxc": lxc_enum(attrs['type'],attrs['name'],attrs['value']) @@ -127,11 +138,8 @@ class docParser(xml.sax.handler.ContentHandler): if tag == 'function': # fuctions come from source files, hence 'virerror.c' if self.function is not None: - if (self.function_module == "libvirt" or - self.function_module == "event" or - self.function_module == "virevent" or - self.function_module == "virerror" or - self.function_module == "virterror"): + if self.function_module in libvirt_headers + \ + ["event", "virevent", "virerror", "virterror"]: function(self.function, self.function_descr, self.function_return, self.function_args, self.function_file, self.function_module, -- 1.9.3

2 3

[libvirt] [PATCH 00/11] RFC: refactor internal driver architecture
by Daniel P. Berrange 29 Oct '14

29 Oct '14

Currently we have a bit of a craz setup for activating drivers for virConnectPtr. We iterate over the primary hypervisor drivers until one matches and activates. This bit is fine. Then we do the same for the network, storage, interface, etc secondary drivers. This is where the fun starts. In the case of the stateless drivers that run outside of libvirtd, they always want the secondary drivers to match the primary driver. Unfortunately if they don't register any dummy secondary driver they'll get given the reomte driver instead. The probing of secondary drivers is really unhelpful here. In the case of stateful drivers that run inside of libvirtd, there is (almost) always only one driver instance that they want to work with. So at best the probing of secondary drivers is a waste of time. With the increasing feature set though we're getting tight dependancies from the hypervisor driver to the secondary driver. eg QEMU depends on the specific network driver we have because it calls various internal APIs it exposes. We also have circular dependancy problems during libvirtd startup: https://www.redhat.com/archives/libvir-list/2014-January/msg01143.html There the storage driver needs a virConnectPtr instance in order to talk to the secret driver. At the same time the storage driver must run before the hypervisor driver because the hypervisor driver needs to use it for autostart of VMs. This is a horrible mess. The solution I think is to remove the concept of probing for secondary drivers entirely. Instead I suggest creating a struct virDriver { ... } which contains a pointer to virHypervisorDriver, virNetworkDriver, virInterfaceDriver, etc. now virConnectPtr will only reference the single virDriverPtr and when we open a hypervisor driver, we immediately get a full set of APIs for everything. This way all the hypervisor drivers will always know exactly what secondary driver they are working with. We'll be able to remove all the xxxPrivateData elements from the virConnectPtr. The stateless drivers can just access the main privateData. The stateful drivers can avoid any use of privateData at all - just access their global static variable with the driverState. This won't quite solve the circular dependancy problem in that mail thread above. By removing the directly reliance on the virConnectPtr driver table though, we'll be able to update the code more easily so it can fetch secrets directly and thus the storage driver wont need to do virConnectOpen("qemu:///system"). The patches that follow illustrate the first part of cleaning up use of the xxxPrivateData fields. This isn't complete but I wanted to show the direction it would go before I spend the time todo the full job. Incidentally our code is buggy as hell when it comes to using the xxxPrivateData fields. The PHyp driver actually abuses the networkPrivateData field for all its APIs. Alot of code wil directly access the global state driver state already ignoring the privateData fields already. So they are really more pain than they are worth and leading to buggy code. Daniel P. Berrange (11): Clean up remote driver connection open code Update remote driver to always use privateData Update ESX driver to always use privateData Update Hyper-V driver to always use privateData Move phyp internal info out of the header file Remove abuse of networkPrivateData in phyp driver Update Parallels driver to always use privateData Update Test driver to always use privateData Remove use of storagePrivateData from storage driver Remove use of networkPrivateData from network driver Remove use of networkPrivateData from netcf driver src/esx/esx_device_monitor.c | 6 +- src/esx/esx_interface_driver.c | 16 +- src/esx/esx_network_driver.c | 20 +- src/esx/esx_nwfilter_driver.c | 6 +- src/esx/esx_secret_driver.c | 6 +- src/esx/esx_storage_backend_iscsi.c | 26 +- src/esx/esx_storage_backend_vmfs.c | 36 +-- src/esx/esx_storage_driver.c | 44 ++-- src/hyperv/hyperv_device_monitor.c | 6 +- src/hyperv/hyperv_interface_driver.c | 6 +- src/hyperv/hyperv_network_driver.c | 6 +- src/hyperv/hyperv_nwfilter_driver.c | 6 +- src/hyperv/hyperv_secret_driver.c | 6 +- src/hyperv/hyperv_storage_driver.c | 6 +- src/interface/interface_backend_netcf.c | 71 ++---- src/network/bridge_driver.c | 435 ++++++++++++++------------------ src/parallels/parallels_network.c | 2 - src/parallels/parallels_storage.c | 12 +- src/parallels/parallels_utils.h | 1 + src/phyp/phyp_driver.c | 225 ++++++++--------- src/phyp/phyp_driver.h | 53 ---- src/remote/remote_driver.c | 153 +++-------- src/rpc/gendispatch.pl | 28 +- src/storage/storage_driver.c | 194 +++++++------- src/test/test_driver.c | 24 +- 25 files changed, 537 insertions(+), 857 deletions(-) -- 2.1.0

3 24

[libvirt] [PATCHv5 0/6] Add non-FreeBSD guest support to Bhyve driver.
by Conrad Meyer 29 Oct '14

29 Oct '14

Drvbhyve hardcodes bhyveload(8) as the host bootloader for guests. bhyveload(8) loader only supports FreeBSD guests. This patch series adds <bootloader> and <bootloader_args> handling to bhyve_command, so libvirt can boot non-FreeBSD guests in Bhyve. Additionally, support for grub-bhyve(1)'s --cons-dev argument is added so that interactive GRUB menus can be manipulated with the domain-configured serial device. See patch logs for further details. Thanks, Conrad Changelog for v5: - Added loader arguments tests to the bhyvexml2argvtest harness - Dropped po/pot file changes per Daniel - Added bootloader, bootloader_args to domain XML schema - Added xml2argv tests for new bhyve bootloader handling - Added xml2argv test for grub with console device Conrad Meyer (6): bhyve: Support /domain/bootloader configuration for non-FreeBSD guests. bhyvexml2argv: Add loader argv tests. domaincommon.rng: Add bootloader, bootloader_arg to os=hvm schema for Bhyve bhyvexml2argv: Add tests for domain-configured bootloader, args bhyve: Add console support for grub-bhyve bootloader bhyvexml2argv: Add test for grub console support docs/drvbhyve.html.in | 94 ++++++++- docs/formatdomain.html.in | 4 +- docs/schemas/domaincommon.rng | 41 ++-- src/bhyve/bhyve_command.c | 215 +++++++++++++++++++-- src/bhyve/bhyve_command.h | 5 +- src/bhyve/bhyve_domain.c | 5 + src/bhyve/bhyve_domain.h | 1 + src/bhyve/bhyve_driver.c | 2 +- src/bhyve/bhyve_process.c | 13 +- .../bhyvexml2argv-acpiapic.ldargs | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs | 1 + .../bhyvexml2argv-bhyveload-explicitargs.args | 3 + .../bhyvexml2argv-bhyveload-explicitargs.ldargs | 1 + .../bhyvexml2argv-bhyveload-explicitargs.xml | 23 +++ .../bhyvexml2argvdata/bhyvexml2argv-console.ldargs | 1 + .../bhyvexml2argv-custom-loader.args | 3 + .../bhyvexml2argv-custom-loader.ldargs | 1 + .../bhyvexml2argv-custom-loader.xml | 24 +++ .../bhyvexml2argv-disk-cdrom-grub.args | 3 + .../bhyvexml2argv-disk-cdrom-grub.ldargs | 2 + .../bhyvexml2argv-disk-cdrom-grub.xml | 23 +++ .../bhyvexml2argv-disk-cdrom.ldargs | 1 + .../bhyvexml2argv-disk-virtio.ldargs | 1 + .../bhyvexml2argv-grub-defaults.args | 3 + .../bhyvexml2argv-grub-defaults.ldargs | 2 + .../bhyvexml2argv-grub-defaults.xml | 23 +++ .../bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs | 1 + .../bhyvexml2argv-serial-grub.args | 4 + .../bhyvexml2argv-serial-grub.ldargs | 2 + .../bhyvexml2argv-serial-grub.xml | 26 +++ .../bhyvexml2argvdata/bhyvexml2argv-serial.ldargs | 1 + tests/bhyvexml2argvtest.c | 41 +++- 32 files changed, 527 insertions(+), 44 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-bhyveload-explicitargs.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-custom-loader.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom-grub.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-cdrom.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-grub-defaults.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial-grub.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial.ldargs -- 1.9.3

3 22

[libvirt] [PATCH] uitl: use "if and only if" instead of "iff" in some comments
by Shanzhi Yu 29 Oct '14

29 Oct '14

>From wiki "In writing, phrases commonly used, with debatable propriety. In logic formulae, logical symbols are used instead of these phrases" In some comments, we use "if and only if", so instead the logical sysbols with phrases in case someone mistaking it as a wrong spelling of "if" Signed-off-by: Shanzhi Yu <shyu(a)redhat.com> --- src/util/virhash.c | 2 +- src/util/virstring.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index e3c1880..e3bf1d3 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -65,7 +65,7 @@ struct _virHashTable { uint32_t seed; size_t size; size_t nbElems; - /* True iff we are iterating over hash entries. */ + /* True if and only if we are iterating over hash entries. */ bool iterating; /* Pointer to the current entry during iteration. */ virHashEntryPtr current; diff --git a/src/util/virstring.c b/src/util/virstring.c index 3dad9dd..974005d 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -519,7 +519,7 @@ virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) return NULL; ret = strncpy(dest, src, n); - /* strncpy NULL terminates iff the last character is \0. Therefore + /* strncpy NULL terminates if and only if the last character is \0. Therefore * force the last byte to be \0 */ dest[n] = '\0'; -- 1.9.3

2 2