[libvirt] [PATCH v2 0/3] Add support for vCPU and I/O thread scheduler setting
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1178986 v2: - No dependency on sched.h unless really needed - Docs fixed according to John - RNG schema matches parsing - Possibility to specify one element for whole mask of vcpus/iothreads - leaks fixed (VIR_FREE(nodes);) O:-) - not accepting other as a scheduler when parsing - Rebased, of course Martin Kletzander (3): util: Add virProcessSetScheduler() function for scheduler settings docs, schema, conf: Add support for setting scheduler parameters of guest threads qemu: Add support for setting vCPU and I/O thread scheduler setting configure.ac | 4 +- docs/formatdomain.html.in | 16 ++ docs/schemas/domaincommon.rng | 44 + src/conf/domain_conf.c | 183 - src/conf/domain_conf.h | 24 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 5 + src/qemu/qemu_process.c| 57 ++- src/qemu/qemu_process.h| 5 +- src/util/virprocess.c | 104 +++- src/util/virprocess.h | 20 ++- .../qemuxml2argv-cputune-iothreadsched-toomuch.xml | 38 + .../qemuxml2argv-cputune-iothreadsched.xml | 39 + .../qemuxml2argv-cputune-vcpusched-overlap.xml | 38 + tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-cputune-iothreadsched.xml | 39 + tests/qemuxml2xmltest.c| 1 + 17 files changed, 612 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-vcpusched-overlap.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreadsched.xml -- 2.3.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] qemu: Add support for setting vCPU and I/O thread scheduler setting
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1178986
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_driver.c | 5 +
src/qemu/qemu_process.c | 57 -
src/qemu/qemu_process.h | 5 -
3 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cf351e6..087a69d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4617,6 +4617,11 @@ static int qemuDomainHotplugVcpus(virQEMUDriverPtr
driver,
}
virCgroupFree(cgroup_vcpu);
+
+if (qemuProcessSetSchedParams(i, cpupids[i],
+ vm-def-cputune.nvcpusched,
+ vm-def-cputune.vcpusched) 0)
+goto cleanup;
}
} else {
for (i = oldvcpus - 1; i = nvcpus; i--) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d5df60d..4773120 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1,7 +1,7 @@
/*
* qemu_process.c: QEMU process management
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -2574,6 +2574,57 @@ qemuProcessSetIOThreadsAffinity(virDomainObjPtr vm)
return ret;
}
+/* Set Scheduler parameters for vCPU or I/O threads. */
+int
+qemuProcessSetSchedParams(int id,
+ pid_t pid,
+ size_t nsp,
+ virDomainThreadSchedParamPtr sp)
+{
+bool val = false;
+size_t i = 0;
+virDomainThreadSchedParamPtr s = NULL;
+
+for (i = 0; i nsp; i++) {
+if (virBitmapGetBit(sp[i].ids, id, val) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot get bit from bitmap));
+}
+if (val) {
+s = sp[i];
+break;
+}
+}
+
+if (!s)
+return 0;
+
+return virProcessSetScheduler(pid, s-scheduler, s-priority);
+}
+
+static int
+qemuProcessSetSchedulers(virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+size_t i = 0;
+
+for (i = 0; i priv-nvcpupids; i++) {
+if (qemuProcessSetSchedParams(i, priv-vcpupids[i],
+ vm-def-cputune.nvcpusched,
+ vm-def-cputune.vcpusched) 0)
+return -1;
+}
+
+for (i = 0; i priv-niothreadpids; i++) {
+if (qemuProcessSetSchedParams(i + 1, priv-iothreadpids[i],
+ vm-def-cputune.niothreadsched,
+ vm-def-cputune.iothreadsched) 0)
+return -1;
+}
+
+return 0;
+}
+
static int
qemuProcessInitPasswords(virConnectPtr conn,
virQEMUDriverPtr driver,
@@ -4869,6 +4920,10 @@ int qemuProcessStart(virConnectPtr conn,
if (qemuProcessSetIOThreadsAffinity(vm) 0)
goto cleanup;
+VIR_DEBUG(Setting scheduler parameters);
+if (qemuProcessSetSchedulers(vm) 0)
+goto cleanup;
+
VIR_DEBUG(Setting any required VM passwords);
if (qemuProcessInitPasswords(conn, driver, vm, asyncJob) 0)
goto cleanup;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 5948ea4..2e1d393 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -1,7 +1,7 @@
/*
* qemu_process.h: QEMU process management
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2012, 2015 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -104,4 +104,7 @@ virBitmapPtr qemuPrepareCpumap(virQEMUDriverPtr driver,
int qemuProcessReadLog(int fd, char *buf, int buflen, int off, bool skipchar);
+int qemuProcessSetSchedParams(int id, pid_t pid, size_t nsp,
+ virDomainThreadSchedParamPtr sp);
+
#endif /* __QEMU_PROCESS_H__ */
--
2.3.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] docs, schema, conf: Add support for setting scheduler parameters of guest threads
In order for QEMU vCPU (and other) threads to run with RT scheduler, libvirt needs to take care of that so QEMU doesn't have to run privileged. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1178986 Signed-off-by: Martin Kletzander mklet...@redhat.com --- docs/formatdomain.html.in | 16 ++ docs/schemas/domaincommon.rng | 44 + src/conf/domain_conf.c | 183 - src/conf/domain_conf.h | 24 +++ .../qemuxml2argv-cputune-iothreadsched-toomuch.xml | 38 + .../qemuxml2argv-cputune-iothreadsched.xml | 39 + .../qemuxml2argv-cputune-vcpusched-overlap.xml | 38 + tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-cputune-iothreadsched.xml | 39 + tests/qemuxml2xmltest.c| 1 + 10 files changed, 423 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-vcpusched-overlap.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreadsched.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d8144ea..3381dfe 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -550,6 +550,8 @@ lt;quotagt;-1lt;/quotagt; lt;emulator_periodgt;100lt;/emulator_periodgt; lt;emulator_quotagt;-1lt;/emulator_quotagt; +lt;vcpusched vcpus='0-4,^3' scheduler='fifo' priority='1'/gt; +lt;iothreadsched iothreads='2' scheduler='batch'/gt; lt;/cputunegt; ... lt;/domaingt; @@ -652,6 +654,20 @@ span class=sinceOnly QEMU driver support since 0.10.0/span /dd + dtcodevcpusched/code and codeiothreadsched/code/dt + dd +The optional codevcpusched/code elements specifie the scheduler +(values codebatch/code, codeidle/code, codefifo/code, +coderr/code) for particular vCPU/IOThread threads (based on +codevcpus/code and codeiothreads/code, leaving out +codevcpus/code/codeiothreads/code sets the default). +For real-time schedulers (codefifo/code, coderr/code), +priority must be specified as well (and is ignored for +non-real-time ones). The value range for the priority depends +on the host kernel (usually 1-99). +span class=sinceSince 1.2.12/span + /dd + /dl diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d467dce..98766dc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -815,10 +815,54 @@ /attribute /element /zeroOrMore +zeroOrMore + element name=vcpusched +optional + attribute name=vcpus +ref name='cpuset'/ + /attribute +/optional +ref name=schedparam/ + /element +/zeroOrMore +zeroOrMore + element name=iothreadsched +optional + attribute name=iothreads +ref name='cpuset'/ + /attribute +/optional +ref name=schedparam/ + /element +/zeroOrMore /interleave /element /define + define name=schedparam +choice + group +attribute name=scheduler + choice +valuebatch/value +valueidle/value + /choice +/attribute + /group + group +attribute name=scheduler + choice +valuefifo/value +valuerr/value + /choice +/attribute +attribute name=priority + ref name=unsignedShort/ +/attribute + /group +/choice + /define + !-- All the NUMA related tunables would go in the numatune -- define name=numatune element name=numatune diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 77319dc..3fb68b3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -772,6 +772,13 @@ VIR_ENUM_IMPL(virDomainLoader, rom, pflash) +VIR_ENUM_IMPL(virDomainThreadSched, VIR_DOMAIN_THREAD_SCHED_LAST, + other, /* default */ + batch, + idle, + fifo, + rr) + /* Internal mapping: subset of block job types that can be present in * mirror XML (remaining types are not two-phase). */ VIR_ENUM_DECL(virDomainBlockJob) @@ -2234,6 +2241,14 @@ void virDomainDefFree(virDomainDefPtr def) virDomainVcpuPinDefArrayFree(def-cputune.iothreadspin, def-cputune.niothreadspin); +for (i = 0; i def-cputune.nvcpusched; i++) +virBitmapFree(def-cputune.vcpusched[i].ids); +
[libvirt] [PATCH] domain: include portgroup in interface status xml
Prior to commit 7d5bf484747 (first appearing in libvirt 1.2.2), the
status XML of a domain's interface was missing a lot of important
information; mainly it just output the config of the interface, plus
the name of the tap device and qemu device alias. Commit 7d5bf484747
changed the status XML to include many important bits of information
that were required to make network hook scripts useful - bandwidth
information, vlan tag, the name of the bridge (or physical device in
the case of macvtap) that the tap/macvtap device was attached to - the
commit log for 7d5bf484747 has a very detailed explanation of the
change. For quick reference - in the example given there, prior to the
change, status XML looked like figure [C]:
interface type='network'
source network='testnet' portgroup='admin'/
target dev='macvtap0'/
alias name='net0'/
address type='pci' domain='0x' bus='0x00'
slot='0x03' function='0x0'/
/interface
and after the change, it looked like figure [E]:
interface type='direct'
source dev='p4p1_0' mode='bridge'/
bandwidth
inbound average='1000' peak='5000' burst='1024'/
outbound average='128' peak='256' burst='256'/
/bandwidth
target dev='macvtap0'/
alias name='net0'/
address type='pci' domain='0x' bus='0x00'
slot='0x03' function='0x0'/
/interface
You'll notice that bandwidth info, physdev, and macvtap mode have been
added, but the network and portgroup names are now missing - I didn't
think that this information was of any use once the needed
bandwidth/vlan/etc config had been pulled from the network/portgroup.
I was wrong.
A few months after that change a user on IRC asked what happened to
portgroup in the status XML and described how he used it (more or less
as a tag to decide what external information to use in a hook script
that was run at startup/migration time - see
http://wiki.libvirt.org/page/OVS_and_PVLANS ). At that time I planned
to make a patch to re-add portgroup, but life intervened as that was
just prior to a transatlantic move involving several weeks of
vacation. During this time I somehow forgot to make the patch, and
also mistakenly remembered that I *had* made it.
Subsequent to this, as a part of mprivozn's work to add support for
network-specific hooks, I did re-add the output of the network name in
status XML, but once again completely forgot about portgroup. This was
in commit a3609121 (first appearing in libvirt 1.2.11). This made the
status XML from the above example look like this:
interface type='direct'
source network='testnet' dev='p4p1_0' mode='bridge'/
bandwidth
inbound average='1000' peak='5000' burst='1024'/
outbound average='128' peak='256' burst='256'/
/bandwidth
target dev='macvtap0'/
alias name='net0'/
address type='pci' domain='0x' bus='0x00'
slot='0x03' function='0x0'/
/interface
*This* patch just adds the portgroup back to the status XML, so the
same example interface will look like this:
interface type='direct'
source network='testnet' portgroup='admin'
dev='p4p1_0' mode='bridge'/
bandwidth
inbound average='1000' peak='5000' burst='1024'/
outbound average='128' peak='256' burst='256'/
/bandwidth
target dev='macvtap0'/
alias name='net0'/
address type='pci' domain='0x' bus='0x00'
slot='0x03' function='0x0'/
/interface
The result is that the status XML now contains all information about
how the interface is setup (bandwidth, physical device, tap device,
etc), in addition to pointers to its origin (the network and
portgroup).
---
I had previously asked about this topic here:
https://www.redhat.com/archives/libvir-list/2014-June/msg00416.html
but then forgot to follow up on it...
src/conf/domain_conf.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8600eff..d870bb6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17591,14 +17591,18 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
if (def-type == VIR_DOMAIN_NET_TYPE_NETWORK !inSubelement) {
/* When we're putting our output into the actual
* subelement rather than the main interface, the
- * network name isn't included in the source because the
- * main interface element's source has the same info
- * already. If we've been called to output directly into
- * the main element's source though (the case here -
- * !inSubElement), we *do* need to output the network
- * name, because the caller won't have done it).
+ * network name and portgroup don't need to be included in
+
Re: [libvirt] [PATCH] storage: rbd: Improve the error when start a pool based on non-exist rados object
On 02/06/2015 08:55 PM, Ján Tomko wrote:
On Fri, Feb 06, 2015 at 07:45:37PM +0800, Shanzhi Yu wrote:
When start/create a pool based on non-exist rados object, the error will be like
$virsh pool-start p-c
error: Failed to start pool p-c
error: failed to create the RBD IoCTX. Does the pool 'libvirt-pool-clone'
exist?: No such file or directory
update it to
error: Failed to start pool p-c
error: internal error: failed to create the RBD IoCTX. the rados pool
'libvirt-pool-clone' is not available
This message is missing the actual error: 'no such file or directory'
returned by rados_ioctx_create. This return value has been added
to the error message by commit 761491eb
Indeed. Just catch that commit
Maybe we could just drop the hint about the pool existence?
error: failed to create the RBD IoCTX: No such file or directory
Would it be better to wrap the error returned by librados? Returning a
libvirt error will be more friendly to a libvirt user
Jan
Signed-off-by: Shanzhi Yu s...@redhat.com
---
src/storage/storage_backend_rbd.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index 57182de..98e7fe7 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -236,8 +236,10 @@ static int
virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virSt
{
int r = rados_ioctx_create(ptr-cluster, pool-def-source.name,
ptr-ioctx);
if (r 0) {
-virReportSystemError(-r, _(failed to create the RBD IoCTX. Does the pool
'%s' exist?),
- pool-def-source.name);
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(failed to create the RBD IoCTX.
+ the rados pool '%s' is not available),
+ pool-def-source.name);
}
return r;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Regards
shyu
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix show the wrong IP address for network type listen address graphic
On 02/11/2015 04:40 AM, Laine Stump wrote: On 02/10/2015 04:35 AM, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1191016 We try to get the IP address in /domain/devices/graphics/@listen, howerver for the network type listen address donnot have this parameter, it will show the address in the /domain/devices/graphics/listen/@address, running XML like this: graphics type='spice' port='5901' autoport='yes' keymap='en-us' listen type='network' address='192.168.122.1' network='default'/ /graphics This patch will try to get the IP address in this path /domain/devices/graphics/listen/@address That will work when the libvirtd being connected to is 0.9.4 or later, but earlier versions of libvirt don't have the listen subelement; instead they just have a 'listen' attribute directly inside graphics that contains the address. All newer versions of libvirt are supposed to populate that from listen[0] for backward compatibility. Oh, right, thanks for your catch, i forgot this thing when i wrote this patch, my patch will cause a issue without the patch you attached when use new virsh client to connect to old libvirtd(older than 0.9.4). The real bug here is that the listen attribute in graphics isn't being filled in in the case of type='network' when the domain is active. On the other hand, fixing the problem there would leave it unfixed for cases where the client is a new libvirt but the server is running libvirt between 0.9.4 and 1.2.12. So I think what is needed is for your patch to check @listen, and if nothing is found there, *then* check listen/@address. I attached a patch to this mail that I propose squashing into your patch before pushing. Let me know if it behaves properly and looks correct. I test with the patch you attached, it works well when use new virsh client connect to new libvirtd and also works well with the old libvirtd (older than 0.9.4 or later ). Thanks for your patch. Beyond that, the server side should still be fixed. I just sent a patch that does that: https://www.redhat.com/archives/libvir-list/2015-February/msg00332.html Yes, if the server have been fixed, old virsh client command will work well with new libvirtd. Between the two patches, we will have fixed the problem for all versions of server, as long as the client is new enough. Thanks for your review and i found you have wrote a patch for the client side, so seems no need write a new version for this patch :) Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix show the wrong IP address for network type listen address graphic
On 02/11/2015 04:40 AM, Laine Stump wrote:
On 02/10/2015 04:35 AM, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1191016
We try to get the IP address in /domain/devices/graphics/@listen, howerver
for the network type listen address donnot have this parameter, it will
show the address in the /domain/devices/graphics/listen/@address, running
XML like this:
graphics type='spice' port='5901' autoport='yes' keymap='en-us'
listen type='network' address='192.168.122.1' network='default'/
/graphics
This patch will try to get the IP address in this path
/domain/devices/graphics/listen/@address
That will work when the libvirtd being connected to is 0.9.4 or later,
but earlier versions of libvirt don't have the listen subelement;
instead they just have a 'listen' attribute directly inside graphics
that contains the address. All newer versions of libvirt are supposed to
populate that from listen[0] for backward compatibility.
The real bug here is that the listen attribute in graphics isn't being
filled in in the case of type='network' when the domain is active. On
the other hand, fixing the problem there would leave it unfixed for
cases where the client is a new libvirt but the server is running
libvirt between 0.9.4 and 1.2.12. So I think what is needed is for your
patch to check @listen, and if nothing is found there, *then* check
listen/@address. I attached a patch to this mail that I propose
squashing into your patch before pushing. Let me know if it behaves
properly and looks correct.
Beyond that, the server side should still be fixed. I just sent a patch
that does that:
https://www.redhat.com/archives/libvir-list/2015-February/msg00332.html
Between the two patches, we will have fixed the problem for all versions
of server, as long as the client is new enough.
Oh, i forgot the vncdisplay, it should also be fixed, i attached patch
to this mail.
Luyao
From fcc4e7194e61788a459674d8041259309b4d867b Mon Sep 17 00:00:00 2001
From: Luyao Huang lhu...@redhat.com
Date: Wed, 11 Feb 2015 10:18:10 +0800
Subject: [PATCH] virsh: fix vncdisplay get wrong ip address when listen type
is network
Just like the fix in domdisplay.
---
tools/virsh-domain.c | 12
1 file changed, 12 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 358d61c..8dd6b5e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10237,6 +10237,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
[libvirt] [PATCH v2 0/4] security: Add check for invalid security models and duplicates
Erik Skultety (4): security: Refactor virSecurityManagerGenLabel security: introduce virSecurityManagerCheckAllLabel function conf: forbid seclabel duplicates for domain devices schema: allow multiple seclabel for devices in domaincommon.rng docs/schemas/domaincommon.rng | 16 +-- src/conf/domain_conf.c | 9 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_process.c| 6 + src/security/security_manager.c| 146 ++--- src/security/security_manager.h| 2 + .../qemuxml2argv-seclabel-device-duplicates.xml| 33 + .../qemuxml2argv-seclabel-device-multiple.xml | 32 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c| 1 + 10 files changed, 220 insertions(+), 27 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-duplicates.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-multiple.xml -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/4] schema: allow multiple seclabel for devices in domaincommon.rng
In our RNG schema we do allow multiple (different) seclabels per-domain, but don't allow this for devices, yet we neither have a check in our XML parser, nor in a post-parse callback. In that case we should allow multiple (different) seclabels for devices as well. --- docs/schemas/domaincommon.rng | 16 +-- .../qemuxml2argv-seclabel-device-multiple.xml | 32 ++ tests/qemuxml2xmltest.c| 1 + 3 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-multiple.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d467dce..b1f4eaa 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1344,9 +1344,9 @@ optional ref name=storageStartupPolicy/ /optional - optional + zeroOrMore ref name='devSeclabel'/ - /optional + /zeroOrMore /element /optional /interleave @@ -1367,9 +1367,9 @@ optional ref name=storageStartupPolicy/ /optional - optional + zeroOrMore ref name='devSeclabel'/ - /optional + /zeroOrMore /element /optional /interleave @@ -1497,9 +1497,9 @@ optional ref name=storageStartupPolicy/ /optional - optional + zeroOrMore ref name='devSeclabel'/ - /optional + /zeroOrMore /element /optional /interleave @@ -3195,9 +3195,9 @@ optional attribute name=slave/ /optional -optional +zeroOrMore ref name='devSeclabel'/ -/optional +/zeroOrMore /element /zeroOrMore optional diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-multiple.xml b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-multiple.xml new file mode 100644 index 000..ce7f4f7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-multiple.xml @@ -0,0 +1,32 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static' cpuset='1-4,8-20,525'1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +disk type='block' device='disk' + source dev='/dev/HostVG/QEMUGuest1' +seclabel model='selinux' relabel='yes' + labelsystem_u:system_r:svirt_custom_t:s0:c192,c392/label +/seclabel +seclabel model='dac' relabel='no'/ + /source + target dev='hda' bus='ide'/ + address type='drive' controller='0' bus='0' target='0' unit='0'/ +/disk +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='pci' index='0' model='pci-root'/ +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index d3dfd9e..cc29083 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -337,6 +337,7 @@ mymain(void) DO_TEST_DIFFERENT(seclabel-none); DO_TEST(seclabel-dac-none); DO_TEST(seclabel-dynamic-none); +DO_TEST(seclabel-device-multiple); DO_TEST_FULL(seclabel-dynamic-none-relabel, true, WHEN_INACTIVE); DO_TEST(numad-static-vcpu-no-numatune); DO_TEST(disk-scsi-lun-passthrough-sgio); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: do upfront check for vcpupids being null when querying pinning
Quoting Daniel P. Berrange (berra...@redhat.com):
The qemuDomainHelperGetVcpus attempted to report an error when the
vcpupids info was NULL. Unfortunately earlier code would clamp the
value of 'maxinfo' to 0 when nvcpupids was 0, so the error reporting
would end up being skipped.
This lead to 'virsh vcpuinfo dom' just returning an empty list
instead of giving the user a clear error.
---
Cool, thanks. (haven't kicked off a test yet but it looks correct :)
Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com
src/qemu/qemu_driver.c | 47 +++
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e3ca437..cd30d9a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1376,6 +1376,12 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info, int maxinfo,
if ((hostcpus = nodeGetCPUCount()) 0)
return -1;
+if (priv-vcpupids == NULL) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(cpu affinity is not supported));
+return -1;
+}
+
maxcpu = maplen * 8;
if (maxcpu hostcpus)
maxcpu = hostcpus;
@@ -1391,8 +1397,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info, int maxinfo,
info[i].number = i;
info[i].state = VIR_VCPU_RUNNING;
-if (priv-vcpupids != NULL
-qemuGetProcessInfo((info[i].cpuTime),
+if (qemuGetProcessInfo((info[i].cpuTime),
(info[i].cpu),
NULL,
vm-pid,
@@ -1406,28 +1411,22 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info, int maxinfo,
if (cpumaps != NULL) {
memset(cpumaps, 0, maplen * maxinfo);
-if (priv-vcpupids != NULL) {
-for (v = 0; v maxinfo; v++) {
-unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen,
v);
-virBitmapPtr map = NULL;
-unsigned char *tmpmap = NULL;
-int tmpmapLen = 0;
-
-if (virProcessGetAffinity(priv-vcpupids[v],
- map, maxcpu) 0)
-return -1;
-virBitmapToData(map, tmpmap, tmpmapLen);
-if (tmpmapLen maplen)
-tmpmapLen = maplen;
-memcpy(cpumap, tmpmap, tmpmapLen);
-
-VIR_FREE(tmpmap);
-virBitmapFree(map);
-}
-} else {
-virReportError(VIR_ERR_OPERATION_INVALID,
- %s, _(cpu affinity is not available));
-return -1;
+for (v = 0; v maxinfo; v++) {
+unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
+virBitmapPtr map = NULL;
+unsigned char *tmpmap = NULL;
+int tmpmapLen = 0;
+
+if (virProcessGetAffinity(priv-vcpupids[v],
+ map, maxcpu) 0)
+return -1;
+virBitmapToData(map, tmpmap, tmpmapLen);
+if (tmpmapLen maplen)
+tmpmapLen = maplen;
+memcpy(cpumap, tmpmap, tmpmapLen);
+
+VIR_FREE(tmpmap);
+virBitmapFree(map);
}
}
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/4] security: Refactor virSecurityManagerGenLabel
Before we generate a security label (security driver with dynamic
labeling) for a domain, we first check for domain's security model
validity. We should also check devices' security model as well,
therefore it might be better to move this chunk of code in a separate function
which would check both the domain's security model and devices' security model.
This function would of course be called right before we try to generate
a security label in qemuProcessStart/qemuProcessAttach
---
src/security/security_manager.c | 20 +---
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 302f54d..000bc82 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -576,33 +576,15 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
virDomainDefPtr vm)
{
int ret = -1;
-size_t i, j;
+size_t i;
virSecurityManagerPtr* sec_managers = NULL;
virSecurityLabelDefPtr seclabel;
bool generated = false;
-if (mgr == NULL || mgr-drv == NULL)
-return ret;
-
if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL)
return ret;
virObjectLock(mgr);
-for (i = 0; i vm-nseclabels; i++) {
-if (!vm-seclabels[i]-model)
-continue;
-
-for (j = 0; sec_managers[j]; j++)
-if (STREQ(vm-seclabels[i]-model, sec_managers[j]-drv-name))
-break;
-
-if (!sec_managers[j]) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _(Unable to find security driver for label %s),
- vm-seclabels[i]-model);
-goto cleanup;
-}
-}
for (i = 0; sec_managers[i]; i++) {
generated = false;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/4] conf: forbid seclabel duplicates for domain devices
Parser checks for per-domain seclabel duplicates, so it would be nice if
it checked for per-device seclabel duplicates the same way
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1165485
---
src/conf/domain_conf.c | 9 ++
.../qemuxml2argv-seclabel-device-duplicates.xml| 33 ++
tests/qemuxml2argvtest.c | 1 +
3 files changed, 43 insertions(+)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-duplicates.xml
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6a57d80..7f242f1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5421,6 +5421,15 @@
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
break;
}
}
+
+/* check for duplicate seclabels */
+for (j = 0; j i; j++) {
+if (STREQ_NULLABLE(model, seclabels[j]-model)) {
+virReportError(VIR_ERR_XML_DETAIL,
+ _(seclabel for model %s is already
provided), model);
+goto error;
+}
+}
seclabels[i]-model = model;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-duplicates.xml
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-duplicates.xml
new file mode 100644
index 000..0ba26b6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-device-duplicates.xml
@@ -0,0 +1,33 @@
+domain type='qemu' id='1'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory unit='KiB'219100/memory
+ currentMemory unit='KiB'219100/currentMemory
+ vcpu placement='static' cpuset='1-4,8-20,525'1/vcpu
+ os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+ /os
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashdestroy/on_crash
+ devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+ source dev='/dev/HostVG/QEMUGuest1'
+seclabel model='selinux' relabel='no'/
+seclabel model='selinux' relabel='no'/
+seclabel model='selinux' relabel='no'/
+ /source
+ target dev='hda' bus='ide'/
+ address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='usb' index='0'/
+controller type='ide' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='virtio'/
+ /devices
+ seclabel type='none' relabel='no'/
+ seclabel type='dynamic' model='dac' relabel='yes'/
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f864c2a..395cab3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1302,6 +1302,7 @@ mymain(void)
DO_TEST(seclabel-none, QEMU_CAPS_NAME);
DO_TEST(seclabel-dac-none, QEMU_CAPS_NAME);
DO_TEST_PARSE_ERROR(seclabel-multiple, QEMU_CAPS_NAME);
+DO_TEST_PARSE_ERROR(seclabel-device-duplicates, QEMU_CAPS_NAME);
DO_TEST(pseries-basic,
QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: fix setting of VM CPU affinity with TCG
If a previous commit I fixed the incorrect handling of vcpu pids
for TCG mode QEMU:
commit b07f3d821dfb11a118ee75ea275fd6ab737d9500
Author: Daniel P. Berrange berra...@redhat.com
Date: Thu Dec 18 16:34:39 2014 +
Don't setup fake CPU pids for old QEMU
The code assumes that def-vcpus == nvcpupids, so when we setup
fake CPU pids for old QEMU with nvcpupids == 1, we cause the
later code to read off the end of the array. This has fun results
like sche_setaffinity(0, ...) which changes libvirtd's own CPU
affinity, or even better sched_setaffinity($RANDOM, ...) which
changes the affinity of a random OS process.
The intent was that this would merely disable the ability to set
per-vCPU affinity. It should still have been possible to set VM
level host CPU affinity.
Unfortunately, when you set vcpu cpuset='0-1'4/vcpu, the XML
parser will internally take this initialize an entry in the
def-cputune.vcpupin array for every VCPU. IOW this is implicitly
being treated as
cputune
vcpupin cpuset='0-1' vcpu='0'/
vcpupin cpuset='0-1' vcpu='1'/
vcpupin cpuset='0-1' vcpu='2'/
vcpupin cpuset='0-1' vcpu='3'/
/cputune
Even more fun, the faked cputune elements are hidden from view when
querying the live XML, because their cpuset mask is the same as the
VM default cpumask.
The upshot was that it was impossible to set VM level CPU affinity.
To fix this we must update qemuProcessSetVcpuAffinities so that it
only reports a fatal error if the per-VCPU cpu mask is different
from the VM level cpu mask.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_process.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
In v2:
- Check for def-cpumask being NULL
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d5df60d..4c35f39 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2497,9 +2497,19 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm)
return 0;
if (priv-vcpupids == NULL) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- %s, _(cpu affinity is not supported));
-return -1;
+/* If any CPU has custom affinity that differs from the
+ * VM default affinity, we must reject it
+ */
+for (n = 0; n def-vcpus; n++) {
+if (!def-cpumask ||
+!virBitmapEqual(def-cpumask,
+def-cputune.vcpupin[n]-cpumask)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(cpu affinity is not supported));
+return -1;
+}
+}
+return 0;
}
for (n = 0; n def-vcpus; n++) {
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virsh vcpuinfo with tcg
On Mon, Feb 09, 2015 at 04:01:55PM +, Daniel P. Berrange wrote: On Mon, Feb 09, 2015 at 03:20:28PM +, Serge Hallyn wrote: Hi, 'virsh vcpuinfo' in 1.2.12 returns an empty line for VMs using tcg. I assume this is due to commit b07f3d821dfb11 which explicitly sets nvcpupids to 0 now. Is 'virsh vcpuinfo' returning nothing just an unfortunate but expected side-effect, or is it a bug and it should return info anyway? My impression is that qemu_driver.c should be using vm-def-vcpus rather than priv-nvcpupids to determine the # of cpus to show? The virsh vcpuinfo command calls virDomainGetVcpus() which in QEMU ends up calling qemuDomainHelperGetVcpus. This method in turn iterates over the vCPUs calling qemuGetProcessInfo to determine the CPU time CPU affinity of the vCPU vs host pCPUs. This only works if there are threads for each vCPU in QEMU. The QEMU TCG emulator is single threaded, so we cannot get the data for individual vCPUs when using TCG. The previous code would result in virDomainGetVcpus() returning only a single vCPU record even when the guest have multiple vCPUs, which is just incorrect. For some reason though qemuDomainHelperGetVcpus just returns no data at all now, when it should be returning an error message cpu affinity is not available. this just looks like a logic error there. I've found another bug in fact. It seems that when the XML parser sees vcpu cpuset='0-1'4/vcpu, it will also create a fake cputune config of cputune vcpupin cpuset='0-1' vcpu='0'/ vcpupin cpuset='0-1' vcpu='1'/ vcpupin cpuset='0-1' vcpu='2'/ vcpupin cpuset='0-1' vcpu='3'/ /cputune So although my intent was to prevent use of per-VCPU pining only, it seems we've in fact prevented use of VM level pinning too :-( I'll send a patch to try and fix that Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/4] security: introduce virSecurityManagerCheckAllLabel function
We do have a check for valid per-domain security model, however we still
do permit an invalid security model for a domain's device (those which
are specified with source element).
This patch introduces a new function virSecurityManagerCheckAllLabel
which compares user specified security model against currently
registered security drivers. That being said, it also permits 'none'
being specified as a device security model.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1165485
---
src/libvirt_private.syms| 1 +
src/qemu/qemu_process.c | 6 ++
src/security/security_manager.c | 126
src/security/security_manager.h | 2 +
4 files changed, 135 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3b4ff41..1b1d891 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -953,6 +953,7 @@ virSecurityDriverLookup;
# security/security_manager.h
+virSecurityManagerCheckAllLabel;
virSecurityManagerClearSocketLabel;
virSecurityManagerGenLabel;
virSecurityManagerGetBaseLabel;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d5df60d..66ae779 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4428,6 +4428,10 @@ int qemuProcessStart(virConnectPtr conn,
NULL) 0)
goto cleanup;
+VIR_DEBUG(Checking domain and device security labels);
+if (virSecurityManagerCheckAllLabel(driver-securityManager, vm-def) 0)
+goto cleanup;
+
/* If you are using a SecurityDriver with dynamic labelling,
then generate a security label for isolation */
VIR_DEBUG(Generating domain security label (if required));
@@ -5424,6 +5428,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
}
}
+if (virSecurityManagerCheckAllLabel(driver-securityManager, vm-def) 0)
+goto error;
if (virSecurityManagerGenLabel(driver-securityManager, vm-def) 0)
goto error;
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 000bc82..32bc9fe 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -685,6 +685,132 @@ virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr,
}
+static int
+virSecurityManagerCheckSecurityDiskLabel(virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDiskDefPtr disk,
+ void *opaque)
+{
+size_t i, j;
+virSecurityManagerPtr mgr = opaque;
+virSecurityManagerPtr *sec_managers = NULL;
+
+if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL)
+return 0;
+
+for (i = 0; i disk-src-nseclabels; i++) {
+if (STREQ_NULLABLE(disk-src-seclabels[i]-model, none))
+continue;
+
+for (j = 0; sec_managers[j]; j++) {
+if (STREQ_NULLABLE(disk-src-seclabels[i]-model,
+ sec_managers[j]-drv-name)) {
+break;
+}
+}
+if (!sec_managers[j]) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unable to find security driver for model %s),
+ disk-src-seclabels[i]-model);
+return -1;
+}
+}
+
+return 0;
+}
+
+
+static int
+virSecurityManagerCheckSecurityChardevLabel(virDomainDefPtr def
ATTRIBUTE_UNUSED,
+virDomainChrDefPtr chrdev,
+void *opaque)
+{
+size_t i, j;
+virSecurityManagerPtr mgr = opaque;
+virSecurityManagerPtr *sec_managers = NULL;
+
+if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL)
+return 0;
+
+for (i = 0; i chrdev-nseclabels; i++) {
+if (STREQ_NULLABLE(chrdev-seclabels[i]-model, none))
+continue;
+
+for (j = 0; sec_managers[j]; j++) {
+if (STREQ_NULLABLE(chrdev-seclabels[i]-model,
+ sec_managers[j]-drv-name)) {
+break;
+}
+}
+if (!sec_managers[j]) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Unable to find security driver for model %s),
+ chrdev-seclabels[i]-model);
+return -1;
+}
+}
+
+return 0;
+}
+
+
+static int
+virSecurityManagerCheckSecurityChardevCallback(virDomainDefPtr def,
+ virDomainChrDefPtr dev,
+ void *opaque)
+{
+return virSecurityManagerCheckSecurityChardevLabel(def, dev, opaque);
+}
+
+
+int virSecurityManagerCheckAllLabel(virSecurityManagerPtr mgr,
+virDomainDefPtr vm)
+{
+size_t i, j;
+virSecurityManagerPtr *sec_managers = NULL;
+
+if (mgr == NULL || mgr-drv == NULL)
+return 0;
+
+if
[libvirt] [PATCH] qemu: do upfront check for vcpupids being null when querying pinning
The qemuDomainHelperGetVcpus attempted to report an error when the
vcpupids info was NULL. Unfortunately earlier code would clamp the
value of 'maxinfo' to 0 when nvcpupids was 0, so the error reporting
would end up being skipped.
This lead to 'virsh vcpuinfo dom' just returning an empty list
instead of giving the user a clear error.
---
src/qemu/qemu_driver.c | 47 +++
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e3ca437..cd30d9a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1376,6 +1376,12 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info, int maxinfo,
if ((hostcpus = nodeGetCPUCount()) 0)
return -1;
+if (priv-vcpupids == NULL) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(cpu affinity is not supported));
+return -1;
+}
+
maxcpu = maplen * 8;
if (maxcpu hostcpus)
maxcpu = hostcpus;
@@ -1391,8 +1397,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info, int maxinfo,
info[i].number = i;
info[i].state = VIR_VCPU_RUNNING;
-if (priv-vcpupids != NULL
-qemuGetProcessInfo((info[i].cpuTime),
+if (qemuGetProcessInfo((info[i].cpuTime),
(info[i].cpu),
NULL,
vm-pid,
@@ -1406,28 +1411,22 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
virVcpuInfoPtr info, int maxinfo,
if (cpumaps != NULL) {
memset(cpumaps, 0, maplen * maxinfo);
-if (priv-vcpupids != NULL) {
-for (v = 0; v maxinfo; v++) {
-unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
-virBitmapPtr map = NULL;
-unsigned char *tmpmap = NULL;
-int tmpmapLen = 0;
-
-if (virProcessGetAffinity(priv-vcpupids[v],
- map, maxcpu) 0)
-return -1;
-virBitmapToData(map, tmpmap, tmpmapLen);
-if (tmpmapLen maplen)
-tmpmapLen = maplen;
-memcpy(cpumap, tmpmap, tmpmapLen);
-
-VIR_FREE(tmpmap);
-virBitmapFree(map);
-}
-} else {
-virReportError(VIR_ERR_OPERATION_INVALID,
- %s, _(cpu affinity is not available));
-return -1;
+for (v = 0; v maxinfo; v++) {
+unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v);
+virBitmapPtr map = NULL;
+unsigned char *tmpmap = NULL;
+int tmpmapLen = 0;
+
+if (virProcessGetAffinity(priv-vcpupids[v],
+ map, maxcpu) 0)
+return -1;
+virBitmapToData(map, tmpmap, tmpmapLen);
+if (tmpmapLen maplen)
+tmpmapLen = maplen;
+memcpy(cpumap, tmpmap, tmpmapLen);
+
+VIR_FREE(tmpmap);
+virBitmapFree(map);
}
}
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix setting of VM CPU affinity with TCG
If a previous commit I fixed the incorrect handling of vcpu pids
for TCG mode QEMU:
commit b07f3d821dfb11a118ee75ea275fd6ab737d9500
Author: Daniel P. Berrange berra...@redhat.com
Date: Thu Dec 18 16:34:39 2014 +
Don't setup fake CPU pids for old QEMU
The code assumes that def-vcpus == nvcpupids, so when we setup
fake CPU pids for old QEMU with nvcpupids == 1, we cause the
later code to read off the end of the array. This has fun results
like sche_setaffinity(0, ...) which changes libvirtd's own CPU
affinity, or even better sched_setaffinity($RANDOM, ...) which
changes the affinity of a random OS process.
The intent was that this would merely disable the ability to set
per-vCPU affinity. It should still have been possible to set VM
level host CPU affinity.
Unfortunately, when you set vcpu cpuset='0-1'4/vcpu, the XML
parser will internally take this initialize an entry in the
def-cputune.vcpupin array for every VCPU. IOW this is implicitly
being treated as
cputune
vcpupin cpuset='0-1' vcpu='0'/
vcpupin cpuset='0-1' vcpu='1'/
vcpupin cpuset='0-1' vcpu='2'/
vcpupin cpuset='0-1' vcpu='3'/
/cputune
Even more fun, the faked cputune elements are hidden from view when
querying the live XML, because their cpuset mask is the same as the
VM default cpumask.
The upshot was that it was impossible to set VM level CPU affinity.
To fix this we must update qemuProcessSetVcpuAffinities so that it
only reports a fatal error if the per-VCPU cpu mask is different
from the VM level cpu mask.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/qemu/qemu_process.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d5df60d..e8c532f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2497,9 +2497,18 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm)
return 0;
if (priv-vcpupids == NULL) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- %s, _(cpu affinity is not supported));
-return -1;
+/* If any CPU has custom affinity that differs from the
+ * VM default affinity, we must reject it
+ */
+for (n = 0; n def-vcpus; n++) {
+if (!virBitmapEqual(def-cpumask,
+def-cputune.vcpupin[n]-cpumask)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(cpu affinity is not supported));
+return -1;
+}
+}
+return 0;
}
for (n = 0; n def-vcpus; n++) {
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: disallow invalid values for video attributes
On Tue, Feb 10, 2015 at 09:06:24AM +0100, Martin Kletzander wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1190956
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c| 8
...v-seclabel-dynamic-none.xml = qemuxml2argv-video-invalid.xml} | 4 +++-
tests/qemuxml2argvtest.c | 1 +
3 files changed, 8 insertions(+), 5 deletions(-)
copy tests/qemuxml2argvdata/{qemuxml2argv-seclabel-dynamic-none.xml =
qemuxml2argv-video-invalid.xml} (90%)
ACK
Jan
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] conf: disallow invalid values for video attributes
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1190956
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/conf/domain_conf.c| 8
...v-seclabel-dynamic-none.xml = qemuxml2argv-video-invalid.xml} | 4 +++-
tests/qemuxml2argvtest.c | 1 +
3 files changed, 8 insertions(+), 5 deletions(-)
copy tests/qemuxml2argvdata/{qemuxml2argv-seclabel-dynamic-none.xml =
qemuxml2argv-video-invalid.xml} (90%)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d5b15db..77319dc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10580,7 +10580,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
_(ram attribute only supported for type of qxl));
goto error;
}
-if (virStrToLong_ui(ram, NULL, 10, def-ram) 0) {
+if (virStrToLong_uip(ram, NULL, 10, def-ram) 0) {
virReportError(VIR_ERR_XML_ERROR,
_(cannot parse video ram '%s'), ram);
goto error;
@@ -10590,7 +10590,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
}
if (vram) {
-if (virStrToLong_ui(vram, NULL, 10, def-vram) 0) {
+if (virStrToLong_uip(vram, NULL, 10, def-vram) 0) {
virReportError(VIR_ERR_XML_ERROR,
_(cannot parse video vram '%s'), vram);
goto error;
@@ -10605,7 +10605,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
_(vgamem attribute only supported for type of
qxl));
goto error;
}
-if (virStrToLong_ui(vgamem, NULL, 10, def-vgamem) 0) {
+if (virStrToLong_uip(vgamem, NULL, 10, def-vgamem) 0) {
virReportError(VIR_ERR_XML_ERROR,
_(cannot parse video vgamem '%s'), vgamem);
goto error;
@@ -10613,7 +10613,7 @@ virDomainVideoDefParseXML(xmlNodePtr node,
}
if (heads) {
-if (virStrToLong_ui(heads, NULL, 10, def-heads) 0) {
+if (virStrToLong_uip(heads, NULL, 10, def-heads) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse video heads '%s'), heads);
goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
b/tests/qemuxml2argvdata/qemuxml2argv-video-invalid.xml
similarity index 90%
copy from tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
copy to tests/qemuxml2argvdata/qemuxml2argv-video-invalid.xml
index cec59f8..e3848e1 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-video-invalid.xml
@@ -22,7 +22,9 @@
controller type='usb' index='0'/
controller type='ide' index='0'/
controller type='pci' index='0' model='pci-root'/
+video
+ model type='qxl' ram='-1' vram='-1' vgamem='-1' heads='-1'/
+/video
memballoon model='virtio'/
/devices
- seclabel type='none' model='none'/
/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 77ee630..f864c2a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1362,6 +1362,7 @@ mymain(void)
QEMU_CAPS_DEVICE_QXL_VGA, QEMU_CAPS_DEVICE_QXL,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_QXL_VGA_VGAMEM,
QEMU_CAPS_QXL_VGAMEM);
+DO_TEST_PARSE_ERROR(video-invalid, NONE);
DO_TEST(virtio-rng-default, QEMU_CAPS_DEVICE,
QEMU_CAPS_DEVICE_VIRTIO_RNG,
QEMU_CAPS_OBJECT_RNG_RANDOM);
--
2.3.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/12] qemu: Implement random number generator hotplug
On Mon, Feb 09, 2015 at 16:57:17 +0100, Ján Tomko wrote:
On Fri, Feb 06, 2015 at 04:32:25PM +0100, Peter Krempa wrote:
From: Luyao Huang lhu...@redhat.com
Export the required helpers and add backend code to hotplug RNG devices.
Signed-off-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Peter Krempa pkre...@redhat.com
---
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_command.h | 4 ++
src/qemu/qemu_driver.c | 8 +++-
src/qemu/qemu_hotplug.c | 99
+
src/qemu/qemu_hotplug.h | 3 ++
5 files changed, 114 insertions(+), 2 deletions(-)
-- 8 8 8 --
+
+/* attach the device - up to a 3 stage process */
+qemuDomainObjEnterMonitor(driver, vm);
+
+if (rng-backend == VIR_DOMAIN_RNG_BACKEND_EGD
+qemuMonitorAttachCharDev(priv-mon, charAlias,
+ rng-source.chardev) 0)
+goto failchardev;
+
+if (qemuMonitorAddObject(priv-mon, type, objAlias, props) 0)
+goto failbackend;
+
+if (qemuMonitorAddDevice(priv-mon, devstr) 0)
+goto failfrontend;
+
+if (qemuDomainObjExitMonitor(driver, vm) 0) {
+vm = NULL;
+goto cleanup;
+}
I know an OOM error at this exact location is not that likely, but
it would be nicer to prealloc the space in vm-def before entering
the monitor so that the operation below cannot fail, like we do
when hotplugging other devices.
Sigh. Okay. I've switched to the preallocation approach as we do usually
in this case.
Jan
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: fix show the wrong IP address for network type listen address graphic
https://bugzilla.redhat.com/show_bug.cgi?id=1191016 We try to get the IP address in /domain/devices/graphics/@listen, howerver for the network type listen address donnot have this parameter, it will show the address in the /domain/devices/graphics/listen/@address, running XML like this: graphics type='spice' port='5901' autoport='yes' keymap='en-us' listen type='network' address='192.168.122.1' network='default'/ /graphics This patch will try to get the IP address in this path /domain/devices/graphics/listen/@address Signed-off-by: Luyao Huang lhu...@redhat.com --- tools/virsh-domain.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 358d61c..4a9b574 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10024,7 +10024,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) int tmp; int flags = 0; bool params = false; -const char *xpath_fmt = string(/domain/devices/graphics[@type='%s']/@%s); +const char *xpath_fmt = string(/domain/devices/graphics[@type='%s']/%s); virSocketAddr addr; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) @@ -10054,7 +10054,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) continue; /* Create our XPATH lookup for the current display's port */ -if (virAsprintf(xpath, xpath_fmt, scheme[iter], port) 0) +if (virAsprintf(xpath, xpath_fmt, scheme[iter], @port) 0) goto cleanup; /* Attempt to get the port number for the current graphics scheme */ @@ -10068,7 +10068,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) /* Create our XPATH lookup for TLS Port (automatically skipped * for unsupported schemes */ -if (virAsprintf(xpath, xpath_fmt, scheme[iter], tlsPort) 0) +if (virAsprintf(xpath, xpath_fmt, scheme[iter], @tlsPort) 0) goto cleanup; /* Attempt to get the TLS port number */ @@ -10081,7 +10081,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) continue; /* Create our XPATH lookup for the current display's address */ -if (virAsprintf(xpath, xpath_fmt, scheme[iter], listen) 0) +if (virAsprintf(xpath, xpath_fmt, scheme[iter], listen/@address) 0) goto cleanup; /* Attempt to get the listening addr if set for the current @@ -10095,7 +10095,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) * care of when getting the XML */ /* Create our XPATH lookup for the password */ -if (virAsprintf(xpath, xpath_fmt, scheme[iter], passwd) 0) +if (virAsprintf(xpath, xpath_fmt, scheme[iter], @passwd) 0) goto cleanup; /* Attempt to get the password */ -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/12] Implement random number generator hot/cold (un)plug
On Mon, Feb 09, 2015 at 16:58:07 +0100, Ján Tomko wrote: On Fri, Feb 06, 2015 at 04:32:14PM +0100, Peter Krempa wrote: Extension of Luyao's series with changes that were possible due to the prepare series for memory hotplug. ACK series, see my replies to individual patches for some minor nits. I've fixed all of them and pushed the series after a final test. Thanks for the review. Jan Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/12] qemu: command: Refactor creation of RNG device commandline
On Mon, Feb 09, 2015 at 16:54:00 +0100, Ján Tomko wrote:
On Fri, Feb 06, 2015 at 04:32:21PM +0100, Peter Krempa wrote:
As the RNG device is using an -object as backend refactor the code to
use the JSON to commandline generator so that we can reuse the code
later in hotplug.
---
src/qemu/qemu_command.c | 108
+---
1 file changed, 84 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6380621..9179c1f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6154,15 +6154,33 @@ qemuBuildSclpDevStr(virDomainChrDefPtr dev)
static int
-qemuBuildRNGBackendArgs(virCommandPtr cmd,
-virDomainRNGDefPtr dev,
-virQEMUCapsPtr qemuCaps)
+qemuBuildRNGBackendChrdevStr(virDomainRNGDefPtr rng,
+ virQEMUCapsPtr qemuCaps,
+ char **chr)
{
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *backend = NULL;
+*chr = NULL;
+
+if (rng-backend != VIR_DOMAIN_RNG_BACKEND_EGD)
+return 0;
Making this a switch ((virDomainRNGBackend)) would check if all the
enum values are handled.
Indeed. Having the complier to complain is useful when adding new stuff.
Consider it added.
Peter
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 5/9] virstoragefile: change backingStore to backingStores.
The backingStore field was a virStorageSourcePtr.
because a quorum can contain more that one backingStore at the same level
it's now a 'virStorageSourcePtr *'.
This patch rename src-backingStore to src-backingStores,
add a static function virStorageSourceExpandBackingStore
(virStorageSourcePushBackingStore in the V2) and made the necessary
modification to
virStorageSourceSetBackingStore and virStorageSourceGetBackingStore.
virStorageSourceSetBackingStore can now expand size of src-backingStores
by calling virStorageSourceExpandBackingStore if necessary.
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
src/util/virstoragefile.c | 69 +--
src/util/virstoragefile.h | 3 ++-
2 files changed, 63 insertions(+), 9 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index d573aba..2a5321c 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1798,21 +1798,66 @@ virStorageSourcePoolDefCopy(const
virStorageSourcePoolDef *src)
}
+/**
+ * virStorageSourceGetBackingStore:
+ * @src: virStorageSourcePtr containing the backing stores
+ * @pos: position of the backing store to get
+ *
+ * return the backingStore at the position of @pos
+ */
virStorageSourcePtr
-virStorageSourceGetBackingStore(const virStorageSource *src,
-size_t pos ATTRIBUTE_UNUSED)
+virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos)
+{
+if (!src || !src-backingStores || pos = src-nBackingStores)
+return NULL;
+return src-backingStores[pos];
+}
+
+
+/**
+ * virStorageSourcePushBackingStore:
+ *
+ * Expand src-backingStores and update src-nBackingStores
+ */
+static bool
+virStorageSourceExpandBackingStore(virStorageSourcePtr src, size_t nbr)
{
-return src-backingStore;
+if (!src)
+return false;
+if (src-nBackingStores 0) {
+if (VIR_EXPAND_N(src-backingStores, src-nBackingStores, nbr) 0)
+return false;
+} else {
+if (VIR_ALLOC_N(src-backingStores, nbr) 0)
+return false;
+src-nBackingStores += nbr;
+}
+return true;
}
+/**
+ * virStorageSourceSetBackingStore:
+ * @src: virStorageSourcePtr to hold @backingStore
+ * @backingStore: backingStore to store
+ * @pos: position of the backing store to store
+ *
+ * Set @backingStore at @pos in src-backingStores.
+ * If src-backingStores don't have the space to contain
+ * @backingStore, we expand src-backingStores
+ */
bool
virStorageSourceSetBackingStore(virStorageSourcePtr src,
virStorageSourcePtr backingStore,
-size_t pos ATTRIBUTE_UNUSED)
+size_t pos)
{
-src-backingStore = backingStore;
-return src-backingStore;
+if (!src)
+return false;
+if (pos = src-nBackingStores
+!virStorageSourceExpandBackingStore(src, pos - src-nBackingStores +
1))
+return false;
+src-backingStores[pos] = backingStore;
+return true;
}
@@ -2023,6 +2068,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src)
void
virStorageSourceBackingStoreClear(virStorageSourcePtr def)
{
+size_t i;
+
if (!def)
return;
@@ -2030,8 +2077,14 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr
def)
VIR_FREE(def-backingStoreRaw);
/* recursively free backing chain */
-virStorageSourceFree(virStorageSourceGetBackingStore(def, 0));
-virStorageSourceSetBackingStore(def, NULL, 0);
+for (i = 0; i def-nBackingStores; ++i)
+virStorageSourceFree(virStorageSourceGetBackingStore(def, i));
+if (def-nBackingStores 0) {
+/* in this case def-backingStores is treat as an array so we have to
free it*/
+VIR_FREE(def-backingStores);
+}
+def-nBackingStores = 0;
+def-backingStores = NULL;
}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 85b8d6e..b94f9d9 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -270,7 +270,8 @@ struct _virStorageSource {
bool shared;
/* backing chain of the storage source */
-virStorageSourcePtr backingStore;
+virStorageSourcePtr *backingStores;
+size_t nBackingStores;
/* metadata for storage driver access to remote and local volumes */
virStorageDriverDataPtr drv;
--
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: Add quorum support in qemuBuildDriveDevStr
Allow to libvirt to build the quorum string used by quemu.
Add 2 static functions: qemuBuildQuorumStr and
qemuBuildAndAppendDriveStrToVirBuffer.
qemuBuildQuorumStr is made because a quorum can have another quorum
as a child, so we may need to call qemuBuildQuorumStr recursively.
qemuBuildQuorumFileSourceStr was basically made to share
the code use to build the source between qemuBuildQuorumStr and
qemuBuildDriveStr, but there is some difference betwin the syntax
use by libvirt to declare a disk and the one qemu need to build a quorum:
a quorum need a syntaxe like:
domaine_name.children.X.file.filename=filename
where libvirt don't use file.filename= but directly file=.
Therfore I use this function only for quorum.
But as explained in the cover letter and here:
http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html
We miss some informations in _virStorageSource to have a complet
quorum support in libvirt.
Ideally I'd like to refactore virDomainDiskDefFormat to allow
qemuBuildQuorumStr to call this function in a loop.
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
src/qemu/qemu_command.c | 110
1 file changed, 110 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d4123a0..acbff19 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3347,6 +3347,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk)
return -1;
}
+static bool
+qemuBuildQuorumFileSourceStr(virConnectPtr conn,
+ virStorageSourcePtr src,
+ virBuffer *opt,
+ const char *toAppend)
+{
+char *source = NULL;
+int actualType = virStorageSourceGetActualType(src);
+
+if (qemuGetDriveSourceString(src, conn, source) 0)
+goto error;
+
+if (source) {
+
+virBufferAsprintf(opt, ,%sfilename=, toAppend);
+
+switch (actualType) {
+case VIR_STORAGE_TYPE_DIR:
+/* QEMU only supports magic FAT format for now */
+if (src-format 0
+src-format != VIR_STORAGE_FILE_FAT) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(unsupported disk driver type for '%s'),
+ virStorageFileFormatTypeToString(src-format));
+goto error;
+}
+
+if (!src-readonly) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot create virtual FAT disks in
read-write mode));
+goto error;
+}
+
+virBufferAddLit(opt, fat:);
+
+break;
+
+default:
+break;
+}
+virBufferAsprintf(opt, %s, source);
+}
+
+return true;
+ error:
+return false;
+}
+
+
+static bool
+qemuBuildQuorumStr(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr src,
+ virBuffer *opt,
+ const char *toAppend)
+{
+char *tmp = NULL;
+int ret;
+virStorageSourcePtr backingStore;
+size_t i;
+
+if (!src-threshold) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(threshold missing in the quorum configuration));
+return false;
+}
+if (src-nBackingStores 2) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(a quorum must have at last 2 children));
+return false;
+}
+if (src-threshold src-nBackingStores) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(threshold must not exceed the number of childrens));
+return false;
+}
+virBufferAsprintf(opt, ,%svote-threshold=%lu,
+ toAppend, src-threshold);
+for (i = 0; i src-nBackingStores; ++i) {
+backingStore = virStorageSourceGetBackingStore(src, i);
+ret = virAsprintf(tmp, %schildren.%lu.file., toAppend, i);
+if (ret 0)
+return false;
+
+virBufferAsprintf(opt, ,%schildren.%lu.driver=%s,
+ toAppend, i,
+
virStorageFileFormatTypeToString(backingStore-format));
+
+if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) ==
false)
+goto error;
+
+/* This operation avoid me to made another copy */
+tmp[ret - sizeof(file)] = '\0';
+if (backingStore-type == VIR_STORAGE_TYPE_QUORUM) {
+if (!qemuBuildQuorumStr(conn, disk, backingStore, opt, tmp))
+goto error;
+}
+VIR_FREE(tmp);
+}
+return true;
+ error:
+VIR_FREE(tmp);
+return false;
+}
+
/* Qemu 1.2 and later have a binary flag -enable-fips that must be
* used for VNC auth to obey FIPS settings; but the flag only
@@ -3819,6 +3924,11 @@ qemuBuildDriveStr(virConnectPtr conn,
[libvirt] [PATCH v3 6/9] virstoragefile: Add quorum in virstoragefile
Add VIR_STORAGE_TYPE_QUORUM in virStorageType. Add VIR_STORAGE_FILE_QUORUM in virStorageFileFormat. Add threshold value in _virStorageSource Signed-off-by: Matthias Gatto matthias.ga...@outscale.com --- docs/formatdomain.html.in | 20 +- docs/schemas/domaincommon.rng | 90 +++--- docs/schemas/storagecommon.rng | 1 + docs/schemas/storagevol.rng| 1 + src/conf/domain_conf.c | 2 + src/qemu/qemu_command.c| 1 + src/qemu/qemu_driver.c | 4 ++ src/qemu/qemu_migration.c | 1 + src/util/virstoragefile.c | 25 src/util/virstoragefile.h | 3 ++ 10 files changed, 106 insertions(+), 42 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 679194f..263baaf 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1835,8 +1835,9 @@ dd Valid values are file, block, dir (span class=sincesince 0.7.5/span), -network (span class=sincesince 0.8.7/span), or -volume (span class=sincesince 1.0.5/span) +network (span class=sincesince 0.8.7/span), +volume (span class=sincesince 1.0.5/span), or +quorum (span class=sincesince 1.2.13/span) and refer to the underlying source for the disk. /dd dtcodedevice/code attribute @@ -1897,6 +1898,14 @@ codesnapshot='yes'/code with a transient disk generally does not make sense. /dd + dtcodethreshold/code attribute + span class=sincesince 1.2.13/span/dt +dd +Only use with a quorum disk. +Indicate the minimum of positive vote a quorum must have to validate +a data to be write. The minimum value is 2. The number of backingStores +contain by the quorum must be superior to the threshold. +/dd /dl /dd dtcodesource/code/dt @@ -1967,6 +1976,11 @@ 'file=/dev/disk/by-path/ip-example.com:3260-iscsi-iqn.2013-07.com.example:iscsi-pool-lun-1'). /p /dd +dtcodetype='quorum'/code +span class=sincesince 1.2.13/span/dt + dd + A quorum contain no source element, but a serie of backingStores instead. + /dd /dl With file, block, and volume, one or more optional sub-elements codeseclabel/code, a href=#seclabeldescribed @@ -2098,6 +2112,8 @@ codebackingStore/code element means the sibling source is self-contained and is not based on any backing store. The following attributes and sub-elements are supported in +span class=sinceSince 1.2.11/span. This elements is used to +describe a quorum child. codebackingStore/code: dl dtcodetype/code attribute/dt diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index d467dce..2556489 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1231,6 +1231,40 @@ /attribute /define + define name='diskDevice' +choice + group +optional + attribute name=device +choice + valuefloppy/value + valuedisk/value + valuecdrom/value +/choice + /attribute +/optional + /group + group +attribute name=device + choice +valuelun/value + /choice +/attribute +optional + ref name=rawIO/ +/optional +optional + attribute name=sgio +choice + valuefiltered/value + valueunfiltered/value +/choice + /attribute +/optional + /group +/choice + /define + !-- A disk description can be either of type file or block The name of the attribute on the source element depends on the type @@ -1238,37 +1272,7 @@ -- define name=disk element name=disk - choice -group - optional -attribute name=device - choice -valuefloppy/value -valuedisk/value -valuecdrom/value - /choice -/attribute - /optional -/group -group - attribute name=device -choice - valuelun/value -/choice - /attribute - optional -ref name=rawIO/ - /optional - optional -attribute name=sgio - choice -valuefiltered/value -valueunfiltered/value - /choice -/attribute - /optional -/group - /choice + ref name=diskDevice/ optional ref name=snapshot/ /optional @@ -1280,9 +1284,15 @@
[libvirt] [PATCH v3 0/9] qemu: Add quorum support to libvirt
The purpose of these patches is to introduce quorum for libvirt I've try to follow this proposal: http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html This feature ask for 6 task: 1) Allow a _virStorageSource to contain more than one backing store. Because all the actual libvirt code use the backingStore field as a pointer and we needs want to change that, I've decide to encapsulate the backingStore field to simplifie the array manipulation. 2) Add the missing field a quorum need in _virStorageSource and the VIR_STORAGE_TYPE_QUORUM and VIR_STORAGE_FILE_QUORUM in their respectives enums. 3) Parse and format the xml Because a quorum allows to have more than one backing store at the same level we need to change virDomainDiskDefFormat and virDomainDiskDefParseXML to call virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse in a loop. virDomainDiskBackingStoreFormat and virDomainDiskBackingStoreParse can call themself recursively in a loop because a quorum can contain another quorum 4) Add nodename We need to add nodename support in _virStorageSource because qemu use them for their child. 5) Build qemu string As for the xml, we have to call the function which create quorum recursively. But this task have the problem explained here: http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html The _virStorageSource missing some informations that can be passed to a child, and therefore this version of quorum is incomplet. 6) Allow to hotplug/change a disk in a quorum This part is not present in these patches because for this task we have to use blockdev-add, and currently libvirt use device_add for hotpluging that doesn't allow to hotplug quorum childs. There is 3 way to handle this problem: 1) create a virDomainBlockDevAdd function in libvirt witch call blockdev-add. 2) use blockdev-add instead of device_add in qemuMonitorJSONAddDevice 3) write a hack which uses blockdev-add when only attaching quorum (but i'm pretty sure this solution is not the good one) V2: -Rebase on master -Add Documentation V3: -Transforme the backingStore field in virStorageSource into an array of pointer instead of a pointer -Modify virStorageSourceSetBackingStore to allow it to expand the backingStore size. Matthias Gatto (9): virstoragefile: Add virStorageSourceGetBackingStore virstoragefile: Always use virStorageSourceGetBackingStore to get backing store virstoragefile: Add virStorageSourceSetBackingStore virstoragefile: Always use virStorageSourceSetBackingStore to set backing store virstoragefile: change backingStore to backingStores. virstoragefile: Add quorum in virstoragefile domain_conf: Read and Write quorum config qemu: Add quorum support in qemuBuildDriveDevStr virstoragefile: Add node-name docs/formatdomain.html.in | 27 - docs/schemas/domaincommon.rng | 96 +++-- docs/schemas/storagecommon.rng| 1 + docs/schemas/storagevol.rng | 1 + src/conf/domain_conf.c| 195 ++ src/conf/storage_conf.c | 23 ++-- src/libvirt_private.syms | 2 + src/qemu/qemu_cgroup.c| 4 +- src/qemu/qemu_command.c | 114 src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_driver.c| 30 +++--- src/qemu/qemu_migration.c | 1 + src/security/security_dac.c | 2 +- src/security/security_selinux.c | 4 +- src/security/virt-aa-helper.c | 2 +- src/storage/storage_backend.c | 33 +++--- src/storage/storage_backend_fs.c | 36 --- src/storage/storage_backend_gluster.c | 10 +- src/storage/storage_backend_logical.c | 15 ++- src/storage/storage_driver.c | 2 +- src/util/virstoragefile.c | 116 +--- src/util/virstoragefile.h | 12 ++- tests/virstoragetest.c| 18 ++-- 23 files changed, 573 insertions(+), 173 deletions(-) -- 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] virstoragefile: Add virStorageSourceSetBackingStore
As explained for virStorageSourceGetBackingStore, quorum allows
multiple backing store, this make the operation to set bs complex
because we have to check if the backingStore is used as an array
or a pointer, and set it differently in both case.
In order to help the manipulation of backing store, I've added a
function virStorageSourceSetBackingStore.
For now virStorageSourceSetBackingStore don't handle the case where
we have more than one backing store in virStorageSource.
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 10 ++
src/util/virstoragefile.h | 3 +++
3 files changed, 14 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 30b7429..c08f6c8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2054,6 +2054,7 @@ virStorageSourceParseRBDColonString;
virStorageSourcePoolDefFree;
virStorageSourcePoolModeTypeFromString;
virStorageSourcePoolModeTypeToString;
+virStorageSourceSetBackingStore;
virStorageTypeFromString;
virStorageTypeToString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index f78f74c..0cc7c5e 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1806,6 +1806,16 @@ virStorageSourceGetBackingStore(const virStorageSource
*src,
}
+bool
+virStorageSourceSetBackingStore(virStorageSourcePtr src,
+virStorageSourcePtr backingStore,
+size_t pos ATTRIBUTE_UNUSED)
+{
+src-backingStore = backingStore;
+return src-backingStore;
+}
+
+
/**
* virStorageSourcePtr:
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index c37ddcf..85b8d6e 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -289,6 +289,9 @@ struct _virStorageSource {
# define DEV_BSIZE 512
# endif
+bool virStorageSourceSetBackingStore(virStorageSourcePtr src,
+virStorageSourcePtr
backingStore,
+size_t pos);
virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource
*src,
size_t pos);
--
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] domain_conf: Read and Write quorum config
Add the capabiltty to libvirt to parse and format the quorum syntax
as described here:
http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
src/conf/domain_conf.c | 164 +++--
1 file changed, 119 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b149065..35ac9f4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5605,20 +5605,56 @@ virDomainDiskSourceParse(xmlNodePtr node,
}
+static bool
+virDomainDiskThresholdParse(virStorageSourcePtr src,
+xmlNodePtr node)
+{
+char *threshold = virXMLPropString(node, threshold);
+int ret;
+
+if (!threshold) {
+virReportError(VIR_ERR_XML_ERROR,
+ %s, _(missing threshold in quorum));
+return false;
+}
+ret = virStrToLong_ul(threshold, NULL, 10, src-threshold);
+if (ret 0 || src-threshold 2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(unexpected threshold %s),
+ threshold must be a decimal number superior to 2
+ and inferior to the number of children);
+VIR_FREE(threshold);
+return false;
+}
+VIR_FREE(threshold);
+return true;
+}
+
+
static int
virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
- virStorageSourcePtr src)
+ virStorageSourcePtr src,
+ xmlNodePtr node,
+ size_t pos)
{
virStorageSourcePtr backingStore = NULL;
xmlNodePtr save_ctxt = ctxt-node;
-xmlNodePtr source;
+xmlNodePtr source = NULL;
char *type = NULL;
char *format = NULL;
int ret = -1;
-if (!(ctxt-node = virXPathNode(./backingStore[*], ctxt))) {
-ret = 0;
-goto cleanup;
+if (src-type == VIR_STORAGE_TYPE_QUORUM) {
+if (!node) {
+ret = 0;
+goto cleanup;
+}
+ctxt-node = node;
+} else {
+if (!(ctxt-node = virXPathNode(./backingStore[*], ctxt))) {
+ret = 0;
+goto cleanup;
+}
}
if (VIR_ALLOC(backingStore) 0)
@@ -5650,6 +5686,25 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
goto cleanup;
}
+if (backingStore-type == VIR_STORAGE_TYPE_QUORUM) {
+xmlNodePtr cur = NULL;
+
+if (!virDomainDiskThresholdParse(backingStore, node))
+goto cleanup;
+
+for (cur = node-children; cur != NULL; cur = cur-next) {
+if (xmlStrEqual(cur-name, BAD_CAST backingStore)) {
+if ((virDomainDiskBackingStoreParse(ctxt,
+backingStore,
+cur,
+
backingStore-nBackingStores) 0)) {
+goto cleanup;
+}
+}
+}
+goto exit;
+}
+
if (!(source = virXPathNode(./source, ctxt))) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(missing disk backing store source));
@@ -5657,10 +5712,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
}
if (virDomainDiskSourceParse(source, ctxt, backingStore) 0 ||
-virDomainDiskBackingStoreParse(ctxt, backingStore) 0)
+virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) 0)
goto cleanup;
-if (!virStorageSourceSetBackingStore(src, backingStore, 0))
+ exit:
+if (!virStorageSourceSetBackingStore(src, backingStore, pos))
goto cleanup;
ret = 0;
@@ -5673,7 +5729,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
return ret;
}
-
#define VENDOR_LEN 8
#define PRODUCT_LEN 16
@@ -6167,6 +6222,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
}
} else if (xmlStrEqual(cur-name, BAD_CAST boot)) {
/* boot is parsed as part of virDomainDeviceInfoParseXML */
+} else if (xmlStrEqual(cur-name, BAD_CAST backingStore)) {
+if (virDomainDiskBackingStoreParse(ctxt, def-src, cur,
+ def-src-nBackingStores)
0)
+goto error;
}
}
cur = cur-next;
@@ -6190,12 +6249,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
def-device = VIR_DOMAIN_DISK_DEVICE_DISK;
}
+if (def-src-type == VIR_STORAGE_TYPE_QUORUM
+!virDomainDiskThresholdParse(def-src, node))
+goto error;
+
+snapshot = virXMLPropString(node, snapshot);
+
/* Only CDROM and Floppy devices are allowed missing source path
* to indicate no media present. LUN is for raw access CD-ROMs
* that are not attached to a physical device
[libvirt] [PATCH v3 1/9] virstoragefile: Add virStorageSourceGetBackingStore
Create virStorageSourceGetBackingStore function in
preparation for quorum:
Actually, if we want to get a backing store inside a virStorageSource
we have to do it manually(src-backingStore = backingStore).
The problem is that with a quorum, a virStorageSource
can contain multiple backing stores, and src-backingStore can
be treated as a virStorageSourcePtr or a virStorageSourcePtrPtr.
Due to these reason, we need to simplify the manipulation of
virStorageSource, and create a function to get the asked
backingStore in a virStorageSource
For now, this function only return the backingStore field
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 8
src/util/virstoragefile.h | 3 +++
3 files changed, 12 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 376c69b..30b7429 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2044,6 +2044,7 @@ virStorageSourceClear;
virStorageSourceCopy;
virStorageSourceFree;
virStorageSourceGetActualType;
+virStorageSourceGetBackingStore;
virStorageSourceGetSecurityLabelDef;
virStorageSourceInitChainElement;
virStorageSourceIsEmpty;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 8d3d1f5..1fc27ef 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1798,6 +1798,14 @@ virStorageSourcePoolDefCopy(const
virStorageSourcePoolDef *src)
}
+virStorageSourcePtr
+virStorageSourceGetBackingStore(const virStorageSource *src,
+size_t pos ATTRIBUTE_UNUSED)
+{
+return src-backingStore;
+}
+
+
/**
* virStorageSourcePtr:
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index b4c3808..c37ddcf 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -289,6 +289,9 @@ struct _virStorageSource {
# define DEV_BSIZE 512
# endif
+virStorageSourcePtr virStorageSourceGetBackingStore(const virStorageSource
*src,
+size_t pos);
+
int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);
int virStorageFileProbeFormatFromBuf(const char *path,
char *buf,
--
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] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store
Replace the parts of the code where a backing store is set manually
with virStorageSourceSetBackingStore
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
src/conf/domain_conf.c| 3 ++-
src/conf/storage_conf.c | 7 +--
src/qemu/qemu_driver.c| 8
src/storage/storage_backend_fs.c | 8 +---
src/storage/storage_backend_gluster.c | 4 ++--
src/storage/storage_backend_logical.c | 9 ++---
src/storage/storage_driver.c | 2 +-
src/util/virstoragefile.c | 8 +---
tests/virstoragetest.c| 4 ++--
9 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 32bf05c..2286665 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5659,7 +5659,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virDomainDiskBackingStoreParse(ctxt, backingStore) 0)
goto cleanup;
-src-backingStore = backingStore;
+if (!virStorageSourceSetBackingStore(src, backingStore, 0))
+goto cleanup;
ret = 0;
cleanup:
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index f4f7e24..a3dac90 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1340,10 +1340,13 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
if ((backingStore = virXPathString(string(./backingStore/path), ctxt))) {
virStorageSourcePtr backingStorePtr;
-if (VIR_ALLOC(ret-target.backingStore) 0)
+if (VIR_ALLOC(backingStorePtr) 0)
goto error;
-backingStorePtr = virStorageSourceGetBackingStore(ret-target, 0);
+if (!virStorageSourceSetBackingStore(ret-target, backingStorePtr,
0)) {
+VIR_FREE(backingStorePtr);
+goto error;
+}
backingStorePtr-path = backingStore;
backingStore = NULL;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8c65fd3..bdca5e0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13590,12 +13590,12 @@
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
/* Update vm in place to match changes. */
need_unlink = false;
-newDiskSrc-backingStore = disk-src;
+virStorageSourceSetBackingStore(newDiskSrc, disk-src, 0);
disk-src = newDiskSrc;
newDiskSrc = NULL;
if (persistDisk) {
-persistDiskSrc-backingStore = persistDisk-src;
+virStorageSourceSetBackingStore(persistDiskSrc, persistDisk-src, 0);
persistDisk-src = persistDiskSrc;
persistDiskSrc = NULL;
}
@@ -13639,13 +13639,13 @@
qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
/* Update vm in place to match changes. */
tmp = disk-src;
disk-src = virStorageSourceGetBackingStore(tmp, 0);
-tmp-backingStore = NULL;
+virStorageSourceSetBackingStore(tmp, NULL, 0);
virStorageSourceFree(tmp);
if (persistDisk) {
tmp = persistDisk-src;
persistDisk-src = virStorageSourceGetBackingStore(tmp, 0);
-tmp-backingStore = NULL;
+virStorageSourceSetBackingStore(tmp, NULL, 0);
virStorageSourceFree(tmp);
}
}
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 445d11d..18e19fe 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -97,10 +97,12 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
goto cleanup;
if (meta-backingStoreRaw) {
-backingStore = virStorageSourceGetBackingStore(target, 0);
-if (!(backingStore = virStorageSourceNewFromBacking(meta)))
+if (!virStorageSourceSetBackingStore(target,
+
virStorageSourceNewFromBacking(meta),
+ 0))
goto cleanup;
+backingStore = virStorageSourceGetBackingStore(target, 0);
backingStore-format = backingStoreFormat;
/* XXX: Remote storage doesn't play nicely with volumes backed by
@@ -112,7 +114,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
if (VIR_ALLOC(backingStore) 0)
goto cleanup;
-target-backingStore = backingStore;
+virStorageSourceSetBackingStore(target, backingStore, 0);
backingStore-type = VIR_STORAGE_TYPE_NETWORK;
backingStore-path = meta-backingStoreRaw;
meta-backingStoreRaw = NULL;
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index fc48f0f..bb79024 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -301,9 +301,9 @@
virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
goto cleanup;
if (meta-backingStoreRaw) {
-if (VIR_ALLOC(vol-target.backingStore) 0)
+if (VIR_ALLOC(backingStore)
[libvirt] [PATCH] Fix qemu job handling in SetSchedulerParameters
Commit c5ee5cf added a job to SetSchedulerParameters, but forgot to change one label in the SCHED_RANGE_CHECK macro. --- Not yet released. 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 1614340..e3ca437 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9740,7 +9740,7 @@ qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, _(value of '%s' is out of range [%lld, %lld]), \ NAME, MIN, MAX); \ rc = -1;\ -goto cleanup; \ +goto endjob;\ } static int -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/9] virstoragefile: Always use virStorageSourceGetBackingStore to get backing store
Uniformize backing store usage by calling virStorageSourceGetBackingStore
instead of setting backing store manually.
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
src/conf/domain_conf.c| 7 ---
src/conf/storage_conf.c | 18 ++
src/qemu/qemu_cgroup.c| 4 ++--
src/qemu/qemu_domain.c| 2 +-
src/qemu/qemu_driver.c| 18 ++
src/security/security_dac.c | 2 +-
src/security/security_selinux.c | 4 ++--
src/security/virt-aa-helper.c | 2 +-
src/storage/storage_backend.c | 33 ++---
src/storage/storage_backend_fs.c | 33 ++---
src/storage/storage_backend_gluster.c | 8 +---
src/storage/storage_backend_logical.c | 10 ++
src/util/virstoragefile.c | 20 ++--
tests/virstoragetest.c| 14 +++---
14 files changed, 95 insertions(+), 80 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d5b15db..32bf05c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16698,7 +16698,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
/* We currently don't output seclabels for backing chain element */
if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) 0 ||
virDomainDiskBackingStoreFormat(buf,
-backingStore-backingStore,
+
virStorageSourceGetBackingStore(backingStore, 0),
backingStore-backingStoreRaw,
idx + 1) 0)
return -1;
@@ -16820,7 +16820,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
/* Don't format backingStore to inactive XMLs until the code for
* persistent storage of backing chains is ready. */
if (!(flags VIR_DOMAIN_DEF_FORMAT_INACTIVE)
-virDomainDiskBackingStoreFormat(buf, def-src-backingStore,
+virDomainDiskBackingStoreFormat(buf,
+
virStorageSourceGetBackingStore(def-src, 0),
def-src-backingStoreRaw, 1) 0)
return -1;
@@ -20850,7 +20851,7 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
}
}
-for (tmp = disk-src; tmp; tmp = tmp-backingStore) {
+for (tmp = disk-src; tmp; tmp = virStorageSourceGetBackingStore(tmp, 0)) {
int actualType = virStorageSourceGetActualType(tmp);
/* execute the callback only for local storage */
if (actualType != VIR_STORAGE_TYPE_NETWORK
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index e9aaa8a..f4f7e24 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1339,20 +1339,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
}
if ((backingStore = virXPathString(string(./backingStore/path), ctxt))) {
+virStorageSourcePtr backingStorePtr;
if (VIR_ALLOC(ret-target.backingStore) 0)
goto error;
-ret-target.backingStore-path = backingStore;
+backingStorePtr = virStorageSourceGetBackingStore(ret-target, 0);
+backingStorePtr-path = backingStore;
backingStore = NULL;
if (options-formatFromString) {
char *format =
virXPathString(string(./backingStore/format/@type), ctxt);
if (format == NULL)
-ret-target.backingStore-format = options-defaultFormat;
+backingStorePtr-format = options-defaultFormat;
else
-ret-target.backingStore-format =
(options-formatFromString)(format);
+backingStorePtr-format = (options-formatFromString)(format);
-if (ret-target.backingStore-format 0) {
+if (backingStorePtr-format 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(unknown volume format type %s), format);
VIR_FREE(format);
@@ -1361,9 +1363,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
VIR_FREE(format);
}
-if (VIR_ALLOC(ret-target.backingStore-perms) 0)
+if (VIR_ALLOC(backingStorePtr-perms) 0)
goto error;
-if (virStorageDefParsePerms(ctxt, ret-target.backingStore-perms,
+if (virStorageDefParsePerms(ctxt, backingStorePtr-perms,
./backingStore/permissions,
DEFAULT_VOL_PERM_MODE) 0)
goto error;
@@ -1641,9 +1643,9 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool,
def-target, target) 0)
goto cleanup;
-if (def-target.backingStore
+if (virStorageSourceGetBackingStore((def-target), 0)
virStorageVolTargetDefFormat(options, buf,
-
Re: [libvirt] [PATCH] Fix qemu job handling in SetSchedulerParameters
On Tue, Feb 10, 2015 at 14:41:49 +0100, Ján Tomko wrote: Commit c5ee5cf added a job to SetSchedulerParameters, but forgot to change one label in the SCHED_RANGE_CHECK macro. --- Not yet released. 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 1614340..e3ca437 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9740,7 +9740,7 @@ qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, _(value of '%s' is out of range [%lld, %lld]), \ NAME, MIN, MAX); \ rc = -1;\ -goto cleanup; \ +goto endjob;\ } static int ACK, I've already installed my bunny ears of shame. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 9/9] virstoragefile: Add node-name
Add nodename inside virstoragefile
During xml backingStore parsing, look for a nodename attribute in the disk
declaration if this one is a quorum, if a nodename is found, add it to
the virStorageSource otherwise create a new one with a random name.
Take inspiration from this patch to create the nodename:
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03209.html
Durring xml backingStore formating, look for a nodename attribute inside the
virStorageSource struct, and add it to the disk element.
Use the nodename to create the quorum in qemuBuildQuorumStr.
Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
docs/formatdomain.html.in | 7 +++
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c| 27 +++
src/qemu/qemu_command.c | 3 +++
src/util/virstoragefile.c | 4
src/util/virstoragefile.h | 1 +
6 files changed, 47 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 263baaf..6429e86 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2131,6 +2131,13 @@
codevda[2]/code refers to the backing store with
codeindex='2'/code of the disk with codevda/code target.
/dd
+ dtcodenodename/code attribute
+ span class=sincesince 1.2.13/span/dt
+ dd
+When the backing store is a quorum child, we can use this attribute
+to define the node-name of a child. If this atribute is undefine,
+a random nodename is generate.
+ /dd
dtcodeformat/code sub-element/dt
dd
The codeformat/code element contains codetype/code
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2556489..a1c5a39 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1304,6 +1304,11 @@
/attribute
interleave
optional
+ attribute name=nodename
+text/
+ /attribute
+/optional
+optional
ref name=diskDevice/
/optional
ref name=diskSource/
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35ac9f4..e3ac5fb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -56,6 +56,7 @@
#include network_conf.h
#include virtpm.h
#include virstring.h
+#include virrandom.h
#define VIR_FROM_THIS VIR_FROM_DOMAIN
@@ -5631,6 +5632,8 @@ virDomainDiskThresholdParse(virStorageSourcePtr src,
}
+#define GEN_NODE_NAME_PREFIXlibvirt
+#define GEN_NODE_NAME_MAX_LEN (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)
static int
virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virStorageSourcePtr src,
@@ -5673,6 +5676,26 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
goto cleanup;
}
+if (src-type == VIR_STORAGE_TYPE_QUORUM) {
+char *nodeName = NULL;
+
+if ((nodeName = virXMLPropString(ctxt-node, nodename))) {
+backingStore-nodeName = nodeName;
+} else {
+size_t len;
+
+if (VIR_ALLOC_N(nodeName, GEN_NODE_NAME_MAX_LEN) 0)
+goto cleanup;
+if (snprintf(nodeName, GEN_NODE_NAME_MAX_LEN,
+ %s%08x, GEN_NODE_NAME_PREFIX, (int)pos) 0)
+goto cleanup;
+for (len = strlen(nodeName); len GEN_NODE_NAME_MAX_LEN - 1;
++len)
+nodeName[len] = virRandomInt('Z' - 'A') + 'A';
+nodeName[GEN_NODE_NAME_MAX_LEN - 1] = '\0';
+backingStore-nodeName = nodeName;
+}
+}
+
if (!(format = virXPathString(string(./format/@type), ctxt))) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(missing disk backing store format));
@@ -5728,6 +5751,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
ctxt-node = save_ctxt;
return ret;
}
+#undef GEN_NODE_NAME_PREFIX
+#undef GEN_NODE_NAME_MAX_LEN
#define VENDOR_LEN 8
#define PRODUCT_LEN 16
@@ -16764,6 +16789,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
type, idx);
if (backingStore-threshold)
virBufferAsprintf(buf, threshold='%lu',
backingStore-threshold);
+if (backingStore-nodeName)
+virBufferAsprintf(buf, nodename='%s', backingStore-nodeName);
virBufferAddLit(buf, \n);
virBufferAdjustIndent(buf, 2);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index acbff19..de971c8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3435,6 +3435,9 @@ qemuBuildQuorumStr(virConnectPtr conn,
toAppend, i,
virStorageFileFormatTypeToString(backingStore-format));
+virBufferAsprintf(opt, ,%schildren.%lu.node-name=%s,
+ toAppend, i, backingStore-nodeName);
+
if
Re: [libvirt] [PATCH] qemu: do upfront check for vcpupids being null when querying pinning
Quoting Serge Hallyn (serge.hal...@ubuntu.com): Quoting Daniel P. Berrange (berra...@redhat.com): The qemuDomainHelperGetVcpus attempted to report an error when the vcpupids info was NULL. Unfortunately earlier code would clamp the value of 'maxinfo' to 0 when nvcpupids was 0, so the error reporting would end up being skipped. This lead to 'virsh vcpuinfo dom' just returning an empty list instead of giving the user a clear error. --- Cool, thanks. (haven't kicked off a test yet but it looks correct :) Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com and Tested-by: Serge E. Hallyn serge.hal...@ubuntu.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix setting of VM CPU affinity with TCG
Quoting Daniel P. Berrange (berra...@redhat.com):
If a previous commit I fixed the incorrect handling of vcpu pids
for TCG mode QEMU:
commit b07f3d821dfb11a118ee75ea275fd6ab737d9500
Author: Daniel P. Berrange berra...@redhat.com
Date: Thu Dec 18 16:34:39 2014 +
Don't setup fake CPU pids for old QEMU
The code assumes that def-vcpus == nvcpupids, so when we setup
fake CPU pids for old QEMU with nvcpupids == 1, we cause the
later code to read off the end of the array. This has fun results
like sche_setaffinity(0, ...) which changes libvirtd's own CPU
affinity, or even better sched_setaffinity($RANDOM, ...) which
changes the affinity of a random OS process.
The intent was that this would merely disable the ability to set
per-vCPU affinity. It should still have been possible to set VM
level host CPU affinity.
Unfortunately, when you set vcpu cpuset='0-1'4/vcpu, the XML
parser will internally take this initialize an entry in the
def-cputune.vcpupin array for every VCPU. IOW this is implicitly
being treated as
cputune
vcpupin cpuset='0-1' vcpu='0'/
vcpupin cpuset='0-1' vcpu='1'/
vcpupin cpuset='0-1' vcpu='2'/
vcpupin cpuset='0-1' vcpu='3'/
/cputune
Even more fun, the faked cputune elements are hidden from view when
querying the live XML, because their cpuset mask is the same as the
VM default cpumask.
The upshot was that it was impossible to set VM level CPU affinity.
To fix this we must update qemuProcessSetVcpuAffinities so that it
only reports a fatal error if the per-VCPU cpu mask is different
from the VM level cpu mask.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
Thanks, confirmed this let's me place qemu in a cpuset. (Noting that
'virsh edit' always ends up squashing the cputune section when the
cpusets are identical, but that should all be reasonable)
Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com
Tested-by: Serge E. Hallyn serge.hal...@ubuntu.com
---
src/qemu/qemu_process.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d5df60d..e8c532f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2497,9 +2497,18 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm)
return 0;
if (priv-vcpupids == NULL) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- %s, _(cpu affinity is not supported));
-return -1;
+/* If any CPU has custom affinity that differs from the
+ * VM default affinity, we must reject it
+ */
+for (n = 0; n def-vcpus; n++) {
+if (!virBitmapEqual(def-cpumask,
+def-cputune.vcpupin[n]-cpumask)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(cpu affinity is not supported));
+return -1;
+}
+}
+return 0;
}
for (n = 0; n def-vcpus; n++) {
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: fix show the wrong IP address for network type listen address graphic
On 02/10/2015 04:35 AM, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1191016
We try to get the IP address in /domain/devices/graphics/@listen, howerver
for the network type listen address donnot have this parameter, it will
show the address in the /domain/devices/graphics/listen/@address, running
XML like this:
graphics type='spice' port='5901' autoport='yes' keymap='en-us'
listen type='network' address='192.168.122.1' network='default'/
/graphics
This patch will try to get the IP address in this path
/domain/devices/graphics/listen/@address
That will work when the libvirtd being connected to is 0.9.4 or later,
but earlier versions of libvirt don't have the listen subelement;
instead they just have a 'listen' attribute directly inside graphics
that contains the address. All newer versions of libvirt are supposed to
populate that from listen[0] for backward compatibility.
The real bug here is that the listen attribute in graphics isn't being
filled in in the case of type='network' when the domain is active. On
the other hand, fixing the problem there would leave it unfixed for
cases where the client is a new libvirt but the server is running
libvirt between 0.9.4 and 1.2.12. So I think what is needed is for your
patch to check @listen, and if nothing is found there, *then* check
listen/@address. I attached a patch to this mail that I propose
squashing into your patch before pushing. Let me know if it behaves
properly and looks correct.
Beyond that, the server side should still be fixed. I just sent a patch
that does that:
https://www.redhat.com/archives/libvir-list/2015-February/msg00332.html
Between the two patches, we will have fixed the problem for all versions
of server, as long as the client is new enough.
From 5f8800d93375ee67dd94bf50a87ed75f44ca2b19 Mon Sep 17 00:00:00 2001
From: Laine Stump la...@laine.org
Date: Tue, 10 Feb 2015 13:32:33 -0500
Subject: [PATCH] virsh: check listen attribute *and* listen subelement in
domdisplay
This should be squashed into Luyao Huang's patch fixing the output of
virsh domdisplay. Without it, virsh would fail when connecting to a
libvirtd older than 0.9.4.
---
tools/virsh-domain.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4a9b574..20bafbe 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1,7 +1,7 @@
/*
* virsh-domain.c: Commands to manage domain
*
- * Copyright (C) 2005, 2007-2014 Red Hat, Inc.
+ * Copyright (C) 2005, 2007-2015 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -10081,7 +10081,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
continue;
/* Create our XPATH lookup for the current display's address */
-if (virAsprintf(xpath, xpath_fmt, scheme[iter], listen/@address) 0)
+if (virAsprintf(xpath, xpath_fmt, scheme[iter], @listen) 0)
goto cleanup;
/* Attempt to get the listening addr if set for the current
@@ -10089,6 +10089,22 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
listen_addr = virXPathString(xpath, ctxt);
VIR_FREE(xpath);
+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)
+ */
+if (virAsprintf(xpath, xpath_fmt, scheme[iter], listen/@address) 0)
+goto cleanup;
+
+listen_addr = virXPathString(xpath, ctxt);
+VIR_FREE(xpath);
+}
+
/* We can query this info for all the graphics types since we'll
* get nothing for the unsupported ones (just rdp for now).
* Also the parameter '--include-password' was already taken
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] domain: backfill listen address to parent graphics listen attribute
Prior to 0.9.4, libvirt only supported a single listen, and it had to
be an IP address:
graphics listen='1.2.3.4' /
Starting with 0.9.4, a graphics element could have a listen
subelement (actually the grammar supports multiples, but all of the
drivers only support a single listen per graphics), and that
listen element can be of type='address' or type='network'. For
type='address', listen also has an attribute called 'address' which
contains the IP address for listening:
graphics
listen type='address' address='1.2.3.4' .../
/graphics
type can also be network, and in that case listen will have a
network attribute which will contain the name of a libvirt
network:
graphics
listen type='network' network='testnet' .../
/graphics
At domain start (or migrate) time, libvirt will attempt to
find an IP address associated with that network (e.g. the IP address
of the bridge device used by the network, or the physical device
listed in forward dev='physdev'/) and fill in that address in the
status XML:
graphics
listen type='network' network='testnet' address='1.2.3.4' .../
/graphics
In the case that a graphics element has a listen subelement of
type='address', that listen subelement's address attribute is
backfilled into the parent graphics element's listen *attribute* for
backward compatibility (so that a management application unaware of
the separate listen element can still learn the listen
address). This backfill should be done with the IP learned from
type='network' as well, and that's what this patch does:
graphics listen='1.2.3.4'
listen type='network' network='testnet' address='1.2.3.4' .../
/graphics
This is a continuation of the fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=1191016
---
src/conf/domain_conf.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6a57d80..6614fb1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18906,17 +18906,23 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
return -1;
}
-/* find the first listen element of type='address' and duplicate
-* its address attribute as the listen attribute of
-* graphics. This is done to improve backward compatibility. */
+/* find the first listen subelement with a valid address and
+* duplicate its address attribute as the listen attribute of
+* graphics. This is done to improve backward compatibility.
+*/
for (i = 0; i def-nListens; i++) {
-if (virDomainGraphicsListenGetType(def, i)
-== VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) {
-if (flags VIR_DOMAIN_DEF_FORMAT_MIGRATABLE
-def-listens[i].fromConfig)
-continue;
-listenAddr = virDomainGraphicsListenGetAddress(def, i);
-break;
+virDomainGraphicsListenType listenType;
+
+if (flags VIR_DOMAIN_DEF_FORMAT_MIGRATABLE
+def-listens[i].fromConfig)
+continue;
+listenType = virDomainGraphicsListenGetType(def, i);
+
+if (listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS ||
+(listenType == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK
+ !(flags VIR_DOMAIN_DEF_FORMAT_INACTIVE))) {
+if ((listenAddr = virDomainGraphicsListenGetAddress(def, i)))
+break;
}
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Two fixes for graphics listen
These area a followup to Luyao Huang's patch to fix incorrect reporting from virsh domdisplay: https://www.redhat.com/archives/libvir-list/2015-February/msg00298.html While his patch works when the remote libvirtd is 0.9.4 or greater, it won't work when talking to older libvirt. Also the code as written isn't what was intended. Patch 1 takes care of that. Patch 2 fixes two theoretical memory leaks (which never happen in real life, but could some day if the functions are used differently). Laine Stump (2): domain: backfill listen address to parent graphics listen attribute domain: avoid potential memory leak in virDomainGraphicsListenSet*() src/conf/domain_conf.c | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] domain: avoid potential memory leak in virDomainGraphicsListenSet*()
virDomainGraphicsListenSetAddress() and
virDomainGraphicsListenSetNetwork() both set their respective char* to
NULL directly when asked to set it to NULL, which is okay as long as
it's already set to NULL. If these functions are ever called to clear
a listen object that has a valid string in address or network, it will
end up leaking the old value. Currently that doesn't happen, so this
is just a preemptive strike.
---
src/conf/domain_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6614fb1..8600eff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21440,7 +21440,7 @@
virDomainGraphicsListenSetAddress(virDomainGraphicsDefPtr def,
listenInfo-type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS;
if (!address) {
-listenInfo-address = NULL;
+VIR_FREE(listenInfo-address);
return 0;
}
@@ -21478,7 +21478,7 @@
virDomainGraphicsListenSetNetwork(virDomainGraphicsDefPtr def,
listenInfo-type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK;
if (!network) {
-listenInfo-network = NULL;
+VIR_FREE(listenInfo-network);
return 0;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix crash in migrate when migrateuri do not have a scheme
https://bugzilla.redhat.com/show_bug.cgi?id=1191355
When we migrate a vm with migrateuri option with a uri do not
have scheme like this:
# virsh migrate test4 --live qemu+ssh://lhuang/system --migrateuri 127.0.0.1
target libvirtd will crashed because uri-scheme is NULL in
qemuMigrationPrepareDirect this line:
if (STRNEQ(uri-scheme, tcp)
add a value check before this line.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_migration.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 879b1bf..5c3b73e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3281,6 +3281,13 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
if (!(uri = qemuMigrationParseURI(uri_in, well_formed_uri)))
goto cleanup;
+if (uri-scheme == NULL) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(missing scheme in migration URI: %s),
+ uri_in);
+goto cleanup;
+}
+
if (STRNEQ(uri-scheme, tcp)
STRNEQ(uri-scheme, rdma)) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
--
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 crash in migrate when migrateuri do not have a scheme
On 02/11/2015 03:41 PM, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1191355
When we migrate a vm with migrateuri option with a uri do not
have scheme like this:
# virsh migrate test4 --live qemu+ssh://lhuang/system --migrateuri 127.0.0.1
target libvirtd will crashed because uri-scheme is NULL in
qemuMigrationPrepareDirect this line:
if (STRNEQ(uri-scheme, tcp)
add a value check before this line.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_migration.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 879b1bf..5c3b73e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3281,6 +3281,13 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
if (!(uri = qemuMigrationParseURI(uri_in, well_formed_uri)))
goto cleanup;
+if (uri-scheme == NULL) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(missing scheme in migration URI: %s),
+ uri_in);
+goto cleanup;
+}
+
if (STRNEQ(uri-scheme, tcp)
STRNEQ(uri-scheme, rdma)) {
Why not just use STRNEQ_NULLABLE instead of STRNEQ directly?
# git diff
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 879b1bf..baca2ed 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3281,8 +3281,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
if (!(uri = qemuMigrationParseURI(uri_in, well_formed_uri)))
goto cleanup;
-if (STRNEQ(uri-scheme, tcp)
-STRNEQ(uri-scheme, rdma)) {
+if (STRNEQ_NULLABLE(uri-scheme, tcp)
+STRNEQ_NULLABLE(uri-scheme, rdma)) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
--
Regards
shyu
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: rbd: Improve the error when start a pool based on non-exist rados object
On Wed, Feb 11, 2015 at 11:02:36AM +0800, Shanzhi Yu wrote:
On 02/06/2015 08:55 PM, Ján Tomko wrote:
On Fri, Feb 06, 2015 at 07:45:37PM +0800, Shanzhi Yu wrote:
When start/create a pool based on non-exist rados object, the error will
be like
$virsh pool-start p-c
error: Failed to start pool p-c
error: failed to create the RBD IoCTX. Does the pool 'libvirt-pool-clone'
exist?: No such file or directory
update it to
error: Failed to start pool p-c
error: internal error: failed to create the RBD IoCTX. the rados pool
'libvirt-pool-clone' is not available
This message is missing the actual error: 'no such file or directory'
returned by rados_ioctx_create. This return value has been added
to the error message by commit 761491eb
Indeed. Just catch that commit
Maybe we could just drop the hint about the pool existence?
error: failed to create the RBD IoCTX: No such file or directory
Would it be better to wrap the error returned by librados? Returning a
libvirt error will be more friendly to a libvirt user
I meant:
virReportSystemError(-r, %s, _(failed to create the RBD IoCTX));
How do you want to wrap it?
Jan
Jan
Signed-off-by: Shanzhi Yu s...@redhat.com
---
src/storage/storage_backend_rbd.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index 57182de..98e7fe7 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -236,8 +236,10 @@ static int
virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virSt
{
int r = rados_ioctx_create(ptr-cluster, pool-def-source.name,
ptr-ioctx);
if (r 0) {
-virReportSystemError(-r, _(failed to create the RBD IoCTX. Does
the pool '%s' exist?),
- pool-def-source.name);
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(failed to create the RBD IoCTX.
+ the rados pool '%s' is not available),
+ pool-def-source.name);
}
return r;
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Regards
shyu
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] util: Add virProcessSetScheduler() function for scheduler settings
This function uses sched_setscheduler() function so it works with
processes and threads as well (even threads not created by us, which is
what we'll need in the future).
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
configure.ac | 4 +-
src/libvirt_private.syms | 1 +
src/util/virprocess.c| 104 ++-
src/util/virprocess.h| 20 -
4 files changed, 124 insertions(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac
index 99a2283..b3e99e7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,6 +1,6 @@
dnl Process this file with autoconf to produce a configure script.
-dnl Copyright (C) 2005-2014 Red Hat, Inc.
+dnl Copyright (C) 2005-2015 Red Hat, Inc.
dnl
dnl This library is free software; you can redistribute it and/or
dnl modify it under the terms of the GNU Lesser General Public
@@ -275,7 +275,7 @@ dnl and various less common threadsafe functions
AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \
getmntent_r getpwuid_r getuid kill mmap newlocale posix_fallocate \
posix_memalign prlimit regexec sched_getaffinity setgroups setns \
- setrlimit symlink sysctlbyname getifaddrs])
+ setrlimit symlink sysctlbyname getifaddrs sched_setscheduler])
dnl Availability of pthread functions. Because of $LIB_PTHREAD, we
dnl cannot use AC_CHECK_FUNCS_ONCE. LIB_PTHREAD and LIBMULTITHREAD
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 376c69b..79fc14f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1915,6 +1915,7 @@ virProcessSetMaxFiles;
virProcessSetMaxMemLock;
virProcessSetMaxProcesses;
virProcessSetNamespaces;
+virProcessSetScheduler;
virProcessTranslateStatus;
virProcessWait;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index d0a1500..c030d74 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1,7 +1,7 @@
/*
* virprocess.c: interaction with processes
*
- * Copyright (C) 2010-2014 Red Hat, Inc.
+ * Copyright (C) 2010-2015 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -32,7 +32,9 @@
# include sys/time.h
# include sys/resource.h
#endif
-#include sched.h
+#if HAVE_SCHED_SETSCHEDULER
+# include sched.h
+#endif
#if defined(__FreeBSD__) || HAVE_BSD_CPU_AFFINITY
# include sys/param.h
@@ -104,6 +106,13 @@ static inline int setns(int fd ATTRIBUTE_UNUSED, int
nstype ATTRIBUTE_UNUSED)
}
#endif
+VIR_ENUM_IMPL(virProcessSchedPolicy, VIR_PROC_POLICY_LAST,
+ none,
+ batch,
+ idle,
+ fifo,
+ rr);
+
/**
* virProcessTranslateStatus:
* @status: child exit status to translate
@@ -1052,3 +1061,94 @@ virProcessExitWithStatus(int status)
}
exit(value);
}
+
+#if HAVE_SCHED_SETSCHEDULER
+
+static int
+virProcessSchedTranslatePolicy(virProcessSchedPolicy policy)
+{
+switch (policy) {
+case VIR_PROC_POLICY_NONE:
+return SCHED_OTHER;
+
+case VIR_PROC_POLICY_BATCH:
+return SCHED_BATCH;
+
+case VIR_PROC_POLICY_IDLE:
+return SCHED_IDLE;
+
+case VIR_PROC_POLICY_FIFO:
+return SCHED_FIFO;
+
+case VIR_PROC_POLICY_RR:
+return SCHED_RR;
+
+case VIR_PROC_POLICY_LAST:
+/* nada */
+break;
+}
+
+return -1;
+}
+
+int
+virProcessSetScheduler(pid_t pid, virProcessSchedPolicy policy, int priority)
+{
+struct sched_param param = {0};
+int pol = virProcessSchedTranslatePolicy(policy);
+
+VIR_DEBUG(pid=%d, policy=%d, priority=%u, pid, policy, priority);
+
+if (!policy)
+return 0;
+
+if (pol == SCHED_FIFO || pol == SCHED_RR) {
+int min = 0;
+int max = 0;
+
+if ((min = sched_get_priority_min(pol)) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot get minimum scheduler
+ priority value));
+return -1;
+}
+
+if ((max = sched_get_priority_max(pol)) 0) {
+virReportSystemError(errno, %s,
+ _(Cannot get maximum scheduler
+ priority value));
+return -1;
+}
+
+if (priority min || priority max) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(Scheduler priority %d out of range [%d, %d]),
+ priority, min, max);
+return -1;
+}
+
+param.sched_priority = priority;
+}
+
+if (sched_setscheduler(pid, pol, param) 0) {
+virReportSystemError(errno,
+ _(Cannot set scheduler parameters for pid %d),
+ pid);
+return -1;
+}
+
+return 0;
+}
+
+#else /* ! HAVE_SCHED_SETSCHEDULER */
+
+int
+virProcessSetScheduler(pid_t pid, int policy, int priority)
+{
+
Re: [libvirt] [PATCH 00/12] Implement random number generator hot/cold (un)plug
On 02/10/2015 08:10 PM, Peter Krempa wrote: On Mon, Feb 09, 2015 at 16:58:07 +0100, Ján Tomko wrote: On Fri, Feb 06, 2015 at 04:32:14PM +0100, Peter Krempa wrote: Extension of Luyao's series with changes that were possible due to the prepare series for memory hotplug. ACK series, see my replies to individual patches for some minor nits. I've fixed all of them and pushed the series after a final test. Thanks for the review. Thanks Peter's help, refactor and new patches :) Also thanks a lot for Jan's review :) Jan Peter Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
