[libvirt] [libvirt-python PATCH] Improve error output when use getTime with a nonzero flags.
When give a nonzero flags to getTime, c_retval will get -1 and goto cleanup.
But py_retval still is NULL,so pass c_retval value to py_retval.
This will make the output message more correct.
error before use this patch:
SystemError: error return without exception set
after use the patch:
libvirtError: unsupported flags (0x1) in function qemuDomainGetTime
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libvirt-override.c b/libvirt-override.c
index 9ba87eb..eed8d50 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7758,6 +7758,7 @@ libvirt_virDomainGetTime(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args) {
LIBVIRT_END_ALLOW_THREADS;
if (c_retval 0)
+ py_retval = libvirt_intWrap(c_retval);
goto cleanup;
if (!(pyobj_seconds = libvirt_longlongWrap(seconds)) ||
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-python v2 PATCH] Improve error output when use getTime with a nonzero flags.
When give a nonzero flags to getTime, c_retval will get -1 and goto cleanup.
But py_retval still is NULL,so pass c_retval value to py_retval.
This will make the output message more correct.
error before use this patch:
SystemError: error return without exception set
after use the patch:
libvirtError: unsupported flags (0x1) in function qemuDomainGetTime
v1:
https://www.redhat.com/archives/libvir-list/2014-October/msg00482.html
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libvirt-override.c b/libvirt-override.c
index 9ba87eb..c779aa3 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7757,9 +7757,11 @@ libvirt_virDomainGetTime(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args) {
c_retval = virDomainGetTime(domain, seconds, nseconds, flags);
LIBVIRT_END_ALLOW_THREADS;
-if (c_retval 0)
+if (c_retval 0){
+ py_retval = libvirt_intWrap(c_retval);
goto cleanup;
-
+}
+
if (!(pyobj_seconds = libvirt_longlongWrap(seconds)) ||
PyDict_SetItemString(dict, seconds, pyobj_seconds) 0)
goto cleanup;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-python PATCH] Add a type check for time in libvirt_virDomainSetTime
When pass a number or other things to setTime,no error output,but set time to 0.
Add a type check and give a clear error messages:
TypeError: time must be dict
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libvirt-override.c b/libvirt-override.c
index 9ba87eb..05552a7 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7795,6 +7795,11 @@ libvirt_virDomainSetTime(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args) {
return NULL;
domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+if (!PyDict_Check(py_dict)) {
+PyErr_Format(PyExc_TypeError, time must be dict);
+return NULL;
+}
+
py_dict_size = PyDict_Size(py_dict);
if (py_dict_size == 2) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-python v2 PATCH] Improve error output when use getTime with a nonzero flags.
Thanks your help and useful messages.
This issue is so small and i just want to fix the ret = NULL with no exception.
So i want to make ret = -1 and make the SystemError: error return without
exception set disappear.
Thanks,
Luyao Huang
- Original Message -
From: Peter Krempa pkre...@redhat.com
To: Luyao Huang lhu...@redhat.com, libvir-list@redhat.com
Sent: Monday, October 20, 2014 2:37:21 PM
Subject: Re: [libvirt] [libvirt-python v2 PATCH] Improve error output when use
getTime with a nonzero flags.
On 10/17/14 04:12, Luyao Huang wrote:
When give a nonzero flags to getTime, c_retval will get -1 and goto cleanup.
But py_retval still is NULL,so pass c_retval value to py_retval.
This will make the output message more correct.
error before use this patch:
SystemError: error return without exception set
after use the patch:
libvirtError: unsupported flags (0x1) in function qemuDomainGetTime
v1:
https://www.redhat.com/archives/libvir-list/2014-October/msg00482.html
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libvirt-override.c b/libvirt-override.c
index 9ba87eb..c779aa3 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7757,9 +7757,11 @@ libvirt_virDomainGetTime(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args) {
c_retval = virDomainGetTime(domain, seconds, nseconds, flags);
LIBVIRT_END_ALLOW_THREADS;
-if (c_retval 0)
+if (c_retval 0){
Missing space before '{'
+ py_retval = libvirt_intWrap(c_retval);
Returning the return value from the C api is useless here. The function
returns a dict on success path thus on error you should return None
(VIR_PY_NONE).
goto cleanup;
-
+}
+
if (!(pyobj_seconds = libvirt_longlongWrap(seconds)) ||
PyDict_SetItemString(dict, seconds, pyobj_seconds) 0)
goto cleanup;
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-python v3 PATCH] Improve error output when use getTime with a nonzero flags.
When give a nonzero flags to getTime, c_retval will get -1 and goto cleanup.
But py_retval still is NULL,so set py_retval = VIR_PY_NONE.
This will make the output message more correct.
error will disappear:
SystemError: error return without exception set
v2:
https://www.redhat.com/archives/libvir-list/2014-October/msg00497.html
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libvirt-override.c b/libvirt-override.c
index 9ba87eb..8690f4f 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7757,9 +7757,11 @@ libvirt_virDomainGetTime(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args) {
c_retval = virDomainGetTime(domain, seconds, nseconds, flags);
LIBVIRT_END_ALLOW_THREADS;
-if (c_retval 0)
+if (c_retval 0) {
+ py_retval = VIR_PY_NONE;
goto cleanup;
-
+}
+
if (!(pyobj_seconds = libvirt_longlongWrap(seconds)) ||
PyDict_SetItemString(dict, seconds, pyobj_seconds) 0)
goto cleanup;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-python PATCH] Fix cannot use VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS flags in domainListGetStats
When set flags=libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, python will report a error: OverflowError: signed integer is greater than maximum Because VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS = 2147483648 (2**31), and it set a signed int in PyArg_ParseTuple function. if (!PyArg_ParseTuple(args, (char *)OOii:virDomainListGetStats, pyobj_conn, py_domlist, stats, flags)) When python = 2.3, 'I' means unsigned int and 'i' means int,so there should use 'I'. From: https://docs.python.org/2/c-api/arg.html I also found a lot of function use 'i' for a unsigned int, but i didn't change them. Signed-off-by: Luyao Huang lhu...@redhat.com --- libvirt-override.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-override.c b/libvirt-override.c index c887b71..6dacdac 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -8126,7 +8126,7 @@ libvirt_virDomainListGetStats(PyObject *self ATTRIBUTE_UNUSED, unsigned int flags; unsigned int stats; -if (!PyArg_ParseTuple(args, (char *)OOii:virDomainListGetStats, +if (!PyArg_ParseTuple(args, (char *)OOII:virDomainListGetStats, pyobj_conn, py_domlist, stats, flags)) return NULL; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [python PATCH] Fix flags cannot get right value for blockCopy function
When use blockCopy, flags cannot get a right value, because
PyArg_ParseTuple want to get 6 parameters and blockCopy only
pass 5.Flags will get a unpredictable value, this will make
this function cannot be used.And error just like:
unsupported flags (0x7f6c) in function qemuDomainBlockCopy
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libvirt-override.c b/libvirt-override.c
index c887b71..4999ac3 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -8175,8 +8175,7 @@ libvirt_virDomainBlockCopy(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args)
int c_retval;
if (!PyArg_ParseTuple(args, (char *) Ozz|Oi:virDomainBlockCopy,
- pyobj_dom, disk, destxml, pyobj_dict, params,
- flags))
+ pyobj_dom, disk, destxml, pyobj_dict, flags))
return VIR_PY_INT_FAIL;
if (PyDict_Check(pyobj_dict)) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [python v2 PATCH] Add dict check for setTime and allow pass one valid parameter
When pass None to time, it will set guest time to 0.
When pass an empty dictionary, it will report error.
Allow a one-element dictionary which contains 'seconds'
or 'nseconds', setting JUST seconds will do the sane
thing of passing 0 for nseconds, instead of erroring out.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override-virDomain.py | 2 ++
libvirt-override.c| 26 +++---
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/libvirt-override-virDomain.py b/libvirt-override-virDomain.py
index a50ec0d..d788657 100644
--- a/libvirt-override-virDomain.py
+++ b/libvirt-override-virDomain.py
@@ -69,6 +69,8 @@
def setTime(self, time=None, flags=0):
Set guest time to the given value. @time is a dict conatining
'seconds' field for seconds and 'nseconds' field for nanosecons
+if time is None:
+time = {'nseconds': 0, 'seconds': 0L}
ret = libvirtmod.virDomainSetTime(self._o, time, flags)
if ret == -1: raise libvirtError ('virDomainSetTime() failed',
dom=self)
return ret
diff --git a/libvirt-override.c b/libvirt-override.c
index a53b46f..5c016f9 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7798,8 +7798,28 @@ libvirt_virDomainSetTime(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args) {
domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
py_dict_size = PyDict_Size(py_dict);
+
+if (py_dict_size == 1) {
+PyObject *pyobj_seconds, *pyobj_nseconds;
+
+if ((pyobj_seconds = PyDict_GetItemString(py_dict, seconds))
+(libvirt_longlongUnwrap(pyobj_seconds, seconds) 0)) {
+PyErr_Format(PyExc_LookupError, malformed 'seconds');
+goto cleanup;
+}
-if (py_dict_size == 2) {
+if ((pyobj_nseconds = PyDict_GetItemString(py_dict, nseconds))
+(libvirt_uintUnwrap(pyobj_nseconds, nseconds) 0)) {
+PyErr_Format(PyExc_LookupError, malformed 'nseconds');
+goto cleanup;
+}
+
+if (!pyobj_nseconds !pyobj_seconds) {
+PyErr_Format(PyExc_LookupError, Dictionary must contain
+ 'seconds' or 'nseconds');
+goto cleanup;
+}
+} else if (py_dict_size == 2) {
PyObject *pyobj_seconds, *pyobj_nseconds;
if (!(pyobj_seconds = PyDict_GetItemString(py_dict, seconds)) ||
@@ -7813,9 +7833,9 @@ libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args) {
PyErr_Format(PyExc_LookupError, malformed or missing 'nseconds');
goto cleanup;
}
-} else if (py_dict_size 0) {
+} else if (py_dict_size = 0) {
PyErr_Format(PyExc_LookupError, Dictionary must contain
- 'seconds' and 'nseconds');
+ 'seconds' or 'nseconds');
goto cleanup;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-python PATCH] Add a type check for time in libvirt_virDomainSetTime
Thanks for your reply and i add check for one-element and 0-element.
But seems this will make the code not succinct enough.
V2:
https://www.redhat.com/archives/libvir-list/2014-October/msg00815.html
BTW,How about move the check to libvirt-override-virDomain.py?
Thanks,
Luyao Huang
- Original Message -
From: Eric Blake ebl...@redhat.com
To: Luyao Huang lhu...@redhat.com, libvir-list@redhat.com
Sent: Saturday, October 25, 2014 2:14:59 AM
Subject: Re: [libvirt] [libvirt-python PATCH] Add a type check for time in
libvirt_virDomainSetTime
On 10/20/2014 03:43 AM, Luyao Huang wrote:
When pass a number or other things to setTime,no error output,but set time to
0.
Add a type check and give a clear error messages:
TypeError: time must be dict
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libvirt-override.c b/libvirt-override.c
index 9ba87eb..05552a7 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7795,6 +7795,11 @@ libvirt_virDomainSetTime(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args) {
return NULL;
domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
+if (!PyDict_Check(py_dict)) {
+PyErr_Format(PyExc_TypeError, time must be dict);
+return NULL;
+}
What happens if py_dict is None or an empty dictionary? The code still
does the wrong thing (it errors out if you have a one-element
dictionary, but not if you have a 0-element dictionary); furthermore, we
SHOULD allow a one-element dictionary (setting JUST seconds should do
the sane thing of passing 0 for nseconds, instead of erroring out).
Looking forward to v2.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix memory leak in cmdNetworkDHCPLeases
After use cidr_format in function virAsprintf and vshPrintExtra, need free cidr_format. Fix the following memory leak from valgrind, like: 18 bytes in 1 blocks are definitely lost in loss record 41 of 192 at 0x4C29BBD: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x85CE36F: __vasprintf_chk (vasprintf_chk.c:80) by 0x4EE52D5: UnknownInlinedFun (stdio2.h:210) by 0x4EE52D5: virVasprintfInternal (virstring.c:459) by 0x4EE53CA: virAsprintfInternal (virstring.c:480) by 0x14FE96: cmdNetworkDHCPLeases (virsh-network.c:1378) by 0x13006B: vshCommandRun (virsh.c:1915) by 0x12A9E1: main (virsh.c:3699) Signed-off-by: Luyao Huang lhu...@redhat.com --- tools/virsh-network.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 90392d3..8ff6fd8 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1381,6 +1381,8 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) expirytime, EMPTYSTR(lease-mac), EMPTYSTR(typestr), cidr_format, EMPTYSTR(lease-hostname), EMPTYSTR(lease-clientid)); + + VIR_FREE(cidr_format); } ret = true; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [python v3 PATCH] Add dict check for setTime and allow pass 'seconds' parameter
When pass None or a empty dictionary to time, it will
report error.Allow a one-element dictionary which
contains 'seconds',setting JUST seconds will do the
sane thing of passing 0 for nseconds, instead of
erroring out.If dict have a unkown key, it will report error.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
libvirt-override-virDomain.py | 6 +++---
libvirt-override.c| 41 +
2 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/libvirt-override-virDomain.py b/libvirt-override-virDomain.py
index a50ec0d..2a4c4c9 100644
--- a/libvirt-override-virDomain.py
+++ b/libvirt-override-virDomain.py
@@ -66,9 +66,9 @@
if ret == -1: raise libvirtError ('virDomainGetTime() failed',
dom=self)
return ret
-def setTime(self, time=None, flags=0):
-Set guest time to the given value. @time is a dict conatining
-'seconds' field for seconds and 'nseconds' field for nanosecons
+def setTime(self, time, flags=0):
+Set guest time to the given value. @time is a dict containing
+'seconds' field for seconds and 'nseconds' field for nanoseconds
ret = libvirtmod.virDomainSetTime(self._o, time, flags)
if ret == -1: raise libvirtError ('virDomainSetTime() failed',
dom=self)
return ret
diff --git a/libvirt-override.c b/libvirt-override.c
index a53b46f..1d1714a 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -7784,6 +7784,8 @@ static PyObject *
libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) {
PyObject *py_retval = NULL;
PyObject *pyobj_domain;
+PyObject *pyobj_seconds;
+PyObject *pyobj_nseconds;
PyObject *py_dict;
virDomainPtr domain;
long long seconds = 0;
@@ -7797,25 +7799,40 @@ libvirt_virDomainSetTime(PyObject *self
ATTRIBUTE_UNUSED, PyObject *args) {
return NULL;
domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
-py_dict_size = PyDict_Size(py_dict);
+if (!PyDict_Check(py_dict)) {
+PyErr_Format(PyExc_TypeError, time must be dict);
+return NULL;
+}
-if (py_dict_size == 2) {
-PyObject *pyobj_seconds, *pyobj_nseconds;
+py_dict_size = PyDict_Size(py_dict);
-if (!(pyobj_seconds = PyDict_GetItemString(py_dict, seconds)) ||
-(libvirt_longlongUnwrap(pyobj_seconds, seconds) 0)) {
-PyErr_Format(PyExc_LookupError, malformed or missing 'seconds');
+if ((pyobj_seconds = PyDict_GetItemString(py_dict, seconds)) != NULL) {
+if (libvirt_longlongUnwrap(pyobj_seconds, seconds) 0) {
+PyErr_Format(PyExc_LookupError, malformed 'seconds');
goto cleanup;
}
+} else {
+PyErr_Format(PyExc_LookupError, Dictionary must contains
+ 'seconds');
+goto cleanup;
+}
-if (!(pyobj_nseconds = PyDict_GetItemString(py_dict, nseconds)) ||
-(libvirt_uintUnwrap(pyobj_nseconds, nseconds) 0)) {
-PyErr_Format(PyExc_LookupError, malformed or missing 'nseconds');
+if ((pyobj_nseconds = PyDict_GetItemString(py_dict, nseconds)) != NULL) {
+if (libvirt_uintUnwrap(pyobj_nseconds, nseconds) 0) {
+PyErr_Format(PyExc_LookupError, malformed 'nseconds');
goto cleanup;
}
-} else if (py_dict_size 0) {
-PyErr_Format(PyExc_LookupError, Dictionary must contain
- 'seconds' and 'nseconds');
+} else if (py_dict_size == 1) {
+nseconds = 0;
+} else {
+PyErr_Format(PyExc_LookupError, Dictionary contains
+ unknown key);
+goto cleanup;
+}
+
+if (py_dict_size 2) {
+PyErr_Format(PyExc_LookupError, Dictionary contains
+ unknown key);
goto cleanup;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] network: fix call virNetworkEventLifecycleNew when networkStartNetwork fail
When start a network fail, libvirt still call virNetworkEventLifecycleNew to send a event. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/network/bridge_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 10ded33..11e86e0 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3253,7 +3253,8 @@ static int networkCreate(virNetworkPtr net) if (virNetworkCreateEnsureACL(net-conn, network-def) 0) goto cleanup; -ret = networkStartNetwork(driver, network); +if ((ret = networkStartNetwork(driver, network)) 0) +goto cleanup; event = virNetworkEventLifecycleNew(network-def-name, network-def-uuid, -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: fix net-dhcp-leases no output in quiet mode
When run net-dhcp-leases in quiet mode, cannot get any output. # virsh -q net-dhcp-leases default Signed-off-by: Luyao Huang lhu...@redhat.com --- tools/virsh-network.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 4610a34..8f9fbd3 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -1377,10 +1377,10 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) ignore_value(virAsprintf(cidr_format, %s/%d, lease-ipaddr, lease-prefix)); -vshPrintExtra(ctl, %-20s %-18s %-9s %-25s %-15s %s\n, - expirytime, EMPTYSTR(lease-mac), - EMPTYSTR(typestr), cidr_format, - EMPTYSTR(lease-hostname), EMPTYSTR(lease-clientid)); +vshPrint(ctl, %-20s %-18s %-9s %-25s %-15s %s\n, + expirytime, EMPTYSTR(lease-mac), + EMPTYSTR(typestr), cidr_format, + EMPTYSTR(lease-hostname), EMPTYSTR(lease-clientid)); VIR_FREE(cidr_format); } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] domain: Improve error output for virDomainListGetStats
When pass flags --domain and --list-* to cmdDomstats,
a unsupport error will output from qemuConnectGetAllDomainStats.
error: unsupported flags (0x1) in function qemuConnectGetAllDomainStats
From manual of virsh: The approaches can't be combined.
Improve error to:
error: --domain and --list-* flags are mutually exclusive
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/libvirt-domain.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7dc3146..6ae6dd2 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11053,6 +11053,19 @@ virDomainListGetStats(virDomainPtr *doms,
goto cleanup;
}
+if (flags (VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_TRANSIENT |
+ VIR_CONNECT_LIST_DOMAINS_RUNNING |
+ VIR_CONNECT_LIST_DOMAINS_SHUTOFF |
+ VIR_CONNECT_LIST_DOMAINS_PAUSED |
+ VIR_CONNECT_LIST_DOMAINS_OTHER)) {
+virReportInvalidArg(flags, %s,
+_(--domain and --list-* flags are mutually exclusive));
+goto cleanup;
+}
+
conn = doms[0]-conn;
virCheckConnectReturn(conn, -1);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain: Improve error output for virDomainListGetStats
Thanks for pointing out the mistake.I will move the check in qemuConnectGetAllDomainStats, this won't make a old client cannot use the future server and will give a good error when use future client to connect to old server. Thanks, Luyao Huang - Original Message - From: Eric Blake ebl...@redhat.com To: Luyao Huang lhu...@redhat.com, libvir-list@redhat.com Sent: Tuesday, November 4, 2014 7:19:41 PM Subject: Re: [libvirt] [PATCH] domain: Improve error output for virDomainListGetStats On 11/04/2014 07:51 AM, Luyao Huang wrote: When pass flags --domain and --list-* to cmdDomstats, a unsupport error will output from qemuConnectGetAllDomainStats. error: unsupported flags (0x1) in function qemuConnectGetAllDomainStats From manual of virsh: The approaches can't be combined. NACK. While they cannot be combined in the current implementation, I see nothing that would prohibit a future implementation from allowing the combination (for example, show me all stats for a given domain, but only if the domain is running). Prohibiting in libvirt-domian.c would prevent an older RPC client from talking to a newer server that supports the combination. We should only prohibit combinations in libvirt-*.c if there is no chance we will ever extend things to allow the combination. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v2 PATCH] qemu: Improve error output for virDomainListGetStats
A unsupport error will output from qemuConnectGetAllDomainStats. Add a
check for the flags in qemuConnectGetAllDomainStats and improve the error
in the current implementation.From manual of virsh:
The approaches can't be combined.
Improve error to:
error: --domain and --list-* flags are mutually exclusive
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6acaea8..60c3882 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18487,13 +18487,18 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
unsigned int privflags = 0;
unsigned int domflags = 0;
-if (ndoms)
-virCheckFlags(VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
-else
-virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
- VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
- VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE |
- VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
+if (ndoms (flags (VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE))) {
+virReportInvalidArg(flags, %s,
+_(--domain and --list-* flags are mutually exclusive));
+return -1;
+}
+
+virCheckFlags(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE |
+ VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS, -1);
if (virConnectGetAllDomainStatsEnsureACL(conn) 0)
return -1;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] domain: Improve error output for virDomainListGetStats
Thanks your advise :) and I have moved the check
in qemuConnectGetAllDomainStats.
And the v2:
https://www.redhat.com/archives/libvir-list/2014-November/msg00069.html
Thanks,
Luyao Huang
- Original Message -
From: Peter Krempa pkre...@redhat.com
To: Luyao Huang lhu...@redhat.com, libvir-list@redhat.com
Sent: Tuesday, November 4, 2014 7:20:45 PM
Subject: Re: [libvirt] [PATCH] domain: Improve error output for
virDomainListGetStats
On 11/04/14 07:51, Luyao Huang wrote:
When pass flags --domain and --list-* to cmdDomstats,
a unsupport error will output from qemuConnectGetAllDomainStats.
error: unsupported flags (0x1) in function qemuConnectGetAllDomainStats
From manual of virsh: The approaches can't be combined.
Improve error to:
error: --domain and --list-* flags are mutually exclusive
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/libvirt-domain.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7dc3146..6ae6dd2 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11053,6 +11053,19 @@ virDomainListGetStats(virDomainPtr *doms,
goto cleanup;
}
+if (flags (VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_PERSISTENT |
+ VIR_CONNECT_LIST_DOMAINS_TRANSIENT |
+ VIR_CONNECT_LIST_DOMAINS_RUNNING |
+ VIR_CONNECT_LIST_DOMAINS_SHUTOFF |
+ VIR_CONNECT_LIST_DOMAINS_PAUSED |
+ VIR_CONNECT_LIST_DOMAINS_OTHER)) {
+virReportInvalidArg(flags, %s,
+_(--domain and --list-* flags are mutually exclusive));
+goto cleanup;
+}
From what I remember it was a deliberate design decision to avoid
reporting this error either from virsh or the library itself so that we
possibly can add the filtering later on.
This should be done in the qemu driver impl of the function so that we
can possibly do it later without breaking old clients
+
conn = doms[0]-conn;
virCheckConnectReturn(conn, -1);
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix a mismatch attribute name
From libvirt.org we know this attribute named:
interface_mac MAC address of the network interface, not unique
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/access/viraccessdriverpolkit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/access/viraccessdriverpolkit.c
b/src/access/viraccessdriverpolkit.c
index 3136be7..0e07053 100644
--- a/src/access/viraccessdriverpolkit.c
+++ b/src/access/viraccessdriverpolkit.c
@@ -206,7 +206,7 @@ virAccessDriverPolkitCheckInterface(virAccessManagerPtr
manager,
const char *attrs[] = {
connect_driver, driverName,
interface_name, iface-name,
-interface_macaddr, iface-mac,
+interface_mac, iface-mac,
NULL,
};
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [v2 PATCH] doc: Fix a mismatch attribute name
From virAccessDriverPolkitCheckInterface function, we know this attribute should named: interface_macaddr Signed-off-by: Luyao Huang lhu...@redhat.com --- docs/aclpolkit.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in index 91b5296..e5a9b16 100644 --- a/docs/aclpolkit.html.in +++ b/docs/aclpolkit.html.in @@ -121,7 +121,7 @@ tdName of the network interface, unique to the local host/td /tr tr - tdinterface_mac/td + tdinterface_macaddr/td tdMAC address of the network interface, not unique/td /tr /tbody -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] security:selinux: Fix crash when tcon is NULL
https://bugzilla.redhat.com/show_bug.cgi?id=1161831
Libvirtd will crash when parameter tcon = NULL in
virSecuritySELinuxSetFileconHelper
function, because libvirt do not check the first parameter when use strcmp().
Add a check for tcon before use strcmp() and output a error in log when tcon is
NULL.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/security/security_selinux.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index f96be50..4fd09b8 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -887,6 +887,11 @@ virSecuritySELinuxSetFileconHelper(const char *path, char
*tcon, bool optional)
int setfilecon_errno = errno;
if (getfilecon_raw(path, econ) = 0) {
+if (tcon == NULL) {
+virReportSystemError(errno,%s,
+ _(Invalid security context : NULL));
+return -1;
+}
if (STREQ(tcon, econ)) {
freecon(econ);
/* It's alright, there's nothing to change anyway. */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Fix crash when src-hosts = NULL in virStorageFileBackendGlusterInit
https://bugzilla.redhat.com/show_bug.cgi?id=1162974
When do external snapshot for a gluster disk with no host name(ip) in
snapshot xml, libvirtd will crash. Because when node do not have a children
in virDomainStorageHostParse, libvirt will return 0, but donnot get hosts for
virStorageFileBackendGlusterInit.
snpahost.xml:
domainsnapshot
namesnapshot_test/name
descriptionSnapshot Test/description
disks
disk name='vda' snapshot='external' type='network'
source protocol='gluster' name='gluster-vol1/gluster.img.snap'/
/disk
/disks
/domainsnapshot
Back trace:
virsh snapshot-create r6 snapshot.xml --disk-only
0 virStorageFileBackendGlusterInit (src=0x7fc760007ca0) at
storage/storage_backend_gluster.c:577
1 0x7fc76d678e22 in virStorageFileInitAs (src=0x7fc760007ca0,
uid=uid@entry=4294967295, gid=gid@entry=4294967295) at
storage/storage_driver.c:2547
2 0x7fc76d678e9c in virStorageFileInit (src=optimized out) at
storage/storage_driver.c:2567
3 0x7fc76bc13f9c in qemuDomainSnapshotPrepareDiskExternal (reuse=false,
active=true, snapdisk=0x7fc7600019b8, disk=0x7fc7641e4880, conn=0x7fc76426cc10)
at qemu/qemu_driver.c:12995
4 qemuDomainSnapshotPrepare (flags=synthetic pointer, def=0x7fc760002570,
vm=0x7fc76422b530, conn=0x7fc76426cc10) at qemu/qemu_driver.c:13156
5 qemuDomainSnapshotCreateXML (domain=0x7fc760001f30, xmlDesc=optimized out,
flags=16) at qemu/qemu_driver.c:13896
6 0x7fc782d4de4d in virDomainSnapshotCreateXML
(domain=domain@entry=0x7fc760001f30,
xmlDesc=0x7fc760001b80
domainsnapshot\nnamesnapshot_test/name\ndescriptionSnapshot
Test/description\ndisks\ndisk name='vda' snapshot='external'
type='network'\nsource protocol='gluster' name='gluster-vol1/gluster,
flags=16) at libvirt.c:18488
7 0x7fc7837cb44c in remoteDispatchDomainSnapshotCreateXML
(server=optimized out, msg=optimized out, ret=0x7fc76a60,
args=0x7fc760001f90, rerr=0x7fc77344dc80,
client=optimized out) at remote_dispatch.h:8605
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2c65276..34c1c12 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4150,7 +4150,12 @@ virDomainStorageHostParse(xmlNodePtr node,
memset(host, 0, sizeof(host));
-child = node-children;
+if ((child = node-children) == NULL) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Can not find a host in xml));
+goto cleanup;
+}
+
while (child != NULL) {
if (child-type == XML_ELEMENT_NODE
xmlStrEqual(child-name, BAD_CAST host)) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix cannot get a hot-unplug disk blockdevio settings
When we try to get a hot-unplug disk blkdevio settings via
qemuDomainGetBlockIoTune, libvirt will output a cannot find
device error.Move the check after confirm vm is running.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 411179d..c717c76 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17031,12 +17031,11 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
goto endjob;
}
-device = qemuDiskPathToAlias(vm, disk, NULL);
-if (!device) {
-goto endjob;
-}
-
if (flags VIR_DOMAIN_AFFECT_LIVE) {
+device = qemuDiskPathToAlias(vm, disk, NULL);
+if (!device) {
+goto endjob;
+}
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetBlockIoThrottle(priv-mon, device, reply,
supportMaxOptions);
qemuDomainObjExitMonitor(driver, vm);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix cannot get a hot-unplug disk blockdevio settings
On 11/15/2014 06:29 AM, John Ferlan wrote:
On 11/13/2014 10:21 PM, Luyao Huang wrote:
It would help to put the bz link in here:
https://bugzilla.redhat.com/show_bug.cgi?id=1164080
When we try to get a hot-unplug disk blkdevio settings via
qemuDomainGetBlockIoTune, libvirt will output a cannot find
device error.Move the check after confirm vm is running.
Technically speaking you're not moving the check after confirming the
vm is running because it is already after that via the call to
virDomainLiveConfigHelperMethod which calls virDomainObjIsActive before
determining how to set *flags.
You're moving the check to only when it's desired to get something from
the active vm (e.g., when the vm is running or the --live flag was
used). Since the 'device' is only ever used within that call - it's
safe/right to move it.
Thanks for the clearly description, i will try to give a more clearly
description next time :)
I updated the bug report with some other details regarding expectations.
Thanks your help in advance.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 411179d..c717c76 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17031,12 +17031,11 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
goto endjob;
}
-device = qemuDiskPathToAlias(vm, disk, NULL);
-if (!device) {
-goto endjob;
-}
-
if (flags VIR_DOMAIN_AFFECT_LIVE) {
+device = qemuDiskPathToAlias(vm, disk, NULL);
+if (!device) {
+goto endjob;
+}
mkletzan's recent changes remove the unnecessary brackets.
I'll clean this up and push shortly
John
Thanks for push the patch.
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorGetBlockIoThrottle(priv-mon, device, reply,
supportMaxOptions);
qemuDomainObjExitMonitor(driver, vm);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: output error when try to hotplug unsupport console
https://bugzilla.redhat.com/show_bug.cgi?id=1164627 When try to use attach-device to hotplug a qemu unsupport sonsole, command will return success, and add a console in vm's xml.But this doesn't work in qemu, qemu doesn't support these console.Add a error output when try to hotplug a unsupport console in qemuBuildConsoleChrDeviceStr. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_command.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1399ce4..2bf4a83 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10069,13 +10069,17 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr, break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: +break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST: -break; +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unsupported console target type %s), + NULLSTR(virDomainChrConsoleTargetTypeToString(chr-targetType))); +return ret; } ret = 0; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Doc: some small issue in the document
When i pasted some xml from libvirt.org, i found some small mistake. Signed-off-by: Luyao Huang lhu...@redhat.com --- docs/formatdomain.html.in | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9c1d0f4..c08b244 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3871,7 +3871,7 @@ lt;sourcegt; lt;address type='pci' domain='0x' bus='0x00' slot='0x07' function='0x0'/gt; lt;/sourcegt; - lt;mac address='52:54:00:6d:90:02'gt; + lt;mac address='52:54:00:6d:90:02'/gt; lt;virtualport type='802.1Qbh'gt; lt;parameters profileid='finance'/gt; lt;/virtualportgt; @@ -3898,7 +3898,7 @@ ... lt;devicesgt; lt;interface type='mcast'gt; - lt;mac address='52:54:00:6d:90:01'gt; + lt;mac address='52:54:00:6d:90:01'/gt; lt;source address='230.0.0.1' port='5558'/gt; lt;/interfacegt; lt;/devicesgt; @@ -4772,11 +4772,11 @@ qemu-kvm -net nic,model=? /dev/null lt;source path='/dev/pts/3'/gt; lt;target port='0'/gt; lt;/serialgt; -lt;serial type='filegt; +lt;serial type='file'gt; lt;source path='/tmp/file'gt; lt;seclabel model='dac' relabel='no'/gt; lt;/sourcegt; - lt;target port='0'gt; + lt;target port='0'/gt; lt;/serialgt; lt;console type='pty'gt; lt;source path='/dev/pts/4'/gt; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] fix attached vm cannot get a right label
When call qemuProcessAttach to attach a qemu process, libvirt will generate a wrong label for DAC, and do not set imagelabel for both of them, no imagelabel will cause some other issue. After this patch guest label will be : seclabel type='static' model='selinux' relabel='yes' labelunconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023/label imagelabelsystem_u:object_r:svirt_image_t:s0-s0:c0.c1023/imagelabel /seclabel seclabel type='static' model='dac' relabel='yes' label+0:+0/label imagelabel+0:+0/imagelabel /seclabel Luyao Huang (2): qemu: fix some small issue in qemuProcessAttach security: Add a new func use stat to get process DAC label src/qemu/qemu_process.c | 10 ++--- src/security/security_dac.c | 50 +++-- 2 files changed, 55 insertions(+), 5 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: fix some small issue in qemuProcessAttach
There are some small issue in qemuProcessAttach:
1.Fix virSecurityManagerGetProcessLabel always get pid = 0,
move 'vm-pid = pid' before call virSecurityManagerGetProcessLabel.
2.Use virSecurityManagerGenLabel to get image label.
3.Fix always set selinux label for other security driver label.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_process.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 382d802..ee95adb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5255,6 +5255,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
if (VIR_STRDUP(priv-pidfile, pidfile) 0)
goto error;
+vm-pid = pid;
+
VIR_DEBUG(Detect security driver config);
sec_managers = virSecurityManagerGetNested(driver-securityManager);
if (sec_managers == NULL)
@@ -5272,7 +5274,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
seclabeldef-type = VIR_DOMAIN_SECLABEL_STATIC;
if (VIR_ALLOC(seclabel) 0)
goto error;
-if (virSecurityManagerGetProcessLabel(driver-securityManager,
+if (virSecurityManagerGetProcessLabel(sec_managers[i],
vm-def, vm-pid, seclabel) 0)
goto error;
@@ -5290,6 +5292,10 @@ int qemuProcessAttach(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
}
+if (virSecurityManagerGenLabel(driver-securityManager, vm-def) 0) {
+goto error;
+}
+
VIR_DEBUG(Creating domain log file);
if ((logfile = qemuDomainCreateLog(driver, vm, false)) 0)
goto error;
@@ -5334,8 +5340,6 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
qemuDomainObjTaint(driver, vm, VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH, logfile);
-vm-pid = pid;
-
VIR_DEBUG(Waiting for monitor to show up);
if (qemuProcessWaitForMonitor(driver, vm, QEMU_ASYNC_JOB_NONE,
priv-qemuCaps, -1) 0)
goto error;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] security: Add a new func use stat to get process DAC label
When use qemuProcessAttach to attach a qemu process, cannot
get a right DAC label. Add a new func to get process label
via stat func. Do not remove virDomainDefGetSecurityLabelDef
before try to use stat to get process DAC label, because
There are some other func call virSecurityDACGetProcessLabel.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/security/security_dac.c | 50 +++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 85253af..2977f71 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1237,17 +1237,63 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
}
static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct stat sb;
+char *path = NULL;
+char *label = NULL;
+int ret = -1;
+
+VIR_INFO(Getting DAC user and group on process '%d', pid);
+
+if (virAsprintf(path, /proc/%d, (int) pid) 0)
+goto cleanup;
+
+if (stat(path, sb) 0)
+goto cleanup;
+
+if (virAsprintf(label, +%u:+%u,
+(unsigned int) sb.st_uid,
+(unsigned int) sb.st_gid) 0)
+goto cleanup;
+
+if (virStrcpy(seclabel-label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL)
+goto cleanup;
+ret = 0;
+
+cleanup:
+VIR_FREE(path);
+VIR_FREE(label);
+return ret;
+}
+
+static int
virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def,
- pid_t pid ATTRIBUTE_UNUSED,
+ pid_t pid,
virSecurityLabelPtr seclabel)
{
virSecurityLabelDefPtr secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (!secdef || !seclabel)
+if (!seclabel)
return -1;
+if (secdef == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(missing label for DAC security
+ driver in domain %s), def-name);
+
+if (virSecurityDACGetProcessLabelInternal(pid, seclabel) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Cannot get process %d DAC label),pid);
+return -1;
+}
+
+return 0;
+}
+
if (secdef-label)
ignore_value(virStrcpy(seclabel-label, secdef-label,
VIR_SECURITY_LABEL_BUFLEN));
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] security: Add a new func use stat to get process DAC label
On 12/01/2014 06:24 PM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 05:54:36PM +0800, Luyao Huang wrote:
When use qemuProcessAttach to attach a qemu process, cannot
get a right DAC label. Add a new func to get process label
via stat func. Do not remove virDomainDefGetSecurityLabelDef
before try to use stat to get process DAC label, because
There are some other func call virSecurityDACGetProcessLabel.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/security/security_dac.c | 50
+++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 85253af..2977f71 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1237,17 +1237,63 @@
virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
}
static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct stat sb;
+char *path = NULL;
+char *label = NULL;
+int ret = -1;
+
+VIR_INFO(Getting DAC user and group on process '%d', pid);
+
+if (virAsprintf(path, /proc/%d, (int) pid) 0)
+goto cleanup;
+
This won't work on systems without /proc.
Okay, i need find a way to check this or find a way to get other system
process uid and gid.
+if (stat(path, sb) 0)
+goto cleanup;
+
Better use lstat.
Thanks your advice.
+if (virAsprintf(label, +%u:+%u,
+(unsigned int) sb.st_uid,
+(unsigned int) sb.st_gid) 0)
+goto cleanup;
+
+if (virStrcpy(seclabel-label, label,VIR_SECURITY_LABEL_BUFLEN)
== NULL)
+goto cleanup;
+ret = 0;
+
+cleanup:
+VIR_FREE(path);
+VIR_FREE(label);
+return ret;
+}
+
+static int
virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
virDomainDefPtr def,
- pid_t pid ATTRIBUTE_UNUSED,
+ pid_t pid,
virSecurityLabelPtr seclabel)
{
virSecurityLabelDefPtr secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (!secdef || !seclabel)
+if (!seclabel)
I wonder whether this won't screw up domain definitions that don't
want to have any seclabel set (those defined with XML), I need to
figure that out.
After check the func wihch call virSecurityManagerGetProcessLabel, both
of them
will report error and go to error when failed get the DAC label (also
for selinux).
The most important reason is the virSecuritySELinuxGetSecurityProcessLabel,
After i see this func, i find it don't use def in this func ( don't use
virSecuritySELinuxGetSecurityProcessLabel to get the labels from def)
And i think this is the right way, because virSecurityManagerGetProcessLabel
should be a func which get the label from the process.
So in my opinion, if we can give a right and good way to get process DAC
label
for the system we support, we can remove virDomainDefGetSecurityLabelDef
and do not use def in this func.
return -1;
+if (secdef == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(missing label for DAC security
+ driver in domain %s), def-name);
+
This should probably be VIR_DEBUG or VIR_INFO, otherwise you report
error without erroring out (returning -1) and it gets saved for the
connection.
Yes, i used wrong func in this place, it should be VIR_DEBUG.
+if (virSecurityDACGetProcessLabelInternal(pid, seclabel) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Cannot get process %d DAC label),pid);
+return -1;
Also two errors will be reported if this fails.
Oh, i didn't notice that, thanks for pointing out.
Martin
+}
+
+return 0;
+}
+
if (secdef-label)
ignore_value(virStrcpy(seclabel-label, secdef-label,
VIR_SECURITY_LABEL_BUFLEN));
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] security: Add a new func use stat to get process DAC label
On 12/01/2014 11:20 PM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 11:05:30PM +0800, Luyao Huang wrote:
On 12/01/2014 06:24 PM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 05:54:36PM +0800, Luyao Huang wrote:
When use qemuProcessAttach to attach a qemu process, cannot
get a right DAC label. Add a new func to get process label
via stat func. Do not remove virDomainDefGetSecurityLabelDef
before try to use stat to get process DAC label, because
There are some other func call virSecurityDACGetProcessLabel.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/security/security_dac.c | 50
+++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 85253af..2977f71 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1237,17 +1237,63 @@
virSecurityDACReserveLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
}
static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct stat sb;
+char *path = NULL;
+char *label = NULL;
+int ret = -1;
+
+VIR_INFO(Getting DAC user and group on process '%d', pid);
+
+if (virAsprintf(path, /proc/%d, (int) pid) 0)
+goto cleanup;
+
This won't work on systems without /proc.
Okay, i need find a way to check this or find a way to get other system
process uid and gid.
Have a look at virProcessGetStartTime(), its implementation for linux
and freebsd should be enough for where
virSecurityDACGetProcessLabelInternal is going to be called.
Thanks a lot! I am worrying about that ; )
+if (stat(path, sb) 0)
+goto cleanup;
+
Better use lstat.
Thanks your advice.
+if (virAsprintf(label, +%u:+%u,
+(unsigned int) sb.st_uid,
+(unsigned int) sb.st_gid) 0)
+goto cleanup;
+
+if (virStrcpy(seclabel-label, label,VIR_SECURITY_LABEL_BUFLEN)
== NULL)
+goto cleanup;
+ret = 0;
+
+cleanup:
+VIR_FREE(path);
+VIR_FREE(label);
+return ret;
+}
+
+static int
virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
virDomainDefPtr def,
- pid_t pid ATTRIBUTE_UNUSED,
+ pid_t pid,
virSecurityLabelPtr seclabel)
{
virSecurityLabelDefPtr secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (!secdef || !seclabel)
+if (!seclabel)
I wonder whether this won't screw up domain definitions that don't
want to have any seclabel set (those defined with XML), I need to
figure that out.
After check the func wihch call virSecurityManagerGetProcessLabel, both
of them
will report error and go to error when failed get the DAC label (also
for selinux).
The most important reason is the
virSecuritySELinuxGetSecurityProcessLabel,
After i see this func, i find it don't use def in this func ( don't use
virSecuritySELinuxGetSecurityProcessLabel to get the labels from def)
And i think this is the right way, because
virSecurityManagerGetProcessLabel
should be a func which get the label from the process.
So in my opinion, if we can give a right and good way to get process DAC
label
for the system we support, we can remove virDomainDefGetSecurityLabelDef
and do not use def in this func.
Well, if there is some, we can output that. And if there is not,
we'll get the current one. I think this function can stay as is
(apart from the problem pointed out below, of course).
I am agree with you :)
return -1;
+if (secdef == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(missing label for DAC security
+ driver in domain %s), def-name);
+
This should probably be VIR_DEBUG or VIR_INFO, otherwise you report
error without erroring out (returning -1) and it gets saved for the
connection.
Yes, i used wrong func in this place, it should be VIR_DEBUG.
+if (virSecurityDACGetProcessLabelInternal(pid, seclabel)
0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Cannot get process %d DAC label),pid);
+return -1;
Also two errors will be reported if this fails.
Oh, i didn't notice that, thanks for pointing out.
Martin
+}
+
+return 0;
+}
+
if (secdef-label)
ignore_value(virStrcpy(seclabel-label, secdef-label,
VIR_SECURITY_LABEL_BUFLEN));
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: fix some small issue in qemuProcessAttach
On 12/01/2014 06:27 PM, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 11:17:54AM +0100, Martin Kletzander wrote:
On Mon, Dec 01, 2014 at 05:54:35PM +0800, Luyao Huang wrote:
There are some small issue in qemuProcessAttach:
1.Fix virSecurityManagerGetProcessLabel always get pid = 0,
move 'vm-pid = pid' before call virSecurityManagerGetProcessLabel.
2.Use virSecurityManagerGenLabel to get image label.
3.Fix always set selinux label for other security driver label.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_process.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
It looks like we were doing everything already, but we just did it
wrong. Nice catch! ACK.
Oh, I spoke too soon. Two minor things are wrong with this patch that
I'll fixup before pushing:
1) The commit message summary is not very descriptive. When someone
will go over the git log he/she won't figure out what was the deal
from fix some small issue:.
Thanks for pointing out, BTW, maybe qemuprocessattach will failed after
this patch
without the patch 2/2, because we should give a way to get process uid
and gid in
virSecurityDACGetProcessLabel otherwise it will always return -1 without
a error
settings.
I will edit Patch 2 and give a v2 in these days (otherwise i cannot use
qemu-attach : )).
2) see below...
Martin
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 382d802..ee95adb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5255,6 +5255,8 @@ int qemuProcessAttach(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (VIR_STRDUP(priv-pidfile, pidfile) 0)
goto error;
+vm-pid = pid;
+
VIR_DEBUG(Detect security driver config);
sec_managers =
virSecurityManagerGetNested(driver-securityManager);
if (sec_managers == NULL)
@@ -5272,7 +5274,7 @@ int qemuProcessAttach(virConnectPtr conn
ATTRIBUTE_UNUSED,
seclabeldef-type = VIR_DOMAIN_SECLABEL_STATIC;
if (VIR_ALLOC(seclabel) 0)
goto error;
-if (virSecurityManagerGetProcessLabel(driver-securityManager,
+if (virSecurityManagerGetProcessLabel(sec_managers[i],
vm-def, vm-pid,
seclabel) 0)
goto error;
@@ -5290,6 +5292,10 @@ int qemuProcessAttach(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
}
+if (virSecurityManagerGenLabel(driver-securityManager,
vm-def) 0) {
+goto error;
+}
+
This won't pass our syntax-check.
Oh that is a accident, i removed the error settings but forgot the {}
when i sent this patch : )
VIR_DEBUG(Creating domain log file);
if ((logfile = qemuDomainCreateLog(driver, vm, false)) 0)
goto error;
@@ -5334,8 +5340,6 @@ int qemuProcessAttach(virConnectPtr conn
ATTRIBUTE_UNUSED,
qemuDomainObjTaint(driver, vm, VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH,
logfile);
-vm-pid = pid;
-
VIR_DEBUG(Waiting for monitor to show up);
if (qemuProcessWaitForMonitor(driver, vm, QEMU_ASYNC_JOB_NONE,
priv-qemuCaps, -1) 0)
goto error;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] security: Add a new func use stat to get process DAC label
When use qemuProcessAttach to attach a qemu process, cannot
get a right DAC label. Add a new func to get process label
via stat func. Do not remove virDomainDefGetSecurityLabelDef
before try to use stat to get process DAC label, because
There are some other func call virSecurityDACGetProcessLabel.
v2 add support freeBSD.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/security/security_dac.c | 95 -
1 file changed, 93 insertions(+), 2 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 85253af..89cafa3 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -23,6 +23,11 @@
#include sys/stat.h
#include fcntl.h
+#ifdef __FreeBSD__
+# include sys/sysctl.h
+# include sys/user.h
+#endif
+
#include security_dac.h
#include virerror.h
#include virfile.h
@@ -1236,18 +1241,104 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
return 0;
}
+#ifdef __linux__
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct stat sb;
+char *path = NULL;
+char *label = NULL;
+int ret = -1;
+
+VIR_INFO(Getting DAC user and group on process '%d', pid);
+
+if (virAsprintf(path, /proc/%d, (int) pid) 0)
+goto cleanup;
+
+if (lstat(path, sb) 0)
+goto cleanup;
+
+if (virAsprintf(label, +%u:+%u,
+(unsigned int) sb.st_uid,
+(unsigned int) sb.st_gid) 0)
+goto cleanup;
+
+if (virStrcpy(seclabel-label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL)
+goto cleanup;
+ret = 0;
+
+cleanup:
+VIR_FREE(path);
+VIR_FREE(label);
+return ret;
+}
+#elif defined(__FreeBSD__)
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct kinfo_proc p;
+int mib[4];
+size_t len = 4;
+char *label = NULL;
+int ret = -1;
+
+sysctlnametomib(kern.proc.pid, mib, len);
+
+len = sizeof(struct kinfo_proc);
+mib[3] = pid;
+
+if (sysctl(mib, 4, p, len, NULL, 0) 0)
+goto cleanup;
+
+if (virAsprintf(label, +%u:+%u,
+(unsigned int) p.ki_ruid,
+(unsigned int) p.ki_rgid) 0)
+goto cleanup;
+
+if (virStrcpy(seclabel-label, label,VIR_SECURITY_LABEL_BUFLEN) == NULL)
+goto cleanup;
+ret = 0;
+
+cleanup:
+VIR_FREE(label);
+return ret;
+}
+#else
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+return -1;
+}
+#endif
+
static int
virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def,
- pid_t pid ATTRIBUTE_UNUSED,
+ pid_t pid,
virSecurityLabelPtr seclabel)
{
virSecurityLabelDefPtr secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (!secdef || !seclabel)
+if (!seclabel)
return -1;
+if (secdef == NULL) {
+VIR_DEBUG(missing label for DAC security
+ driver in domain %s, def-name);
+
+if (virSecurityDACGetProcessLabelInternal(pid, seclabel) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Cannot get process %d DAC label),pid);
+return -1;
+}
+
+return 0;
+}
+
if (secdef-label)
ignore_value(virStrcpy(seclabel-label, secdef-label,
VIR_SECURITY_LABEL_BUFLEN));
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: fix some small issue in qemuProcessAttach
Sorry i didn't know that, i will pay attention next time. And i have send a v2(but i haven't work in FreeBSD before, so...): https://www.redhat.com/archives/libvir-list/2014-December/msg00207.html Thanks, Luyao Huang - Original Message - From: Martin Kletzander mklet...@redhat.com To: Luyao Huang lhu...@redhat.com Cc: libvir-list@redhat.com Sent: Monday, December 1, 2014 11:41:59 PM Subject: Re: [libvirt] [PATCH 1/2] qemu: fix some small issue in qemuProcessAttach On Mon, Dec 01, 2014 at 11:30:09PM +0800, Luyao Huang wrote: On 12/01/2014 06:27 PM, Martin Kletzander wrote: On Mon, Dec 01, 2014 at 11:17:54AM +0100, Martin Kletzander wrote: On Mon, Dec 01, 2014 at 05:54:35PM +0800, Luyao Huang wrote: There are some small issue in qemuProcessAttach: 1.Fix virSecurityManagerGetProcessLabel always get pid = 0, move 'vm-pid = pid' before call virSecurityManagerGetProcessLabel. 2.Use virSecurityManagerGenLabel to get image label. 3.Fix always set selinux label for other security driver label. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_process.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) It looks like we were doing everything already, but we just did it wrong. Nice catch! ACK. Oh, I spoke too soon. Two minor things are wrong with this patch that I'll fixup before pushing: 1) The commit message summary is not very descriptive. When someone will go over the git log he/she won't figure out what was the deal from fix some small issue:. Thanks for pointing out, BTW, maybe qemuprocessattach will failed after this patch without the patch 2/2, because we should give a way to get process uid and gid in virSecurityDACGetProcessLabel otherwise it will always return -1 without a error settings. I missed that, those patches should've been sent in reverse order to indicate that this one is dependant on the second one. I will edit Patch 2 and give a v2 in these days (otherwise i cannot use qemu-attach : )). Looking forward to v2. -- 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
[libvirt] [PATCH] storage: fix crash caused by no check return before set close
https://bugzilla.redhat.com/show_bug.cgi?id=1087104#c5 When try to use a invalid offset do volupload, libvirt will get failed in virFDStreamOpenFileInternal, but seems libvirt do not check the return in storageVolUpload, and call virFDStreamSetInternalCloseCb , but stream doesn't have a privateData (is NULL), then crash. Although libvirt have a check for invalid offset in cmdVolUpload, but i think there should have some other way to touch off this crash. 0 0x7f09429a9c10 in pthread_mutex_lock () from /lib64/libpthread.so.0 1 0x7f094514dbf5 in virMutexLock (m=optimized out) at util/virthread.c:88 2 0x7f09451cb211 in virFDStreamSetInternalCloseCb at fdstream.c:795 3 0x7f092ff2c9eb in storageVolUpload at storage/storage_driver.c:2098 4 0x7f09451f46e0 in virStorageVolUpload at libvirt.c:14000 5 0x7f0945c78fa1 in remoteDispatchStorageVolUpload at remote_dispatch.h:14339 6 remoteDispatchStorageVolUploadHelper at remote_dispatch.h:14309 7 0x7f094524a192 in virNetServerProgramDispatchCall at rpc/virnetserverprogram.c:437 Signed-off-by: Luyao Huang lhu...@redhat.com --- src/storage/storage_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 7f33d6f..7f4de19 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2111,8 +2111,9 @@ storageVolUpload(virStorageVolPtr obj, goto cleanup; } -ret = backend-uploadVol(obj-conn, pool, vol, stream, - offset, length, flags); +if ((ret = backend-uploadVol(obj-conn, pool, vol, stream, + offset, length, flags)) 0) +goto cleanup; /* Add cleanup callback - call after uploadVol since the stream * is then fully set up -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 2/2] security: Add a new func use stat to get process DAC label
When use qemuProcessAttach to attach a qemu process, cannot
get a right DAC label. Add a new func to get process label
via stat func. Do not remove virDomainDefGetSecurityLabelDef
before try to use stat to get process DAC label, because
There are some other func call virSecurityDACGetProcessLabel.
v2 add support freeBSD.
v3 use snprintf instead of VirAsprintf and move the error
settings in virSecurityDACGetProcessLabelInternal.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/security/security_dac.c | 85 +++--
1 file changed, 82 insertions(+), 3 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 85253af..8fd1e6e 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -22,6 +22,12 @@
#include sys/types.h
#include sys/stat.h
#include fcntl.h
+#include errno.h
+
+#ifdef __FreeBSD__
+# include sys/sysctl.h
+# include sys/user.h
+#endif
#include security_dac.h
#include virerror.h
@@ -1236,17 +1242,90 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
return 0;
}
+#ifdef __linux__
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct stat sb;
+char *path = NULL;
+int ret = -1;
+
+VIR_INFO(Getting DAC user and group on process '%d', pid);
+
+if (virAsprintf(path, /proc/%d, (int) pid) 0)
+goto cleanup;
+
+if (lstat(path, sb) 0) {
+virReportSystemError(errno,
+ _(unable to get PID %d uid and gid via stat),
+ pid);
+goto cleanup;
+}
+
+snprintf(seclabel-label, VIR_SECURITY_LABEL_BUFLEN,
+ +%u:+%u, (unsigned int) sb.st_uid, (unsigned int) sb.st_gid);
+ret = 0;
+
+cleanup:
+VIR_FREE(path);
+return ret;
+}
+#elif defined(__FreeBSD__)
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct kinfo_proc p;
+int mib[4];
+size_t len = 4;
+
+sysctlnametomib(kern.proc.pid, mib, len);
+
+len = sizeof(struct kinfo_proc);
+mib[3] = pid;
+
+if (sysctl(mib, 4, p, len, NULL, 0) 0) {
+virReportSystemError(errno,
+ _(unable to get PID %d uid and gid via sysctl),
+ pid);
+return -1;
+}
+
+snprintf(seclabel-label, VIR_SECURITY_LABEL_BUFLEN,
+ +%u:+%u, (unsigned int) p.ki_ruid, (unsigned int) p.ki_rgid);
+
+return 0;
+}
+#else
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ Cannot get proccess DAC label for pid %d on this platform,
+ (int) pid);
+return -1;
+}
+#endif
+
static int
virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def,
- pid_t pid ATTRIBUTE_UNUSED,
+ pid_t pid,
virSecurityLabelPtr seclabel)
{
virSecurityLabelDefPtr secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (!secdef || !seclabel)
-return -1;
+if (secdef == NULL) {
+VIR_DEBUG(missing label for DAC security
+ driver in domain %s, def-name);
+
+if (virSecurityDACGetProcessLabelInternal(pid, seclabel) 0)
+return -1;
+return 0;
+}
if (secdef-label)
ignore_value(virStrcpy(seclabel-label, secdef-label,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 2/2] security: Add a new func use stat to get process DAC label
On 12/06/2014 12:50 AM, Eric Blake wrote: On 12/05/2014 01:20 AM, Luyao Huang wrote: When use qemuProcessAttach to attach a qemu process, cannot get a right DAC label. Add a new func to get process label via stat func. Do not remove virDomainDefGetSecurityLabelDef before try to use stat to get process DAC label, because There are some other func call virSecurityDACGetProcessLabel. v2 add support freeBSD. v3 use snprintf instead of VirAsprintf and move the error settings in virSecurityDACGetProcessLabelInternal. v2 and v3 comments belong... Signed-off-by: Luyao Huang lhu...@redhat.com --- ...here, where they are visible to reviewers, but will not be included in git history (when the committer uses 'git am', anything after --- is stripped). A year from now, we won't care how many preliminary versions of a patch went to the list, only the one version that is in git. Got it, i will move the version information after --- in next version thanks a lot for your help :) src/security/security_dac.c | 85 +++-- 1 file changed, 82 insertions(+), 3 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 85253af..8fd1e6e 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -22,6 +22,12 @@ #include sys/types.h #include sys/stat.h #include fcntl.h +#include errno.h Isn't errno.h implicitly included by our generic headers, such as internal.h? Yes, i notice errno.h included in internal.h, and internal.h include in virerror.h. So this place include is unnecessary, thanks for pointing out. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: forbid negative number in address(like controller, bus, slot...)
https://bugzilla.redhat.com/show_bug.cgi?id=1171582
When we edit a negative controller address number to a device, some
of them will auto generate a controller with invalid index number. This
will make guest disappear after restart libvirtd. Instead of allow
negative number for controller index, I think we should forbid negative
number in these place (we did this before, but after f18c02ec,
virStrToLong_ui changed to allow negative number). So changed to use
virStrToLong_uip in these place.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/device_conf.c | 4 ++--
src/conf/domain_conf.c | 32
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index b3b04e1..a7ec8a6 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -71,14 +71,14 @@ virDevicePCIAddressParseXML(xmlNodePtr node,
}
if (bus
-virStrToLong_ui(bus, NULL, 0, addr-bus) 0) {
+virStrToLong_uip(bus, NULL, 0, addr-bus) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'bus' attribute));
goto cleanup;
}
if (slot
-virStrToLong_ui(slot, NULL, 0, addr-slot) 0) {
+virStrToLong_uip(slot, NULL, 0, addr-slot) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'slot' attribute));
goto cleanup;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d81c37..961ec67 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3453,28 +3453,28 @@ virDomainDeviceDriveAddressParseXML(xmlNodePtr node,
unit = virXMLPropString(node, unit);
if (controller
-virStrToLong_ui(controller, NULL, 10, addr-controller) 0) {
+virStrToLong_uip(controller, NULL, 10, addr-controller) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'controller' attribute));
goto cleanup;
}
if (bus
-virStrToLong_ui(bus, NULL, 10, addr-bus) 0) {
+virStrToLong_uip(bus, NULL, 10, addr-bus) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'bus' attribute));
goto cleanup;
}
if (target
-virStrToLong_ui(target, NULL, 10, addr-target) 0) {
+virStrToLong_uip(target, NULL, 10, addr-target) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'target' attribute));
goto cleanup;
}
if (unit
-virStrToLong_ui(unit, NULL, 10, addr-unit) 0) {
+virStrToLong_uip(unit, NULL, 10, addr-unit) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'unit' attribute));
goto cleanup;
@@ -3507,21 +3507,21 @@ virDomainDeviceVirtioSerialAddressParseXML(
port = virXMLPropString(node, port);
if (controller
-virStrToLong_ui(controller, NULL, 10, addr-controller) 0) {
+virStrToLong_uip(controller, NULL, 10, addr-controller) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'controller' attribute));
goto cleanup;
}
if (bus
-virStrToLong_ui(bus, NULL, 10, addr-bus) 0) {
+virStrToLong_uip(bus, NULL, 10, addr-bus) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'bus' attribute));
goto cleanup;
}
if (port
-virStrToLong_ui(port, NULL, 10, addr-port) 0) {
+virStrToLong_uip(port, NULL, 10, addr-port) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'port' attribute));
goto cleanup;
@@ -3553,19 +3553,19 @@ virDomainDeviceCCWAddressParseXML(xmlNodePtr node,
if (cssid ssid devno) {
if (cssid
-virStrToLong_ui(cssid, NULL, 0, addr-cssid) 0) {
+virStrToLong_uip(cssid, NULL, 0, addr-cssid) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'cssid' attribute));
goto cleanup;
}
if (ssid
-virStrToLong_ui(ssid, NULL, 0, addr-ssid) 0) {
+virStrToLong_uip(ssid, NULL, 0, addr-ssid) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'ssid' attribute));
goto cleanup;
}
if (devno
-virStrToLong_ui(devno, NULL, 0, addr-devno) 0) {
+virStrToLong_uip(devno, NULL, 0, addr-devno) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'devno' attribute));
goto cleanup;
@@ -3607,14 +3607,14 @@ virDomainDeviceCcidAddressParseXML(xmlNodePtr node,
slot
[libvirt] [PATCHv2 1/2] qemu: output error when try to hotplug unsupport console
https://bugzilla.redhat.com/show_bug.cgi?id=1164627 When try to use attach-device to hotplug a qemu unsupport sonsole, command will return success, and add a console in vm's xml.But this doesn't work in qemu, qemu doesn't support these console.Add a error output when try to hotplug a unsupport console in qemuBuildConsoleChrDeviceStr. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_command.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1399ce4..7dced5f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10069,13 +10069,17 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr, break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: +break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST: -break; +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unsupported console target type %s), + NULLSTR(virDomainChrConsoleTargetTypeToString(chr-targetType))); +return ret; } ret = 0; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: add a check when cold-plug a Chr device
Add a func just check the base target type which
qemu support. But i still doubt this will be useful
, we already have a check when we try to start the
vm. And this check only check the target type, and
the other things will be done in virDomainChrDefParseXML.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_hotplug.c | 64 +
1 file changed, 64 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b9a0cee..fe91ec7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1384,10 +1384,74 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr
driver,
}
+static int
+qemuDomainChrCheckDefSupport(virDomainChrDefPtr chr)
+{
+int ret = -1;
+
+switch (chr-deviceType) {
+case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
+switch ((virDomainChrSerialTargetType) chr-targetType) {
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
+ret = 0;
+break;
+
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unsupported serial target type %s for qemu),
+
NULLSTR(virDomainChrSerialTargetTypeToString(chr-targetType)));
+break;
+}
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
+ret = 0;
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
+switch ((virDomainChrChannelTargetType) chr-targetType) {
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
+ret = 0;
+break;
+
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unsupported channel target type %s for qemu),
+
NULLSTR(virDomainChrChannelTargetTypeToString(chr-targetType)));
+break;
+}
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
+switch ((virDomainChrConsoleTargetType) chr-targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+ret = 0;
+break;
+
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unsupported console target type %s for qemu),
+
NULLSTR(virDomainChrConsoleTargetTypeToString(chr-targetType)));
+break;
+}
+break;
+}
+
+return ret;
+}
+
int
qemuDomainChrInsert(virDomainDefPtr vmdef,
virDomainChrDefPtr chr)
{
+if (qemuDomainChrCheckDefSupport(chr) 0)
+return -1;
+
if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2]qemu: output error when try to hotplug/coldplug a unsupported device
When use attach-device to hotplug a qemu unsupported console, command will success and add a XML to the running guest, but donnot do anything in qemu side. Add a check in qemuBuildConsoleChrDeviceStr, and output a error when try to attach a qemu unsupport console. About report error for qemu unsupported Chr device when cold-plug, I think this maybe unnessary in this place, because we will check it when we start the guest and it will report a clear error.But if we use qemu* header func add a qemu unsupported things to qemu guest, it seems strange. Luyao Huang (2): qemu: output error when try to hotplug unsupport console qemu: add a check when cold-plug a Chr device src/qemu/qemu_command.c | 6 - src/qemu/qemu_hotplug.c | 64 + 2 files changed, 69 insertions(+), 1 deletion(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 2/2] security: Add a new func use stat to get process DAC label
When use qemuProcessAttach to attach a qemu process, cannot
get a right DAC label. Add a new func to get process label
via stat func. Do not remove virDomainDefGetSecurityLabelDef
before try to use stat to get process DAC label, because
There are some other func call virSecurityDACGetProcessLabel.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
v2 add support freeBSD.
v3 use snprintf instead of VirAsprintf and move the error
settings in virSecurityDACGetProcessLabelInternal.
v4 remove errno.h include and thanks Eric advice move this
version comment to this place.
src/security/security_dac.c | 84 +++--
1 file changed, 81 insertions(+), 3 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 85253af..300b245 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -23,6 +23,11 @@
#include sys/stat.h
#include fcntl.h
+#ifdef __FreeBSD__
+# include sys/sysctl.h
+# include sys/user.h
+#endif
+
#include security_dac.h
#include virerror.h
#include virfile.h
@@ -1236,17 +1241,90 @@ virSecurityDACReserveLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
return 0;
}
+#ifdef __linux__
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct stat sb;
+char *path = NULL;
+int ret = -1;
+
+VIR_INFO(Getting DAC user and group on process '%d', pid);
+
+if (virAsprintf(path, /proc/%d, (int) pid) 0)
+goto cleanup;
+
+if (lstat(path, sb) 0) {
+virReportSystemError(errno,
+ _(unable to get PID %d uid and gid via stat),
+ pid);
+goto cleanup;
+}
+
+snprintf(seclabel-label, VIR_SECURITY_LABEL_BUFLEN,
+ +%u:+%u, (unsigned int) sb.st_uid, (unsigned int) sb.st_gid);
+ret = 0;
+
+cleanup:
+VIR_FREE(path);
+return ret;
+}
+#elif defined(__FreeBSD__)
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+struct kinfo_proc p;
+int mib[4];
+size_t len = 4;
+
+sysctlnametomib(kern.proc.pid, mib, len);
+
+len = sizeof(struct kinfo_proc);
+mib[3] = pid;
+
+if (sysctl(mib, 4, p, len, NULL, 0) 0) {
+virReportSystemError(errno,
+ _(unable to get PID %d uid and gid via sysctl),
+ pid);
+return -1;
+}
+
+snprintf(seclabel-label, VIR_SECURITY_LABEL_BUFLEN,
+ +%u:+%u, (unsigned int) p.ki_ruid, (unsigned int) p.ki_rgid);
+
+return 0;
+}
+#else
+static int
+virSecurityDACGetProcessLabelInternal(pid_t pid,
+ virSecurityLabelPtr seclabel)
+{
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ Cannot get proccess DAC label for pid %d on this platform,
+ (int) pid);
+return -1;
+}
+#endif
+
static int
virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
virDomainDefPtr def,
- pid_t pid ATTRIBUTE_UNUSED,
+ pid_t pid,
virSecurityLabelPtr seclabel)
{
virSecurityLabelDefPtr secdef =
virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (!secdef || !seclabel)
-return -1;
+if (secdef == NULL) {
+VIR_DEBUG(missing label for DAC security
+ driver in domain %s, def-name);
+
+if (virSecurityDACGetProcessLabelInternal(pid, seclabel) 0)
+return -1;
+return 0;
+}
if (secdef-label)
ignore_value(virStrcpy(seclabel-label, secdef-label,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: forbid negative number in address(like controller, bus, slot...)
On 12/09/2014 06:38 PM, Michal Privoznik wrote:
On 08.12.2014 09:27, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1171582
When we edit a negative controller address number to a device, some
of them will auto generate a controller with invalid index number. This
will make guest disappear after restart libvirtd. Instead of allow
negative number for controller index, I think we should forbid negative
number in these place (we did this before, but after f18c02ec,
virStrToLong_ui changed to allow negative number). So changed to use
virStrToLong_uip in these place.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/device_conf.c | 4 ++--
src/conf/domain_conf.c | 32
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index b3b04e1..a7ec8a6 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -71,14 +71,14 @@ virDevicePCIAddressParseXML(xmlNodePtr node,
}
if (bus
-virStrToLong_ui(bus, NULL, 0, addr-bus) 0) {
+virStrToLong_uip(bus, NULL, 0, addr-bus) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'bus' attribute));
goto cleanup;
}
if (slot
-virStrToLong_ui(slot, NULL, 0, addr-slot) 0) {
+virStrToLong_uip(slot, NULL, 0, addr-slot) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'slot' attribute));
goto cleanup;
There are other attributes that you missed: domain and function. Even
though they are tested in virDevicePCIAddressIsValid() it is not
sufficient. For instance, for function you can have a number from 0 to
7 (including). Since function is an unsigned int, entering the correct
negative value that is equal to unsigned value from the range would
accept the value. But it shouldn't.
Thanks a lot for pointing out, I forgot them when i wrote this patch.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2d81c37..961ec67 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3453,28 +3453,28 @@
virDomainDeviceDriveAddressParseXML(xmlNodePtr node,
unit = virXMLPropString(node, unit);
if (controller
-virStrToLong_ui(controller, NULL, 10, addr-controller) 0) {
+virStrToLong_uip(controller, NULL, 10, addr-controller)
0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'controller'
attribute));
goto cleanup;
}
if (bus
-virStrToLong_ui(bus, NULL, 10, addr-bus) 0) {
+virStrToLong_uip(bus, NULL, 10, addr-bus) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'bus' attribute));
goto cleanup;
}
if (target
-virStrToLong_ui(target, NULL, 10, addr-target) 0) {
+virStrToLong_uip(target, NULL, 10, addr-target) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'target'
attribute));
goto cleanup;
}
if (unit
-virStrToLong_ui(unit, NULL, 10, addr-unit) 0) {
+virStrToLong_uip(unit, NULL, 10, addr-unit) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'unit' attribute));
goto cleanup;
@@ -3507,21 +3507,21 @@ virDomainDeviceVirtioSerialAddressParseXML(
port = virXMLPropString(node, port);
if (controller
-virStrToLong_ui(controller, NULL, 10, addr-controller) 0) {
+virStrToLong_uip(controller, NULL, 10, addr-controller)
0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'controller'
attribute));
goto cleanup;
}
if (bus
-virStrToLong_ui(bus, NULL, 10, addr-bus) 0) {
+virStrToLong_uip(bus, NULL, 10, addr-bus) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'bus' attribute));
goto cleanup;
}
if (port
-virStrToLong_ui(port, NULL, 10, addr-port) 0) {
+virStrToLong_uip(port, NULL, 10, addr-port) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'port' attribute));
goto cleanup;
@@ -3553,19 +3553,19 @@ virDomainDeviceCCWAddressParseXML(xmlNodePtr
node,
if (cssid ssid devno) {
if (cssid
-virStrToLong_ui(cssid, NULL, 0, addr-cssid) 0) {
+virStrToLong_uip(cssid, NULL, 0, addr-cssid) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Cannot parse address 'cssid'
attribute));
goto cleanup;
}
if (ssid
-virStrToLong_ui(ssid, NULL, 0, addr-ssid) 0
[libvirt] [PATCH] conf: goto error when value max_sectors too large
Output error when we try to set a too large max_sectors.
Just like queues and cmd_per_lun here.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2965d8d..d4ac301 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6681,6 +6681,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
if (max_sectors virStrToLong_ui(max_sectors, NULL, 10,
def-max_sectors) 0) {
virReportError(VIR_ERR_XML_ERROR,
_(Malformed 'max_sectors' value %s'), max_sectors);
+goto error;
}
if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_USB
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Ignore device address for model=none usb controller and memballon
It make no sense at all to have it there.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ec45b8c..2965d8d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6683,8 +6683,12 @@ virDomainControllerDefParseXML(xmlNodePtr node,
_(Malformed 'max_sectors' value %s'), max_sectors);
}
-if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0)
+if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_USB
+def-model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) {
+VIR_DEBUG(Ignoring device address for none model usb controller);
+} else if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0)
{
goto error;
+}
switch (def-type) {
case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: {
@@ -9989,7 +9993,9 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
goto error;
}
-if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0)
+if (def-model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)
+VIR_DEBUG(Ignoring device address for none model Memballoon);
+else if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0)
goto error;
cleanup:
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: fix crash when match a network iscsi hostdev with a host iscsi hostdev
https://bugzilla.redhat.com/show_bug.cgi?id=1174053
When we use attach-device to coldplug a network iscsi hostdev,
libvirt will check if there is already a device in XML. But if
the 'b' is a host iscsi hostdev and 'a' is a network iscsi hostdev
, libvirtd will crash in virDomainHostdevMatchSubsysSCSIiSCSI,
because 'b' doesn't have a hostname.
Add a check in virDomainHostdevMatchSubsys, if the a's protocol
and b's protocol is not the same.
backtrace like this:
0 0x7f850d6bc307 in virDomainHostdevMatchSubsysSCSIiSCSI at
conf/domain_conf.c:10889
1 virDomainHostdevMatchSubsys at conf/domain_conf.c:10911
2 virDomainHostdevMatch at conf/domain_conf.c:10973
3 virDomainHostdevFind at conf/domain_conf.c:10998
4 0x7f84f6a10560 in qemuDomainAttachDeviceConfig at qemu/qemu_driver.c:7223
5 qemuDomainAttachDeviceFlags at qemu/qemu_driver.c:7554
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5cf0b1a..eb63c93 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11184,7 +11184,9 @@ static int
virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
virDomainHostdevDefPtr b)
{
-if (a-source.subsys.type != b-source.subsys.type)
+if (a-source.subsys.type != b-source.subsys.type ||
+(a-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI
+ a-source.subsys.u.scsi.protocol != b-source.subsys.u.scsi.protocol))
return 0;
switch (a-source.subsys.type) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: fix virDomainLeaseIndex cannot work when both parameter have lockspaces present
https://bugzilla.redhat.com/show_bug.cgi?id=1174096
When both parameter have lockspaces present, virDomainLeaseIndex
will always -1 even there is a lease the same with the one we check.
I think we shouldn't do 'continue' when the two lockspaces are the same.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5cf0b1a..f36affc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11667,13 +11667,14 @@ int virDomainLeaseIndex(virDomainDefPtr def,
for (i = 0; i def-nleases; i++) {
vlease = def-leases[i];
-/* Either both must have lockspaces present which match.. */
-if (vlease-lockspace lease-lockspace
-STRNEQ(vlease-lockspace, lease-lockspace))
-continue;
+/* Either both must have lockspaces present which match.. */
+if (vlease-lockspace lease-lockspace) {
+if (STRNEQ(vlease-lockspace, lease-lockspace))
+continue;
/* ...or neither must have a lockspace present */
-if (vlease-lockspace || lease-lockspace)
+} else if (vlease-lockspace || lease-lockspace)
continue;
+
if (STREQ(vlease-key, lease-key))
return i;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix use a nonexist address in qemuDomainAttachDeviceConfig
We free them before, then use it. This make we always do virDomainDefAddImplicitControllers when attach a disk. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df3ba6d..a9afa5d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7223,10 +7223,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (virDomainDiskInsert(vmdef, disk)) return -1; /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ -dev-data.disk = NULL; if (disk-bus != VIR_DOMAIN_DISK_BUS_VIRTIO) if (virDomainDefAddImplicitControllers(vmdef) 0) return -1; +dev-data.disk = NULL; if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) 0) return -1; break; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Auto generate a controller when attach hostdev and chr device
https://bugzilla.redhat.com/show_bug.cgi?id=1174154 When we use attach-device add a hostdev or chr device which have a iscsi address or others (just like guest agent, subsys iscsi disk...), we will find there is no basic controller for our new attached device. Somtimes this will make guest cannot start after we add them (although they can start at the second time). Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_driver.c | 4 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df3ba6d..62fb784 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7250,6 +7250,8 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (virDomainHostdevInsert(vmdef, hostdev)) return -1; dev-data.hostdev = NULL; +if (virDomainDefAddImplicitControllers(vmdef) 0) +return -1; if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) 0) return -1; break; @@ -7290,6 +7292,8 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (qemuDomainChrInsert(vmdef, dev-data.chr) 0) return -1; dev-data.chr = NULL; +if (virDomainDefAddImplicitControllers(vmdef) 0) +return -1; break; case VIR_DOMAIN_DEVICE_FS: -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix crash when match a network iscsi hostdev with a host iscsi hostdev
On 12/15/2014 07:58 PM, John Ferlan wrote:
On 12/14/2014 10:09 PM, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1174053
When we use attach-device to coldplug a network iscsi hostdev,
libvirt will check if there is already a device in XML. But if
the 'b' is a host iscsi hostdev and 'a' is a network iscsi hostdev
, libvirtd will crash in virDomainHostdevMatchSubsysSCSIiSCSI,
because 'b' doesn't have a hostname.
Add a check in virDomainHostdevMatchSubsys, if the a's protocol
and b's protocol is not the same.
backtrace like this:
0 0x7f850d6bc307 in virDomainHostdevMatchSubsysSCSIiSCSI at
conf/domain_conf.c:10889
1 virDomainHostdevMatchSubsys at conf/domain_conf.c:10911
2 virDomainHostdevMatch at conf/domain_conf.c:10973
3 virDomainHostdevFind at conf/domain_conf.c:10998
4 0x7f84f6a10560 in qemuDomainAttachDeviceConfig at qemu/qemu_driver.c:7223
5 qemuDomainAttachDeviceFlags at qemu/qemu_driver.c:7554
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5cf0b1a..eb63c93 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11184,7 +11184,9 @@ static int
virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
virDomainHostdevDefPtr b)
{
-if (a-source.subsys.type != b-source.subsys.type)
+if (a-source.subsys.type != b-source.subsys.type ||
+(a-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI
+ a-source.subsys.u.scsi.protocol != b-source.subsys.u.scsi.protocol))
While the check works - it's in the wrong place. It should be in the
subsequent switch. I'll clean it up a bit and also reference the commit
id that introduced the issue as part of the commit message
Thanks a lot for your help and i will try to fix in a right place next
time :)
John
return 0;
switch (a-source.subsys.type) {
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix virDomainLeaseIndex cannot work when both parameter have lockspaces present
On 12/15/2014 09:27 PM, Michal Privoznik wrote:
On 15.12.2014 07:46, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1174096
When both parameter have lockspaces present, virDomainLeaseIndex
will always -1 even there is a lease the same with the one we check.
I think we shouldn't do 'continue' when the two lockspaces are the same.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5cf0b1a..f36affc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11667,13 +11667,14 @@ int virDomainLeaseIndex(virDomainDefPtr def,
for (i = 0; i def-nleases; i++) {
vlease = def-leases[i];
-/* Either both must have lockspaces present which match.. */
-if (vlease-lockspace lease-lockspace
-STRNEQ(vlease-lockspace, lease-lockspace))
-continue;
+/* Either both must have lockspaces present which match.. */
+if (vlease-lockspace lease-lockspace) {
+if (STRNEQ(vlease-lockspace, lease-lockspace))
+continue;
/* ...or neither must have a lockspace present */
-if (vlease-lockspace || lease-lockspace)
+} else if (vlease-lockspace || lease-lockspace)
continue;
When an 'if' statement has body enclosed in brackets, so must have the
'else' branch.
Ohhh, i see. Thanks a lot for your pointing out :)
+
if (STREQ(vlease-key, lease-key))
return i;
}
ACked, fixed and pushed.
Thanks
Michal
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: fix cannot start a guest have a shareable network iscsi hostdev
https://bugzilla.redhat.com/show_bug.cgi?id=1174569 We should do nothing for the shareable network iscsi hostdev in qemuAddSharedHostdev and qemuRemoveSharedHostdev. Shareable for a network iscsi hostdev is not valid, so just ignore it. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_conf.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8d3818e..9539231 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1158,7 +1158,8 @@ qemuAddSharedHostdev(virQEMUDriverPtr driver, if (!hostdev-shareable || !(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS - hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI + hostdev-source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)) return 0; if (!(key = qemuGetSharedHostdevKey(hostdev))) @@ -1261,7 +1262,8 @@ qemuRemoveSharedHostdev(virQEMUDriverPtr driver, if (!hostdev-shareable || !(hostdev-mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS - hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) + hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI + hostdev-source.subsys.u.scsi.protocol != VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)) return 0; if (!(key = qemuGetSharedHostdevKey(hostdev))) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix use a nonexist address in qemuDomainAttachDeviceConfig
On 12/16/2014 08:00 PM, Peter Krempa wrote: On 12/15/14 10:10, Luyao Huang wrote: We free them before, then use it. This make we always do We clear the pointer, not free. virDomainDefAddImplicitControllers when attach a disk. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df3ba6d..a9afa5d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7223,10 +7223,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (virDomainDiskInsert(vmdef, disk)) return -1; /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ -dev-data.disk = NULL; The comment even explains why the line is there. From that point on, the pointer is owned by 'vmdef' and thus the original needs to be cleared so that it isn't freed (and doulbe freed) later on. Oh, i see. Thanks for your review and i found seems i don't check if gdb is real go to virDomainDefAddImplicitControllers, just check the pointer's value is optimized out. (gdb) 7204dev-data.disk = NULL; (gdb) 7205if (disk-bus != VIR_DOMAIN_DISK_BUS_VIRTIO) (gdb) p disk $3 = optimized out Thanks a lot for your help :) if (disk-bus != VIR_DOMAIN_DISK_BUS_VIRTIO) Additionally, 'disk' contains still a valid copy of the pointer so the code should run just fine. if (virDomainDefAddImplicitControllers(vmdef) 0) return -1; +dev-data.disk = NULL; So moving the line here doesn't make sense. if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) 0) return -1; break; NACK, Peter Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix use a nonexist address in qemuDomainAttachDeviceConfig
On 12/16/2014 08:33 PM, Peter Krempa wrote: On 12/16/14 13:32, Luyao Huang wrote: On 12/16/2014 08:00 PM, Peter Krempa wrote: On 12/15/14 10:10, Luyao Huang wrote: We free them before, then use it. This make we always do We clear the pointer, not free. virDomainDefAddImplicitControllers when attach a disk. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df3ba6d..a9afa5d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7223,10 +7223,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (virDomainDiskInsert(vmdef, disk)) return -1; /* vmdef has the pointer. Generic codes for vmdef will do all jobs */ -dev-data.disk = NULL; The comment even explains why the line is there. From that point on, the pointer is owned by 'vmdef' and thus the original needs to be cleared so that it isn't freed (and doulbe freed) later on. Oh, i see. Thanks for your review and i found seems i don't check if gdb is real go to virDomainDefAddImplicitControllers, just check the pointer's value is optimized out. (gdb) 7204dev-data.disk = NULL; (gdb) 7205if (disk-bus != VIR_DOMAIN_DISK_BUS_VIRTIO) (gdb) p disk $3 = optimized out optimized out means that GDB can't access the variable as it is stored in a register or for other reason caused by the code optimizer. It does not mean that the variable is cleared or whatever, just that GDB can't print it. Got it, thanks in advance for your help. I should be more carefully when i wrote the patch. Peter Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix cannot start a guest have a shareable network iscsi hostdev
On 12/17/2014 06:27 PM, Michal Privoznik wrote: On 17.12.2014 11:04, lhuang wrote: On 12/16/2014 11:46 PM, Michal Privoznik wrote: On 16.12.2014 04:16, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1174569 We should do nothing for the shareable network iscsi hostdev in qemuAddSharedHostdev and qemuRemoveSharedHostdev. Shareable for a network iscsi hostdev is not valid, so just ignore it. If it is invalid, can't we just forbid it in the parsing phase? Thanks for your review. Maybe 'invalid' this words is not correct here, after i check the code in qemuRemoveSharedHostdev there are some words in there: Currently the only conflicts we have to care about for the shared disk and shared host device is sgio setting, which is only valid for block disk and scsi host device. I think this means we should do nothing for the network iscsi hostdev (just like what we do for the network disk, usb hostdev...). And i don't think it means the shareable/ is invalid for these disk, because they are already 'shareable' in guests even libvirt do nothing. But i cannot make sure i am right :) Okay, I've reworded the commit message and pushed. ACK Thanks a lot for your review and help. It is my fault i should write a clearly description when i sent this patch. Michal Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] lxc: fix show the wrong xml when guest start failed
https://bugzilla.redhat.com/show_bug.cgi?id=1176503
When guest start failed, libvirt will keep the current vm-def,
this will make a issue that we cannot get a right xml after guest
start failed. Pass the newDef to def will make it work well.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/lxc/lxc_process.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 1c0d4e5..b7171ac 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1353,10 +1353,6 @@ int virLXCProcessStart(virConnectPtr conn,
VIR_FREE(veths[i]);
}
if (rc != 0) {
-if (vm-newDef) {
-virDomainDefFree(vm-newDef);
-vm-newDef = NULL;
-}
if (priv-monitor) {
virObjectUnref(priv-monitor);
priv-monitor = NULL;
@@ -1373,6 +1369,12 @@ int virLXCProcessStart(virConnectPtr conn,
VIR_FREE(vm-def-seclabels[0]-label);
VIR_FREE(vm-def-seclabels[0]-imagelabel);
}
+if (vm-newDef) {
+virDomainDefFree(vm-def);
+vm-def = vm-newDef;
+vm-def-id = -1;
+vm-newDef = NULL;
+}
}
for (i = 0; i nttyFDs; i++)
VIR_FORCE_CLOSE(ttyFDs[i]);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: Don't use the current state in def-data.network.actual when migrate
https://bugzilla.redhat.com/show_bug.cgi?id=1177194 When migrate a vm, we will generate a xml via qemuDomainDefFormatLive and pass this xml to target libvirtd. Libvirt will use the current network state in def-data.network.actual to generate the xml, this will make migrate failed when we set a network type guest interface use a macvtap network as a source in a vm then migrate vm to another host(which has the different macvtap network settings: different interface name, bridge name...) Add a flag check in virDomainNetDefFormat, if we set a VIR_DOMAIN_XML_MIGRATABLE flag when call virDomainNetDefFormat, we won't get the current vm interface state. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index aafc05e..fffd6cd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -17436,7 +17436,9 @@ virDomainNetDefFormat(virBufferPtr buf, unsigned int actualType = virDomainNetGetActualType(def); bool publicActual = (def-type == VIR_DOMAIN_NET_TYPE_NETWORK def-data.network.actual - !(flags (VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET))); + !(flags (VIR_DOMAIN_XML_INACTIVE | + VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET | + VIR_DOMAIN_XML_MIGRATABLE))); const char *typeStr; virDomainHostdevDefPtr hostdef = NULL; char macstr[VIR_MAC_STRING_BUFLEN]; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix tc old rules will be cleaned after set tc new rules fail
https://bugzilla.redhat.com/show_bug.cgi?id=1177723
If tc cmd failed when we use qemuDomainSetInterfaceParameters, the old
rules will be clean. Restore the old rules if tc failed.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c01da6c..810bd35 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9915,8 +9915,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
if (virNetDevBandwidthSet(net-ifname, newBandwidth, false) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot set bandwidth limits on %s),
+ _(cannot set bandwidth limits on %s
+ - attempting to restore old settings),
device);
+ignore_value(virNetDevBandwidthSet(net-ifname,
+ net-bandwidth,
+ false));
goto cleanup;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix tc old rules will be cleaned after set tc new rules fail
Please ignore this patch, i worked in a old upstream libvirt.
Sorry for the noisy.
Luyao
- Original Message -
From: Luyao Huang lhu...@redhat.com
To: libvir-list@redhat.com
Cc: Luyao Huang lhu...@redhat.com
Sent: Tuesday, December 30, 2014 5:00:03 PM
Subject: [libvirt] [PATCH] qemu: fix tc old rules will be cleaned after set
tc new rules fail
https://bugzilla.redhat.com/show_bug.cgi?id=1177723
If tc cmd failed when we use qemuDomainSetInterfaceParameters, the old
rules will be clean. Restore the old rules if tc failed.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c01da6c..810bd35 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9915,8 +9915,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
if (virNetDevBandwidthSet(net-ifname, newBandwidth, false) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(cannot set bandwidth limits on %s),
+ _(cannot set bandwidth limits on %s
+ - attempting to restore old settings),
device);
+ignore_value(virNetDevBandwidthSet(net-ifname,
+ net-bandwidth,
+ false));
goto cleanup;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] qemu: fix tc old rules will be cleaned if set tc new rules fail
https://bugzilla.redhat.com/show_bug.cgi?id=1177723
If tc cmd failed (maybe value too large) when we use
virDomainSetInterfaceParameters
, the old rules will be clean. Restore the old rules if tc failed.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
v1 make a big mistake that i used a old libvirt
src/qemu/qemu_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 73a825d..650e0dd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10494,8 +10494,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
sizeof(*newBandwidth-out));
}
-if (virNetDevBandwidthSet(net-ifname, newBandwidth, false) 0)
+if (virNetDevBandwidthSet(net-ifname, newBandwidth, false) 0) {
+ignore_value(virNetDevBandwidthSet(net-ifname,
+ net-bandwidth,
+ false));
goto endjob;
+}
virNetDevBandwidthFree(net-bandwidth);
if (newBandwidth-in || newBandwidth-out) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/12]qemu: support hot-plug/unplug RNG device
qemu already support hot-plug and hot-unplug RNG device. These patch will make libvirt support hot-plug/unplug RNG device for qemu driver. Luyao Huang (12): qemu: introduce a new func qemuAssignDeviceRNGAlias for rng device qemu: rename qemuBuildRNGDeviceArgs to qemuBuildRNGDevStr and change something conf: introduce a new func virDomainRNGEquals conf: introduce 3 functions for RNG device libvirt_private: add 4 new func in libvirt_private.syms qemu: add id when build RNG device and rename object id qemu: introduce 2 func qemuDomainRNGInsert and qemuDomainRNGRemove qemu: introduce 2 functions for attach a rng object in json monitor qemu_monitor: add 2 functions qemuMonitorDetachRNGDev and qemuMonitorAttachRNGDev audit: make function virDomainAuditRNG global qemu: Implement RNG device hotplug on live level qemu: Implement RNG device hotunplug on live level src/conf/domain_audit.c | 2 +- src/conf/domain_audit.h | 7 ++ src/conf/domain_conf.c | 78 src/conf/domain_conf.h | 12 +++ src/libvirt_private.syms | 6 ++ src/qemu/qemu_command.c | 70 +- src/qemu/qemu_command.h | 5 + src/qemu/qemu_driver.c | 12 ++- src/qemu/qemu_hotplug.c | 212 ++- src/qemu/qemu_hotplug.h | 14 ++- src/qemu/qemu_monitor.c | 43 + src/qemu/qemu_monitor.h | 7 ++ src/qemu/qemu_monitor_json.c | 58 src/qemu/qemu_monitor_json.h | 5 + 14 files changed, 502 insertions(+), 29 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/12] conf: introduce a new func virDomainRNGEquals
virDomainRNGEquals is a func which check if two rng device
are the same.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 34 ++
src/conf/domain_conf.h | 3 +++
2 files changed, 37 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aafc05e..91c114e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11922,6 +11922,40 @@ virDomainChrRemove(virDomainDefPtr vmdef,
return ret;
}
+bool
+virDomainRNGEquals(virDomainRNGDefPtr src,
+ virDomainRNGDefPtr tgt)
+{
+if (!src || !tgt)
+return src == tgt;
+
+if (src-model != tgt-model)
+return false;
+
+if (src-rate != tgt-rate || src-period != tgt-period)
+return false;
+
+switch ((virDomainRNGModel) src-model) {
+case VIR_DOMAIN_RNG_MODEL_VIRTIO:
+switch ((virDomainRNGBackend) src-backend) {
+case VIR_DOMAIN_RNG_BACKEND_RANDOM:
+return STREQ_NULLABLE(src-source.file, tgt-source.file);
+break;
+case VIR_DOMAIN_RNG_BACKEND_EGD:
+return virDomainChrSourceDefIsEqual((virDomainChrSourceDef *)
src-source.chardev,
+(virDomainChrSourceDef *)
tgt-source.chardev);
+break;
+case VIR_DOMAIN_RNG_BACKEND_LAST:
+break;
+}
+break;
+
+case VIR_DOMAIN_RNG_MODEL_LAST:
+break;
+}
+return false;
+}
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 57297cd..c197095 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2597,6 +2597,9 @@ virDomainChrInsert(virDomainDefPtr vmdef,
virDomainChrDefPtr
virDomainChrRemove(virDomainDefPtr vmdef,
virDomainChrDefPtr chr);
+bool
+virDomainRNGEquals(virDomainRNGDefPtr src,
+ virDomainRNGDefPtr tgt);
int virDomainSaveXML(const char *configDir,
virDomainDefPtr def,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/12] qemu: Implement RNG device hotplug on live level
We have enough patches for hotplug RNG device, maybe we can
implement live hotplug of a RNG device.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 8 -
src/qemu/qemu_hotplug.c | 92 +
src/qemu/qemu_hotplug.h | 3 ++
3 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 73a825d..2ad6e01 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6936,6 +6936,13 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
dev-data.chr = NULL;
break;
+case VIR_DOMAIN_DEVICE_RNG:
+ret = qemuDomainAttachRNGDevice(driver, vm,
+dev-data.rng);
+if (!ret)
+dev-data.rng = NULL;
+break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
@@ -6947,7 +6954,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_RNG:
case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f9327b4..7f1e612 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1523,6 +1523,98 @@ qemuDomainRNGRemove(virDomainDefPtr vmdef,
return ret;
}
+int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+virDomainDefPtr vmdef = vm-def;
+char *devstr = NULL;
+char *charAlias = NULL;
+char *objAlias = NULL;
+bool need_remove = false;
+bool releaseaddr = false;
+
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(qemu does not support -device));
+return ret;
+}
+
+if (qemuAssignDeviceRNGAlias(vmdef, rng, -1) 0)
+return ret;
+
+if (rng-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+if (STRPREFIX(vm-def-os.machine, s390-ccw)
+virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
+rng-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
+} else if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
+rng-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
+}
+}
+
+if (rng-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+rng-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+if (virDomainPCIAddressEnsureAddr(priv-pciaddrs, rng-info) 0)
+return ret;
+} else if (rng-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+if (virDomainCCWAddressAssign(rng-info, priv-ccwaddrs,
+ !rng-info.addr.ccw.assigned) 0)
+return ret;
+}
+releaseaddr = true;
+if (!(devstr = qemuBuildRNGDevStr(vmdef, rng, priv-qemuCaps)))
+return ret;
+
+if (virAsprintf(objAlias, obj%s, rng-info.alias) 0)
+goto cleanup;
+
+if (rng-backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
+if (virAsprintf(charAlias, char%s, rng-info.alias) 0)
+goto cleanup;
+
+qemuDomainObjEnterMonitor(driver, vm);
+if (qemuMonitorAttachCharDev(priv-mon, charAlias,
rng-source.chardev) 0) {
+qemuDomainObjExitMonitor(driver, vm);
+goto audit;
+}
+need_remove = true;
+} else {
+qemuDomainObjEnterMonitor(driver, vm);
+}
+
+if (qemuMonitorAttachRNGDev(priv-mon, charAlias, objAlias, rng) 0) {
+if (need_remove)
+qemuMonitorDetachCharDev(priv-mon, charAlias);
+qemuDomainObjExitMonitor(driver, vm);
+goto audit;
+}
+
+if (devstr qemuMonitorAddDevice(priv-mon, devstr) 0) {
+qemuMonitorDetachRNGDev(priv-mon, objAlias);
+if (need_remove)
+qemuMonitorDetachCharDev(priv-mon, charAlias);
+qemuDomainObjExitMonitor(driver, vm);
+goto audit;
+}
+qemuDomainObjExitMonitor(driver, vm);
+
+if (qemuDomainRNGInsert(vmdef, rng) 0)
+goto cleanup;
+
+ret = 0;
+ audit:
+virDomainAuditRNG(vm, NULL, rng, attach, ret == 0);
+ cleanup:
+if (releaseaddr)
+qemuDomainReleaseDeviceAddress(vm, rng-info, NULL);
+VIR_FREE(charAlias);
+VIR_FREE(objAlias);
+VIR_FREE(devstr);
+return ret;
+}
+
static int
qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 7b838ee..fbf009f 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -98,6 +98,9 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
int qemuDomainDetachChrDevice(virQEMUDriverPtr
[libvirt] [PATCH 07/12] qemu: introduce 2 func qemuDomainRNGInsert and qemuDomainRNGRemove
qemu side functions, call virDomainRNGInsert and virDomainRNGRemove
to help us.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_hotplug.c | 23 +++
src/qemu/qemu_hotplug.h | 7 +++
2 files changed, 30 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7f93b9b..f9327b4 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1501,6 +1501,29 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
return ret;
}
+int
+qemuDomainRNGInsert(virDomainDefPtr vmdef,
+virDomainRNGDefPtr rng)
+{
+return virDomainRNGInsert(vmdef, rng);
+}
+
+virDomainRNGDefPtr
+qemuDomainRNGRemove(virDomainDefPtr vmdef,
+virDomainRNGDefPtr rng)
+{
+virDomainRNGDefPtr ret;
+
+if (!(ret = virDomainRNGRemove(vmdef, rng))) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(device not present in domain configuration));
+return NULL;
+}
+
+return ret;
+}
+
+
static int
qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index d13c532..7b838ee 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -107,6 +107,13 @@ virDomainChrDefPtr
qemuDomainChrRemove(virDomainDefPtr vmdef,
virDomainChrDefPtr chr);
+int
+qemuDomainRNGInsert(virDomainDefPtr vmdef,
+virDomainRNGDefPtr rng);
+virDomainRNGDefPtr
+qemuDomainRNGRemove(virDomainDefPtr vmdef,
+virDomainRNGDefPtr rng);
+
void qemuDomainRemoveDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/12] libvirt_private: add 4 new func in libvirt_private.syms
Signed-off-by: Luyao Huang lhu...@redhat.com --- src/libvirt_private.syms | 5 + 1 file changed, 5 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index aa776b4..deab4cf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -375,7 +375,12 @@ virDomainPMSuspendedReasonTypeToString; virDomainRedirdevBusTypeFromString; virDomainRedirdevBusTypeToString; virDomainRNGBackendTypeToString; +virDomainRNGDefFree; +virDomainRNGEquals; +virDomainRNGFind; +virDomainRNGInsert; virDomainRNGModelTypeToString; +virDomainRNGRemove; virDomainRunningReasonTypeFromString; virDomainRunningReasonTypeToString; virDomainSaveConfig; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/12] qemu_monitor: add 2 functions qemuMonitorDetachRNGDev and qemuMonitorAttachRNGDev
These 2 functions just do some basic check and then call
qemuMonitorJSONAttachRNGDev and qemuMonitorDelObject to
help us.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_monitor.c | 43 +++
src/qemu/qemu_monitor.h | 7 +++
2 files changed, 50 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 100bbd0..9ac5934 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4140,6 +4140,49 @@ int qemuMonitorDetachCharDev(qemuMonitorPtr mon,
return qemuMonitorJSONDetachCharDev(mon, chrID);
}
+int qemuMonitorAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID,
+virDomainRNGDefPtr rng)
+{
+VIR_DEBUG(mon=%p chrID=%s objID=%s rng=%p, mon, chrID, objID, rng);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(monitor must not be NULL));
+return -1;
+}
+
+if (!mon-json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(JSON monitor is required));
+return -1;
+}
+
+return qemuMonitorJSONAttachRNGDev(mon, chrID, objID, rng);
+}
+
+int qemuMonitorDetachRNGDev(qemuMonitorPtr mon,
+ const char *objID)
+{
+VIR_DEBUG(mon=%p objID=%s, mon, objID);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(monitor must not be NULL));
+return -1;
+}
+
+if (!mon-json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(JSON monitor is required));
+return -1;
+}
+
+return qemuMonitorDelObject(mon, objID);
+}
+
+
int
qemuMonitorGetDeviceAliases(qemuMonitorPtr mon,
char ***aliases)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index edab66f..99587e8 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -865,6 +865,13 @@ int qemuMonitorAttachCharDev(qemuMonitorPtr mon,
int qemuMonitorDetachCharDev(qemuMonitorPtr mon,
const char *chrID);
+int qemuMonitorAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID,
+virDomainRNGDefPtr rng);
+int qemuMonitorDetachRNGDev(qemuMonitorPtr mon,
+const char *objID);
+
int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon,
char ***aliases);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/12] qemu: add id when build RNG device and rename object id
We didn't set a id when we build RNG device cmdline before.
Give a id to every RNG device and we can hotunplug it via
QMP cmd device_del.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 46e289d..a4073ee 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5761,7 +5761,7 @@ qemuBuildRNGBackendArgs(virCommandPtr cmd,
goto cleanup;
}
-virBufferAsprintf(buf, rng-random,id=%s,filename=%s,
+virBufferAsprintf(buf, rng-random,id=obj%s,filename=%s,
dev-info.alias, dev-source.file);
virCommandAddArg(cmd, -object);
@@ -5784,7 +5784,7 @@ qemuBuildRNGBackendArgs(virCommandPtr cmd,
virCommandAddArgList(cmd, -chardev, backend, NULL);
virCommandAddArg(cmd, -object);
-virCommandAddArgFormat(cmd, rng-egd,chardev=char%s,id=%s,
+virCommandAddArgFormat(cmd, rng-egd,chardev=char%s,id=obj%s,
dev-info.alias, dev-info.alias);
break;
@@ -5816,13 +5816,13 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
}
if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
-virBufferAsprintf(buf, virtio-rng-ccw,rng=%s, dev-info.alias);
+virBufferAsprintf(buf, virtio-rng-ccw,rng=obj%s,id=%s,
dev-info.alias, dev-info.alias);
else if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
-virBufferAsprintf(buf, virtio-rng-s390,rng=%s, dev-info.alias);
+virBufferAsprintf(buf, virtio-rng-s390,rng=obj%s,id=%s,
dev-info.alias, dev-info.alias);
else if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
-virBufferAsprintf(buf, virtio-rng-device,rng=%s, dev-info.alias);
+virBufferAsprintf(buf, virtio-rng-device,rng=obj%s,id=%s,
dev-info.alias, dev-info.alias);
else
-virBufferAsprintf(buf, virtio-rng-pci,rng=%s, dev-info.alias);
+virBufferAsprintf(buf, virtio-rng-pci,rng=obj%s,id=%s,
dev-info.alias, dev-info.alias);
if (dev-rate 0) {
virBufferAsprintf(buf, ,max-bytes=%u, dev-rate);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] conf: introduce 3 functions for RNG device
the 3 functions are:
virDomainRNGInsert: Insert a RNG device to vm-def.
virDomainRNGRemove: remove a RNG device in vm-def.
virDomainRNGFind: find a RNG device in vm-def.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 44
src/conf/domain_conf.h | 9 +
2 files changed, 53 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 91c114e..37c4569 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11956,6 +11956,50 @@ virDomainRNGEquals(virDomainRNGDefPtr src,
return false;
}
+int
+virDomainRNGInsert(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng)
+{
+return VIR_APPEND_ELEMENT(vmdef-rngs, vmdef-nrngs, rng);
+}
+
+virDomainRNGDefPtr
+virDomainRNGRemove(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng)
+{
+virDomainRNGDefPtr ret;
+size_t i;
+
+for (i = 0; i vmdef-nrngs; i++) {
+ret = vmdef-rngs[i];
+
+if (virDomainRNGEquals(ret, rng))
+break;
+}
+
+if (i == vmdef-nrngs)
+return NULL;
+
+VIR_DELETE_ELEMENT(vmdef-rngs, i, vmdef-nrngs);
+return ret;
+}
+
+virDomainRNGDefPtr
+virDomainRNGFind(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng)
+{
+virDomainRNGDefPtr ret;
+size_t i;
+
+for (i = 0; i vmdef-nrngs; i++) {
+ret = vmdef-rngs[i];
+
+if (virDomainRNGEquals(ret, rng))
+return ret;
+}
+return NULL;
+}
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c197095..cb87fad 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2600,6 +2600,15 @@ virDomainChrRemove(virDomainDefPtr vmdef,
bool
virDomainRNGEquals(virDomainRNGDefPtr src,
virDomainRNGDefPtr tgt);
+int
+virDomainRNGInsert(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng);
+virDomainRNGDefPtr
+virDomainRNGRemove(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng);
+virDomainRNGDefPtr
+virDomainRNGFind(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng);
int virDomainSaveXML(const char *configDir,
virDomainDefPtr def,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/12] qemu: Implement RNG device hotunplug on live level
We have enough patches for hotunplug RNG device, maybe we can
implement live hotunplug of a RNG device.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 4 +-
src/qemu/qemu_hotplug.c | 97 -
src/qemu/qemu_hotplug.h | 4 +-
3 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2ad6e01..f7600f3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7017,6 +7017,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_CHR:
ret = qemuDomainDetachChrDevice(driver, vm, dev-data.chr);
break;
+case VIR_DOMAIN_DEVICE_RNG:
+ret = qemuDomainDetachRNGDevice(driver, vm, dev-data.rng);
+break;
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
@@ -7027,7 +7030,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_RNG:
case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7f1e612..d61e2a1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2921,6 +2921,52 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
return ret;
}
+static int
+qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+virObjectEventPtr event;
+char *charAlias = NULL;
+char *objAlias = NULL;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int ret = -1;
+int rc;
+
+VIR_DEBUG(Removing RNG device %s from domain %p %s,
+ rng-info.alias, vm, vm-def-name);
+
+if (virAsprintf(objAlias, obj%s, rng-info.alias) 0)
+goto cleanup;
+
+if (virAsprintf(charAlias, char%s, rng-info.alias) 0)
+goto cleanup;
+
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorDetachRNGDev(priv-mon, objAlias);
+if (rc == 0 rng-backend == VIR_DOMAIN_RNG_BACKEND_EGD)
+ignore_value(qemuMonitorDetachCharDev(priv-mon, charAlias));
+qemuDomainObjExitMonitor(driver, vm);
+
+virDomainAuditRNG(vm, rng, NULL, detach, rc == 0);
+
+if (rc 0)
+goto cleanup;
+
+event = virDomainEventDeviceRemovedNewFromObj(vm, rng-info.alias);
+if (event)
+qemuDomainEventQueue(driver, event);
+
+qemuDomainRNGRemove(vm-def, rng);
+qemuDomainReleaseDeviceAddress(vm, rng-info, NULL);
+virDomainRNGDefFree(rng);
+ret = 0;
+
+ cleanup:
+VIR_FREE(charAlias);
+VIR_FREE(objAlias);
+return ret;
+}
void
qemuDomainRemoveDevice(virQEMUDriverPtr driver,
@@ -2944,6 +2990,9 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_DEVICE_CHR:
qemuDomainRemoveChrDevice(driver, vm, dev-data.chr);
break;
+case VIR_DOMAIN_DEVICE_RNG:
+qemuDomainRemoveRNGDevice(driver, vm, dev-data.rng);
+break;
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LEASE:
@@ -2958,7 +3007,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_RNG:
case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
@@ -3830,3 +3878,50 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
VIR_FREE(devstr);
return ret;
}
+
+int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+virDomainDefPtr vmdef = vm-def;
+virDomainRNGDefPtr tmpRNG;
+int rc;
+
+if (!(tmpRNG = virDomainRNGFind(vmdef, rng))) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(device not present in domain configuration));
+return ret;
+}
+
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(qemu does not support -device));
+return ret;
+}
+
+if (!tmpRNG-info.alias qemuAssignDeviceRNGAlias(vmdef, tmpRNG, -1) 0)
+return ret;
+
+sa_assert(tmpRNG-info.alias);
+
+qemuDomainMarkDeviceForRemoval(vm, tmpRNG-info);
+
+qemuDomainObjEnterMonitor(driver, vm);
+if (qemuMonitorDelDevice(priv-mon, tmpRNG-info.alias) 0) {
+qemuDomainObjExitMonitor(driver, vm);
+goto cleanup;
+}
+qemuDomainObjExitMonitor(driver, vm);
+
+rc = qemuDomainWaitForDeviceRemoval(vm);
+if (rc == 0 || rc == 1)
+ret = qemuDomainRemoveRNGDevice(driver, vm, tmpRNG);
+else
[libvirt] [PATCH 10/12] audit: make function virDomainAuditRNG global
Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_audit.c | 2 +- src/conf/domain_audit.h | 7 +++ src/libvirt_private.syms | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index fcf9df7..159ebf5 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -242,7 +242,7 @@ virDomainAuditDisk(virDomainObjPtr vm, } -static void +void virDomainAuditRNG(virDomainObjPtr vm, virDomainRNGDefPtr oldDef, virDomainRNGDefPtr newDef, const char *reason, bool success) diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 3434feb..4c1ef90 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -117,5 +117,12 @@ void virDomainAuditChardev(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditRNG(virDomainObjPtr vm, + virDomainRNGDefPtr oldDef, + virDomainRNGDefPtr newDef, + const char *reason, + bool success) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); + #endif /* __VIR_DOMAIN_AUDIT_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index deab4cf..9ed4583 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -123,6 +123,7 @@ virDomainAuditMemory; virDomainAuditNet; virDomainAuditNetDevice; virDomainAuditRedirdev; +virDomainAuditRNG; virDomainAuditSecurityLabel; virDomainAuditStart; virDomainAuditStop; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/12] qemu: introduce 2 functions for attach a rng object in json monitor
We need a new function to build a RNG device object, and need a
function to build a props which will be used in qemuMonitorJSONAddObject.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_monitor_json.c | 58
src/qemu/qemu_monitor_json.h | 5
2 files changed, 63 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e567aa7..4430819 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6166,6 +6166,64 @@ qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon,
return ret;
}
+static virJSONValuePtr
+qemuMonitorJSONRNGPropsCommand(const char *name,
+ const char *data)
+{
+virJSONValuePtr ret;
+
+if (!(ret = virJSONValueNewObject()))
+goto error;
+
+if (virJSONValueObjectAppendString(ret, name, data) 0)
+goto error;
+
+return ret;
+
+ error:
+virJSONValueFree(ret);
+return NULL;
+}
+
+int
+qemuMonitorJSONAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID,
+virDomainRNGDefPtr rng)
+{
+const char *type = NULL;
+virJSONValuePtr props = NULL;
+
+switch ((virDomainRNGBackend) rng-backend) {
+case VIR_DOMAIN_RNG_BACKEND_RANDOM:
+type = rng-random;
+if (!(props = qemuMonitorJSONRNGPropsCommand(filename,
rng-source.file)))
+goto cleanup;
+break;
+
+case VIR_DOMAIN_RNG_BACKEND_EGD:
+if (!chrID) {
+virReportError(VIR_ERR_INTERNAL_ERROR,%s,
+ _(miss chardev id));
+goto cleanup;
+}
+type = rng-egd;
+if (!(props = qemuMonitorJSONRNGPropsCommand(chardev, chrID)))
+goto cleanup;
+break;
+
+case VIR_DOMAIN_RNG_BACKEND_LAST:
+/*shouldn't happen*/
+goto cleanup;
+}
+
+return qemuMonitorJSONAddObject(mon, type, objID, props);
+
+ cleanup:
+virJSONValueFree(props);
+return -1;
+}
+
int
qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 222f11e..66c519d 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -459,6 +459,11 @@ int qemuMonitorJSONAttachCharDev(qemuMonitorPtr mon,
virDomainChrSourceDefPtr chr);
int qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon,
const char *chrID);
+int qemuMonitorJSONAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID,
+virDomainRNGDefPtr rng);
+
int qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon,
char ***aliases);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/12] qemu: rename qemuBuildRNGDeviceArgs to qemuBuildRNGDevStr and change something
rename qemuBuildRNGDeviceArgs to qemuBuildRNGDevStr, we need this function
to build a cmdline.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c | 33 -
src/qemu/qemu_command.h | 4
2 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c1e9bca..46e289d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5800,22 +5800,19 @@ qemuBuildRNGBackendArgs(virCommandPtr cmd,
return ret;
}
-
-static int
-qemuBuildRNGDeviceArgs(virCommandPtr cmd,
- virDomainDefPtr def,
- virDomainRNGDefPtr dev,
- virQEMUCapsPtr qemuCaps)
+char *
+qemuBuildRNGDevStr(virDomainDefPtr def,
+ virDomainRNGDefPtr dev,
+ virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
-int ret = -1;
if (dev-model != VIR_DOMAIN_RNG_MODEL_VIRTIO ||
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_RNG)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(this qemu doesn't support RNG device type '%s'),
virDomainRNGModelTypeToString(dev-model));
-goto cleanup;
+goto error;
}
if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
@@ -5836,16 +5833,14 @@ qemuBuildRNGDeviceArgs(virCommandPtr cmd,
}
if (qemuBuildDeviceAddressStr(buf, def, dev-info, qemuCaps) 0)
-goto cleanup;
-
-virCommandAddArg(cmd, -device);
-virCommandAddArgBuffer(cmd, buf);
-
-ret = 0;
+goto error;
+if (virBufferCheckError(buf) 0)
+goto error;
- cleanup:
+return virBufferContentAndReset(buf);
+ error:
virBufferFreeAndReset(buf);
-return ret;
+return NULL;
}
@@ -9802,13 +9797,17 @@ qemuBuildCommandLine(virConnectPtr conn,
}
for (i = 0; i def-nrngs; i++) {
+char *devstr;
/* add the RNG source backend */
if (qemuBuildRNGBackendArgs(cmd, def-rngs[i], qemuCaps) 0)
goto error;
/* add the device */
-if (qemuBuildRNGDeviceArgs(cmd, def, def-rngs[i], qemuCaps) 0)
+virCommandAddArg(cmd, -device);
+if (!(devstr = qemuBuildRNGDevStr(def, def-rngs[i], qemuCaps)))
goto error;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
}
if (def-nvram) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 9f3f249..4264215 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -163,6 +163,10 @@ char *qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
const char *configfd,
virQEMUCapsPtr qemuCaps);
+char *qemuBuildRNGDevStr(virDomainDefPtr def,
+ virDomainRNGDefPtr dev,
+ virQEMUCapsPtr qemuCaps);
+
int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
/* Legacy, pre device support */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/12] qemu: introduce a new func qemuAssignDeviceRNGAlias for rng device
This function used to set a alias name for RNG device, usage just
like other named qemuAssignDevice***Alias functions.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c | 25 -
src/qemu/qemu_command.h | 1 +
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d5679de..c1e9bca 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -909,6 +909,29 @@ qemuGetNextChrDevIndex(virDomainDefPtr def,
return idx;
}
+int
+qemuAssignDeviceRNGAlias(virDomainDefPtr def, virDomainRNGDefPtr rng, int idx)
+{
+if (idx == -1) {
+size_t i;
+idx = 0;
+for (i = 0; i def-nrngs; i++) {
+int thisidx;
+if ((thisidx = qemuDomainDeviceAliasIndex(def-rngs[i]-info,
rng)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to determine device index for RNG
device));
+return -1;
+}
+if (thisidx = idx)
+idx = thisidx + 1;
+}
+}
+
+if (virAsprintf(rng-info.alias, rng%d, idx) 0)
+return -1;
+
+return 0;
+}
int
qemuAssignDeviceChrAlias(virDomainDefPtr def,
@@ -1035,7 +1058,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps)
return -1;
}
for (i = 0; i def-nrngs; i++) {
-if (virAsprintf(def-rngs[i]-info.alias, rng%zu, i) 0)
+if (qemuAssignDeviceRNGAlias(def, def-rngs[i], i) 0)
return -1;
}
if (def-tpm) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index f7d3c2d..9f3f249 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -262,6 +262,7 @@ int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def,
virDomainRedirdevDefPtr r
int qemuAssignDeviceChrAlias(virDomainDefPtr def,
virDomainChrDefPtr chr,
ssize_t idx);
+int qemuAssignDeviceRNGAlias(virDomainDefPtr def, virDomainRNGDefPtr rng, int
idx);
int
qemuParseKeywords(const char *str,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/2] qemu: output error when try to hotplug unsupport console
On 01/22/2015 06:19 PM, Peter Krempa wrote: On Thu, Jan 22, 2015 at 10:28:18 +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1164627 When using 'virsh attach-device' to hotplug an unsupported console type into a qemu guest, the attachment will erroneously allows the attachment. This patch will check to ensure that only the support target types are used for a running guest. NB, Although a guest can be defined with an improper target, startup will cause failure. I tweaked the subject and commit message as some sentences didn't make sense. Thanks for your review and help. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_command.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c041ee7..df87e1e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10067,13 +10067,17 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr, break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: +break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST: -break; +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unsupported console target type %s), + NULLSTR(virDomainChrConsoleTargetTypeToString(chr-targetType))); +goto cleanup; } ACK, although the rest of the chardev command and hotplug code is a big mess and would benefit from a cleanup/refactor. Yes, i noticed this mess and i will try to fix this in next step. Pushed. Peter Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: forget goto error when the node-name is not valid
Although it won't cause a issue now, the code won't touch this place. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 706e5d2..3a0b13e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8609,6 +8609,7 @@ virDomainChrDefParseXML(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(unknown character device type: %s), nodeName); +goto error; } cur = node-children; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: fix forget goto error when report a unsupport error
https://bugzilla.redhat.com/show_bug.cgi?id=1188914
This will cause a issue that add a unsupport input device
to a hypervisor which unsupport it, also will cause a wrong
error message when the input device is not a mouse or keyboard.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 706e5d2..09b8e9b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8968,6 +8968,7 @@ virDomainInputDefParseXML(const virDomainDef *dom,
virReportError(VIR_ERR_INTERNAL_ERROR,
_(unsupported input bus %s),
bus);
+goto error;
}
if (def-type != VIR_DOMAIN_INPUT_TYPE_MOUSE
def-type != VIR_DOMAIN_INPUT_TYPE_KBD) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: recheck the validating backend when the firewalld start/stop
On 02/02/2015 07:38 PM, Daniel P. Berrange wrote: On Mon, Feb 02, 2015 at 11:40:44AM +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1188088 When the firewalld is running and then start the libvirtd, libvirt will set the current backend as VIR_FIREWALL_BACKEND_FIREWALLD. But when firewalld is stop, we still try to use firewalld even it is stopped, this will make the vm which has nwfilter cannot start because systemd cannot find a running firewalld service. We already have a Dbus callback functions before, add a recheck for the validating backend in firewalld_dbus_filter_bridge and nwfilterFirewalldDBusFilter callback functions to help us dynamic change the validating backend. NACK, this is not desirable IMHO. Just because firewalld is stopped does not imply that it should not be used by libvirt. It may simply be in the process of being restarted, either by the admin or due to an RPM upgrade. Switching a host between firewalld non-firewalld managmenet is not something that is typically done - the decision to use firewalld is something taken at time of initial provisioning. So I don't think libvirt should optimize for that scenario. We should optimize for a host always using one or the other exclusively and not try to dynamically switch. Got it, i hadn't thought about this when i wrote this patch. And thanks a lot for your clearly explanation. Regards, Daniel Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] lxc: fix double close handshakefds[1]
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/lxc/lxc_process.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 01da344..b385423 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1270,11 +1270,6 @@ int virLXCProcessStart(virConnectPtr conn,
goto error;
}
-if (VIR_CLOSE(handshakefds[1]) 0) {
-virReportSystemError(errno, %s, _(could not close handshake fd));
-goto error;
-}
-
if (virCommandHandshakeWait(cmd) 0)
goto error;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] lxc: fix show the wrong xml when guest start failed
https://bugzilla.redhat.com/show_bug.cgi?id=1176503
When guest start failed, libvirt will keep the current vm-def,
this will make a issue that we cannot get a right xml after guest
start failed. And don't call the stop/release hook to do some
other clean work.
Call virLXCProcessCleanup to help us clean the source and call
the hooks if start a vm failed
Signed-off-by: Luyao Huang lhu...@redhat.com
---
v2: use virLXCProcessCleanup to free the source and call the hook.
v3: rework the patch to suit the virLXCProcessStart code changed.
src/lxc/lxc_process.c | 76 ++-
1 file changed, 32 insertions(+), 44 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 01da344..1a6cfbb 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1022,6 +1022,7 @@ int virLXCProcessStart(virConnectPtr conn,
virCgroupPtr selfcgroup;
int status;
char *pidfile = NULL;
+bool need_stop = false;
if (virCgroupNewSelf(selfcgroup) 0)
return -1;
@@ -1259,6 +1260,7 @@ int virLXCProcessStart(virConnectPtr conn,
goto cleanup;
}
+need_stop = true;
priv-stopReason = VIR_DOMAIN_EVENT_STOPPED_FAILED;
priv-wantReboot = false;
vm-def-id = vm-pid;
@@ -1272,20 +1274,20 @@ int virLXCProcessStart(virConnectPtr conn,
if (VIR_CLOSE(handshakefds[1]) 0) {
virReportSystemError(errno, %s, _(could not close handshake fd));
-goto error;
+goto cleanup;
}
if (virCommandHandshakeWait(cmd) 0)
-goto error;
+goto cleanup;
/* Write domain status to disk for the controller to
* read when it starts */
if (virDomainSaveStatus(driver-xmlopt, cfg-stateDir, vm) 0)
-goto error;
+goto cleanup;
/* Allow the child to exec the controller */
if (virCommandHandshakeNotify(cmd) 0)
-goto error;
+goto cleanup;
if (virAtomicIntInc(driver-nactive) == 1 driver-inhibitCallback)
driver-inhibitCallback(true, driver-inhibitOpaque);
@@ -1298,7 +1300,7 @@ int virLXCProcessStart(virConnectPtr conn,
_(guest failed to start: %s), out);
}
-goto error;
+goto cleanup;
}
/* We know the cgroup must exist by this synchronization
@@ -1310,13 +1312,13 @@ int virLXCProcessStart(virConnectPtr conn,
vm-def-resource-partition :
NULL,
-1, priv-cgroup) 0)
-goto error;
+goto cleanup;
if (!priv-cgroup) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(No valid cgroup for machine %s),
vm-def-name);
-goto error;
+goto cleanup;
}
/* And we can get the first monitor connection now too */
@@ -1329,17 +1331,17 @@ int virLXCProcessStart(virConnectPtr conn,
virReportError(VIR_ERR_INTERNAL_ERROR,
_(guest failed to start: %s), ebuf);
}
-goto error;
+goto cleanup;
}
if (autoDestroy
virCloseCallbacksSet(driver-closeCallbacks, vm,
conn, lxcProcessAutoDestroy) 0)
-goto error;
+goto cleanup;
if (virDomainObjSetDefTransient(caps, driver-xmlopt,
vm, false) 0)
-goto error;
+goto cleanup;
/* We don't need the temporary NIC names anymore, clear them */
virLXCProcessCleanInterfaces(vm-def);
@@ -1358,47 +1360,38 @@ int virLXCProcessStart(virConnectPtr conn,
* If the script raised an error abort the launch
*/
if (hookret 0)
-goto error;
+goto cleanup;
}
rc = 0;
cleanup:
-if (rc != 0 !err)
-err = virSaveLastError();
-virCommandFree(cmd);
if (VIR_CLOSE(logfd) 0) {
virReportSystemError(errno, %s, _(could not close logfile));
rc = -1;
}
-for (i = 0; i nveths; i++) {
-if (rc != 0 veths[i])
-ignore_value(virNetDevVethDelete(veths[i]));
-VIR_FREE(veths[i]);
-}
if (rc != 0) {
-if (vm-newDef) {
-virDomainDefFree(vm-newDef);
-vm-newDef = NULL;
-}
-if (priv-monitor) {
-virObjectUnref(priv-monitor);
-priv-monitor = NULL;
-}
-virDomainConfVMNWFilterTeardown(vm);
-
-virSecurityManagerRestoreAllLabel(driver-securityManager,
- vm-def, false);
-virSecurityManagerReleaseLabel(driver-securityManager, vm-def);
-/* Clear out dynamically assigned labels */
-if (vm-def-nseclabels
-vm-def-seclabels[0]-type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
-VIR_FREE(vm-def-seclabels[0]-model);
-VIR_FREE(vm-def-seclabels[0]-label);
-VIR_FREE
[libvirt] [PATCH] qemu: fix cannot set graphic passwd via qemuDomainSaveImageDefineXML
https://bugzilla.redhat.com/show_bug.cgi?id=1183890
When we try to update a xml to a image file, we will clear the
graphics passwd settings, because we do not pass VIR_DOMAIN_XML_SECURE
to qemuDomainDefCopy, qemuDomainDefFormatBuf won't format the passwd.
Add VIR_DOMAIN_XML_SECURE flag when we call qemuDomainDefCopy
in qemuDomainSaveImageUpdateDef.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5994558..abe3b9f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5613,7 +5613,8 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,
if (!(newdef_migr = qemuDomainDefCopy(driver,
newdef,
- VIR_DOMAIN_XML_MIGRATABLE)))
+ VIR_DOMAIN_XML_MIGRATABLE |
+ VIR_DOMAIN_XML_SECURE)))
goto cleanup;
if (!virDomainDefCheckABIStability(def, newdef_migr)) {
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/2] qemu: output error when try to hotplug unsupport console
https://bugzilla.redhat.com/show_bug.cgi?id=1164627 When using 'virsh attach-device' to hotplug an unsupported console type into a qemu guest, the attachment will erroneously allows the attachment. This patch will check to ensure that only the support target types are used for a running guest. NB, Although a guest can be defined with an improper target, startup will cause failure. Signed-off-by: Luyao Huang lhu...@redhat.com --- src/qemu/qemu_command.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c041ee7..df87e1e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10067,13 +10067,17 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr, break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: +break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ: case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST: -break; +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(unsupported console target type %s), + NULLSTR(virDomainChrConsoleTargetTypeToString(chr-targetType))); +goto cleanup; } ret = 0; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/2] qemu: add a target type check when hot/cold-plug a Chr device
https://bugzilla.redhat.com/show_bug.cgi?id=1164627
Add a func just check the base target type which
qemu support. And this check will help to avoid add a qemu
unsupport target type chr device to a running guest(hotplug)
or to the guest inactive XML (coldplug, will effect next boot)
And this check only check the target type, and other things
have been checked in virDomainChrDefParseXML.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/libvirt_private.syms | 2 ++
src/qemu/qemu_hotplug.c | 67
2 files changed, 69 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a2eec83..7fb722d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -145,6 +145,8 @@ virDomainBlockedReasonTypeFromString;
virDomainBlockedReasonTypeToString;
virDomainCapabilitiesPolicyTypeToString;
virDomainCapsFeatureTypeToString;
+virDomainChrChannelTargetTypeFromString;
+virDomainChrChannelTargetTypeToString;
virDomainChrConsoleTargetTypeFromString;
virDomainChrConsoleTargetTypeToString;
virDomainChrDefForeach;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9ed96dc..2e02d05 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1401,10 +1401,77 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr
driver,
}
+static int
+qemuDomainChrCheckDefSupport(virDomainChrDefPtr chr)
+{
+switch (chr-deviceType) {
+case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
+switch ((virDomainChrSerialTargetType) chr-targetType) {
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
+break;
+
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unsupported serial target type %s for qemu),
+
NULLSTR(virDomainChrSerialTargetTypeToString(chr-targetType)));
+return -1;
+break;
+}
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
+switch ((virDomainChrChannelTargetType) chr-targetType) {
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
+break;
+
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE:
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unsupported channel target type %s for qemu),
+
NULLSTR(virDomainChrChannelTargetTypeToString(chr-targetType)));
+return -1;
+break;
+}
+break;
+
+case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
+switch ((virDomainChrConsoleTargetType) chr-targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+break;
+
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_UML:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unsupported console target type %s for qemu),
+
NULLSTR(virDomainChrConsoleTargetTypeToString(chr-targetType)));
+return -1;
+break;
+}
+break;
+}
+
+return 0;
+}
+
int
qemuDomainChrInsert(virDomainDefPtr vmdef,
virDomainChrDefPtr chr)
{
+if (qemuDomainChrCheckDefSupport(chr) 0)
+return -1;
+
if (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE
chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/9] qemu: support hot-plug/unplug RNG device
qemu already support hot-plug and hot-unplug RNG device. These patch will make libvirt support hot-plug/unplug RNG device for qemu driver. v3: -fix a wrong use of virJSONValueObjectCreate -merge some new functions use qemu monitor in one patch -merge tests fix in the patch which need fix tests v2: -remove a commit about add 4 new func in libvirt_private.syms and move them to the commit which use these functions -impove some commit subject -improve the logic in virDomainRNGRemove -remove qemuDomainRNGInsert -unexport qemuDomainRNGRemove -add a check if chardev and backend object ID have the same basic alias name -rename need_remove to remove_chardev in qemuDomainAttachRNGDevice -add qemu capability check for RNG device backend Luyao Huang (9): qemu: Add helper to assign RNG device aliases qemu: refactor qemuBuildRNGDeviceArgs to allow reuse in RNG hotplug conf: Introduce function to compare RNG devices. conf: add 3 functions for insert,remove and find a RNG device qemu: add id when build RNG device and rename object id qemu: add functions for attach/detach RNG device via qemu monitor audit: export virDomainAuditRNG qemu: Implement RNG device hotplug on live level qemu: Implement RNG device hotunplug on live level src/conf/domain_audit.c| 2 +- src/conf/domain_audit.h| 7 + src/conf/domain_conf.c | 76 +++ src/conf/domain_conf.h | 9 + src/libvirt_private.syms | 6 + src/qemu/qemu_command.c| 69 --- src/qemu/qemu_command.h| 5 + src/qemu/qemu_driver.c | 12 +- src/qemu/qemu_hotplug.c| 218 - src/qemu/qemu_hotplug.h| 7 +- src/qemu/qemu_monitor.c| 43 src/qemu/qemu_monitor.h| 7 + src/qemu/qemu_monitor_json.c | 43 src/qemu/qemu_monitor_json.h | 5 + .../qemuxml2argv-aarch64-virt-virtio.args | 4 +- .../qemuxml2argv-arm-vexpressa9-virtio.args| 4 +- .../qemuxml2argv-arm-virt-virtio.args | 4 +- .../qemuxml2argv-s390-piix-controllers.args| 2 +- .../qemuxml2argv-s390-usb-none.args| 2 +- .../qemuxml2argv-virtio-rng-ccw.args | 4 +- .../qemuxml2argv-virtio-rng-default.args | 4 +- .../qemuxml2argv-virtio-rng-egd.args | 4 +- .../qemuxml2argv-virtio-rng-multiple.args | 8 +- .../qemuxml2argv-virtio-rng-random.args| 4 +- 24 files changed, 501 insertions(+), 48 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 8/9] qemu: Implement RNG device hotplug on live level
We have enough patches for hotplug RNG device, maybe we can
implement live hotplug of a RNG device.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_driver.c | 8 +++-
src/qemu/qemu_hotplug.c | 106
src/qemu/qemu_hotplug.h | 3 ++
3 files changed, 116 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9eed81d..28fbf15 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6976,6 +6976,13 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
dev-data.chr = NULL;
break;
+case VIR_DOMAIN_DEVICE_RNG:
+ret = qemuDomainAttachRNGDevice(driver, vm,
+dev-data.rng);
+if (!ret)
+dev-data.rng = NULL;
+break;
+
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
@@ -6987,7 +6994,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_RNG:
case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6f62345..c20a4ee 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1510,6 +1510,112 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
return ret;
}
+int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+virDomainDefPtr vmdef = vm-def;
+char *devstr = NULL;
+char *charAlias = NULL;
+char *objAlias = NULL;
+bool remove_chardev = false;
+bool releaseaddr = false;
+
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(qemu does not support -device));
+return ret;
+}
+
+if (rng-backend == VIR_DOMAIN_RNG_BACKEND_RANDOM
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_RANDOM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support the rng-random
+ backend));
+return ret;
+} else if (rng-backend == VIR_DOMAIN_RNG_BACKEND_EGD
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_RNG_EGD)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support the rng-egd
+ backend));
+return ret;
+}
+
+if (qemuAssignDeviceRNGAlias(vmdef, rng, -1) 0)
+return ret;
+
+if (rng-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+if (STRPREFIX(vm-def-os.machine, s390-ccw)
+virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
+rng-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
+} else if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_VIRTIO_S390)) {
+rng-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
+}
+}
+
+if (rng-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+rng-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+if (virDomainPCIAddressEnsureAddr(priv-pciaddrs, rng-info) 0)
+return ret;
+} else if (rng-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+if (virDomainCCWAddressAssign(rng-info, priv-ccwaddrs,
+ !rng-info.addr.ccw.assigned) 0)
+return ret;
+}
+releaseaddr = true;
+if (!(devstr = qemuBuildRNGDevStr(vmdef, rng, priv-qemuCaps)))
+goto cleanup;
+
+if (virAsprintf(objAlias, obj%s, rng-info.alias) 0)
+goto cleanup;
+
+if (rng-backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
+if (virAsprintf(charAlias, char%s, rng-info.alias) 0)
+goto cleanup;
+
+qemuDomainObjEnterMonitor(driver, vm);
+if (qemuMonitorAttachCharDev(priv-mon, charAlias,
rng-source.chardev) 0) {
+qemuDomainObjExitMonitor(driver, vm);
+goto audit;
+}
+remove_chardev = true;
+} else {
+qemuDomainObjEnterMonitor(driver, vm);
+}
+
+if (qemuMonitorAttachRNGDev(priv-mon, charAlias, objAlias, rng) 0) {
+if (remove_chardev)
+qemuMonitorDetachCharDev(priv-mon, charAlias);
+qemuDomainObjExitMonitor(driver, vm);
+goto audit;
+}
+
+if (devstr qemuMonitorAddDevice(priv-mon, devstr) 0) {
+qemuMonitorDetachRNGDev(priv-mon, objAlias);
+if (remove_chardev)
+qemuMonitorDetachCharDev(priv-mon, charAlias);
+qemuDomainObjExitMonitor(driver, vm);
+goto audit;
+}
+qemuDomainObjExitMonitor(driver, vm);
+
+if (virDomainRNGInsert(vmdef, rng
[libvirt] [PATCH v3 9/9] qemu: Implement RNG device hotunplug on live level
We have enough patches for hotunplug RNG device, maybe we can
implement live hotunplug of a RNG device.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 4 +-
src/qemu/qemu_hotplug.c | 112 ++-
src/qemu/qemu_hotplug.h | 4 +-
4 files changed, 118 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d3754af..6c76786 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -378,6 +378,7 @@ virDomainPMSuspendedReasonTypeToString;
virDomainRedirdevBusTypeFromString;
virDomainRedirdevBusTypeToString;
virDomainRNGBackendTypeToString;
+virDomainRNGDefFree;
virDomainRNGEquals;
virDomainRNGFind;
virDomainRNGInsert;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 28fbf15..b3d9613 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7057,6 +7057,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_CHR:
ret = qemuDomainDetachChrDevice(driver, vm, dev-data.chr);
break;
+case VIR_DOMAIN_DEVICE_RNG:
+ret = qemuDomainDetachRNGDevice(driver, vm, dev-data.rng);
+break;
case VIR_DOMAIN_DEVICE_FS:
case VIR_DOMAIN_DEVICE_INPUT:
case VIR_DOMAIN_DEVICE_SOUND:
@@ -7067,7 +7070,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_RNG:
case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_REDIRDEV:
case VIR_DOMAIN_DEVICE_NONE:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c20a4ee..2f90534 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1510,6 +1510,21 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
return ret;
}
+static virDomainRNGDefPtr
+qemuDomainRNGRemove(virDomainDefPtr vmdef,
+virDomainRNGDefPtr rng)
+{
+virDomainRNGDefPtr ret;
+
+if (!(ret = virDomainRNGRemove(vmdef, rng))) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(device not present in domain configuration));
+return NULL;
+}
+
+return ret;
+}
+
int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainRNGDefPtr rng)
@@ -2922,6 +2937,52 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
return ret;
}
+static int
+qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+virObjectEventPtr event;
+char *charAlias = NULL;
+char *objAlias = NULL;
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int ret = -1;
+int rc;
+
+VIR_DEBUG(Removing RNG device %s from domain %p %s,
+ rng-info.alias, vm, vm-def-name);
+
+if (virAsprintf(objAlias, obj%s, rng-info.alias) 0)
+goto cleanup;
+
+if (virAsprintf(charAlias, char%s, rng-info.alias) 0)
+goto cleanup;
+
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorDetachRNGDev(priv-mon, objAlias);
+if (rc == 0 rng-backend == VIR_DOMAIN_RNG_BACKEND_EGD)
+ignore_value(qemuMonitorDetachCharDev(priv-mon, charAlias));
+qemuDomainObjExitMonitor(driver, vm);
+
+virDomainAuditRNG(vm, rng, NULL, detach, rc == 0);
+
+if (rc 0)
+goto cleanup;
+
+event = virDomainEventDeviceRemovedNewFromObj(vm, rng-info.alias);
+if (event)
+qemuDomainEventQueue(driver, event);
+
+qemuDomainRNGRemove(vm-def, rng);
+qemuDomainReleaseDeviceAddress(vm, rng-info, NULL);
+virDomainRNGDefFree(rng);
+ret = 0;
+
+ cleanup:
+VIR_FREE(charAlias);
+VIR_FREE(objAlias);
+return ret;
+}
void
qemuDomainRemoveDevice(virQEMUDriverPtr driver,
@@ -2945,6 +3006,9 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_DEVICE_CHR:
qemuDomainRemoveChrDevice(driver, vm, dev-data.chr);
break;
+case VIR_DOMAIN_DEVICE_RNG:
+qemuDomainRemoveRNGDevice(driver, vm, dev-data.rng);
+break;
case VIR_DOMAIN_DEVICE_NONE:
case VIR_DOMAIN_DEVICE_LEASE:
@@ -2959,7 +3023,6 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
case VIR_DOMAIN_DEVICE_SMARTCARD:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_RNG:
case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_TPM:
case VIR_DOMAIN_DEVICE_PANIC:
@@ -3831,3 +3894,50 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
VIR_FREE(devstr);
return ret;
}
+
+int qemuDomainDetachRNGDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainRNGDefPtr rng)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm
[libvirt] [PATCH v3 6/9] qemu: add functions for attach/detach RNG device via qemu monitor
qemuMonitorAttachRNGDev and qemuMonitorDetachRNGDev functions
just do some basic check and then call qemuMonitorJSONAttachRNGDev
and qemuMonitorDelObject to help us.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_monitor.c | 43 +++
src/qemu/qemu_monitor.h | 7 +++
src/qemu/qemu_monitor_json.c | 43 +++
src/qemu/qemu_monitor_json.h | 5 +
4 files changed, 98 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6882a50..1e3f8ea 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4206,6 +4206,49 @@ int qemuMonitorDetachCharDev(qemuMonitorPtr mon,
return qemuMonitorJSONDetachCharDev(mon, chrID);
}
+int qemuMonitorAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID,
+virDomainRNGDefPtr rng)
+{
+VIR_DEBUG(mon=%p chrID=%s objID=%s rng=%p, mon, chrID, objID, rng);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(monitor must not be NULL));
+return -1;
+}
+
+if (!mon-json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(JSON monitor is required));
+return -1;
+}
+
+return qemuMonitorJSONAttachRNGDev(mon, chrID, objID, rng);
+}
+
+int qemuMonitorDetachRNGDev(qemuMonitorPtr mon,
+ const char *objID)
+{
+VIR_DEBUG(mon=%p objID=%s, mon, objID);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(monitor must not be NULL));
+return -1;
+}
+
+if (!mon-json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(JSON monitor is required));
+return -1;
+}
+
+return qemuMonitorDelObject(mon, objID);
+}
+
+
int
qemuMonitorGetDeviceAliases(qemuMonitorPtr mon,
char ***aliases)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 133d42d..a767e3e 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -869,6 +869,13 @@ int qemuMonitorAttachCharDev(qemuMonitorPtr mon,
int qemuMonitorDetachCharDev(qemuMonitorPtr mon,
const char *chrID);
+int qemuMonitorAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID,
+virDomainRNGDefPtr rng);
+int qemuMonitorDetachRNGDev(qemuMonitorPtr mon,
+const char *objID);
+
int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon,
char ***aliases);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index da5c14d..33c3866 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6235,6 +6235,49 @@ qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon,
return ret;
}
+int
+qemuMonitorJSONAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID,
+virDomainRNGDefPtr rng)
+{
+const char *type = NULL;
+virJSONValuePtr props;
+
+if (!(props = virJSONValueNewObject()))
+goto cleanup;
+
+switch ((virDomainRNGBackend) rng-backend) {
+case VIR_DOMAIN_RNG_BACKEND_RANDOM:
+type = rng-random;
+if (virJSONValueObjectCreate(props, s:filename, rng-source.file,
NULL) 0)
+goto cleanup;
+break;
+
+case VIR_DOMAIN_RNG_BACKEND_EGD:
+if (STRNEQ_NULLABLE(strstr(chrID, rng), strstr(objID, rng))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(chardev id '%s' basic alias name is different
from '%s',
+ chrID, objID));
+goto cleanup;
+}
+type = rng-egd;
+if (virJSONValueObjectCreate(props, s:chardev, chrID, NULL) 0)
+goto cleanup;
+break;
+
+case VIR_DOMAIN_RNG_BACKEND_LAST:
+/*shouldn't happen*/
+goto cleanup;
+}
+
+return qemuMonitorJSONAddObject(mon, type, objID, props);
+
+ cleanup:
+virJSONValueFree(props);
+return -1;
+}
+
int
qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 1da1a00..04f7ca2 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -462,6 +462,11 @@ int qemuMonitorJSONAttachCharDev(qemuMonitorPtr mon,
virDomainChrSourceDefPtr chr);
int qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon,
const char *chrID);
+int qemuMonitorJSONAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID
[libvirt] [PATCH v3 5/9] qemu: add id when build RNG device and rename object id
We didn't set a id when we build RNG device cmdline before.
Give a id to every RNG device and we can hotunplug it via
QMP cmd device_del.
And fix the tests.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c | 12 ++--
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-virtio.args | 4 ++--
.../qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args | 4 ++--
tests/qemuxml2argvdata/qemuxml2argv-arm-virt-virtio.args | 4 ++--
.../qemuxml2argvdata/qemuxml2argv-s390-piix-controllers.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-s390-usb-none.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-ccw.args | 4 ++--
tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-default.args | 4 ++--
tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-egd.args | 4 ++--
tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.args | 8
tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args | 4 ++--
11 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c0cd3eb..57f73be 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5780,7 +5780,7 @@ qemuBuildRNGBackendArgs(virCommandPtr cmd,
goto cleanup;
}
-virBufferAsprintf(buf, rng-random,id=%s,filename=%s,
+virBufferAsprintf(buf, rng-random,id=obj%s,filename=%s,
dev-info.alias, dev-source.file);
virCommandAddArg(cmd, -object);
@@ -5803,7 +5803,7 @@ qemuBuildRNGBackendArgs(virCommandPtr cmd,
virCommandAddArgList(cmd, -chardev, backend, NULL);
virCommandAddArg(cmd, -object);
-virCommandAddArgFormat(cmd, rng-egd,chardev=char%s,id=%s,
+virCommandAddArgFormat(cmd, rng-egd,chardev=char%s,id=obj%s,
dev-info.alias, dev-info.alias);
break;
@@ -5836,13 +5836,13 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
}
if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
-virBufferAsprintf(buf, virtio-rng-ccw,rng=%s, dev-info.alias);
+virBufferAsprintf(buf, virtio-rng-ccw,rng=obj%s,id=%s,
dev-info.alias, dev-info.alias);
else if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
-virBufferAsprintf(buf, virtio-rng-s390,rng=%s, dev-info.alias);
+virBufferAsprintf(buf, virtio-rng-s390,rng=obj%s,id=%s,
dev-info.alias, dev-info.alias);
else if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
-virBufferAsprintf(buf, virtio-rng-device,rng=%s, dev-info.alias);
+virBufferAsprintf(buf, virtio-rng-device,rng=obj%s,id=%s,
dev-info.alias, dev-info.alias);
else
-virBufferAsprintf(buf, virtio-rng-pci,rng=%s, dev-info.alias);
+virBufferAsprintf(buf, virtio-rng-pci,rng=obj%s,id=%s,
dev-info.alias, dev-info.alias);
if (dev-rate 0) {
virBufferAsprintf(buf, ,max-bytes=%u, dev-rate);
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-virtio.args
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-virtio.args
index 05f3629..d12e6e2 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-virtio.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-virtio.args
@@ -11,5 +11,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-net user,vlan=0,name=hostnet0 -serial pty -chardev pty,id=charconsole1 \
-device virtconsole,chardev=charconsole1,id=console1 \
-device virtio-balloon-device,id=balloon0 \
--object rng-random,id=rng0,filename=/dev/random \
--device virtio-rng-device,rng=rng0
+-object rng-random,id=objrng0,filename=/dev/random \
+-device virtio-rng-device,rng=objrng0,id=rng0
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args
b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args
index 62de9d3..dba74e1 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-vexpressa9-virtio.args
@@ -10,5 +10,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-net user,vlan=0,name=hostnet0 -serial pty -chardev pty,id=charconsole1 \
-device virtconsole,chardev=charconsole1,id=console1 \
-device virtio-balloon-device,id=balloon0 \
--object rng-random,id=rng0,filename=/dev/random \
--device virtio-rng-device,rng=rng0
+-object rng-random,id=objrng0,filename=/dev/random \
+-device virtio-rng-device,rng=objrng0,id=rng0
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-arm-virt-virtio.args
b/tests/qemuxml2argvdata/qemuxml2argv-arm-virt-virtio.args
index 5206ad8..c73022b 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-arm-virt-virtio.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-arm-virt-virtio.args
@@ -10,5 +10,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test
QEMU_AUDIO_DRV=none \
-net user,vlan=0,name=hostnet0 -serial pty -chardev pty,id
[libvirt] [PATCH v3 1/9] qemu: Add helper to assign RNG device aliases
This function is used to assign an alias for a RNG device. It will be
later reused when hotplugging RNGs.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c | 25 -
src/qemu/qemu_command.h | 1 +
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c041ee7..52741fe 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -910,6 +910,29 @@ qemuGetNextChrDevIndex(virDomainDefPtr def,
return idx;
}
+int
+qemuAssignDeviceRNGAlias(virDomainDefPtr def, virDomainRNGDefPtr rng, int idx)
+{
+if (idx == -1) {
+size_t i;
+idx = 0;
+for (i = 0; i def-nrngs; i++) {
+int thisidx;
+if ((thisidx = qemuDomainDeviceAliasIndex(def-rngs[i]-info,
rng)) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to determine device index for RNG
device));
+return -1;
+}
+if (thisidx = idx)
+idx = thisidx + 1;
+}
+}
+
+if (virAsprintf(rng-info.alias, rng%d, idx) 0)
+return -1;
+
+return 0;
+}
int
qemuAssignDeviceChrAlias(virDomainDefPtr def,
@@ -1036,7 +1059,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps)
return -1;
}
for (i = 0; i def-nrngs; i++) {
-if (virAsprintf(def-rngs[i]-info.alias, rng%zu, i) 0)
+if (qemuAssignDeviceRNGAlias(def, def-rngs[i], i) 0)
return -1;
}
if (def-tpm) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index dcc7127..fc5f50a 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -264,6 +264,7 @@ int qemuAssignDeviceRedirdevAlias(virDomainDefPtr def,
virDomainRedirdevDefPtr r
int qemuAssignDeviceChrAlias(virDomainDefPtr def,
virDomainChrDefPtr chr,
ssize_t idx);
+int qemuAssignDeviceRNGAlias(virDomainDefPtr def, virDomainRNGDefPtr rng, int
idx);
int
qemuParseKeywords(const char *str,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/9] qemu: refactor qemuBuildRNGDeviceArgs to allow reuse in RNG hotplug
Rename qemuBuildRNGDeviceArgs to qemuBuildRNGDevStr and change the
return type so that it can be reused in the device hotplug code later.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c | 32
src/qemu/qemu_command.h | 4
2 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 52741fe..c0cd3eb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5820,21 +5820,19 @@ qemuBuildRNGBackendArgs(virCommandPtr cmd,
}
-static int
-qemuBuildRNGDeviceArgs(virCommandPtr cmd,
- virDomainDefPtr def,
- virDomainRNGDefPtr dev,
- virQEMUCapsPtr qemuCaps)
+char *
+qemuBuildRNGDevStr(virDomainDefPtr def,
+ virDomainRNGDefPtr dev,
+ virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
-int ret = -1;
if (dev-model != VIR_DOMAIN_RNG_MODEL_VIRTIO ||
!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_RNG)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(this qemu doesn't support RNG device type '%s'),
virDomainRNGModelTypeToString(dev-model));
-goto cleanup;
+goto error;
}
if (dev-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
@@ -5855,16 +5853,14 @@ qemuBuildRNGDeviceArgs(virCommandPtr cmd,
}
if (qemuBuildDeviceAddressStr(buf, def, dev-info, qemuCaps) 0)
-goto cleanup;
-
-virCommandAddArg(cmd, -device);
-virCommandAddArgBuffer(cmd, buf);
-
-ret = 0;
+goto error;
+if (virBufferCheckError(buf) 0)
+goto error;
- cleanup:
+return virBufferContentAndReset(buf);
+ error:
virBufferFreeAndReset(buf);
-return ret;
+return NULL;
}
@@ -9845,13 +9841,17 @@ qemuBuildCommandLine(virConnectPtr conn,
}
for (i = 0; i def-nrngs; i++) {
+char *devstr;
/* add the RNG source backend */
if (qemuBuildRNGBackendArgs(cmd, def-rngs[i], qemuCaps) 0)
goto error;
/* add the device */
-if (qemuBuildRNGDeviceArgs(cmd, def, def-rngs[i], qemuCaps) 0)
+virCommandAddArg(cmd, -device);
+if (!(devstr = qemuBuildRNGDevStr(def, def-rngs[i], qemuCaps)))
goto error;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
}
if (def-nvram) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index fc5f50a..3959677 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -165,6 +165,10 @@ char *qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
const char *configfd,
virQEMUCapsPtr qemuCaps);
+char *qemuBuildRNGDevStr(virDomainDefPtr def,
+ virDomainRNGDefPtr dev,
+ virQEMUCapsPtr qemuCaps);
+
int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
/* Legacy, pre device support */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 4/9] conf: add 3 functions for insert, remove and find a RNG device
the 3 functions are:
virDomainRNGInsert: Insert a RNG device to vm-def.
virDomainRNGRemove: remove a RNG device in vm-def.
virDomainRNGFind: find a RNG device in vm-def.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 42 ++
src/conf/domain_conf.h | 6 ++
src/libvirt_private.syms | 3 +++
3 files changed, 51 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 81e4030..659864c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12095,6 +12095,48 @@ virDomainRNGEquals(virDomainRNGDefPtr src,
return false;
}
+int
+virDomainRNGInsert(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng)
+{
+return VIR_APPEND_ELEMENT(vmdef-rngs, vmdef-nrngs, rng);
+}
+
+virDomainRNGDefPtr
+virDomainRNGRemove(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng)
+{
+virDomainRNGDefPtr ret;
+size_t i;
+
+for (i = 0; i vmdef-nrngs; i++) {
+ret = vmdef-rngs[i];
+
+if (virDomainRNGEquals(ret, rng)) {
+VIR_DELETE_ELEMENT(vmdef-rngs, i, vmdef-nrngs);
+return ret;
+}
+}
+
+return NULL;
+}
+
+virDomainRNGDefPtr
+virDomainRNGFind(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng)
+{
+virDomainRNGDefPtr ret;
+size_t i;
+
+for (i = 0; i vmdef-nrngs; i++) {
+ret = vmdef-rngs[i];
+
+if (virDomainRNGEquals(ret, rng))
+return ret;
+}
+return NULL;
+}
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b3843e8..10cb09b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2655,6 +2655,12 @@ virDomainChrRemove(virDomainDefPtr vmdef,
bool virDomainRNGEquals(virDomainRNGDefPtr src,
virDomainRNGDefPtr tgt);
+int virDomainRNGInsert(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng);
+virDomainRNGDefPtr virDomainRNGRemove(virDomainDefPtr vmdef,
+ virDomainRNGDefPtr rng);
+virDomainRNGDefPtr virDomainRNGFind(virDomainDefPtr vmdef,
+virDomainRNGDefPtr rng);
int virDomainSaveXML(const char *configDir,
virDomainDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 653655f..a635652 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -378,7 +378,10 @@ virDomainRedirdevBusTypeFromString;
virDomainRedirdevBusTypeToString;
virDomainRNGBackendTypeToString;
virDomainRNGEquals;
+virDomainRNGFind;
+virDomainRNGInsert;
virDomainRNGModelTypeToString;
+virDomainRNGRemove;
virDomainRunningReasonTypeFromString;
virDomainRunningReasonTypeToString;
virDomainSaveConfig;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/9] conf: Introduce function to compare RNG devices.
virDomainRNGEquals is a func which check if two rng device
are the same.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/domain_conf.c | 34 ++
src/conf/domain_conf.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 38 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8792f5e..81e4030 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12061,6 +12061,40 @@ virDomainChrRemove(virDomainDefPtr vmdef,
return ret;
}
+bool
+virDomainRNGEquals(virDomainRNGDefPtr src,
+ virDomainRNGDefPtr tgt)
+{
+if (!src || !tgt)
+return src == tgt;
+
+if (src-model != tgt-model)
+return false;
+
+if (src-rate != tgt-rate || src-period != tgt-period)
+return false;
+
+switch ((virDomainRNGModel) src-model) {
+case VIR_DOMAIN_RNG_MODEL_VIRTIO:
+switch ((virDomainRNGBackend) src-backend) {
+case VIR_DOMAIN_RNG_BACKEND_RANDOM:
+return STREQ_NULLABLE(src-source.file, tgt-source.file);
+break;
+case VIR_DOMAIN_RNG_BACKEND_EGD:
+return virDomainChrSourceDefIsEqual(src-source.chardev,
+tgt-source.chardev);
+break;
+case VIR_DOMAIN_RNG_BACKEND_LAST:
+break;
+}
+break;
+
+case VIR_DOMAIN_RNG_MODEL_LAST:
+break;
+}
+return false;
+}
+
char *
virDomainDefGetDefaultEmulator(virDomainDefPtr def,
virCapsPtr caps)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 09ab194..b3843e8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2653,6 +2653,9 @@ virDomainChrDefPtr
virDomainChrRemove(virDomainDefPtr vmdef,
virDomainChrDefPtr chr);
+bool virDomainRNGEquals(virDomainRNGDefPtr src,
+virDomainRNGDefPtr tgt);
+
int virDomainSaveXML(const char *configDir,
virDomainDefPtr def,
const char *xml);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a2eec83..653655f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -377,6 +377,7 @@ virDomainPMSuspendedReasonTypeToString;
virDomainRedirdevBusTypeFromString;
virDomainRedirdevBusTypeToString;
virDomainRNGBackendTypeToString;
+virDomainRNGEquals;
virDomainRNGModelTypeToString;
virDomainRunningReasonTypeFromString;
virDomainRunningReasonTypeToString;
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 7/9] audit: export virDomainAuditRNG
Signed-off-by: Luyao Huang lhu...@redhat.com --- src/conf/domain_audit.c | 2 +- src/conf/domain_audit.h | 7 +++ src/libvirt_private.syms | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index fcf9df7..159ebf5 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -242,7 +242,7 @@ virDomainAuditDisk(virDomainObjPtr vm, } -static void +void virDomainAuditRNG(virDomainObjPtr vm, virDomainRNGDefPtr oldDef, virDomainRNGDefPtr newDef, const char *reason, bool success) diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h index 3434feb..4c1ef90 100644 --- a/src/conf/domain_audit.h +++ b/src/conf/domain_audit.h @@ -117,5 +117,12 @@ void virDomainAuditChardev(virDomainObjPtr vm, const char *reason, bool success) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); +void virDomainAuditRNG(virDomainObjPtr vm, + virDomainRNGDefPtr oldDef, + virDomainRNGDefPtr newDef, + const char *reason, + bool success) +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); + #endif /* __VIR_DOMAIN_AUDIT_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a635652..d3754af 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -123,6 +123,7 @@ virDomainAuditMemory; virDomainAuditNet; virDomainAuditNetDevice; virDomainAuditRedirdev; +virDomainAuditRNG; virDomainAuditSecurityLabel; virDomainAuditStart; virDomainAuditStop; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix fail to start vm with one cell use memdev and another not
https://bugzilla.redhat.com/show_bug.cgi?id=1182467
We changed our build numa qemu cmd line way in f309db,
But qemu cannot accept one cell use memdev and another not,
so this cause a issue when we start a vm with one cell
have hugpage or nodemask settings and others not.
After this patch we will use memdev for all cells if there
is at least one cell need this.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c| 88 +-
.../qemuxml2argv-numatune-memnode-no-memory.args | 3 +-
2 files changed, 55 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3cafbfb..11ca355 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6613,14 +6613,18 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
virBitmapPtr nodeset)
{
size_t i, j;
-virBuffer buf = VIR_BUFFER_INITIALIZER;
+virBuffer buf[def-cpu-ncells];
virDomainHugePagePtr master_hugepage = NULL;
char *cpumask = NULL, *tmpmask = NULL, *next = NULL;
char *nodemask = NULL;
char *mem_path = NULL;
int ret = -1;
+bool need_memdev = false;
const long system_page_size = sysconf(_SC_PAGESIZE) / 1024;
+for (i = 0; i def-cpu-ncells; i++)
+memset(buf[i], 0, sizeof(buf[i]));
+
if (virDomainNumatuneHasPerNodeBinding(def-numatune)
!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE))) {
@@ -6672,20 +6676,8 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
def-cpu-cells[i].mem = cellmem * 1024;
virMemAccess memAccess = def-cpu-cells[i].memAccess;
-VIR_FREE(cpumask);
VIR_FREE(nodemask);
-if (!(cpumask = virBitmapFormat(def-cpu-cells[i].cpumask)))
-goto cleanup;
-
-if (strchr(cpumask, ',')
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(disjoint NUMA cpu ranges are not supported
- with this QEMU));
-goto cleanup;
-}
-
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_RAM) ||
virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE)) {
virDomainNumatuneMemMode mode;
@@ -6752,17 +6744,17 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
if (!(mem_path = qemuGetHugepagePath(cfg-hugetlbfs[j])))
goto cleanup;
-virBufferAsprintf(buf,
+virBufferAsprintf(buf[i],
memory-backend-file,prealloc=yes,mem-path=%s,
mem_path);
switch (memAccess) {
case VIR_MEM_ACCESS_SHARED:
-virBufferAddLit(buf, ,share=on);
+virBufferAddLit(buf[i], ,share=on);
break;
case VIR_MEM_ACCESS_PRIVATE:
-virBufferAddLit(buf, ,share=off);
+virBufferAddLit(buf[i], ,share=off);
break;
case VIR_MEM_ACCESS_DEFAULT:
@@ -6777,10 +6769,10 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
only with hugepages));
goto cleanup;
}
-virBufferAddLit(buf, memory-backend-ram);
+virBufferAddLit(buf[i], memory-backend-ram);
}
-virBufferAsprintf(buf, ,size=%lluM,id=ram-node%zu, cellmem, i);
+virBufferAsprintf(buf[i], ,size=%lluM,id=ram-node%zu, cellmem,
i);
if (virDomainNumatuneMaybeFormatNodeset(def-numatune, nodeset,
nodemask, i) 0)
@@ -6798,19 +6790,15 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
for (tmpmask = nodemask; tmpmask; tmpmask = next) {
if ((next = strchr(tmpmask, ',')))
*(next++) = '\0';
-virBufferAddLit(buf, ,host-nodes=);
-virBufferAdd(buf, tmpmask, -1);
+virBufferAddLit(buf[i], ,host-nodes=);
+virBufferAdd(buf[i], tmpmask, -1);
}
-virBufferAsprintf(buf, ,policy=%s, policy);
+virBufferAsprintf(buf[i], ,policy=%s, policy);
}
-if (hugepage || nodemask) {
-virCommandAddArg(cmd, -object);
-virCommandAddArgBuffer(cmd, buf);
-} else {
-virBufferFreeAndReset(buf);
-}
+if (hugepage || nodemask)
+need_memdev = true;
} else {
if (memAccess) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
@@ -6819,23 +6807,52 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg
Re: [libvirt] [PATCH v3 1/2] qemu: output error when try to hotplug unsupport console
On 01/22/2015 09:25 PM, Peter Krempa wrote: On Thu, Jan 22, 2015 at 21:20:46 +0800, Luyao Huang wrote: On 01/22/2015 06:19 PM, Peter Krempa wrote: On Thu, Jan 22, 2015 at 10:28:18 +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1164627 ... ACK, although the rest of the chardev command and hotplug code is a big mess and would benefit from a cleanup/refactor. Yes, i noticed this mess and i will try to fix this in next step. I don't insist on you doing that. It was just an observation from looking through the code. yes, I know that, i am just interested in this part of code :) Although i know there are some thorny problems when do a refactor, and seems this will take some time to do it. Peter Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: add a check when cold-plug a Chr device
On 01/21/2015 08:00 PM, Ján Tomko wrote:
On 01/21/2015 04:10 AM, lhuang wrote:
On 01/21/2015 03:00 AM, John Ferlan wrote:
On 12/09/2014 01:48 AM, Luyao Huang wrote:
Add a func just check the base target type which
qemu support. But i still doubt this will be useful
, we already have a check when we try to start the
vm. And this check only check the target type, and
the other things will be done in virDomainChrDefParseXML.
The commit message needs some massaging.
Essentially, this patch fixes the condition where if a guest has been
started and someone uses attach-device with (or without) the --config
option, then these checks will avoid the next guest being modified,
correct?
Right
This will also cause an error earlier that patch 1/2 as
qemuDomainChrInsert is called in the path before qemuDomainAttachDeviceLive
Yes and if this patch have been pushed then the patch 1/2 will be a patch for
improving exist code.
ChrInsert is called after qemuBuildConsoleChrDeviceStr in AttachDevice. We
should error out earlier and include the first patch too.
Thanks for pointing out, error output more earlier more better :)
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_hotplug.c | 64
+
1 file changed, 64 insertions(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b9a0cee..fe91ec7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1384,10 +1384,74 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr
driver,
}
+static int
+qemuDomainChrCheckDefSupport(virDomainChrDefPtr chr)
+{
+int ret = -1;
+
+switch (chr-deviceType) {
+case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
+switch ((virDomainChrSerialTargetType) chr-targetType) {
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
+case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
+ret = 0;
+break;
+
+default:
Typically in switches listing other options rather than default:
The point of casting it to virDomainChrSerialTargetType is to catch all the
unhandled values and it only works if there's no default: clause.
Also I don't think we need the ret variable at all. Just return 0 at the end
of the function, or -1 if we reached an unsupported combination.
Good idea ! i will remove the ret and use return in these place.
I think perhaps this one is better than 1/2, but will see if my
responses result in other opinions...
Even if this one fixes the bug, 1/2 is nice to have.
Thanks for pointing out and i forgot cc Jan and Pavel when sent this patch,
maybe they have some other opinions.
No need to cc me, I am subscribed to the list. :)
I just saw other people cc the people who help review the patch when
send a new version
patch to upstream, so i think it is better to cc the people who help
review the patch when write
a new version.
Thanks a lot for your review.
Jan
Luyao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] util: introduce a new helper for get interface IPv6 address
Introduce a new function to help to get interface IPv6 address.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/virnetdev.c | 70
src/util/virnetdev.h | 2 ++
3 files changed, 73 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 645aef1..f60911c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1659,6 +1659,7 @@ virNetDevDelMulti;
virNetDevExists;
virNetDevGetIndex;
virNetDevGetIPv4Address;
+virNetDevGetIPv6Address;
virNetDevGetLinkInfo;
virNetDevGetMAC;
virNetDevGetMTU;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 2a0eb43..c1a588e 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -33,6 +33,10 @@
#include virstring.h
#include virutil.h
+#if defined(HAVE_GETIFADDRS)
+# include ifaddrs.h
+#endif
+
#include sys/ioctl.h
#include net/if.h
#include fcntl.h
@@ -1432,6 +1436,72 @@ int virNetDevGetIPv4Address(const char *ifname
ATTRIBUTE_UNUSED,
#endif /* ! SIOCGIFADDR */
+/**
+ *virNetDevGetIPv6Address:
+ *@ifname: name of the interface whose IP address we want
+ *@addr: filled with the IPv6 address
+ *
+ *This function gets the IPv6 address for the interface @ifname
+ *and stores it in @addr
+ *
+ *Returns 0 on success, -1 on failure.
+ */
+#if defined(HAVE_GETIFADDRS)
+int virNetDevGetIPv6Address(const char *ifname,
+virSocketAddrPtr addr)
+{
+struct ifaddrs *ifap, *ifa;
+int ret = -1;
+bool found_one = false;
+
+if (getifaddrs(ifap) 0) {
+virReportSystemError(errno, %s,
+ _(Could not get interface list));
+return -1;
+}
+
+for (ifa = ifap; ifa; ifa = ifa-ifa_next) {
+if (STRNEQ(ifa-ifa_name, ifname))
+continue;
+found_one = true;
+if (ifa-ifa_addr-sa_family == AF_INET6)
+break;
+}
+
+if (!ifa) {
+if (found_one)
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Interface '%s' do not have a IPv6 address),
+ ifname);
+else
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Interface '%s' not found),
+ ifname);
+goto cleanup;
+}
+
+addr-data.stor.ss_family = AF_INET6;
+addr-len = sizeof(addr-data.inet6);
+memcpy(addr-data.inet6, ifa-ifa_addr, addr-len);
+ret = 0;
+
+ cleanup:
+freeifaddrs(ifap);
+return ret;
+}
+
+#else
+
+int virNetDevGetIPv6Address(const char *ifname ATTRIBUTE_UNUSED,
+virSocketAddrPtr addr ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, %s,
+ _(Unable to get IPv6 address on this platform));
+return -1;
+}
+
+#endif
+
/**
* virNetDevValidateConfig:
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index de8b480..c065381 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -104,6 +104,8 @@ int virNetDevClearIPAddress(const char *ifname,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
int virNetDevGetIPv4Address(const char *ifname, virSocketAddrPtr addr)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virNetDevGetIPv6Address(const char *ifname, virSocketAddrPtr addr)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
int virNetDevSetMAC(const char *ifname,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] conf: introduce new family attribute in graphics listen for chose IP family
If a interface or network have both ipv6 and ipv4 address which can be used,
we do not know use which be a listen address. So introduce a new attribute
to help us chose this.
graphics XML will like this after this commit:
graphics type='spice' port='5900' autoport='yes'
listen type='network' address='192.168.0.1' network='vepa-net'
family='default'/
/graphics
and this ip family can be set 3 type:
default: check if the interface or network have a can be used ipv4 address,
if yes use this address, if not will try to get a ipv6 address.
ipv4: check if the interface or network have a can be used ipv4 address
ipv6: check if the interface or network have a can be used ipv6 address
fix some test which will be break by this commit.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
docs/formatdomain.html.in | 10 +-
docs/schemas/domaincommon.rng | 9 +
src/conf/domain_conf.c | 21 +
src/conf/domain_conf.h | 10 ++
.../qemuxml2argv-graphics-listen-network.xml| 2 +-
.../qemuxml2argv-graphics-listen-network2.xml | 2 +-
.../qemuxml2xmlout-graphics-listen-network2.xml | 2 +-
7 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fcf5984..7a07ca0 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4512,7 +4512,7 @@ qemu-kvm -net nic,model=? /dev/null
lt;graphics type='rdp' autoport='yes' multiUser='yes' /gt;
lt;graphics type='desktop' fullscreen='yes'/gt;
lt;graphics type='spice'gt;
- lt;listen type='network' network='rednet'/gt;
+ lt;listen type='network' network='rednet' family='default'/gt;
lt;/graphicsgt;
lt;/devicesgt;
.../pre
@@ -4752,6 +4752,14 @@ qemu-kvm -net nic,model=? /dev/null
the first forward dev will be used.
/dd
/dl
+dl
+ dtcodefamily/code/dt
+ ddif codetype='network'/code, the codefamily/code
+attribute will contain an IP family. This tells which IP address
+will be got for the network. IP family can be set to default, ipv4
+or ipv6.
+ /dd
+/dl
h4a name=elementsVideoVideo devices/a/h4
p
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7a1d299..fb8d084 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2888,6 +2888,15 @@
ref name=addrIPorName/
/attribute
/optional
+optional
+ attribute name=family
+choice
+ valuedefault/value
+ valueipv4/value
+ valueipv6/value
+/choice
+ /attribute
+/optional
/group
/choice
/element
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fd36063..307801d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -522,6 +522,12 @@ VIR_ENUM_IMPL(virDomainGraphicsListen,
VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST,
address,
network)
+VIR_ENUM_IMPL(virDomainGraphicsListenFamily,
+ VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST,
+ default,
+ ipv4,
+ ipv6)
+
VIR_ENUM_IMPL(virDomainGraphicsAuthConnected,
VIR_DOMAIN_GRAPHICS_AUTH_CONNECTED_LAST,
default,
@@ -9396,6 +9402,7 @@
virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
char *address = virXMLPropString(node, address);
char *network = virXMLPropString(node, network);
char *fromConfig = virXMLPropString(node, fromConfig);
+char *family = virXMLPropString(node, family);
int tmp;
if (!type) {
@@ -9433,6 +9440,16 @@
virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
network = NULL;
}
+if (def-type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) {
+if (!family) {
+def-family = VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_DEFAULT;
+} else if ((def-family =
virDomainGraphicsListenFamilyTypeFromString(family)) 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unknown graphics listen IP family '%s'),
family);
+goto error;
+}
+}
+
if (fromConfig
flags VIR_DOMAIN_DEF_PARSE_STATUS) {
if (virStrToLong_i(fromConfig, NULL, 10, tmp) 0) {
@@ -9452,6 +9469,7 @@
virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def,
VIR_FREE(address);
VIR_FREE(network);
VIR_FREE(fromConfig);
+VIR_FREE(family);
return ret;
}
@@ -19044,6 +19062,9 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf,
if (def-network
(def-type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) {
virBufferEscapeString(buf, network='%s', def
[libvirt] [PATCH 0/4] add support for graphics to get a IPv6 address
https://bugzilla.redhat.com/show_bug.cgi?id=1192318 We already support add a IPv6 address to graphics listen address and xml like this: graphics type='spice' port='5900' autoport='yes' listen='2001b8:ca2:2::1' keymap='en-us' listen type='address' address='2001b8:ca2:2::1'/ /graphics However we do not support get a IPv6 address for the network. add some helpers and rework networkGetNetworkAddress to make it can get a IPv6 address when we need. Luyao Huang (4): util: introduce a new helper for get interface IPv6 address conf: introduce new family attribute in graphics listen for chose IP family network: rework networkGetNetworkAddress to make it can get IPv6 address qemu: fix a error coverd issue in 2 place docs/formatdomain.html.in | 10 +++- docs/schemas/domaincommon.rng | 9 +++ src/conf/domain_conf.c | 21 +++ src/conf/domain_conf.h | 10 src/conf/network_conf.c| 2 +- src/conf/network_conf.h| 1 + src/libvirt_private.syms | 2 + src/network/bridge_driver.c| 50 +++- src/network/bridge_driver.h| 6 +- src/qemu/qemu_command.c| 18 ++ src/util/virnetdev.c | 70 ++ src/util/virnetdev.h | 2 + .../qemuxml2argv-graphics-listen-network.xml | 2 +- .../qemuxml2argv-graphics-listen-network2.xml | 2 +- .../qemuxml2xmlout-graphics-listen-network2.xml| 2 +- 15 files changed, 174 insertions(+), 33 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] network: rework networkGetNetworkAddress to make it can get IPv6 address
Export the required helpers and rework networkGetNetworkAddress to
make it can get IPv6 address.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/conf/network_conf.c | 2 +-
src/conf/network_conf.h | 1 +
src/libvirt_private.syms| 1 +
src/network/bridge_driver.c | 50 -
src/network/bridge_driver.h | 6 --
src/qemu/qemu_command.c | 4 ++--
6 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index f947d89..9eb640b 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -148,7 +148,7 @@ virNetworkDHCPHostDefClear(virNetworkDHCPHostDefPtr def)
VIR_FREE(def-name);
}
-static void
+void
virNetworkIpDefClear(virNetworkIpDefPtr def)
{
VIR_FREE(def-family);
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 484522e..0c4b559 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -310,6 +310,7 @@ virNetworkObjPtr virNetworkFindByName(virNetworkObjListPtr
nets,
void virNetworkDefFree(virNetworkDefPtr def);
void virNetworkObjFree(virNetworkObjPtr net);
void virNetworkObjListFree(virNetworkObjListPtr vms);
+void virNetworkIpDefClear(virNetworkIpDefPtr def);
typedef bool (*virNetworkObjListFilter)(virConnectPtr conn,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f60911c..ff942d8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -559,6 +559,7 @@ virNetworkDeleteConfig;
virNetworkFindByName;
virNetworkFindByUUID;
virNetworkForwardTypeToString;
+virNetworkIpDefClear;
virNetworkIpDefNetmask;
virNetworkIpDefPrefix;
virNetworkLoadAllConfigs;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 2798010..d1afd16 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4487,6 +4487,7 @@ networkReleaseActualDevice(virDomainDefPtr dom,
* networkGetNetworkAddress:
* @netname: the name of a network
* @netaddr: string representation of IP address for that network.
+ * @family: IP family
*
* Attempt to return an IP (v4) address associated with the named
* network. If a libvirt virtual network, that will be provided in the
@@ -4503,12 +4504,13 @@ networkReleaseActualDevice(virDomainDefPtr dom,
* completely unsupported.
*/
int
-networkGetNetworkAddress(const char *netname, char **netaddr)
+networkGetNetworkAddress(const char *netname, char **netaddr, int family)
{
int ret = -1;
virNetworkObjPtr network;
virNetworkDefPtr netdef;
-virNetworkIpDefPtr ipdef;
+virNetworkIpDefPtr ipdef = NULL;
+virNetworkIpDefPtr ipdef6 = NULL;
virSocketAddr addr;
virSocketAddrPtr addrptr = NULL;
char *dev_name = NULL;
@@ -4529,15 +4531,9 @@ networkGetNetworkAddress(const char *netname, char
**netaddr)
case VIR_NETWORK_FORWARD_NONE:
case VIR_NETWORK_FORWARD_NAT:
case VIR_NETWORK_FORWARD_ROUTE:
-/* if there's an ipv4def, get it's address */
+/* if there's an ipv4def or ipv6def, get it's address */
ipdef = virNetworkDefGetIpByIndex(netdef, AF_INET, 0);
-if (!ipdef) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(network '%s' doesn't have an IPv4 address),
- netdef-name);
-break;
-}
-addrptr = ipdef-address;
+ipdef6 = virNetworkDefGetIpByIndex(netdef, AF_INET6, 0);
break;
case VIR_NETWORK_FORWARD_BRIDGE:
@@ -4561,10 +4557,32 @@ networkGetNetworkAddress(const char *netname, char
**netaddr)
break;
}
-if (dev_name) {
-if (virNetDevGetIPv4Address(dev_name, addr) 0)
-goto error;
-addrptr = addr;
+switch ((virDomainGraphicsListenFamily) family) {
+case VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_DEFAULT:
+case VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV4:
+if (ipdef)
+addrptr = ipdef-address;
+if (dev_name) {
+if (virNetDevGetIPv4Address(dev_name, addr) 0)
+goto error;
+addrptr = addr;
+}
+/*if the family is default and we do not get a ipv4 address
+ *in this place, we will try to get a ipv6 address
+ */
+if (addrptr || family == VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV4)
+break;
+case VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV6:
+if (ipdef6)
+addrptr = ipdef6-address;
+if (dev_name) {
+if (virNetDevGetIPv6Address(dev_name, addr) 0)
+goto error;
+addrptr = addr;
+}
+break;
+case VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST:
+break;
}
if (!(addrptr
@@ -4579,6 +4597,10 @@ networkGetNetworkAddress(const char *netname, char
**netaddr)
return ret;
error:
+if (ipdef6)
+virNetworkIpDefClear(ipdef6);
+if (ipdef)
+virNetworkIpDefClear(ipdef);
goto cleanup
[libvirt] [PATCH 4/4] qemu: fix a error coverd issue in 2 place
we already set a more clearly error in networkGetNetworkAddress,
so this error will cover our error in networkGetNetworkAddress.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_command.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6bd8f69..b42e0f4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7284,12 +7284,9 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr
cfg,
network driver not present));
goto error;
}
-if (ret 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _(listen network '%s' had no usable address),
- listenNetwork);
+if (ret 0)
goto error;
-}
+
listenAddr = netAddr;
/* store the address we found in the graphics element so it will
* show up in status. */
@@ -7448,12 +7445,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr
cfg,
network driver not present));
goto error;
}
-if (ret 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _(listen network '%s' had no usable address),
- listenNetwork);
+if (ret 0)
goto error;
-}
+
listenAddr = netAddr;
/* store the address we found in the graphics element so it will
* show up in status. */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: fix IP address in vncdisplay for listen type='network'
Just like the fix for domdisplay in commit 1ba815.
---
tools/virsh-domain.c | 12
1 file changed, 12 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index dc4a863..2506b89 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10269,6 +10269,18 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd)
listen_addr = virXPathString(string(/domain/devices/graphics
[@type='vnc']/@listen), ctxt);
+if (!listen_addr) {
+/* The subelement address - listen address='xyz'/ -
+ * *should* have been automatically backfilled into its
+ * parent graphics listen='xyz' (which we just tried to
+ * retrieve into listen_addr above) but in some cases it
+ * isn't, so we also do an explicit check for the
+ * subelement (which, by the way, doesn't exist on libvirt
+ * 0.9.4, so we really do need to check both places)
+ */
+listen_addr = virXPathString(string(/domain/devices/graphics
+ [@type='vnc']/listen/@address), ctxt);
+}
if (listen_addr == NULL || STREQ(listen_addr, 0.0.0.0))
vshPrint(ctl, :%d\n, port-5900);
else
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
