Re: [libvirt] [PATCH v2 1/4] Introduce virDomainFSTrim() public API
On 29.11.2012 00:41, Eric Blake wrote: This will call FITRIM within guest. The API has 4 arguments, however, only 2 will be used for now (@dom and @minumum). The rest two are there if in future qemu guest agent learns them. s/in future/in the future/ +/** + * virDomainFSTrim: + * @dom: a domain object + * @mountPoint: which mount point trim s/trim/to trim/ + * @minimum: Minimum contiguous free range to discard in bytes + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Calls FITRIM within the guest (hence guest agent may be + * required depending on hypervisor used). Either call it on each + * mounted filesystem (@mountPoint is NULL) or just on specified + * @mountPoint. @minimum tell that free ranges smaller than this s/tell/hints/ + * may be ignored (this is a hint and the guest may not respect + * it). By increasing this value, the fstrim operation will + * complete more quickly for filesystems with badly fragmented + * free space, although not all blocks will be discarded. Maybe mention that the command may fail if @minimum is not 0. Right. But since I've already pushed the patches I am pushing a separate commit to fix those nits under trivial rule: commit 5049b53689128701e5bb72fa578467ac34071be9 Author: Michal Privoznik mpriv...@redhat.com AuthorDate: Thu Nov 29 09:30:58 2012 +0100 Commit: Michal Privoznik mpriv...@redhat.com CommitDate: Thu Nov 29 09:30:58 2012 +0100 libvirt.c: Fix wording and grammar in virDomainFSTrim The documentation to this API has some defects from grammar and wording POV. These were raised after I've pushed the patches, so they are in a separate commit. diff --git a/src/libvirt.c b/src/libvirt.c index b1854e3..4581394 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20229,18 +20229,19 @@ error: /** * virDomainFSTrim: * @dom: a domain object - * @mountPoint: which mount point trim + * @mountPoint: which mount point to trim * @minimum: Minimum contiguous free range to discard in bytes * @flags: extra flags, not used yet, so callers should always pass 0 * * Calls FITRIM within the guest (hence guest agent may be * required depending on hypervisor used). Either call it on each * mounted filesystem (@mountPoint is NULL) or just on specified - * @mountPoint. @minimum tell that free ranges smaller than this + * @mountPoint. @minimum hints that free ranges smaller than this * may be ignored (this is a hint and the guest may not respect * it). By increasing this value, the fstrim operation will * complete more quickly for filesystems with badly fragmented * free space, although not all blocks will be discarded. + * If @minimum is not zero, the command may fail. * * Returns 0 on success, -1 otherwise. */ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [test-API][PATCH v2] Add test case of set vcpus with flags
On 11/28/2012 08:10 PM, Wayne Sun wrote:
v2: break down the case to small cases with separate flags
* Use setVcpusFlags API to set domain vcpus with flags
* 3 cases added, each only deal with one set flag value as in
config, live or maximum
* cases are independent on domain states, API will report error
if not suitable for certain states
* the sample conf is only one scenario of hotplug domain vcpus
Signed-off-by: Wayne Sun g...@redhat.com
---
cases/set_vcpus_flags.conf | 64 +++
repos/setVcpus/set_vcpus_config.py | 69 +
repos/setVcpus/set_vcpus_live.py| 96 +++
repos/setVcpus/set_vcpus_maximum.py | 62 ++
4 files changed, 291 insertions(+), 0 deletions(-)
create mode 100644 cases/set_vcpus_flags.conf
create mode 100644 repos/setVcpus/__init__.py
create mode 100644 repos/setVcpus/set_vcpus_config.py
create mode 100644 repos/setVcpus/set_vcpus_live.py
create mode 100644 repos/setVcpus/set_vcpus_maximum.py
diff --git a/cases/set_vcpus_flags.conf b/cases/set_vcpus_flags.conf
new file mode 100644
index 000..d346735
--- /dev/null
+++ b/cases/set_vcpus_flags.conf
@@ -0,0 +1,64 @@
+domain:install_linux_cdrom
+guestname
+$defaultname
+guestos
+$defaultos
+guestarch
+$defaultarch
+vcpu
+$defaultvcpu
+memory
+$defaultmem
+hddriver
+$defaulthd
+nicdriver
+$defaultnic
+imageformat
+qcow2
+
+
+domain:destroy
+guestname
+$defaultname
+
+setVcpus:set_vcpus_maximum
+guestname
+$defaultname
+vcpu
+4
+
+setVcpus:set_vcpus_config
+guestname
+$defaultname
+vcpu
+1
It's better to merge set_vcpus_maximum and set_vcpus_config
into one testcase
setVcpus:set_vcpus_config
guestname
$defaultname
maxvcpu(optional)
4
vcpu(or optional)
1
+
+domain:start
+guestname
+$defaultname
+
+setVcpus:set_vcpus_live
+guestname
+$defaultname
+vcpu
+3
+username
+$username
+password
+$password
+
+setVcpus:set_vcpus_config
+guestname
+$defaultname
+vcpu
+2
+
+domain:destroy
+guestname
+$defaultname
+
+domain:undefine
+guestname
+$defaultname
+
+options cleanup=enable
diff --git a/repos/setVcpus/__init__.py b/repos/setVcpus/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/setVcpus/set_vcpus_config.py
b/repos/setVcpus/set_vcpus_config.py
new file mode 100644
index 000..2b8f5e7
--- /dev/null
+++ b/repos/setVcpus/set_vcpus_config.py
@@ -0,0 +1,69 @@
+#!/usr/bin/env python
+# Test set domain vcpu with flag VIR_DOMAIN_AFFECT_CONFIG. Check
+# domain config xml to get 'current' vcpu number.
+
+from xml.dom import minidom
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+
+required_params = ('guestname', 'vcpu', )
+optional_params = {}
+
+def get_current_vcpu(domobj):
+dump domain config xml description to get current vcpu number
+
+try:
+guestxml = domobj.XMLDesc(2)
+logger.debug(domain %s xml is :\n%s %(domobj.name(), guestxml))
+xml = minidom.parseString(guestxml)
+vcpu = xml.getElementsByTagName('vcpu')[0]
+
+if vcpu.hasAttribute('current'):
+attr = vcpu.getAttributeNode('current')
+current = int(attr.nodeValue)
+else:
+logger.info(no 'current' atrribute for element vcpu)
+current = int(vcpu.childNodes[0].data)
+
+logger.info(domain current vcpu number is: %s % current)
+
+except libvirtError, e:
+logger.error(libvirt call failed: + str(e))
+return False
+
+return current
+
+def set_vcpus_config(params):
+set domain vcpu with config flag and check
+
+global logger
+logger = params['logger']
+params.pop('logger')
+guestname = params['guestname']
+vcpu = int(params['vcpu'])
+
+logger.info(the name of virtual machine is %s % guestname)
+logger.info(the given vcpu number is %s % vcpu)
+
+conn = sharedmod.libvirtobj['conn']
+
+try:
+domobj = conn.lookupByName(guestname)
+logger.info(set domain vcpu as %s with flag: %s %
+(vcpu, libvirt.VIR_DOMAIN_AFFECT_CONFIG))
+domobj.setVcpusFlags(vcpu, libvirt.VIR_DOMAIN_AFFECT_CONFIG)
+except libvirtError, e:
+logger.error(libvirt call failed: + str(e))
+return 1
+
+logger.info(check domain config xml to get current vcpu)
+ret = get_current_vcpu(domobj)
+if ret == vcpu:
+logger.info(domain current vcpu is equal as set)
+return 0
+else:
+logger.error(domain current vcpu is not equal as set)
+return 1
diff --git a/repos/setVcpus/set_vcpus_live.py
Re: [libvirt] [PATCH 0/3] qemu: QMP Capability Probing Fixes
On 11/28/2012 03:58 PM, Daniel P. Berrange wrote: Viktor Mihajlovski (3): qemu: Wait for monitor socket even without pid qemu: Fix QMP Capabability Probing Failure qemu: Add QEMU version computation to QMP probing I pushed this series now. Daniel Thx -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [test-API][PATCH v3] Add test case of set vcpus with flags
v2: break down the case to small cases with separate flags
* Use setVcpusFlags API to set domain vcpus with flags
* 3 cases added, each only deal with one set flag value as in
config, live or maximum
* cases are independent on domain states, API will report error
if not suitable for certain states
* the sample conf is only one scenario of hotplug domain vcpus
v3: merge config and maximum case to config
* maximum flag can only work when domain is shutoff, merge it
to config case to simplify code
Signed-off-by: Wayne Sun g...@redhat.com
---
cases/set_vcpus_flags.conf | 67 +
repos/setVcpus/set_vcpus_config.py | 93 ++
repos/setVcpus/set_vcpus_live.py | 96
3 files changed, 256 insertions(+), 0 deletions(-)
create mode 100644 cases/set_vcpus_flags.conf
create mode 100644 repos/setVcpus/__init__.py
create mode 100644 repos/setVcpus/set_vcpus_config.py
create mode 100644 repos/setVcpus/set_vcpus_live.py
diff --git a/cases/set_vcpus_flags.conf b/cases/set_vcpus_flags.conf
new file mode 100644
index 000..6cf595f
--- /dev/null
+++ b/cases/set_vcpus_flags.conf
@@ -0,0 +1,67 @@
+domain:install_linux_cdrom
+guestname
+$defaultname
+guestos
+$defaultos
+guestarch
+$defaultarch
+vcpu
+$defaultvcpu
+memory
+$defaultmem
+hddriver
+$defaulthd
+nicdriver
+$defaultnic
+imageformat
+qcow2
+
+domain:destroy
+guestname
+$defaultname
+
+setVcpus:set_vcpus_config
+guestname
+$defaultname
+vcpu
+1
+maxvcpu
+8
+
+domain:start
+guestname
+$defaultname
+
+setVcpus:set_vcpus_live
+guestname
+$defaultname
+vcpu
+3
+username
+$username
+password
+$password
+
+setVcpus:set_vcpus_config
+guestname
+$defaultname
+vcpu
+5
+
+domain:destroy
+guestname
+$defaultname
+
+domain:start
+guestname
+$defaultname
+
+domain:destroy
+guestname
+$defaultname
+
+domain:undefine
+guestname
+$defaultname
+
+options cleanup=enable
diff --git a/repos/setVcpus/__init__.py b/repos/setVcpus/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/setVcpus/set_vcpus_config.py
b/repos/setVcpus/set_vcpus_config.py
new file mode 100644
index 000..08eb53f
--- /dev/null
+++ b/repos/setVcpus/set_vcpus_config.py
@@ -0,0 +1,93 @@
+#!/usr/bin/env python
+# Test set domain vcpu with flag VIR_DOMAIN_AFFECT_CONFIG, also set
+# and check max vcpu with flag VIR_DOMAIN_VCPU_MAXIMUM if maxvcpu
+# param is given
+
+from xml.dom import minidom
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+
+required_params = ('guestname', 'vcpu', )
+optional_params = {'maxvcpu': 8,
+ }
+
+def get_vcpu_number(domobj):
+dump domain config xml description to get vcpu number, return
+ current vcpu and maximum vcpu number
+
+try:
+guestxml = domobj.XMLDesc(2)
+logger.debug(domain %s xml is :\n%s %(domobj.name(), guestxml))
+xml = minidom.parseString(guestxml)
+vcpu = xml.getElementsByTagName('vcpu')[0]
+maxvcpu = int(vcpu.childNodes[0].data)
+logger.info(domain max vcpu number is: %s % maxvcpu)
+
+if vcpu.hasAttribute('current'):
+attr = vcpu.getAttributeNode('current')
+current = int(attr.nodeValue)
+else:
+logger.info(no 'current' atrribute for element vcpu)
+current = int(vcpu.childNodes[0].data)
+
+logger.info(domain current vcpu number is: %s % current)
+
+except libvirtError, e:
+logger.error(libvirt call failed: + str(e))
+return False
+
+return current, maxvcpu
+
+def set_vcpus_config(params):
+set domain vcpu with config flag and check, also set and check
+ max vcpu with maximum flag if optional param maxvcpu is given
+
+global logger
+logger = params['logger']
+params.pop('logger')
+guestname = params['guestname']
+vcpu = int(params['vcpu'])
+maxvcpu = params.get('maxvcpu', None)
+
+logger.info(the name of virtual machine is %s % guestname)
+logger.info(the given vcpu number is %s % vcpu)
+
+conn = sharedmod.libvirtobj['conn']
+
+try:
+domobj = conn.lookupByName(guestname)
+logger.info(set domain vcpu as %s with flag: %s %
+(vcpu, libvirt.VIR_DOMAIN_AFFECT_CONFIG))
+domobj.setVcpusFlags(vcpu, libvirt.VIR_DOMAIN_AFFECT_CONFIG)
+logger.info(set domain vcpu succeed)
+
+if maxvcpu:
+logger.info(the given max vcpu number is %s % maxvcpu)
+logger.info(set domain maximum vcpu as %s with flag: %s %
+(maxvcpu, libvirt.VIR_DOMAIN_VCPU_MAXIMUM))
+domobj.setVcpusFlags(int(maxvcpu),
Re: [libvirt] [test-API][PATCH v3] Add test case of set vcpus with flags
On 11/29/2012 07:07 PM, Wayne Sun wrote:
v2: break down the case to small cases with separate flags
* Use setVcpusFlags API to set domain vcpus with flags
* 3 cases added, each only deal with one set flag value as in
config, live or maximum
* cases are independent on domain states, API will report error
if not suitable for certain states
* the sample conf is only one scenario of hotplug domain vcpus
v3: merge config and maximum case to config
* maximum flag can only work when domain is shutoff, merge it
to config case to simplify code
Signed-off-by: Wayne Sun g...@redhat.com
---
cases/set_vcpus_flags.conf | 67 +
repos/setVcpus/set_vcpus_config.py | 93 ++
repos/setVcpus/set_vcpus_live.py | 96
3 files changed, 256 insertions(+), 0 deletions(-)
create mode 100644 cases/set_vcpus_flags.conf
create mode 100644 repos/setVcpus/__init__.py
create mode 100644 repos/setVcpus/set_vcpus_config.py
create mode 100644 repos/setVcpus/set_vcpus_live.py
diff --git a/cases/set_vcpus_flags.conf b/cases/set_vcpus_flags.conf
new file mode 100644
index 000..6cf595f
--- /dev/null
+++ b/cases/set_vcpus_flags.conf
@@ -0,0 +1,67 @@
+domain:install_linux_cdrom
+guestname
+$defaultname
+guestos
+$defaultos
+guestarch
+$defaultarch
+vcpu
+$defaultvcpu
+memory
+$defaultmem
+hddriver
+$defaulthd
+nicdriver
+$defaultnic
+imageformat
+qcow2
+
+domain:destroy
+guestname
+$defaultname
+
+setVcpus:set_vcpus_config
+guestname
+$defaultname
+vcpu
+1
+maxvcpu
+8
+
+domain:start
+guestname
+$defaultname
+
+setVcpus:set_vcpus_live
+guestname
+$defaultname
+vcpu
+3
+username
+$username
+password
+$password
+
+setVcpus:set_vcpus_config
+guestname
+$defaultname
+vcpu
+5
+
+domain:destroy
+guestname
+$defaultname
+
+domain:start
+guestname
+$defaultname
+
+domain:destroy
+guestname
+$defaultname
+
+domain:undefine
+guestname
+$defaultname
+
+options cleanup=enable
diff --git a/repos/setVcpus/__init__.py b/repos/setVcpus/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/setVcpus/set_vcpus_config.py
b/repos/setVcpus/set_vcpus_config.py
new file mode 100644
index 000..08eb53f
--- /dev/null
+++ b/repos/setVcpus/set_vcpus_config.py
@@ -0,0 +1,93 @@
+#!/usr/bin/env python
+# Test set domain vcpu with flag VIR_DOMAIN_AFFECT_CONFIG, also set
+# and check max vcpu with flag VIR_DOMAIN_VCPU_MAXIMUM if maxvcpu
+# param is given
+
+from xml.dom import minidom
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+
+required_params = ('guestname', 'vcpu', )
+optional_params = {'maxvcpu': 8,
+ }
+
+def get_vcpu_number(domobj):
+dump domain config xml description to get vcpu number, return
+ current vcpu and maximum vcpu number
+
+try:
+guestxml = domobj.XMLDesc(2)
+logger.debug(domain %s xml is :\n%s %(domobj.name(), guestxml))
+xml = minidom.parseString(guestxml)
+vcpu = xml.getElementsByTagName('vcpu')[0]
+maxvcpu = int(vcpu.childNodes[0].data)
+logger.info(domain max vcpu number is: %s % maxvcpu)
+
+if vcpu.hasAttribute('current'):
+attr = vcpu.getAttributeNode('current')
+current = int(attr.nodeValue)
+else:
+logger.info(no 'current' atrribute for element vcpu)
+current = int(vcpu.childNodes[0].data)
+
+logger.info(domain current vcpu number is: %s % current)
+
+except libvirtError, e:
+logger.error(libvirt call failed: + str(e))
+return False
+
+return current, maxvcpu
+
+def set_vcpus_config(params):
+set domain vcpu with config flag and check, also set and check
+ max vcpu with maximum flag if optional param maxvcpu is given
+
+global logger
+logger = params['logger']
+params.pop('logger')
+guestname = params['guestname']
+vcpu = int(params['vcpu'])
+maxvcpu = params.get('maxvcpu', None)
Either vcpu or maxvcpu could be optional, if both are given, we
set them all.
In your case, only maxvcpu is optional.
Guannan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Misc LXC related fixes
On Tue, Nov 27, 2012 at 12:44:43PM -0500, Eric Blake wrote: - Original Message - The following is a list of random bug fixes I've identified while working on LXC ACK 1 through 8, and 10. Nit in the commit message for 8/10. See my proposal for an alternate fix for 9. This series is pushed with the change you mention Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: Refactor config parameter retrieval
This patch adds macros to help retrieve configuration values from qemu
driver's configuration. Some configuration options are grouped
together in the process.
---
src/qemu/qemu_conf.c | 303 +--
1 file changed, 73 insertions(+), 230 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b7f249d..84859bf 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -153,50 +153,30 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
return -1; \
}
-p = virConfGetValue(conf, vnc_auto_unix_socket);
-CHECK_TYPE(vnc_auto_unix_socket, VIR_CONF_LONG);
-if (p) driver-vncAutoUnixSocket = p-l;
-
-p = virConfGetValue(conf, vnc_tls);
-CHECK_TYPE(vnc_tls, VIR_CONF_LONG);
-if (p) driver-vncTLS = p-l;
-
-p = virConfGetValue(conf, vnc_tls_x509_verify);
-CHECK_TYPE(vnc_tls_x509_verify, VIR_CONF_LONG);
-if (p) driver-vncTLSx509verify = p-l;
-
-p = virConfGetValue(conf, vnc_tls_x509_cert_dir);
-CHECK_TYPE(vnc_tls_x509_cert_dir, VIR_CONF_STRING);
-if (p p-str) {
-VIR_FREE(driver-vncTLSx509certdir);
-if (!(driver-vncTLSx509certdir = strdup(p-str))) {
-virReportOOMError();
-virConfFree(conf);
-return -1;
-}
-}
-
-p = virConfGetValue(conf, vnc_listen);
-CHECK_TYPE(vnc_listen, VIR_CONF_STRING);
-if (p p-str) {
-VIR_FREE(driver-vncListen);
-if (!(driver-vncListen = strdup(p-str))) {
-virReportOOMError();
-virConfFree(conf);
-return -1;
-}
-}
-
-p = virConfGetValue(conf, vnc_password);
-CHECK_TYPE(vnc_password, VIR_CONF_STRING);
-if (p p-str) {
-VIR_FREE(driver-vncPassword);
-if (!(driver-vncPassword = strdup(p-str))) {
-virReportOOMError();
-virConfFree(conf);
-return -1;
-}
-}
+#define GET_VALUE_LONG(NAME, VAR) p = virConfGetValue(conf, NAME); \
+ CHECK_TYPE(NAME, VIR_CONF_LONG); \
+ if (p) VAR = p-l;
+
+#define GET_VALUE_STR(NAME, VAR) p = virConfGetValue(conf, NAME); \
+ CHECK_TYPE(NAME, VIR_CONF_STRING); \
+ if (p p-str) { \
+ VIR_FREE(VAR); \
+ if (!(VAR = strdup(p-str))) { \
+ virReportOOMError(); \
+ virConfFree(conf); \
+ return -1; \
+ } \
+ }
+
+GET_VALUE_LONG(vnc_auto_unix_socket, driver-vncAutoUnixSocket);
+GET_VALUE_LONG(vnc_tls, driver-vncTLS);
+GET_VALUE_LONG(vnc_tls_x509_verify, driver-vncTLSx509verify);
+GET_VALUE_STR(vnc_tls_x509_cert_dir, driver-vncTLSx509certdir);
+GET_VALUE_STR(vnc_listen, driver-vncListen);
+GET_VALUE_STR(vnc_password, driver-vncPassword);
+GET_VALUE_LONG(vnc_sasl, driver-vncSASL);
+GET_VALUE_STR(vnc_sasl_dir, driver-vncSASLdir);
+GET_VALUE_LONG(vnc_allow_host_audio, driver-vncAllowHostAudio);
p = virConfGetValue(conf, security_driver);
if (p p-type == VIR_CONF_LIST) {
@@ -240,104 +220,47 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
}
}
-p = virConfGetValue(conf, security_default_confined);
-CHECK_TYPE(security_default_confined, VIR_CONF_LONG);
-if (p) driver-securityDefaultConfined = p-l;
-
-p = virConfGetValue(conf, security_require_confined);
-CHECK_TYPE(security_require_confined, VIR_CONF_LONG);
-if (p) driver-securityRequireConfined = p-l;
-
-
-p = virConfGetValue(conf, vnc_sasl);
-CHECK_TYPE(vnc_sasl, VIR_CONF_LONG);
-if (p) driver-vncSASL = p-l;
-
-p = virConfGetValue(conf, vnc_sasl_dir);
-CHECK_TYPE(vnc_sasl_dir, VIR_CONF_STRING);
-if (p p-str) {
-VIR_FREE(driver-vncSASLdir);
-if (!(driver-vncSASLdir = strdup(p-str))) {
-virReportOOMError();
-virConfFree(conf);
-return -1;
-}
-}
-
-p = virConfGetValue(conf, spice_tls);
-CHECK_TYPE(spice_tls, VIR_CONF_LONG);
-if (p) driver-spiceTLS = p-l;
+GET_VALUE_LONG(security_default_confined,
driver-securityDefaultConfined);
+GET_VALUE_LONG(security_require_confined,
driver-securityRequireConfined);
-p = virConfGetValue(conf, spice_tls_x509_cert_dir);
-CHECK_TYPE(spice_tls_x509_cert_dir, VIR_CONF_STRING);
-if (p p-str) {
-VIR_FREE(driver-spiceTLSx509certdir);
-if (!(driver-spiceTLSx509certdir = strdup(p-str))) {
-virReportOOMError();
-virConfFree(conf);
-
[libvirt] [PATCH 0/2] Clean up qemu driver configuration parser.
Done while hacking on other series (that is not yet ready). Peter Krempa (2): qemu: Refactor config parameter retrieval qemu: Refactor error reporting in qemu driver configuration parser src/qemu/qemu_conf.c | 479 +++ 1 file changed, 142 insertions(+), 337 deletions(-) -- 1.8.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: Refactor error reporting in qemu driver configuration parser
This patch adds two labels and gets rid of a ton of duplicated code.
This patch also fixes some error message and swtiches most of them to
proper error reporting functions.
---
src/qemu/qemu_conf.c | 194 +--
1 file changed, 78 insertions(+), 116 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 84859bf..a086397 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -74,10 +74,11 @@ void qemuDriverUnlock(virQEMUDriverPtr driver)
int qemuLoadDriverConfig(virQEMUDriverPtr driver,
const char *filename) {
-virConfPtr conf;
+virConfPtr conf = NULL;
virConfValuePtr p;
-char *user;
-char *group;
+char *user = NULL;
+char *group = NULL;
+int ret = -1;
int i;
/* Setup critical defaults */
@@ -86,28 +87,21 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
driver-dynamicOwnership = 1;
driver-clearEmulatorCapabilities = 1;
-if (!(driver-vncListen = strdup(127.0.0.1))) {
-virReportOOMError();
-return -1;
-}
+if (!(driver-vncListen = strdup(127.0.0.1)))
+goto no_memory;
driver-remotePortMin = QEMU_REMOTE_PORT_MIN;
driver-remotePortMax = QEMU_REMOTE_PORT_MAX;
-if (!(driver-vncTLSx509certdir = strdup(SYSCONFDIR /pki/libvirt-vnc))) {
-virReportOOMError();
-return -1;
-}
+if (!(driver-vncTLSx509certdir = strdup(SYSCONFDIR /pki/libvirt-vnc)))
+goto no_memory;
+
+if (!(driver-spiceListen = strdup(127.0.0.1)))
+goto no_memory;
-if (!(driver-spiceListen = strdup(127.0.0.1))) {
-virReportOOMError();
-return -1;
-}
if (!(driver-spiceTLSx509certdir
- = strdup(SYSCONFDIR /pki/libvirt-spice))) {
-virReportOOMError();
-return -1;
-}
+ = strdup(SYSCONFDIR /pki/libvirt-spice)))
+goto no_memory;
#if defined HAVE_MNTENT_H defined HAVE_GETMNTENT_R
/* For privileged driver, try and find hugepage mount automatically.
@@ -118,14 +112,13 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
if (errno != ENOENT) {
virReportSystemError(errno, %s,
_(unable to find hugetlbfs mountpoint));
-return -1;
+goto cleanup;
}
}
#endif
-if (!(driver-lockManager =
- virLockManagerPluginNew(nop, NULL, 0)))
-return -1;
+if (!(driver-lockManager = virLockManagerPluginNew(nop, NULL, 0)))
+goto cleanup;
driver-keepAliveInterval = 5;
driver-keepAliveCount = 5;
@@ -136,21 +129,18 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
*/
if (access(filename, R_OK) == -1) {
VIR_INFO(Could not read qemu config file %s, filename);
-return 0;
-}
-
-conf = virConfReadFile(filename, 0);
-if (!conf) {
-return -1;
+ret = 0;
+goto cleanup;
}
+if (!(conf = virConfReadFile(filename, 0)))
+goto cleanup;
#define CHECK_TYPE(name,typ) if (p p-type != (typ)) { \
virReportError(VIR_ERR_INTERNAL_ERROR, \
%s: %s: expected type #typ, \
filename, (name)); \
-virConfFree(conf); \
-return -1; \
+goto cleanup; \
}
#define GET_VALUE_LONG(NAME, VAR) p = virConfGetValue(conf, NAME); \
@@ -161,11 +151,8 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
CHECK_TYPE(NAME, VIR_CONF_STRING); \
if (p p-str) { \
VIR_FREE(VAR); \
- if (!(VAR = strdup(p-str))) { \
- virReportOOMError(); \
- virConfFree(conf); \
- return -1; \
- } \
+ if (!(VAR = strdup(p-str))) \
+goto no_memory; \
}
GET_VALUE_LONG(vnc_auto_unix_socket, driver-vncAutoUnixSocket);
@@ -186,36 +173,27 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver,
/* Calc length and check items */
for (len = 0, pp = p-list; pp; len++, pp = pp-next) {
if (pp-type != VIR_CONF_STRING) {
-VIR_ERROR(_(security_driver be a list of strings));
-virConfFree(conf);
-return -1;
+virReportError(VIR_ERR_CONF_SYNTAX, %s,
+
[libvirt] [PATCH] Don't fail hard when we can't connect to the monitor
As of 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 we fail to connect to the
monitor instead of getting an exit status != 0 from qemu itself. This
breaks capabilities probing for the non QMP case.
---
src/qemu/qemu_capabilities.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d92947f..d6affb9 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2345,8 +2345,10 @@ qemuCapsInitQMP(qemuCapsPtr caps,
goto cleanup;
}
-if (!(mon = qemuMonitorOpen(NULL, config, true, callbacks)))
+if (!(mon = qemuMonitorOpen(NULL, config, true, callbacks))) {
+ret = 0;
goto cleanup;
+}
qemuMonitorLock(mon);
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/4] Support for sending signals to guest processes
A follow up to https://www.redhat.com/archives/libvir-list/2012-November/msg01283.html Changed in this version - Switch pid to long long to allow process groups 64-bit pids - Add explicit enum entries for RT signals - Do case insensitive parsing in virsh - Fix version numbers - Improve docs -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/4] Implement virDomainSendProcessSignal for LXC driver
From: Daniel P. Berrange berra...@redhat.com
Implement the new API for sending signals to processes in a guest
for the LXC driver. Only support sending signals to the init
process for now, because
- The kernel does not appear to expose the mapping between
container PID numbers and host PID numbers anywhere in the
host OS namespace
- There is no race-free validate whether a host PID corresponds
to a process in a container.
* src/lxc/lxc_driver.c: Allow sending processes signals
---
src/lxc/lxc_driver.c | 73
1 file changed, 73 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 991b593..96d5c3e 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2666,6 +2666,78 @@ cleanup:
return ret;
}
+
+static int
+lxcDomainSendProcessSignal(virDomainPtr dom,
+ long long pid_value,
+ unsigned int signum,
+ unsigned int flags)
+{
+virLXCDriverPtr driver = dom-conn-privateData;
+virDomainObjPtr vm = NULL;
+virLXCDomainObjPrivatePtr priv;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+pid_t victim;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+lxcDriverLock(driver);
+virUUIDFormat(dom-uuid, uuidstr);
+vm = virDomainFindByUUID(driver-domains, dom-uuid);
+lxcDriverUnlock(driver);
+if (!vm) {
+virReportError(VIR_ERR_NO_DOMAIN,
+ _(no domain with matching uuid '%s'), uuidstr);
+goto cleanup;
+}
+priv = vm-privateData;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(domain is not running));
+goto cleanup;
+}
+
+/*
+ * XXX if the kernel has /proc/$PID/ns/pid we can
+ * switch into container namespace that way be
+ * able to kill any PID. Alternatively if there
+ * is a way to find a mapping of guest-host PIDs
+ * we can kill that way.
+ */
+if (pid_value != 1) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+ _(Only the init process may be killed));
+goto cleanup;
+}
+
+if (!priv-initpid) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Init pid is not yet available));
+goto cleanup;
+}
+victim = priv-initpid;
+
+/* We're relying on fact libvirt header signal numbers
+ * are taken from Linux, to avoid mapping
+ */
+if (kill(victim, signum) 0) {
+virReportSystemError(errno,
+ _(Unable to send %d signal to process %d),
+ signum, victim);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+if (vm)
+virDomainObjUnlock(vm);
+return ret;
+}
+
+
static int
lxcListAllDomains(virConnectPtr conn,
virDomainPtr **domains,
@@ -2751,6 +2823,7 @@ static virDriver lxcDriver = {
.nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */
.nodeGetMemoryParameters = nodeGetMemoryParameters, /* 0.10.2 */
.nodeSetMemoryParameters = nodeSetMemoryParameters, /* 0.10.2 */
+.domainSendProcessSignal = lxcDomainSendProcessSignal, /* 1.0.1 */
};
static virStateDriver lxcStateDriver = {
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Remove full stop from error messages
---
Pushed as trivial.
src/qemu/qemu_agent.c | 2 +-
src/qemu/qemu_monitor.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 7062d53..893f7f2 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -242,7 +242,7 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool
*inProgress)
if (ret != 0) {
virReportSystemError(errno, %s,
- _(monitor socket did not show up.));
+ _(monitor socket did not show up));
goto error;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fe8424f..aef5044 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -297,7 +297,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
if (ret != 0) {
virReportSystemError(errno, %s,
- _(monitor socket did not show up.));
+ _(monitor socket did not show up));
goto error;
}
--
1.8.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] Add virDomainSendProcessSignal API
From: Daniel P. Berrange berra...@redhat.com
Add an API for sending signals to arbitrary processes in the
guest OS. This is primarily useful for container based virt,
but can be used for machine virt too, if there is a suitable
guest agent,
* include/libvirt/libvirt.h.in: Add virDomainSendProcessSignal
and virDomainProcessSignal enum
* src/driver.h: Driver entry point
* src/libvirt.c, src/libvirt_public.syms: Impl for new API
---
include/libvirt/libvirt.h.in | 97
src/driver.h | 7
src/libvirt.c| 84 ++
src/libvirt_public.syms | 1 +
4 files changed, 189 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index bd03315..0f6ff4c 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2989,6 +2989,103 @@ int virDomainSendKey(virDomainPtr domain,
unsigned int flags);
/*
+ * These just happen to match Linux signal numbers. The numbers
+ * will be mapped to whatever the SIGNUM is in the guest OS in
+ * question by the agent delivering the signal. The names are
+ * based on the POSIX / XSI signal standard though.
+ *
+ * Do not rely on all values matching Linux though. It is possible
+ * this enum might be extended with new signals which have no
+ * mapping in Linux.
+ */
+typedef enum {
+VIR_DOMAIN_PROCESS_SIGNAL_NOP= 0, /* No constant in POSIX/Linux */
+VIR_DOMAIN_PROCESS_SIGNAL_HUP= 1, /* SIGHUP */
+VIR_DOMAIN_PROCESS_SIGNAL_INT= 2, /* SIGINT */
+VIR_DOMAIN_PROCESS_SIGNAL_QUIT = 3, /* SIGQUIT */
+VIR_DOMAIN_PROCESS_SIGNAL_ILL= 4, /* SIGILL */
+VIR_DOMAIN_PROCESS_SIGNAL_TRAP = 5, /* SIGTRAP */
+VIR_DOMAIN_PROCESS_SIGNAL_ABRT = 6, /* SIGABRT */
+VIR_DOMAIN_PROCESS_SIGNAL_BUS= 7, /* SIGBUS */
+VIR_DOMAIN_PROCESS_SIGNAL_FPE= 8, /* SIGFPE */
+VIR_DOMAIN_PROCESS_SIGNAL_KILL = 9, /* SIGKILL */
+
+VIR_DOMAIN_PROCESS_SIGNAL_USR1 = 10, /* SIGUSR1 */
+VIR_DOMAIN_PROCESS_SIGNAL_SEGV = 11, /* SIGSEGV */
+VIR_DOMAIN_PROCESS_SIGNAL_USR2 = 12, /* SIGUSR2 */
+VIR_DOMAIN_PROCESS_SIGNAL_PIPE = 13, /* SIGPIPE */
+VIR_DOMAIN_PROCESS_SIGNAL_ALRM = 14, /* SIGALRM */
+VIR_DOMAIN_PROCESS_SIGNAL_TERM = 15, /* SIGTERM */
+VIR_DOMAIN_PROCESS_SIGNAL_STKFLT = 16, /* Not in POSIX (SIGSTKFLT on
Linux )*/
+VIR_DOMAIN_PROCESS_SIGNAL_CHLD = 17, /* SIGCHLD */
+VIR_DOMAIN_PROCESS_SIGNAL_CONT = 18, /* SIGCONT */
+VIR_DOMAIN_PROCESS_SIGNAL_STOP = 19, /* SIGSTOP */
+
+VIR_DOMAIN_PROCESS_SIGNAL_TSTP = 20, /* SIGTSTP */
+VIR_DOMAIN_PROCESS_SIGNAL_TTIN = 21, /* SIGTTIN */
+VIR_DOMAIN_PROCESS_SIGNAL_TTOU = 22, /* SIGTTOU */
+VIR_DOMAIN_PROCESS_SIGNAL_URG= 23, /* SIGURG */
+VIR_DOMAIN_PROCESS_SIGNAL_XCPU = 24, /* SIGXCPU */
+VIR_DOMAIN_PROCESS_SIGNAL_XFSZ = 25, /* SIGXFSZ */
+VIR_DOMAIN_PROCESS_SIGNAL_VTALRM = 26, /* SIGVTALRM */
+VIR_DOMAIN_PROCESS_SIGNAL_PROF = 27, /* SIGPROF */
+VIR_DOMAIN_PROCESS_SIGNAL_WINCH = 28, /* Not in POSIX (SIGWINCH on
Linux) */
+VIR_DOMAIN_PROCESS_SIGNAL_POLL = 29, /* SIGPOLL (also known as SIGIO
on Linux) */
+
+VIR_DOMAIN_PROCESS_SIGNAL_PWR= 30, /* Not in POSIX (SIGPWR on
Linux */
+VIR_DOMAIN_PROCESS_SIGNAL_SYS= 31, /* SIGSYS (also known as
SIGUNUSED on Linux ) */
+VIR_DOMAIN_PROCESS_SIGNAL_RT0= 32, /* SIGRTMIN */
+VIR_DOMAIN_PROCESS_SIGNAL_RT1= 33, /* SIGRTMIN + 1 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT2= 34, /* SIGRTMIN + 2 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT3= 35, /* SIGRTMIN + 3 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT4= 36, /* SIGRTMIN + 4 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT5= 37, /* SIGRTMIN + 5 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT6= 38, /* SIGRTMIN + 6 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT7= 39, /* SIGRTMIN + 7 */
+
+VIR_DOMAIN_PROCESS_SIGNAL_RT8= 40, /* SIGRTMIN + 8 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT9= 41, /* SIGRTMIN + 9 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT10 = 42, /* SIGRTMIN + 10 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT11 = 43, /* SIGRTMIN + 11 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT12 = 44, /* SIGRTMIN + 12 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT13 = 45, /* SIGRTMIN + 13 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT14 = 46, /* SIGRTMIN + 14 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT15 = 47, /* SIGRTMIN + 15 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT16 = 48, /* SIGRTMIN + 16 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT17 = 49, /* SIGRTMIN + 17 */
+
+VIR_DOMAIN_PROCESS_SIGNAL_RT18 = 50, /* SIGRTMIN + 18 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT19 = 51, /* SIGRTMIN + 19 */
+VIR_DOMAIN_PROCESS_SIGNAL_RT20 = 52, /* SIGRTMIN + 20 */
+
[libvirt] [PATCH v2 2/4] Specify remote protocol for virDomainSendProcessSignal
From: Daniel P. Berrange berra...@redhat.com
* src/remote/remote_protocol.x: message definition
* src/remote/remote_driver.c: Register driver function
* src/remote_protocol-structs: Test case
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/remote/remote_driver.c | 1 +
src/remote/remote_protocol.x | 10 +-
src/remote_protocol-structs | 7 +++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 49d89be..25f20fc 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -6124,6 +6124,7 @@ static virDriver remote_driver = {
.domainMigrateFinish3 = remoteDomainMigrateFinish3, /* 0.9.2 */
.domainMigrateConfirm3 = remoteDomainMigrateConfirm3, /* 0.9.2 */
.domainSendKey = remoteDomainSendKey, /* 0.9.3 */
+.domainSendProcessSignal = remoteDomainSendProcessSignal, /* 1.0.1 */
.domainBlockJobAbort = remoteDomainBlockJobAbort, /* 0.9.4 */
.domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */
.domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 31567e2..bdad9f0 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1019,6 +1019,13 @@ struct remote_domain_send_key_args {
unsigned int flags;
};
+struct remote_domain_send_process_signal_args {
+remote_nonnull_domain dom;
+hyper pid_value;
+unsigned int signum;
+unsigned int flags;
+};
+
struct remote_domain_set_vcpus_args {
remote_nonnull_domain dom;
unsigned int nvcpus;
@@ -3034,7 +3041,8 @@ enum remote_procedure {
REMOTE_PROC_NETWORK_UPDATE = 291, /* autogen autogen priority:high */
REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292, /* autogen autogen */
REMOTE_PROC_NODE_GET_CPU_MAP = 293, /* skipgen skipgen */
-REMOTE_PROC_DOMAIN_FSTRIM = 294 /* autogen autogen */
+REMOTE_PROC_DOMAIN_FSTRIM = 294, /* autogen autogen */
+REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295 /* autogen autogen */
/*
* Notice how the entries are grouped in sets of 10 ?
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index d0d4f53..e7d05b8 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -672,6 +672,12 @@ struct remote_domain_send_key_args {
} keycodes;
u_int flags;
};
+struct remote_domain_send_process_signal_args {
+remote_nonnull_domain dom;
+int64_tpid_value;
+u_int signum;
+u_int flags;
+};
struct remote_domain_set_vcpus_args {
remote_nonnull_domain dom;
u_int nvcpus;
@@ -2440,4 +2446,5 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND_DISK = 292,
REMOTE_PROC_NODE_GET_CPU_MAP = 293,
REMOTE_PROC_DOMAIN_FSTRIM = 294,
+REMOTE_PROC_DOMAIN_SEND_PROCESS_SIGNAL = 295,
};
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] Add a 'send-process-signal' command to virsh
From: Daniel P. Berrange berra...@redhat.com
* tools/virsh.c: Add send-process-signal
* tools/virsh.pod: Document new command
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
tools/virsh-domain.c | 103 +++
tools/virsh.pod | 27 ++
2 files changed, 130 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6d5a0ec..e22255a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5901,6 +5901,108 @@ cleanup:
}
/*
+ * send-process-signal command
+ */
+static const vshCmdInfo info_send_process_signal[] = {
+{help, N_(Send signals to processes) },
+{desc, N_(Send signals to processes in the guest) },
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_send_process_signal[] = {
+{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
+{pid, VSH_OT_DATA, VSH_OFLAG_REQ, N_(the process ID) },
+{signame, VSH_OT_DATA, VSH_OFLAG_REQ, N_(the signal number or name) },
+{NULL, 0, 0, NULL}
+};
+
+VIR_ENUM_DECL(virDomainProcessSignal)
+VIR_ENUM_IMPL(virDomainProcessSignal,
+ VIR_DOMAIN_PROCESS_SIGNAL_LAST,
+ nop, hup, int, quit, ill, /* 0-4 */
+ trap, abrt, bus, fpe, kill, /* 5-9 */
+ usr1, segv, usr2, pipe, alrm, /* 10-14 */
+ term, stkflt, chld, cont, stop, /* 15-19 */
+ tstp, ttin, ttou, urg, xcpu, /* 20-24 */
+ xfsz, vtalrm, prof, winch, poll, /* 25-29 */
+ pwr, sys, rt0,rt1, rt2, /* 30-34 */
+ rt3, rt4, rt5, rt6, rt7, /* 35-39 */
+ rt8, rt9, rt10, rt11, rt12, /* 40-44 */
+ rt13, rt14, rt15, rt16, rt17, /* 45-49 */
+ rt18, rt19, rt20, rt21, rt22, /* 50-54 */
+ rt23, rt24, rt25, rt26, rt27, /* 55-59 */
+ rt28, rt29, rt30, rt31, rt32) /* 60-64 */
+
+static int getSignalNumber(vshControl *ctl, const char *signame)
+{
+size_t i;
+int signum;
+char *lower = vshStrdup(ctl, signame);
+
+for (i = 0 ; signame[i] ; i++)
+lower[i] = c_tolower(signame[i]);
+
+if (virStrToLong_i(signame, NULL, 10, signum) = 0)
+goto cleanup;
+
+if (STRPREFIX(signame, sig))
+signame += 3;
+else if (STRPREFIX(signame, sig_))
+signame += 4;
+
+if ((signum = virDomainProcessSignalTypeFromString(signame)) = 0)
+goto cleanup;
+
+signum = -1;
+cleanup:
+VIR_FREE(lower);
+return signum;
+}
+
+static bool
+cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+int ret = false;
+const char *pidstr;
+const char *signame;
+long long pid_value;
+int signum;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (vshCommandOptString(cmd, pid, pidstr) = 0) {
+vshError(ctl, %s, _(missing argument));
+return false;
+}
+
+if (vshCommandOptString(cmd, signame, signame) = 0) {
+vshError(ctl, %s, _(missing argument));
+return false;
+}
+
+if (virStrToLong_ll(pidstr, NULL, 10, pid_value) 0) {
+vshError(ctl, _(malformed PID value: %s), pidstr);
+goto cleanup;
+}
+
+if ((signum = getSignalNumber(ctl, signame)) 0) {
+vshError(ctl, _(malformed signal name: %s), signame);
+goto cleanup;
+}
+
+if (virDomainSendProcessSignal(dom, pid_value, signum, 0) 0)
+goto cleanup;
+
+ret = true;
+
+cleanup:
+virDomainFree(dom);
+return ret;
+}
+
+/*
* setmem command
*/
static const vshCmdInfo info_setmem[] = {
@@ -8398,6 +8500,7 @@ const vshCmdDef domManagementCmds[] = {
{edit, cmdEdit, opts_edit, info_edit, 0},
{inject-nmi, cmdInjectNMI, opts_inject_nmi, info_inject_nmi, 0},
{send-key, cmdSendKey, opts_send_key, info_send_key, 0},
+{send-process-signal, cmdSendProcessSignal, opts_send_process_signal,
info_send_process_signal, 0},
{managedsave, cmdManagedSave, opts_managedsave, info_managedsave, 0},
{managedsave-remove, cmdManagedSaveRemove, opts_managedsaveremove,
info_managedsaveremove, 0},
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 353af66..1708cc4 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1393,6 +1393,33 @@ BExamples
# send a tab, held for 1 second
virsh send-key --holdtime 1000 0xf
+=item Bsend-process-signal Idomain-id [I--host-pid] Ipid Isigname
+
+Send a signal Isigname to the process identified by Ipid running in
+the virtual domain Idomain-id. The Ipid is a process ID in the virtual
+domain namespace.
+
+The Isigname argument may be either an integer signal constant number,
+or one of the symbolic names:
+
+nop, hup, int, quit, ill,
+trap, abrt, bus, fpe, kill,
+usr1, segv, usr2, pipe, alrm,
+term, stkflt, chld, cont, stop,
+tstp, ttin, ttou, urg, xcpu,
+xfsz, vtalrm, prof, winch, poll,
+pwr, sys, rtmin
+
+The symbol name may optionally be prefixed with 'sig'.
Re: [libvirt] [PATCH] network: fix crash when portgroup has no name
On 11/28/2012 05:55 PM, Laine Stump wrote:
On 11/28/2012 04:19 AM, Martin Kletzander wrote:
On 11/28/2012 06:08 AM, Laine Stump wrote:
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=879473
The name attribute is required for portgroup elements (yes, the RNG
specifies that), and there is code in libvirt that assumes it is
non-null. Unfortunately, the portgroup parsing function wasn't
checking for lack of portgroup. One adverse result of this was that
attempts to update a network by adding a portgroup with no name would
cause libvirtd to segfault. For example:
virsh net-update default add portgroup portgroup default='yes'/
This patch causes virNetworkPortGroupParseXML to fail if no name is
specified, thus avoiding any later problems.
Looking at the code, I see it's required on more places, yes. And
according to the documentation the name is needed.
---
src/conf/network_conf.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 228951d..6ce2e63 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1175,6 +1175,12 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def,
/* grab raw data from XML */
def-name = virXPathString(string(./@name), ctxt);
+if (!def-name) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Missing required name attribute in portgroup));
+goto error;
+}
+
isDefault = virXPathString(string(./@default), ctxt);
def-isDefault = isDefault STRCASEEQ(isDefault, yes);
Just a question; there's a similar check for (!def-name), for networks
particularly, and that one uses VIR_ERR_NO_NAME (specified as a error
for missing _domain_ name). Should one of these be changed (in a
separate patch, of course)?
According to the comment with the definition of VIR_ERR_NO_NAME, it's
intended only for when the name is missing from a domain. The error
printed out though is missing name information (with optional in %s).
So if you look at the comment, it seems that it shouldn't be used for
network (or node device) name, but if you look at the message generated,
it looks like it could be used for any missing name. Personally I don't
see the use of having a separate error code for missing name vs. missing
[anything else]. To me it's just like any other XML error.
That's exactly what I meant, so you assured me that I haven't
misunderstood the second appearance of that.
Thanks,
Martin
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] Add virDomainSendProcessSignal API
Add an API for sending signals to arbitrary processes in the
guest OS. This is primarily useful for container based virt,
but can be used for machine virt too, if there is a suitable
guest agent,
* include/libvirt/libvirt.h.in: Add virDomainSendProcessSignal
and virDomainProcessSignal enum
* src/driver.h: Driver entry point
* src/libvirt.c, src/libvirt_public.syms: Impl for new API
---
include/libvirt/libvirt.h.in | 97
src/driver.h | 7
src/libvirt.c| 84
++
src/libvirt_public.syms | 1 +
4 files changed, 189 insertions(+)
+ *
+ * Do not rely on all values matching Linux though. It is possible
Why two spaces mid-sentence?
+VIR_DOMAIN_PROCESS_SIGNAL_RT32 = 64, /* SIGRTMIN + 32 /
SIGRTMAX */
Yes, this works better for me.
+
+#ifdef VIR_ENUM_SENTINELS
+/* This is the last signal only in so much as none
+ * of the latter ones have named constants. They are
+ * just numeric offsets from RTMIN upwards
This comment is now out of date.
+/**
+ * virDomainSendProcessSignal:
+ * @domain: pointer to domain object
+ * @pid_value: a positive integer process ID, or negative integer
process group ID
+ * @signum: a signal from the virDomainProcessSignal enum
+ * @flags: one of the virDomainProcessSignalFlag values
+ *
+ * Send a signal to the designated process in the guest
+ *
+ * The signal numbers must be taken from the virDomainProcessSignal
+ * enum. These will be translated to the corresponding signal
+ * number for the guest OS, by the guest agent delivering the
+ * signal. If there is no mapping from virDomainProcessSignal to
+ * the native OS signals, this API will report an error
s/$/./
+ *
+ * If @pid_value is an integer greater than zero, it is
+ * treated as a process ID. If @pid_value is an integer
+ * less than zero, it is treated as a process group ID.
+ * All the @pid_value numbers are from the container/guest
+ * namespace. The value zero is not valid.
+ *
+ * Not all hypervisors will support sending signals to
+ * arbitrary processes or process groups. If this API is
+ * implemented the minimum requirement is to be able to
+ * use @pid_value==1 (ie kill init). No other value is
s/ie/i.e./
+
+if (signum VIR_DOMAIN_PROCESS_SIGNAL_LAST) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
Drop this hunk (rather, move it to patch 4/4). Such a check
should be done in the hypervisor driver, not in the entry
point. Otherwise, if we add a new signal later, this hunk
would prevent an older virsh from sending the new signal.
ACK with those changes.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] Specify remote protocol for virDomainSendProcessSignal
* src/remote/remote_protocol.x: message definition * src/remote/remote_driver.c: Register driver function * src/remote_protocol-structs: Test case Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 10 +- src/remote_protocol-structs | 7 +++ 3 files changed, 17 insertions(+), 1 deletion(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/4] Add a 'send-process-signal' command to virsh
* tools/virsh.c: Add send-process-signal * tools/virsh.pod: Document new command Signed-off-by: Daniel P. Berrange berra...@redhat.com --- tools/virsh-domain.c | 103 +++ tools/virsh.pod | 27 ++ 2 files changed, 130 insertions(+) +VIR_ENUM_DECL(virDomainProcessSignal) +VIR_ENUM_IMPL(virDomainProcessSignal, + VIR_DOMAIN_PROCESS_SIGNAL_LAST, + nop, hup, int, quit, ill, /* 0-4 */ + trap, abrt, bus, fpe, kill, /* 5-9 */ + usr1, segv, usr2, pipe, alrm, /* 10-14 */ + term, stkflt, chld, cont, stop, /* 15-19 */ + tstp, ttin, ttou, urg, xcpu, /* 20-24 */ + xfsz, vtalrm, prof, winch, poll, /* 25-29 */ + pwr, sys, rt0,rt1, rt2, /* 30-34 */ This doesn't include rtmin... + +if (STRPREFIX(signame, sig)) +signame += 3; +else if (STRPREFIX(signame, sig_)) +signame += 4; The else will never be reached. Swap these two conditions. + +if ((signum = virDomainProcessSignalTypeFromString(signame)) = 0) +goto cleanup; + ...and you lost your handling of rtmin+1 here... +++ b/tools/virsh.pod @@ -1393,6 +1393,33 @@ BExamples # send a tab, held for 1 second virsh send-key --holdtime 1000 0xf +=item Bsend-process-signal Idomain-id [I--host-pid] Ipid Isigname --host-pid is unsupported; drop it from the synopsis. +The Isigname argument may be either an integer signal constant number, +or one of the symbolic names: Maybe mention case-insensitive. + +nop, hup, int, quit, ill, +trap, abrt, bus, fpe, kill, +usr1, segv, usr2, pipe, alrm, +term, stkflt, chld, cont, stop, +tstp, ttin, ttou, urg, xcpu, +xfsz, vtalrm, prof, winch, poll, +pwr, sys, rtmin This list doesn't mention rt0 and friends. + +The symbol name may optionally be prefixed with 'sig'. The 'rtmin' signal +also allows an optional suffix +number to refer to other real time +signals. ...so this part of the docs no longer matches the code... + +BExamples + virsh send-process-signal myguest 1 15 + virsh send-process-signal myguest 1 term + virsh send-process-signal myguest 1 sigterm + virsh send-process-signal myguest 1 rtmin+12 ...and this last example needs fixing. But I'm okay giving ACK; I trust you to clean things up appropriately, and this patch only touches virsh. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] Add virDomainSendProcessSignal API
Add an API for sending signals to arbitrary processes in the
guest OS. This is primarily useful for container based virt,
but can be used for machine virt too, if there is a suitable
guest agent,
+
+if (signum VIR_DOMAIN_PROCESS_SIGNAL_LAST) {
+virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
Drop this hunk (rather, move it to patch 4/4). Such a check
should be done in the hypervisor driver, not in the entry
point. Otherwise, if we add a new signal later, this hunk
would prevent an older virsh from sending the new signal.
On the other hand, you _should_ add a check here that prevents
signum==0, since we have documented that signum must be non-zero
as part of the API contract.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] Add virDomainSendProcessSignal API
Drop this hunk (rather, move it to patch 4/4). Such a check should be done in the hypervisor driver, not in the entry point. Otherwise, if we add a new signal later, this hunk would prevent an older virsh from sending the new signal. On the other hand, you _should_ add a check here that prevents signum==0, since we have documented that signum must be non-zero as part of the API contract. Arg. Can't type. signum==0 is valid, pid_value==0 is the forbidden item. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] Implement virDomainSendProcessSignal for LXC driver
Implement the new API for sending signals to processes in a guest for the LXC driver. Only support sending signals to the init process for now, because - The kernel does not appear to expose the mapping between container PID numbers and host PID numbers anywhere in the host OS namespace - There is no race-free validate whether a host PID corresponds s/validate/way to validate/ ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] conf: fix NULL check in virNetDevBandwidthParse
Found by coverity:
Error: REVERSE_INULL (CWE-476):
libvirt-0.10.2/src/conf/netdev_bandwidth_conf.c:99: deref_ptr:
Directly dereferencing pointer node.
libvirt-0.10.2/src/conf/netdev_bandwidth_conf.c:107:
check_after_deref: Null-checking node suggests that it may be
null, but it has already been dereferenced on all paths leading to
the check.
---
src/conf/netdev_bandwidth_conf.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/conf/netdev_bandwidth_conf.c b/src/conf/netdev_bandwidth_conf.c
index 261718f..5802eba 100644
--- a/src/conf/netdev_bandwidth_conf.c
+++ b/src/conf/netdev_bandwidth_conf.c
@@ -96,7 +96,7 @@ virNetDevBandwidthPtr
virNetDevBandwidthParse(xmlNodePtr node)
{
virNetDevBandwidthPtr def = NULL;
-xmlNodePtr cur = node-children;
+xmlNodePtr cur;
xmlNodePtr in = NULL, out = NULL;
if (VIR_ALLOC(def) 0) {
@@ -110,6 +110,8 @@ virNetDevBandwidthParse(xmlNodePtr node)
goto error;
}
+cur = node-children;
+
while (cur) {
if (cur-type == XML_ELEMENT_NODE) {
if (xmlStrEqual(cur-name, BAD_CAST inbound)) {
--
1.7.8.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] More coverity issues
REVERSE_INULL, SIZEOF_MISMATCH and some uninitialized variables. Ján Tomko (6): conf: fix NULL check in virNetDevBandwidthParse util: fix virBitmap allocation in virProcessInfoGetAffinity virsh: use correct sizeof when allocating cpumap virsh: do timing even for unusable connections rpc: don't destroy xdr before creating it in virNetMessageEncodeHeader conf: fix uninitialized variable in virDomainListSnapshots src/conf/netdev_bandwidth_conf.c |4 +++- src/conf/snapshot_conf.c |2 +- src/rpc/virnetmessage.c |2 +- src/util/processinfo.c |2 +- tools/virsh-domain.c |4 ++-- tools/virsh.c| 12 ++-- 6 files changed, 14 insertions(+), 12 deletions(-) -- 1.7.8.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] util: fix virBitmap allocation in virProcessInfoGetAffinity
Found by coverity:
Error: REVERSE_INULL (CWE-476):
libvirt-0.10.2/src/util/processinfo.c:141: deref_ptr: Directly
dereferencing pointer map.
libvirt-0.10.2/src/util/processinfo.c:142: check_after_deref:
Null-checking map suggests that it may be null, but it has already
been dereferenced on all paths leading to the check.
---
src/util/processinfo.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/util/processinfo.c b/src/util/processinfo.c
index b8c60eb..b1db049 100644
--- a/src/util/processinfo.c
+++ b/src/util/processinfo.c
@@ -140,7 +140,7 @@ realloc:
}
*map = virBitmapNew(maxcpu);
-if (!map) {
+if (!*map) {
virReportOOMError();
return -1;
}
--
1.7.8.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] virsh: use correct sizeof when allocating cpumap
Found by coverity:
Error: SIZEOF_MISMATCH (CWE-569):
libvirt-0.10.2/tools/virsh-domain.c:4754: suspicious_sizeof: Passing
argument 8UL /* sizeof (cpumap) */ to function
_vshCalloc(vshControl *, size_t, size_t, char const *, int) and
then casting the return value to unsigned char * is suspicious.
Error: SIZEOF_MISMATCH (CWE-569):
libvirt-0.10.2/tools/virsh-domain.c:4942: suspicious_sizeof: Passing
argument 8UL /* sizeof (cpumap) */ to function
_vshCalloc(vshControl *, size_t, size_t, char const *, int) and
then casting the return value to unsigned char * is suspicious.
---
tools/virsh-domain.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6d5a0ec..12da00d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4768,7 +4768,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
/* Pin mode: pinning specified vcpu to specified physical cpus*/
-cpumap = vshCalloc(ctl, cpumaplen, sizeof(cpumap));
+cpumap = vshCalloc(ctl, cpumaplen, sizeof(*cpumap));
/* Parse cpulist */
cur = cpulist;
if (*cur == 0) {
@@ -4954,7 +4954,7 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
/* Pin mode: pinning emulator threads to specified physical cpus*/
-cpumap = vshCalloc(ctl, cpumaplen, sizeof(cpumap));
+cpumap = vshCalloc(ctl, cpumaplen, sizeof(*cpumap));
/* Parse cpulist */
cur = cpulist;
if (*cur == 0) {
--
1.7.8.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] rpc: don't destroy xdr before creating it in virNetMessageEncodeHeader
On OOM, xdr_destroy got called on xdr even though it wasn't created yet.
Found by coverity:
Error: UNINIT (CWE-457):
libvirt-0.10.2/src/rpc/virnetmessage.c:214: var_decl: Declaring
variable xdr without initializer.
libvirt-0.10.2/src/rpc/virnetmessage.c:219: cond_true: Condition
virReallocN(msg-buffer, 1UL /* sizeof (*msg-buffer) */,
msg-bufferLength) 0, taking true branch
libvirt-0.10.2/src/rpc/virnetmessage.c:221: goto: Jumping to label
cleanup
libvirt-0.10.2/src/rpc/virnetmessage.c:257: label: Reached label
cleanup
libvirt-0.10.2/src/rpc/virnetmessage.c:258: uninit_use: Using
uninitialized value xdr.x_ops.
---
src/rpc/virnetmessage.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c
index ce5f9d8..2fbd603 100644
--- a/src/rpc/virnetmessage.c
+++ b/src/rpc/virnetmessage.c
@@ -218,7 +218,7 @@ int virNetMessageEncodeHeader(virNetMessagePtr msg)
msg-bufferLength = VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX;
if (VIR_REALLOC_N(msg-buffer, msg-bufferLength) 0) {
virReportOOMError();
-goto cleanup;
+return ret;
}
msg-bufferOffset = 0;
--
1.7.8.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] conf: fix uninitialized variable in virDomainListSnapshots
If allocation of names fails, list is uninitialized.
---
src/conf/snapshot_conf.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 06be34d..bba1bb7 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -1021,7 +1021,7 @@ virDomainListSnapshots(virDomainSnapshotObjListPtr
snapshots,
unsigned int flags)
{
int count = virDomainSnapshotObjListNum(snapshots, from, flags);
-virDomainSnapshotPtr *list;
+virDomainSnapshotPtr *list = NULL;
char **names;
int ret = -1;
int i;
--
1.7.8.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] virsh: do timing even for unusable connections
Time values were uninitialized if the connection wasn't usable.
---
tools/virsh.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 6372177..dea3f82 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1562,20 +1562,20 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd)
!(cmd-def-flags VSH_CMD_FLAG_NOCONNECT))
vshReconnect(ctl);
+if (enable_timing)
+GETTIMEOFDAY(before);
+
if ((cmd-def-flags VSH_CMD_FLAG_NOCONNECT) ||
vshConnectionUsability(ctl, ctl-conn)) {
-if (enable_timing)
-GETTIMEOFDAY(before);
-
ret = cmd-def-handler(ctl, cmd);
-
-if (enable_timing)
-GETTIMEOFDAY(after);
} else {
/* connection is not usable, return error */
ret = false;
}
+if (enable_timing)
+GETTIMEOFDAY(after);
+
/* try to automatically catch disconnections */
if (!ret
((last_error != NULL)
--
1.7.8.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Globally Reserve Resources for Host
On Wed, Nov 28, 2012 at 3:26 PM, Daniel P. Berrange berra...@redhat.com wrote: On Wed, Nov 28, 2012 at 02:27:53PM -0500, Dusty Mabe wrote: On Wed, Nov 14, 2012 at 11:22 AM, Dusty Mabe dustym...@gmail.com wrote: On Thu, Nov 1, 2012 at 11:32 PM, Dusty Mabe dustym...@gmail.com wrote: Sorry for not replying before. I've been thinking about this today and am specifically wondering about the possible implications for a change we need to make to cgroups setup in libvirt No worries, thank you for responding!! The core issue is that it has become apparent that nesting cgroups can cause some very significant performance / scalability problems for the kernel. They have requested / recommended that libvirt make its cgroup hierarchy as flat as possible to avoid this problem. That makes sense. Does this affect how you feel about https://www.redhat.com/archives/libvir-list/2011-March/msg01546.html ? 1. Match current hardcoded layout: cgroup_layout=/::PROCESS::/libvirt/::DRIVER::/::VMNAME:: 2. Remove first level when used with systemd: cgroup_layout=/::PROCESS::/::DRIVER::/::VMNAME:: 3. Combine 3rd/4th levels too cgroup_layout=/::PROCESS::/::DRIVER::-::VMNAME:: 4. Ignore current libvirtd placement completely and create in the root: cgroup_layout=/libvirt/::DRIVER::-::VMNAME:: 5. Use UUID instead of VM name cgroup_layout=/libvirt/::DRIVER::-::VMUUID:: A couple more options: 6. cgroup_layout=/libvirt/::DRIVER::/VMNAME 7. cgroup_layout=/libvirt/::DRIVER::/VMUUID I'm sure you've noticed that this plan doesn't leave much scope for the approach you outlined above, since the 'qemu' level will disappear by default. It looks like you are right, by default there would be no driver level cgroup. What if we still had settings in qemu.conf for something like reservedHostMem and reservedHostCpus (maybe those names don't make sense, but I think you know what i mean) that would only be enforced if the cgroup_layout had a */::DRIVER::/* component (cases 1,2,6,7). In this case there would be a 'qemu' level. We could explain this dependency in the conf file and print out an error/warning in the logs if it wasn't followed. It is on my plate to get these changes done for Fedora 19 / RHEL-7 releases. Daniel Thanks again for responding! I appreciate your input and it is helpful to know the path that libvirt is going down. I know what I am suggesting above may be a long shot but i figure it's worth at least airing the idea. I like it because too often my hosts get oversubscribed from other people creating guests and can cause wacky behavior (my problem, not yours). Dusty Mabe -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] conf: prevent crash with no uuid in cephx auth secret
On 11/28/12 15:31, Osier Yang wrote:
On 2012年11月28日 21:34, Ján Tomko wrote:
Also remove the pointles check for NULL in auth.cephx.secret.uuid,
since this is a static array.
It's nice if there is log of coverity.
Error: FORWARD_NULL (CWE-476):
libvirt-0.10.2/src/conf/storage_conf.c:447: cond_false: Condition
auth-username == NULL, taking false branch
libvirt-0.10.2/src/conf/storage_conf.c:451: if_end: End of if statement
libvirt-0.10.2/src/conf/storage_conf.c:455: cond_true: Condition uuid
== NULL, taking true branch
libvirt-0.10.2/src/conf/storage_conf.c:455: var_compare_op: Comparing
uuid to null implies that uuid might be null.
libvirt-0.10.2/src/conf/storage_conf.c:455: cond_false: Condition
auth-secret.usage == NULL, taking false branch
libvirt-0.10.2/src/conf/storage_conf.c:459: if_end: End of if statement
libvirt-0.10.2/src/conf/storage_conf.c:461: var_deref_model: Passing
null pointer uuid to function virUUIDParse(char const *, unsigned
char *), which dereferences it. (The dereference is assumed on the
basis of the 'nonnull' parameter attribute.)
Error: NO_EFFECT (CWE-398):
libvirt-0.10.2/src/conf/storage_conf.c:979: array_null: Comparing an
array to null is not useful: src-auth.cephx.secret.uuid != NULL.
...
This change forces both of uuid and usage to be specified.
But from the RNG schema:
define name='sourceinfoauthsecret'
element name='secret'
choice
attribute name='uuid'
text/
/attribute
attribute name='usage'
text/
/attribute
/choice
/element
/define
Means that it allows only one of them specified.
Hm, from the schema, it should error out if both of them are
specified too. So either there is problem of either the schema
or the codes.
I think we have to figure out if the schema is correct first.
Looking at the code in storage_backend_rbd.c it looks like if both are
specified, only usage is taken into account:
if (pool-def-source.auth.cephx.secret.uuid != NULL) {
virUUIDFormat(pool-def-source.auth.cephx.secret.uuid, secretUuid);
VIR_DEBUG(Looking up secret by UUID: %s, secretUuid);
secret = virSecretLookupByUUIDString(conn, secretUuid);
}
if (pool-def-source.auth.cephx.secret.usage != NULL) {
VIR_DEBUG(Looking up secret by usage: %s,
pool-def-source.auth.cephx.secret.usage);
secret = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_CEPH,
pool-def-source.auth.cephx.secret.usage);
}
I'll send another version shortly.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Don't fail hard when we can't connect to the monitor
On Thu, Nov 29, 2012 at 12:39:24PM +, Daniel P. Berrange wrote:
On Thu, Nov 29, 2012 at 01:29:38PM +0100, Guido Günther wrote:
As of 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 we fail to connect to the
monitor instead of getting an exit status != 0 from qemu itself. This
breaks capabilities probing for the non QMP case.
---
src/qemu/qemu_capabilities.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d92947f..d6affb9 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2345,8 +2345,10 @@ qemuCapsInitQMP(qemuCapsPtr caps,
goto cleanup;
}
-if (!(mon = qemuMonitorOpen(NULL, config, true, callbacks)))
+if (!(mon = qemuMonitorOpen(NULL, config, true, callbacks))) {
+ret = 0;
goto cleanup;
+}
qemuMonitorLock(mon);
ACK
Pushed. Thanks. This unbreaks libvirt-tck on wheezy:
http://honk.sigxcpu.org:8001/job/libvirt-tck-debian-wheezy-qemu-session/202/
Cheers,
-- Guido
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] conf: prevent crash with no uuid in cephx auth secret
Also remove the pointless check for NULL in auth.cephx.secret.uuid,
since this is a static array.
---
src/conf/storage_conf.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 3fdc5b6..99c2e52 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -458,7 +458,7 @@ virStoragePoolDefParseAuthCephx(xmlXPathContextPtr ctxt,
return -1;
}
-if (virUUIDParse(uuid, auth-secret.uuid) 0) {
+if (uuid virUUIDParse(uuid, auth-secret.uuid) 0) {
virReportError(VIR_ERR_XML_ERROR,
%s, _(invalid auth secret uuid));
return -1;
@@ -979,10 +979,8 @@ virStoragePoolSourceFormat(virBufferPtr buf,
src-auth.cephx.username);
virBufferAsprintf(buf, %s, secret);
-if (src-auth.cephx.secret.uuid != NULL) {
-virUUIDFormat(src-auth.cephx.secret.uuid, uuid);
-virBufferAsprintf(buf, uuid='%s', uuid);
-}
+virUUIDFormat(src-auth.cephx.secret.uuid, uuid);
+virBufferAsprintf(buf, uuid='%s', uuid);
if (src-auth.cephx.secret.usage != NULL) {
virBufferAsprintf(buf, usage='%s', src-auth.cephx.secret.usage);
--
1.7.8.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/6] util: fix virBitmap allocation in virProcessInfoGetAffinity
Found by coverity: Error: REVERSE_INULL (CWE-476): libvirt-0.10.2/src/util/processinfo.c:141: deref_ptr: Directly dereferencing pointer map. libvirt-0.10.2/src/util/processinfo.c:142: check_after_deref: Null-checking map suggests that it may be null, but it has already been dereferenced on all paths leading to the check. --- src/util/processinfo.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/6] virsh: do timing even for unusable connections
Time values were uninitialized if the connection wasn't usable. --- tools/virsh.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/6] conf: fix NULL check in virNetDevBandwidthParse
Found by coverity: Error: REVERSE_INULL (CWE-476): libvirt-0.10.2/src/conf/netdev_bandwidth_conf.c:99: deref_ptr: Directly dereferencing pointer node. libvirt-0.10.2/src/conf/netdev_bandwidth_conf.c:107: check_after_deref: Null-checking node suggests that it may be null, but it has already been dereferenced on all paths leading to the check. --- src/conf/netdev_bandwidth_conf.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/6] rpc: don't destroy xdr before creating it in virNetMessageEncodeHeader
On OOM, xdr_destroy got called on xdr even though it wasn't created yet. Found by coverity: Error: UNINIT (CWE-457): libvirt-0.10.2/src/rpc/virnetmessage.c:214: var_decl: Declaring variable xdr without initializer. libvirt-0.10.2/src/rpc/virnetmessage.c:219: cond_true: Condition virReallocN(msg-buffer, 1UL /* sizeof (*msg-buffer) */, msg-bufferLength) 0, taking true branch libvirt-0.10.2/src/rpc/virnetmessage.c:221: goto: Jumping to label cleanup libvirt-0.10.2/src/rpc/virnetmessage.c:257: label: Reached label cleanup libvirt-0.10.2/src/rpc/virnetmessage.c:258: uninit_use: Using uninitialized value xdr.x_ops. --- src/rpc/virnetmessage.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/6] virsh: use correct sizeof when allocating cpumap
- Original Message - Found by coverity: Error: SIZEOF_MISMATCH (CWE-569): libvirt-0.10.2/tools/virsh-domain.c:4754: suspicious_sizeof: Passing argument 8UL /* sizeof (cpumap) */ to function _vshCalloc(vshControl *, size_t, size_t, char const *, int) and then casting the return value to unsigned char * is suspicious. Error: SIZEOF_MISMATCH (CWE-569): libvirt-0.10.2/tools/virsh-domain.c:4942: suspicious_sizeof: Passing argument 8UL /* sizeof (cpumap) */ to function _vshCalloc(vshControl *, size_t, size_t, char const *, int) and then casting the return value to unsigned char * is suspicious. --- tools/virsh-domain.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) ACK. -cpumap = vshCalloc(ctl, cpumaplen, sizeof(cpumap)); +cpumap = vshCalloc(ctl, cpumaplen, sizeof(*cpumap)); Bugs like this show why we prefer VIR_ALLOC_N. Maybe we should fix vshCalloc to be a thin wrapper around VIR_ALLOC_N, but that's a bigger task for another day. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/6] conf: fix uninitialized variable in virDomainListSnapshots
If allocation of names fails, list is uninitialized. --- src/conf/snapshot_conf.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) ACK. And looks like this one was introduced fairly recently, by commit 0361917. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] More coverity issues
- Original Message - REVERSE_INULL, SIZEOF_MISMATCH and some uninitialized variables. Series now pushed to libvirt.org. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] Make QEMU perform managed save of all VMs on stop of libvirtd
On Tue, Nov 27, 2012 at 03:06:45PM -0500, Eric Blake wrote: How does this interact with the libvirt-guests init script? I guess that if that is installed, it gets to run first before the system libvirtd is shutdown, so the qemu:///system won't see any running guests, and this patch would just affect qemu:///session? In theory it can do both qemu:///system and qemu:///session, however, the later patches only wire this up for qemu:///session, precisely because libvirt-guests already exists. In other words, right now, there is no interaction, but we may convert libvirt-guests in the future to use this method instead. Fair enough, and shouldn't hold up this series. Is managed save always the right thing, or should we allow the user to configure for a graceful shutdown and/or a migration to other host? Do we need timeouts, as managed save may take a while per guest? Similar to how /etc/sysconfig/libvirt-guests can request O_DIRECT to avoid file system pollution, do we need a way for the user to configure the use of that flag? Is doing the same thing for all guests wise, or do we need to allow for the possibility of a per-guest choice of what action to take (maybe via a new on_host_shutdown element?) where we only fall back to the configured default if the guest XML didn't request something? Do we correctly and automatically restart all the guests that were saved by this hook the next time libvirtd restarts? We don't attempt to automatically restart all guests - we will only restart those marked as autostart, which I think is the right thing todo in general. Well, with 'libvirt-guests', we restarted everything that had been saved, not just those guests marked autostart. (Or another way of looking at it: saving a guest on libvirtd shutdown resulted in the guest behaving as if it had a one-shot autostart). I think it may be a bit awkward if session behaves differently from libvirt-guests regarding the resume of guests that are not autostart. I think managed save is the only reasonable choice here, because we want something that is going to be reliable, and zero-conf for the user. Migration requires the user to pick a dest host, and is not guaranteed to converge. Graceful shutdown relies on a co-operating guest. So IMHO, neither of those are really satisfactory options for running on desktop logout / host shutdown. I agree that managed save is the only reasonable default, but I still wonder if we need to allow for non-default configurations, rather than forcing managed save as the only solution. Not sure about O_DIRECT - I'm inclined to say we should just *always* use O_DIRECT - unless someone can point out a downside with it ? About the only possible downside I can see is that it _might_ be slightly slower, as it forces libvirt to take all the I/O through a pipe before playing it out to the file system; which is why we exposed the option to request O_DIRECT when I first implemented it. But later, we switched to always using a pipe anyways, because current qemu doesn't know how to throttle when writing to regular files (it only properly does bandwidth throttling on a file descriptor that can report EAGAIN, such as a pipe), so now that we are always using a pipe, always using O_DIRECT might make sense as a default (and make the existing flag for requesting O_DIRECT become a no-op because we are automatically using direct mode to begin with). Thoughts? If we make O_DIRECT unconditional, then we have several followup patches to write. Long term, I make no secret of the fact that I want to see libvirt-guests die in horribly painful way. This kind of functionality should be brought in house to libvirtd and obviously this new code is a start in that direction. Agree. +virDomainSuspend(domains[i]); Should you be checking for errors here? Yes no. Pausing the guests is just a performance tweak, so if it fails it isn't critical. If pausing fails, chances are the managed save willl fail too, so we'll likely get an erorr regardless. Fair enough. +} + +ret = 0; +/* Then we save the VMs to disk */ +for (i = 0 ; i numDomains ; i++) +if (virDomainManagedSave(domains[i], flags[i]) 0) +ret = -1; What happens if a guest managed to exit itself after the initial virConnectListAllDomains() and this point? The virDomainManagedSave will fail, but that is not an error since a stopped guest has nothing to save after all. Do you need to be checking specific error codes here before treating this as a failure? I guess we could handle that - though the caller will ignore all the errors anyway :-) At this point, I think you've answered my questions. I'm not sure if you made any minor tweaks based on my feedback, but it looks like this patch is ready to push and any future work can be in followup patches (since we are already planning for
Re: [libvirt] [PATCH 08/10] Replace polling for active VMs with signalling by drivers
Currently to deal with auto-shutdown libvirtd must periodically
poll all stateful drivers. Thus sucks because it requires
acquiring both the driver lock and locks on every single virtual
machine. Instead pass in a inhibit callback to
virStateInitialize
which drivers can invoke whenever they want to inhibit shutdown
due to existance of active VMs.
+nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED,
+ virStateInhibitCallback callback
ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
{
Here, you aren't remembering the callback...
Yes, this is technically a semantic change, I could have pulled
into a separate patch.
Basically there is no compelling reason for the nwfilter driver
to inhibit shutdown. Active NWfilters are all associated with
active domains, which will already be inhibiting shutdown.
True.
+storageDriverStartup(bool privileged,
+ virStateInhibitCallback callback
ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
Another case of ignoring the callback...
Again this is because, IMHO, there is no compelling reason for active
storage pools to inhibit libvirtd shutdown. Indeed inhibiting it
makes
auto-shutdown pretty much useless, since you'll almost always have an
active directory based storage pool.
Probably worth mentioning these intentional changes in the commit
log, but you've convinced me that the code is sane. ACK.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10] Inhibit desktop shutdown while any virtual machines are running
Use the freedesktop inhibition DBus service to prevent host
shutdown or session logout while any VMs are running.
+#ifdef HAVE_DBUS
+# include dbus/dbus.h
Do we really need this header...
No, its obsolete.
Well, I _did_ see some dbus_ calls in this file:
+#ifdef HAVE_DBUS
+static void virNetServerGotInhibitReply(DBusPendingCall *pending,
+void *opaque)
+{
+
+reply = dbus_pending_call_steal_reply(pending);
...or is this local header sufficient? (That is, should you rework
this patch to put the raw dbus_* calls isolated into virdbus.[ch],
rathar than having this file have to use conditional compilation)?
Correct.
So I'm assuming here I should wait for you to post a v2 that actually
does this refactoring.
+VIR_DEBUG(srv=%p inhibitions=%zu, srv,
srv-autoShutdownInhibitions);
Again, should this debug be hoisted into an earlier patch?
I don't think it really matters - this patch is the first time that
it
is interesting to see the debug info :-)
Fair enough on this point, though :)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] Shut down session libvirtd cleanly on host shutdown/user logout
When the session dies or when the system is going to be shut down we issue a virStateStop() call to instruct drivers to prepare to be stopped. This will remove any previously acquire inhibitions. +#ifdef HAVE_DBUS +# include dbus/dbus.h Again, is this necessary, +# include virdbus.h or should we be hiding the interface into virdbus.h, which can be unconditionally included? Correct, it is bogus But that still raises the question of whether this file should be making raw dbus_* calls, or whether that should be factored into virdbus.h. Again, looks slick; and my objection earlier in the series about qemu:///system vs. libvirt-guests init script may have been premature, seeing as how you are tying this inhibition handling only to sessions. Still, I'd be interested in your responses to my questions before granting ack. I think at this point I have given ack to 1-8, and am waiting to see if you plan a v2 for patches 9 and 10 to push more raw dbus_* actions into virdbus.c. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Add APIs for talking to init via /dev/initctl
To be able todo controlled shutdown/reboot of containers an
s/todo/to do/
API to talk to init via /dev/initctl is required. Fortunately
this is quite straightforward to implement, and is supported
by both sysvinit and systemd. Upstart support for /dev/initctl
is unclear.
+++ b/src/util/virinitctl.c
@@ -0,0 +1,161 @@
+
+/* These constants struct definitions are taken from
+ * systemd, under terms of LGPLv2+
+ *
+ * initreq.hInterface to talk to init through /dev/initctl.
+ *
+ * Copyright (C) 1995-2004 Miquel van Smoorenburg
+ */
Thankfully, compatible with our usage.
+
+#if defined(__FreeBSD_kernel__)
+# define VIR_INITCTL_FIFO /etc/.initctl
+#else
+# define VIR_INITCTL_FIFO /dev/initctl
+#endif
+
+#define VIR_INITCTL_MAGIC 0x03091969
Wonder if the original author of this code picked a birthdate. Gotta
love the Easter eggs present in open source software :)
+
+/*
+ * Because of legacy interfaces, runlevel and sleeptime
+ * aren't in a separate struct in the union.
+ *
+ * The weird sizes are because init expects the whole
+ * struct to be 384 bytes.
+ */
+struct virInitctlRequest {
+int magic; /* Magic number
*/
'int' is not necessarily 4 bytes; I would feel slightly more
comfortable had upstream used int32_t. I know you are just copying
code from an existing header (so don't change the struct itself),
but wonder if we should at least add our own sanity checking:
verify(sizeof(virInitctlRequest)) == 384);
+
+if ((fd = open(path, O_WRONLY|O_NDELAY|O_CLOEXEC|O_NOCTTY)) 0)
O_NDELAY is non-standard. I would spell it according to POSIX as
O_NONBLOCK.
+typedef enum virInitctlRunLevel virInitctlRunLevel;
+enum virInitctlRunLevel {
+VIR_INITCTL_RUNLEVEL_POWEROFF = 0,
+VIR_INITCTL_RUNLEVEL_1 = 1,
+VIR_INITCTL_RUNLEVEL_2 = 2,
+VIR_INITCTL_RUNLEVEL_3 = 3,
+VIR_INITCTL_RUNLEVEL_4 = 4,
+VIR_INITCTL_RUNLEVEL_5 = 5,
+VIR_INITCTL_RUNLEVEL_REBOOT = 6,
Should you add VIR_INITCTL_RUNLEVEL_LAST, in case we ever
expand this list?
I've never used messages over the socket to initctl myself, but
I'm assuming your code works. ACK once you fix the nits I've
pointed out above.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] qemu: support live update of an interface's filter
These two patches enable making a live change to the nwfilter of a guest's interface via virDomainUpdateDeviceFlags (virsh update-device). Differences from V1: 1) add patch from Stefan Berger to do a proper comparison of the values stored in the filterparams hashtable. 2) simplify virNWFilterHashTableEqual to use Stefan's new function, and remove a couple of pointless comparisons based on Stefan's review. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] nwfilter: utility function virNWFilterVarValueEqual
From: Stefan Berger stef...@us.ibm.com
To detect if an interface's nwfilter has changed, we need to also
compare the filterparams, which is a hashtable of virNWFilterVarValue.
virHashEqual can do this nicely, but requires a pointer to a function
that will compare two of the items being stored in the hashes.
---
src/conf/nwfilter_params.c | 31 +++
src/conf/nwfilter_params.h | 2 ++
src/libvirt_private.syms | 1 +
3 files changed, 34 insertions(+)
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index 6dc4baa..3ac1303 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -189,6 +189,37 @@ virNWFilterVarValueGetCardinality(const
virNWFilterVarValuePtr val)
return 0;
}
+bool
+virNWFilterVarValueEqual(const virNWFilterVarValuePtr a,
+ const virNWFilterVarValuePtr b)
+{
+unsigned int card, i, j;
+const char *s;
+
+if (!a || !b)
+return false;
+
+card = virNWFilterVarValueGetCardinality(a);
+if (card != virNWFilterVarValueGetCardinality(b))
+return false;
+
+/* brute force O(n^2) comparison */
+for (i = 0; i card; i++) {
+bool eq = false;
+
+s = virNWFilterVarValueGetNthValue(a, i);
+for (j = 0; j card; j++) {
+if (STREQ_NULLABLE(s, virNWFilterVarValueGetNthValue(b, j))) {
+ eq = true;
+ break;
+}
+}
+if (!eq)
+return false;
+}
+return true;
+}
+
int
virNWFilterVarValueAddValue(virNWFilterVarValuePtr val, char *value)
{
diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h
index cedf9cd..96d3033 100644
--- a/src/conf/nwfilter_params.h
+++ b/src/conf/nwfilter_params.h
@@ -57,6 +57,8 @@ const char *virNWFilterVarValueGetSimple(const
virNWFilterVarValuePtr val);
const char *virNWFilterVarValueGetNthValue(virNWFilterVarValuePtr val,
unsigned int idx);
unsigned int virNWFilterVarValueGetCardinality(const virNWFilterVarValuePtr);
+bool virNWFilterVarValueEqual(const virNWFilterVarValuePtr a,
+ const virNWFilterVarValuePtr b);
int virNWFilterVarValueAddValue(virNWFilterVarValuePtr val, char *value);
int virNWFilterVarValueDelValue(virNWFilterVarValuePtr val, const char *value);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2573b8a..ada73fb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -968,6 +968,7 @@ virNWFilterVarValueCopy;
virNWFilterVarValueCreateSimple;
virNWFilterVarValueCreateSimpleCopyValue;
virNWFilterVarValueDelValue;
+virNWFilterVarValueEqual;
virNWFilterVarValueFree;
virNWFilterVarValueGetCardinality;
virNWFilterVarValueGetNthValue;
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] qemu: support live update of an interface's filter
Since we can't (currently) rely on the ability to provide blanket
support for all possible network changes by calling the toplevel
netdev hostside disconnect/connect functions (due to qemu only
supporting a lockstep between initialization of host side and guest
side of devices), in order to support live change of an interface's
nwfilter we need to make a special purpose function to only call the
nwfilter teardown and setup functions if the filter for an interface
(or its parameters) changes. The pattern is nearly identical to that
used to change the bridge that an interface is connected to.
This patch was inspired by a request from Guido Winkelmann
gu...@sagersystems.de, who tested an earlier version.
---
src/conf/nwfilter_params.c | 21 ++
src/conf/nwfilter_params.h | 4 +++-
src/libvirt_private.syms | 1 +
src/qemu/qemu_hotplug.c| 55 +++---
4 files changed, 77 insertions(+), 4 deletions(-)
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index 3ac1303..7254519 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -795,6 +795,27 @@ err_exit:
return -1;
}
+/* The general purpose function virNWFilterVarValueEqual returns a
+ * bool, but the comparison callback for virHashEqual (called below)
+ * needs to return an int of 0 for == and non-0 for !=
+ */
+static int
+virNWFilterVarValueCompare(const void *a, const void *b)
+{
+return virNWFilterVarValueEqual((const virNWFilterVarValuePtr) a,
+(const virNWFilterVarValuePtr) b) ? 0 : 1;
+}
+
+bool
+virNWFilterHashTableEqual(virNWFilterHashTablePtr a,
+ virNWFilterHashTablePtr b)
+{
+if (!(a || b))
+return true;
+if (!(a b))
+return false;
+return virHashEqual(a-hashTable, b-hashTable,
virNWFilterVarValueCompare);
+}
static bool
isValidVarName(const char *var)
diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h
index 96d3033..6c3ce54 100644
--- a/src/conf/nwfilter_params.h
+++ b/src/conf/nwfilter_params.h
@@ -1,7 +1,7 @@
/*
* nwfilter_params.h: parsing and data maintenance of filter parameters
*
- * Copyright (C) 2011 Red Hat, Inc.
+ * Copyright (C) 2011-2012 Red Hat, Inc.
* Copyright (C) 2010 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -87,6 +87,8 @@ void *virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr
table,
const char *name);
int virNWFilterHashTablePutAll(virNWFilterHashTablePtr src,
virNWFilterHashTablePtr dest);
+bool virNWFilterHashTableEqual(virNWFilterHashTablePtr a,
+ virNWFilterHashTablePtr b);
# define VALID_VARNAME \
abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ada73fb..5598682 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -952,6 +952,7 @@ virNWFilterIPAddrMapShutdown;
# nwfilter_params.h
virNWFilterHashTableCreate;
+virNWFilterHashTableEqual;
virNWFilterHashTableFree;
virNWFilterHashTablePut;
virNWFilterHashTablePutAll;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 0bc2259..5058198 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1330,6 +1330,43 @@ cleanup:
return ret;
}
+static int
+qemuDomainChangeNetFilter(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virDomainNetDefPtr olddev,
+ virDomainNetDefPtr newdev)
+{
+/* make sure this type of device supports filters. */
+switch (virDomainNetGetActualType(newdev)) {
+case VIR_DOMAIN_NET_TYPE_ETHERNET:
+case VIR_DOMAIN_NET_TYPE_BRIDGE:
+case VIR_DOMAIN_NET_TYPE_NETWORK:
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(filters not supported on interfaces of type %s),
+
virDomainNetTypeToString(virDomainNetGetActualType(newdev)));
+return -1;
+}
+
+virDomainConfNWFilterTeardown(olddev);
+
+if (virDomainConfNWFilterInstantiate(conn, vm-def-uuid, newdev) 0) {
+virErrorPtr errobj;
+
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(failed to add new filter rules to '%s'
+ - attempting to restore old rules),
+ olddev-ifname);
+errobj = virSaveLastError();
+ignore_value(virDomainConfNWFilterInstantiate(conn, vm-def-uuid,
olddev));
+virSetError(errobj);
+virFreeError(errobj);
+return -1;
+}
+return 0;
+}
+
int qemuDomainChangeNetLinkState(struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainNetDefPtr dev,
@@ -1373,6 +1410,7 @@
Re: [libvirt] [PATCH 2/3] Introduce two new methods for triggering controlled shutdown/reboot
The virDomainShutdownFlags and virDomainReboot APIs allow the caller
to request the operation is implemented via either acpi button press
or a guest agent. For containers, a couple of other methods make
sense, a message to /dev/initctl, and direct kill(SIGTERM|HUP) of
the container init process.
Indeed - this is a nice way to tie in your earlier patches.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
include/libvirt/libvirt.h.in | 4
tools/virsh-domain.c | 18 ++
2 files changed, 18 insertions(+), 4 deletions(-)
Alas, you are missing documentation of the new flags in
src/libvirt.c (not that the existing flags were documented
there yet), as well as missing an update to this portion
of libvirt.c:virDomainShutdownFlags() (and its reboot
counterpart):
/* At most one of these two flags should be set. */
if ((flags VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN)
(flags VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) {
virReportInvalidArg(flags, %s,
_(flags for acpi power button and guest agent are
mutually exclusive));
goto error;
}
That should be updated to check that now at most one of the
four flags are present.
Everything else looks okay, but to ensure we get decent
documentation, I'll defer ack until you post v2.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] Add support for shutdown / reboot APIs in LXC driver
Add support for doing controlled shutdown / reboot in the LXC
driver. The default behaviour is to try talking to /dev/initctl
inside the container's virtual root (/proc/$INITPID/root). This
works with sysvinit or systemd. If that file does not exist
then send SIGTERM (for shutdown) or SIGHUP (for reboot). These
signals are not any kind of particular standard for shutdown
or reboot, just something apps can choose to handle. The new
virDomainSendProcessSignal allows for sending custom signals.
We might allow the choice of SIGTERM/HUP to be configured for
LXC containers via the XML in the future.
Maybe even via new attributes or sub-elements of the existing
on_reboot XML.
+
+if (flags == 0 ||
+(flags VIR_DOMAIN_SHUTDOWN_INITCTL)) {
+if ((rc =
virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
+vroot)) 0) {
+goto cleanup;
+}
+if (rc == 0 flags != 0
Based on libvirt.c (well, if you follow my advice in 2/3 about
enforcing the mutual exclusion between the four explicit
shutdown methods), if 'flags != 0', then we are guaranteed
that 'flags == VIR_DOMAIN_SHUTDOWN_INITCTL)' at this point...
+((flags ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
...which means this leg of the conditional is always true.
Or is your intent to allow the user to specify multiple flags
rather than enforcing mutual exclusion between the flags? And
if so, you need to fix libvirt.c to drop the mutual exclusion,
as well as to document that when multiple flags are specified,
it is up to the hypervisor which method is attempted first.
+static int
+lxcDomainReboot(virDomainPtr dom,
+unsigned int flags)
+{
+virLXCDriverPtr driver = dom-conn-privateData;
+virLXCDomainObjPrivatePtr priv;
+virDomainObjPtr vm;
+char *vroot = NULL;
+int ret = -1;
+int rc;
+
+virCheckFlags(VIR_DOMAIN_REBOOT_INITCTL |
+ VIR_DOMAIN_REBOOT_SIGNAL, -1);
This code is very similar to the shutdown; is it worth factoring
out a helper method that takes as additional arguments the choice
of initctl message (_POWEROFF vs. _REBOOT) and signal (SIGKILL
vs. SIGHUP) to use?
+if (flags == 0 ||
+(flags VIR_DOMAIN_REBOOT_INITCTL)) {
+if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
+vroot)) 0) {
+goto cleanup;
+}
+if (rc == 0 flags != 0
+((flags ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
Same review comments apply regarding whether we enforce mutual
exclusion of flags at the API level.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Refactor config parameter retrieval
This patch adds macros to help retrieve configuration values from qemu driver's configuration. Some configuration options are grouped together in the process. --- src/qemu/qemu_conf.c | 303 +-- 1 file changed, 73 insertions(+), 230 deletions(-) Always fun to see refactoring that reduces code size. +#define GET_VALUE_LONG(NAME, VAR) p = virConfGetValue(conf, NAME); \ This feels like a long line; I'd format it slightly differently: #define GET_VALUE_LONG(NAME, VAR) \ p = virConfGetValue(conf, NAME);\ ... + CHECK_TYPE(NAME, VIR_CONF_LONG); \ + if (p) VAR = p-l; As long as you are refactoring, can you please split this line into two: if (p) \ VAR = p-l per our coding conventions? + +#define GET_VALUE_STR(NAME, VAR) p = virConfGetValue(conf, NAME); \ Again, this feels like a lot of indentation; starting the first line of the macro body after a continuation will get rid of a lot of this whitespace. +/* increasing the value by 1 makes all the loops going through +the bitmap (i = remotePortMin; i remotePortMax; i++), work as +expected. */ +driver-remotePortMax += 1; Why ' += 1' instead of the shorter '++'? +GET_VALUE_LONG(max_queued, driver-max_queued); +GET_VALUE_LONG(keepalive_interval, driver-keepAliveInterval); +GET_VALUE_LONG(keepalive_count, driver-keepAliveCount); +GET_VALUE_LONG(seccomp_sandbox, driver-seccompSandbox); virConfFree(conf); return 0; Should we add #undef GET_VALUE_LONG #undef GET_VALUE_STRING at the end, since our macros are merely local helpers for this function? ACK with nits addressed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] IPv6 enhancements; put dnsmasq parameters in conf-file
These three patch files are packaged together because they serially depend on each other. These files have been rebased to v3 of the dnsmasq capabilities and bind-dynamic patches. The DHCPv6 support checks dnsmasq's version and requires a minimum of 2.64. Also, using dnsmasq for providing the RA service is checked against the dnsmasq version and is currently 2.64. As with IPv4, IPv6 DHCP is only one subnetwork on an interface. Additionally, if other IPv6 addresses are defined, a warning message is issued since the Router Advertisement service will support only state-full (DHCP) or state-less (SLAAC) addressing on a network interface (not both). Thus, the additional subnetworks will need to be manually configured to properly function. If dnsmasq provides the RA service, it also points to itself as a RDNSS (Recursive DNS Server) as part of the information is supplies. If IPv6 DHCP is not being run, then SLAAC addressing is supported for any IPv6 addresses specified. Gene Czarcinski (3): v1: allow guest to guest IPv6 without gateway definition v8.1 add support for DHCPv6 v7.9: put dnsmasq parameters into conf-file docs/formatnetwork.html.in | 126 - docs/schemas/network.rng | 12 +- src/conf/network_conf.c| 100 ++-- src/network/bridge_driver.c| 590 ++--- src/network/bridge_driver.h| 7 +- src/util/dnsmasq.c | 9 +- tests/networkxml2argvdata/dhcp6-nat-network.argv | 14 + tests/networkxml2argvdata/dhcp6-nat-network.xml| 24 + tests/networkxml2argvdata/dhcp6-network.argv | 14 + tests/networkxml2argvdata/dhcp6-network.xml| 14 + .../dhcp6host-routed-network.argv | 12 + .../dhcp6host-routed-network.xml | 19 + tests/networkxml2argvdata/isolated-network.argv| 25 +- .../networkxml2argvdata/nat-network-dns-hosts.argv | 14 +- .../nat-network-dns-srv-record-minimal.argv| 34 +- .../nat-network-dns-srv-record.argv| 24 +- .../nat-network-dns-txt-record.argv| 22 +- tests/networkxml2argvdata/nat-network.argv | 22 +- tests/networkxml2argvdata/netboot-network.argv | 28 +- .../networkxml2argvdata/netboot-proxy-network.argv | 26 +- tests/networkxml2argvdata/routed-network.argv | 11 +- tests/networkxml2argvtest.c| 47 +- 22 files changed, 838 insertions(+), 356 deletions(-) create mode 100644 tests/networkxml2argvdata/dhcp6-nat-network.argv create mode 100644 tests/networkxml2argvdata/dhcp6-nat-network.xml create mode 100644 tests/networkxml2argvdata/dhcp6-network.argv create mode 100644 tests/networkxml2argvdata/dhcp6-network.xml create mode 100644 tests/networkxml2argvdata/dhcp6host-routed-network.argv create mode 100644 tests/networkxml2argvdata/dhcp6host-routed-network.xml -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] v1: allow guest to guest IPv6 without gateway definition
This patch adds the capability for virtual guests to do IPv6
communication via a virtual network interface with no IPv6
(gateway) addresses specified. This capability currently
exists for IPv4.
This patch allows creation of a completely isolated IPv6 network.
Note that virtual guests cannot communication with the virtualization
host via this interface. Also note that:
net.ipv6.conf.interface_name.disable_ipv6 = 1
Also not that starting libvirtd has set the following:
net.bridge.bridge-nf-call-arptables = 1
net.bridge.bridge-nf-call-ip6tables = 1
net.bridge.bridge-nf-call-iptables = 1
although /etc/syslog.conf has them all set to 0.
Note: If it is desired to control this behavior by having something like
ipv6='yes' on the network statement, then this should also be done for ipv4.
---
docs/formatnetwork.html.in | 18 ++
src/network/bridge_driver.c | 22 ++
2 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 49206dd..7b3b25c 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -773,5 +773,23 @@
lt;/forwardgt;
lt;/networkgt;/pre
+h3a name=examplesNoGatewayNetwork config with no gateway
addresses/a/h3
+
+p
+A valid network definition can contain no IPv4 or IPv6 addresses. Such a
definition
+can be used for a very private or very isolated network since it will
not be
+possible to communicate with the virtualization host via this network.
However,
+this virtual network interface can be used for communication between
virtual guest
+systems. This works for IPv4 and span class=since(Since 1.0.1)/span
IPv6.
+/p
+
+pre
+ lt;networkgt;
+lt;namegt;nogwlt;/namegt;
+lt;uuidgt;7a3b7497-1ec7-8aef-6d5c-38dff9109e93lt;/uuidgt;
+lt;bridge name=virbr2 stp=on delay=0 /gt;
+lt;mac address='00:16:3E:5D:C7:9E'/gt;
+ lt;/networkgt;/pre
+
/body
/html
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 479ff29..c306d46 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1616,15 +1616,16 @@ networkRemoveRoutingIptablesRules(struct network_driver
*driver,
}
}
-/* Add all once/network rules required for IPv6 (if any IPv6 addresses are
defined) */
+/* Add all once/network rules required for IPv6.
+ * Even if no IPv6 addresses are defined, allow IPv6 commuinications
+ * between virtual systems. If any IPv6 addresses are defined, then
+ * add the rules for regular operation.
+ */
static int
networkAddGeneralIp6tablesRules(struct network_driver *driver,
virNetworkObjPtr network)
{
-if (!virNetworkDefGetIpByIndex(network-def, AF_INET6, 0))
-return 0;
-
/* Catch all rules to block forwarding to/from bridges */
if (iptablesAddForwardRejectOut(driver-iptables, AF_INET6,
@@ -1652,6 +1653,10 @@ networkAddGeneralIp6tablesRules(struct network_driver
*driver,
goto err3;
}
+/* if no IPv6 addresses are defined, we are done. */
+if (!virNetworkDefGetIpByIndex(network-def, AF_INET6, 0))
+return 0;
+
/* allow DNS over IPv6 */
if (iptablesAddTcpInput(driver-iptables, AF_INET6,
network-def-bridge, 53) 0) {
@@ -1688,11 +1693,12 @@ static void
networkRemoveGeneralIp6tablesRules(struct network_driver *driver,
virNetworkObjPtr network)
{
-if (!virNetworkDefGetIpByIndex(network-def, AF_INET6, 0))
-return;
+if (virNetworkDefGetIpByIndex(network-def, AF_INET6, 0)) {
+iptablesRemoveUdpInput(driver-iptables, AF_INET6,
network-def-bridge, 53);
+iptablesRemoveTcpInput(driver-iptables, AF_INET6,
network-def-bridge, 53);
+}
-iptablesRemoveUdpInput(driver-iptables, AF_INET6, network-def-bridge,
53);
-iptablesRemoveTcpInput(driver-iptables, AF_INET6, network-def-bridge,
53);
+/* the following rules are there even if no IPv6 address has been defined
*/
iptablesRemoveForwardAllowCross(driver-iptables, AF_INET6,
network-def-bridge);
iptablesRemoveForwardRejectIn(driver-iptables, AF_INET6,
network-def-bridge);
iptablesRemoveForwardRejectOut(driver-iptables, AF_INET6,
network-def-bridge);
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] v7.9: put dnsmasq parameters into conf-file
This patch changes how parameters are passed to dnsmasq. Instead of
being on the command line, the parameters are put into a file (one
parameter per line) and a commandline --conf-file= specifies the
location of the file. The file is located in the same directory as
the leases file.
This also adds the dnsmasq parameter interface=net-name
Putting the dnsmasq parameters into a configuration file
allows them to be examined and more easily understood than
examining the command lines displayed by ps ax. This is
especially true when a number of networks have been started.
I suspect that when the use of dnsmasq was originally done,
the command line was simple but has gotten more complicated
over time and will likely become even more complicated in the
future.
I believe that if use of dnsmasq was done today with
the current requirements for dnsmasq, a configuration file
would have been used. Many (most?) daemons use configuration
files as oppose to large number of command line parameters.
One potential addition to dnsmasq parameters is to specify
the reverse-lookup queries which are not to be passed on up
the chain. For IPv4, such queries are rather simple and
take the form:
ipv4_address.in-addr.arpa
but ipv6 queries involve a lot longer specification. The
IPv6 query will be of the form:
ipv6_address.ip6.arpa
where the above query expands to a 44 character string which is
specified by --local=/ipv6_address.ip6.arpa/ which is
certainly a lot of clutter to add to the command line.
---
src/network/bridge_driver.c| 189 +
src/network/bridge_driver.h| 7 +-
tests/networkxml2argvdata/dhcp6-nat-network.argv | 29 ++--
tests/networkxml2argvdata/dhcp6-network.argv | 29 ++--
.../dhcp6host-routed-network.argv | 25 ++-
tests/networkxml2argvdata/isolated-network.argv| 31 ++--
.../networkxml2argvdata/nat-network-dns-hosts.argv | 19 +--
.../nat-network-dns-srv-record-minimal.argv| 37 ++--
.../nat-network-dns-srv-record.argv| 27 ++-
.../nat-network-dns-txt-record.argv| 26 +--
tests/networkxml2argvdata/nat-network.argv | 29 ++--
tests/networkxml2argvdata/netboot-network.argv | 37 ++--
.../networkxml2argvdata/netboot-proxy-network.argv | 33 ++--
tests/networkxml2argvdata/routed-network.argv | 15 +-
tests/networkxml2argvtest.c| 40 +
15 files changed, 286 insertions(+), 287 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b793d78..8360354 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -142,6 +142,16 @@ networkDnsmasqLeaseFileNameFunc
networkDnsmasqLeaseFileName =
networkDnsmasqLeaseFileNameDefault;
static char *
+networkDnsmasqConfigFileName(const char *netname)
+{
+char *conffile;
+
+ignore_value(virAsprintf(conffile, DNSMASQ_STATE_DIR /%s.conf,
+ netname));
+return conffile;
+}
+
+static char *
networkRadvdPidfileBasename(const char *netname)
{
/* this is simple but we want to be sure it's consistently done */
@@ -168,6 +178,7 @@ networkRemoveInactive(struct network_driver *driver,
{
char *leasefile = NULL;
char *radvdconfigfile = NULL;
+char *configfile = NULL;
char *radvdpidbase = NULL;
dnsmasqContext *dctx = NULL;
virNetworkDefPtr def = virNetworkObjGetPersistentDef(net);
@@ -187,9 +198,13 @@ networkRemoveInactive(struct network_driver *driver,
if (!(radvdpidbase = networkRadvdPidfileBasename(def-name)))
goto no_memory;
+if (!(configfile = networkDnsmasqConfigFileName(def-name)))
+goto no_memory;
+
/* dnsmasq */
dnsmasqDelete(dctx);
unlink(leasefile);
+unlink(configfile);
/* radvd */
unlink(radvdconfigfile);
@@ -202,6 +217,7 @@ networkRemoveInactive(struct network_driver *driver,
cleanup:
VIR_FREE(leasefile);
+VIR_FREE(configfile);
VIR_FREE(radvdconfigfile);
VIR_FREE(radvdpidbase);
dnsmasqContextFree(dctx);
@@ -635,12 +651,13 @@ networkBuildDnsmasqHostsList(dnsmasqContext *dctx,
static int
-networkBuildDnsmasqArgv(virNetworkObjPtr network,
+networkDnsmasqConfContents(virNetworkObjPtr network,
const char *pidfile,
-virCommandPtr cmd,
+char **configstr,
dnsmasqContext *dctx,
dnsmasqCapsPtr caps ATTRIBUTE_UNUSED)
{
+virBuffer configbuf = VIR_BUFFER_INITIALIZER;
int r, ret = -1;
int nbleases = 0;
int ii;
@@ -651,46 +668,43 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
virNetworkIpDefPtr tmpipdef, ipdef, ipv4def, ipv6def;
bool dhcp4flag, dhcp6flag, ipv6SLAAC;
+*configstr = NULL;
+
/*
- * NB, be careful about syntax for dnsmasq options in long format.
- *
- * If the flag has a
Re: [libvirt] [PATCHv3 1/3] util: capabilities detection for dnsmasq
On 11/28/2012 06:13 PM, Eric Blake wrote:
In order to optionally take advantage of new features in dnsmasq when
the host's version of dnsmasq supports them, but still be able to run
on hosts that don't support the new features, we need to be able to
detect the version of dnsmasq running on the host, and possibly
determine from the help output what options are in this dnsmasq.
networkxml2argvtest.c creates 2 artificial dnsmasqCaps objects at
startup - one restricted (doesn't support --bind-dynamic) and one
full (does support --bind-dynamic). Some of the test cases use one
and some the other, to make sure both code pathes are tested.
Should we also follow the lead of tests/qemuhelpdata/ and do an actual
capture from released dnsmasq binaries, to ensure that our parser for
those files matches our artificial dnsmasqCaps objects? More on this
below...[1]
I had thought about that when making this patch, but wanted to keep
things as simple as possible, since I'll probably have to backport it
all the way to 0.8.7 :-/ Because of that, I'd like to keep things as
they are for this patch, and expand it later.
+++ b/src/libvirt_private.syms
@@ -246,13 +246,19 @@ virDevicePCIAddressParseXML;
dnsmasqSave;
-
# domain_audit.h
Spurious whitespace change? Then again, we haven't been very
consistent on one vs. two blank lines between .h files.
Fixed.
@@ -891,7 +899,8 @@ cleanup:
}
static int
-networkStartDhcpDaemon(virNetworkObjPtr network)
+networkStartDhcpDaemon(struct network_driver *driver,
Given Dan's recent rename of 'struct qemud_driver *driver' to
the friendlier 'virQEMUDriverPtr driver', should we do a similar
change here? But if so, separate it to its own patch.
Yes, but as you say, in a separate patch.
@@ -935,7 +944,9 @@ networkStartDhcpDaemon(virNetworkObjPtr network)
if (dctx == NULL)
goto cleanup;
-ret = networkBuildDhcpDaemonCommandLine(network, cmd, pidfile,
dctx);
+dnsmasqCapsRefresh(driver-dnsmasqCaps, false);
+
+ret = networkBuildDhcpDaemonCommandLine(network, cmd, pidfile,
dctx, driver-dnsmasqCaps);
Long line; worth wrapping?
Fixed.
+++ b/src/network/bridge_driver.h
@@ -1,7 +1,7 @@
/*
* network_driver.h: core driver methods for managing networks
*
- * Copyright (C) 2006, 2007, 2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
You asked me about this one on IRC. If libvirt had a single
copyright holder, then I'd say this is okay (it follows the
convention[2] given by the FSF, where if a repository is publicly
available, then touching any one file in that repository counts
as a copyrightable claim on the package as a whole, and that
all files in the package may claim the same range of copyright
years as the package as a whole, even for files that were not
touched in a particular year). It is a bit more questionable
here (since we have NOT required anyone to assign copyright,
libvirt has a multitude of copyright holders), but I still
think you can get away with this change (Red Hat does indeed
own the copyrights on a vast majority of the code base, and
has made copyrightable contributions in every single year in
this range).
That last bit is the important part - the change is actually correct
since (as I confirmed by looking through the git history) this file (or
one of its predecessors, as it was moved/split a couple times) *was*
modified by someone from Red Hat at least once in every year in that range.
I guess it all boils down to how likely we are
to ever try and defend the copyright in court (FSF has a much
easier job of that task, thanks to them requiring copyright
assignment; whereas with libvirt, it is up to individual
copyright holders over their individual portions of the
overall work to decide what they would defend in court).
[2] https://www.gnu.org/prep/maintain/maintain.html#Copyright-Notices
To update the list of year numbers, add each year in which you have made
nontrivial changes to the package. (Here we assume you’re using a publicly
accessible revision control server, so that every revision installed is also
immediately and automatically published.) When you add the new year, it is
not required to keep track of which files have seen significant changes in
the new year and which have not. It is recommended and simpler to add the new
year to all files in the package, and be done with it for the rest of the
year.
You can use a range (‘2008-2010’) instead of listing individual years
(‘2008, 2009, 2010’) if and only if: 1) every year in the range, inclusive,
really is a “copyrightable” year that would be listed individually; and 2)
you make an explicit statement in a README file about this usage.
+
+#define SKIP_BLANKS(p) do { while ((*(p) == ' ') || (*(p) == '\t'))
(p)++; } while (0)
Could we use virSkipSpaces() instead of open-coding this?
I was just copying exactly what's already in qemu_capabilities.c,
without even
Re: [libvirt] [PATCHv3 3/3] network: use dnsmasq --bind-dynamic when available
On 11/28/2012 06:32 PM, Eric Blake wrote: This bug resolves CVE-2012-3411, which is described in the following bugzilla report: https://bugzilla.redhat.com/show_bug.cgi?id=833033 The following report is specifically for libvirt on Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=874702 In short, a dnsmasq instance run with the intention of listening for DHCP/DNS requests only on a libvirt virtual network (which is constructed using a Linux host bridge) would also answer queries sent from outside the virtualization host. snip It's always nice to fully explain things in the commit message, as you have done here - not only does it make the reviewer's job easier today, but down the road, it will make it much easier to answer what the CVE was all about and who is impacted (or more specifically, that default installation is NOT impacted). Thanks for taking the time to write it up. ACK. And let's get this in, so distros can start backporting the CVE fix for the sake of those people who ARE impacted. Thanks! I've pushed the entire series. I suppose I should now get to the backports... -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 2/3] util: new virSocketAddrIsPrivate function
On 11/28/2012 06:23 PM, Eric Blake wrote:
- Original Message -
This new function returns true if the given address is in the range
of
any private or local networks as defined in RFC1918 (IPv4) or
RFC3484/RFC4193 (IPv6), otherwise they return false.
These ranges are:
192.168.0.0/16
172.16.0.0/16
10.0.0.0/24
FC00::/7
FEC0::/10
---
src/libvirt_private.syms | 1 +
src/util/virsocketaddr.c | 34 +-
src/util/virsocketaddr.h | 3 ++-
3 files changed, 36 insertions(+), 2 deletions(-)
/*
+ * virSocketAddrIsPrivate:
+ * @s: the location of the IP address
+ *
+ * Return true if this address is in its family's defined
+ * private/local address space. For IPv4, private addresses are in
+ * the range of 192.168.0.0/16, 172.16.0.0/16, or 10.0.0.0/8. For
+ * IPv6, local addresses are in the range of FC00::/7.
+ *
+ * See RFC1918 and RFC4193 for details.
Did you intend to update this comment to match the commit message
regarding FEC0::/10?
Yes. Thanks for noticing!
+ */
+bool
+virSocketAddrIsPrivate(const virSocketAddrPtr addr)
+{
+unsigned long val;
+
+switch (addr-data.stor.ss_family) {
+case AF_INET:
+ val = ntohl(addr-data.inet4.sin_addr.s_addr);
+
+ return ((val 0x) == ((192L 24) + (168 16)) ||
+ (val 0x) == ((172L 24) + (16 16)) ||
+ (val 0xFF00) == ((10L 24)));
+
+case AF_INET6:
+return ((addr-data.inet6.sin6_addr.s6_addr[0] 0xFC) ==
0xFC ||
Isn't this FC00::/6? You should be using ' 0xFE' for /7.
Gulp. Yes. Fixed.
ACK with those fixes.
Pushed with those fixes.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: Refactor error reporting in qemu driver configuration parser
This patch adds two labels and gets rid of a ton of duplicated code. This patch also fixes some error message and swtiches most of them to s/swtiches/switches/ proper error reporting functions. --- src/qemu/qemu_conf.c | 194 +-- 1 file changed, 78 insertions(+), 116 deletions(-) And even more cleanup - nice. ACK. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] problems with current libvirt and qemu-kvm
When I rebased my DHCPv6, etc. patches, I did that not only on to of Laine Stump's dnsmasq-capabiliotes/bind-dynamic updates but I also rebased to current git master. Well, the problem with the large number/frequent RTR-ADVERT syslog messages from dnsmasq that I had previously seen were fixed ... they stopped and dnsmasq produced expected results. But, starting virt-manager hung and there were new error messages in syslog about internal error cannot find suitable emulator for X86_64. So I downgraded to a previous version that I knew worked and it did but my RTR-ADVERT messages are back. The good version uses a tarball created from git on November 19th and the bad version has a tarball created from git on November 29th. OK, before I start shuffling through the patches applied between those two date, does anyone have an idea what patches might be guilty ... for both issues ... fixing the RTR-ADVERT problem and giving libvirtd/qemu-kvm indigestion? I am not griping. When you run on the leading edge, you expect a few arrows in your back. Any suggesting on testing will also be appreciated. Gene -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] problems with current libvirt and qemu-kvm
So I downgraded to a previous version that I knew worked and it did but my RTR-ADVERT messages are back. The good version uses a tarball created from git on November 19th and the bad version has a tarball created from git on November 29th. Have you ever used 'git bisect' before? It makes the task of whittling down to an offending commit very easy, particularly if you can write a shell script that will automate the steps needed to prove if the bug is present for that particular git commit when you have two other known commits where one is bad and the other is good. 'man git-bisect', and look at the details about 'bisect run'. Meanwhile, are you comfortable using gdb? The error message of internal error cannot find suitable emulator for X86_64. seems like it occurs early enough in the startup that it should be possible to find the function that prints that message, stick a gdb breakpoint at the start of that function, run libvirtd, and inspect the backtrace, to at least get an idea of what file and its callers are worth narrowing down on your search for a culprit that prevent you from using latest git sources. Alas, I'm not having any problems using qemu-kvm from a just-built libvirt.git on my machine, so I'm not sure why it is failing for you. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Refactor config parameter retrieval
On 11/29/12 20:04, Eric Blake wrote: This patch adds macros to help retrieve configuration values from qemu driver's configuration. Some configuration options are grouped together in the process. --- src/qemu/qemu_conf.c | 303 +-- 1 file changed, 73 insertions(+), 230 deletions(-) Always fun to see refactoring that reduces code size. Indeed! +#define GET_VALUE_LONG(NAME, VAR) p = virConfGetValue(conf, NAME); \ This feels like a long line; I'd format it slightly differently: #define GET_VALUE_LONG(NAME, VAR) \ p = virConfGetValue(conf, NAME);\ ... + CHECK_TYPE(NAME, VIR_CONF_LONG); \ + if (p) VAR = p-l; As long as you are refactoring, can you please split this line into two: if (p) \ VAR = p-l per our coding conventions? Ah yeah ... I was doing the conversions in a kind of braindead way. + +#define GET_VALUE_STR(NAME, VAR) p = virConfGetValue(conf, NAME); \ Again, this feels like a lot of indentation; starting the first line of the macro body after a continuation will get rid of a lot of this whitespace. +/* increasing the value by 1 makes all the loops going through +the bitmap (i = remotePortMin; i remotePortMax; i++), work as +expected. */ +driver-remotePortMax += 1; Why ' += 1' instead of the shorter '++'? It was pre-existing but I'm touching the line anyways so I'll change that. +GET_VALUE_LONG(max_queued, driver-max_queued); +GET_VALUE_LONG(keepalive_interval, driver-keepAliveInterval); +GET_VALUE_LONG(keepalive_count, driver-keepAliveCount); +GET_VALUE_LONG(seccomp_sandbox, driver-seccompSandbox); virConfFree(conf); return 0; Should we add #undef GET_VALUE_LONG #undef GET_VALUE_STRING at the end, since our macros are merely local helpers for this function? Definitely. ACK with nits addressed. Pushed, thanks. -- 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
Re: [libvirt] [PATCH 2/2] qemu: Refactor error reporting in qemu driver configuration parser
On 11/29/12 21:14, Eric Blake wrote: This patch adds two labels and gets rid of a ton of duplicated code. This patch also fixes some error message and swtiches most of them to s/swtiches/switches/ proper error reporting functions. --- src/qemu/qemu_conf.c | 194 +-- 1 file changed, 78 insertions(+), 116 deletions(-) And even more cleanup - nice. ACK. Pushed; Thanks! Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] storage: fix scsi detach regression with cgroup ACLs
https://bugzilla.redhat.com/show_bug.cgi?id=876828 Commit 38c4a9cc introduced a regression in hot unplugging of disks from qemu, where cgroup device ACLs were no longer being revoked (thankfully not a security hole: cgroup ACLs only prevent open() of the disk; so reverting the ACL prevents future abuse but doesn't stop abuse from an fd that was already opened before the ACL change). Commit 1b2ebf95 overlooked that there were two spots affected. * src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Transfer backing chain before deletion. * src/qemu/qemu_driver.c (qemuDomainDetachDeviceDiskLive): Fix spacing (partly to ensure a different-looking patch). --- I blame git for letting me find this - I did a 'pull --rebase' on top of libvirt.git, and noticed that my working patch was still on the tree - it turns out that the hunk for qemu_hotplug.c is _identical_ except for the context of the function name needing a fix. I still wish git would be more vocal when it finds an alternate place to apply a patch when function names don't match. src/qemu/qemu_driver.c |2 +- src/qemu/qemu_hotplug.c |2 ++ 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c37bdb9..ae98dbf 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6070,7 +6070,7 @@ qemuDomainDetachDeviceDiskLive(virQEMUDriverPtr driver, if (disk-bus == VIR_DOMAIN_DISK_BUS_VIRTIO) ret = qemuDomainDetachPciDiskDevice(driver, vm, dev); else if (disk-bus == VIR_DOMAIN_DISK_BUS_SCSI) -ret = qemuDomainDetachDiskDevice(driver, vm, dev); +ret = qemuDomainDetachDiskDevice(driver, vm, dev); else if (dev-data.disk-bus == VIR_DOMAIN_DISK_BUS_USB) ret = qemuDomainDetachDiskDevice(driver, vm, dev); else diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index cfeae68..2d4a822 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2091,6 +2091,8 @@ int qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, virDomainDiskRemove(vm-def, i); +dev-data.disk-backingChain = detach-backingChain; +detach-backingChain = NULL; virDomainDiskDefFree(detach); if (virSecurityManagerRestoreImageLabel(driver-securityManager, -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] Make QEMU perform managed save of all VMs on stop of libvirtd
Not sure about O_DIRECT - I'm inclined to say we should just *always* use O_DIRECT - unless someone can point out a downside with it ? About the only possible downside I can see is that it _might_ be slightly slower, as it forces libvirt to take all the I/O through a pipe before playing it out to the file system; which is why we exposed the option to request O_DIRECT when I first implemented it. Another downside - some people like to play with virtual machines in /tmp, but if /tmp is mounted on tmpfs, then O_DIRECT does not work. Seeing as how O_DIRECT is not mandated by POSIX, and several important file systems like tmpfs do not support it, we have to choose whether to default to something that fails on some file systems, and/or make our O_DIRECT code gracefully fall back rather than erroring out when not available. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: don't attempt undefined QMP commands
https://bugzilla.redhat.com/show_bug.cgi?id=872292
Libvirt should not attempt to call a QMP command that has not been
documented in qemu.git - if future qemu introduces a command by the
same name but with subtly different semantics, then libvirt will be
broken when trying to use that command.
See also this attempt to convert the three snapshot commands to QMP:
https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg01597.html
although it looks like that will still not happen before qemu 1.3.
That thread eventually decided that qemu would use the name
'save-vm' rather than 'savevm', which mitigates the fact that
libvirt's attempt to use a QMP 'savevm' would be broken, but we
might not be as lucky on the other commands.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONSetCPU)
(qemuMonitorJSONAddDrive, qemuMonitorJSONDriveDel)
(qemuMonitorJSONCreateSnapshot, qemuMonitorJSONLoadSnapshot)
(qemuMonitorJSONDeleteSnapshot): Use only HMP fallback for now.
(qemuMonitorJSONAddHostNetwork, qemuMonitorJSONRemoveHostNetwork)
(qemuMonitorJSONAttachDrive, qemuMonitorJSONGetGuestDriveAddress):
Delete; QMP implies QEMU_CAPS_DEVICE, which prefers AddNetdev,
RemoveNetdev, and AddDrive anyways.
* src/qemu/qemu_monitor.c (qemuMonitorAddHostNetwork)
(qemuMonitorRemoveHostNetwork, qemuMonitorAttachDrive): Reflect
deleted commands.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONAddHostNetwork)
(qemuMonitorJSONRemoveHostNetwork, qemuMonitorJSONAttachDrive):
Likewise.
---
src/qemu/qemu_monitor.c |9 +-
src/qemu/qemu_monitor_json.c | 311 --
src/qemu/qemu_monitor_json.h | 12 --
3 files changed, 31 insertions(+), 301 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index aef5044..43e45ef 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2387,7 +2387,8 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon,
}
if (mon-json)
-ret = qemuMonitorJSONAddHostNetwork(mon, netstr);
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(JSON monitor should be using netdev_add));
else
ret = qemuMonitorTextAddHostNetwork(mon, netstr);
@@ -2418,7 +2419,8 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon,
}
if (mon-json)
-ret = qemuMonitorJSONRemoveHostNetwork(mon, vlan, netname);
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(JSON monitor should be using netdev_del));
else
ret = qemuMonitorTextRemoveHostNetwork(mon, vlan, netname);
return ret;
@@ -2548,7 +2550,8 @@ int qemuMonitorAttachDrive(qemuMonitorPtr mon,
}
if (mon-json)
-ret = qemuMonitorJSONAttachDrive(mon, drivestr, controllerAddr,
driveAddr);
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(JSON monitor should be using AddDrive));
else
ret = qemuMonitorTextAttachDrive(mon, drivestr, controllerAddr,
driveAddr);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9fcd7e8..195d89f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2094,44 +2094,9 @@ cleanup:
int qemuMonitorJSONSetCPU(qemuMonitorPtr mon,
int cpu, int online)
{
-int ret;
-virJSONValuePtr cmd = qemuMonitorJSONMakeCommand(cpu_set,
- U:cpu, (unsigned long
long)cpu,
- s:state, online ?
online : offline,
- NULL);
-virJSONValuePtr reply = NULL;
-if (!cmd)
-return -1;
-
-if ((ret = qemuMonitorJSONCommand(mon, cmd, reply)) 0)
-goto cleanup;
-
-if (qemuMonitorJSONHasError(reply, CommandNotFound)) {
-VIR_DEBUG(cpu_set command not found, trying HMP);
-ret = qemuMonitorTextSetCPU(mon, cpu, online);
-goto cleanup;
-}
-
-if (ret == 0) {
-/* XXX See if CPU soft-failed due to lack of ACPI */
-#if 0
-if (qemuMonitorJSONHasError(reply, DeviceNotActive) ||
-qemuMonitorJSONHasError(reply, KVMMissingCap))
-goto cleanup;
-#endif
-
-/* See if any other fatal error occurred */
-ret = qemuMonitorJSONCheckError(cmd, reply);
-
-/* Real success */
-if (ret == 0)
-ret = 1;
-}
-
-cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
-return ret;
+/* XXX Update to use QMP, if QMP ever adds support for cpu hotplug */
+VIR_DEBUG(no QMP support for cpu_set, trying HMP);
+return qemuMonitorTextSetCPU(mon, cpu, online);
}
@@ -2674,52 +2639,6 @@ int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon,
}
-int qemuMonitorJSONAddHostNetwork(qemuMonitorPtr mon,
- const char *netstr)
-{
-int ret;
-virJSONValuePtr cmd = qemuMonitorJSONMakeCommand(host_net_add,
-
