[libvirt] [PATCHv3] util: make it more robust to calculate timeout value
From: Zhang Bo oscar.zhan...@huawei.com
When we change system clock to years ago, a certain CPU may use up 100% cputime.
The reason is that in function virEventPollCalculateTimeout(), we assign the
unsigned long long result to an INT variable,
*timeout = then - now; // timeout is INT, and then/now are long long
if (*timeout 0)
*timeout = 0;
there's a chance that variable @then minus variable @now may be a very large
number
that overflows INT value expression, then *timeout will be negative and be
assigned to 0.
Next the 'poll' in function virEventPollRunOnce() will get into an 'endless'
while loop there.
thus, the cpu that virEventPollRunOnce() thread runs on will go up to 100%.
Although as we discussed before in
https://www.redhat.com/archives/libvir-list/2015-May/msg00400.html
it should be prohibited to set-time while other applications are running, but
it does
seems to have no harm to make the codes more robust.
Signed-off-by: Wang Yufei james.wangyu...@huawei.com
Signed-off-by: Zhang Bo oscar.zhan...@huawei.com
---
src/util/vireventpoll.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index ffda206..4e4f407 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -357,9 +357,10 @@ static int virEventPollCalculateTimeout(int *timeout)
return -1;
EVENT_DEBUG(Schedule timeout then=%llu now=%llu, then, now);
-*timeout = then - now;
-if (*timeout 0)
+if (then = now)
*timeout = 0;
+else
+*timeout = ((then-now) INT_MAX) ? INT_MAX : (then-now);
} else {
*timeout = -1;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: Deal with panic device on pSeries.
On Fri, 2015-05-15 at 11:35 +0200, Andrea Bolognani wrote: The guest firmware provides the same functionality as the pvpanic device, which is not available in QEMU on pSeries: make sure the XML reflects this fact by automatically adding a panic/ element when not already present. On the other hand, unlike the pvpanic device, the guest firmware can't be configured, so report an error if an address has been provided in the XML. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1182388 Ping :) -- Andrea Bolognani abolo...@redhat.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] threshold: new API virDomainBlockSetWriteThreshold
On Sat, May 23, 2015 at 21:45:56 -0600, Eric Blake wrote:
qemu 2.3 added a new QMP command block-set-write-threshold,
which allows callers to get an interrupt when a file hits a
write threshold, rather than the current approach of repeatedly
polling for file allocation. This patch prepares the API for
callers to register to receive the event, as well as a way
to query the threshold.
The event is one-shot in qemu - a guest must re-register a new
threshold after each time it triggers. However, the
virConnectDomainEventRegisterAny() call does not allow
parameterization, so callers must use a pair of APIs - one
to register the callback (one-time call), and another to
repeatedly set the desired threshold (repeated each time a
threshold changes).
Note that the threshold is registered as a double, but reported
as an unsigned long long. This is intentional, as it allows
the use of a flag for registering a threshold via a percentage,
but once registered, the threshold is normalized according to
the current size of the disk.
To make the patch series more digestible, this patch
intentionally omits remote support, by using a couple of
placeholders at a point where the compiler forces the addition
of a case within a switch statement.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Posting now to get feedback on the proposed API, before the actual
implementation is complete.
daemon/remote.c | 2 +
include/libvirt/libvirt-domain.h | 48 +++
src/conf/domain_event.c | 4 +-
src/driver-hypervisor.h | 7 +++
src/libvirt-domain.c | 101
+++
src/libvirt_public.syms | 1 +
tools/virsh-domain.c | 23 +
tools/virsh.pod | 1 +
8 files changed, 186 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index e259a76..51d7de5 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1102,6 +1102,8 @@ static virConnectDomainEventGenericCallback
domainEventCallbacks[] = {
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTunable),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventAgentLifecycle),
VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventDeviceAdded),
+/* TODO: Implement RPC support for this */
+VIR_DOMAIN_EVENT_CALLBACK(NULL),
};
verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST);
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index d851225..0c4fd62 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -515,6 +515,15 @@ typedef virDomainBlockStatsStruct
*virDomainBlockStatsPtr;
# define VIR_DOMAIN_BLOCK_STATS_ERRS errs
/**
+ * VIR_DOMAIN_BLOCK_STATS_WRITE_THRESHOLD:
+ *
+ * Macro represents the typed parameter, as an llong, that reports
Signed? The rest of the block stats fields is signed for a reason, is
there a reason why it's here?
+ * the threshold in bytes at which the block device will trigger a
+ * VIR_DOMAIN_EVENT_ID_WRITE_THRESHOLD, or 0 if no threshold is registered.
In case it remains signed, the negative values should be somehow
described.
+ */
+# define VIR_DOMAIN_BLOCK_STATS_WRITE_THRESHOLD write-threshold
+
+/**
* virDomainInterfaceStats:
*
* Network interface stats for virDomainInterfaceStats.
@@ -1297,6 +1306,16 @@ int virDomainBlockStatsFlags
(virDomainPtr dom,
virTypedParameterPtr
params,
int *nparams,
unsigned int flags);
+
+typedef enum {
+/* threshold is a percentage rather than byte limit */
+VIR_DOMAIN_BLOCK_SET_WRITE_THRESHOLD_PERCENTAGE = (1 0),
+} virDomainBlockSetWriteThresholdFlags;
+int virDomainBlockSetWriteThreshold(virDomainPtr dom,
+const char *disk,
+double threshold,
Double? Your last proposal used unsigned long long. Using double with
byte sizes seems a bit odd to me. The file size won't exceed unsigned
long long for a while so I don't see a reason to use double here.
+unsigned int flags);
+
int virDomainInterfaceStats (virDomainPtr dom,
const char *path,
virDomainInterfaceStatsPtr
stats,
@@ -3246,6 +3265,34 @@ typedef void
(*virConnectDomainEventDeviceAddedCallback)(virConnectPtr conn,
void *opaque);
/**
+ * virConnectDomainEventWriteThresholdCallback:
+ * @conn: connection object
+ * @dom: domain on which the event occurred
+ * @devAlias: device alias
+ * @threshold: threshold that was exceeded, in
Re: [libvirt] [PATCH 01/10] util: Introduce thread queues
On Fri, May 22, 2015 at 13:48:16 -0400, John Ferlan wrote:
On 05/22/2015 10:31 AM, Jiri Denemark wrote:
On Fri, May 22, 2015 at 10:16:26 -0400, John Ferlan wrote:
On 05/21/2015 06:42 PM, Jiri Denemark wrote:
Our usage of pthread conditions does not allow a single thread to wait
for several events from different sources. This is because the condition
is bound to the source of the event. We can invert the usage by giving
each thread its own condition and providing APIs for registering this
thread condition with several sources. Each of the sources can then
signal the thread condition.
Thread queues also support several threads to be registered with a
single event source, which can either wakeup all waiting threads or just
the first one.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
po/POTFILES.in| 1 +
src/Makefile.am | 2 +
src/libvirt_private.syms | 15 ++
src/util/virthreadqueue.c | 343
++
src/util/virthreadqueue.h | 54
5 files changed, 415 insertions(+)
create mode 100644 src/util/virthreadqueue.c
create mode 100644 src/util/virthreadqueue.h
Ran the series through Coverity and came back with this gem
+
+void
+virThreadQueueFree(virThreadQueuePtr queue)
+{
+if (!queue)
+return;
+
+while (queue-head)
+virThreadQueueRemove(queue, queue-head);
(3) Event cond_true: Condition queue-head, taking true branch
(6) Event loop_begin: Jumped back to beginning of loop
(7) Event cond_true: Condition queue-head, taking true branch
283while (queue-head)
(4) Event freed_arg: virThreadQueueRemove frees queue-head.
[details]
(5) Event loop:Jumping back to the beginning of the loop
(8) Event deref_arg: Calling virThreadQueueRemove dereferences
freed pointer queue-head. [details]
284virThreadQueueRemove(queue, queue-head);
False positive. If virThreadQueueRemove frees queue-head, it also
updates it with queue-head-next.
I understand and looked at the code, but I think this is a case where
pass by reference and pass by value makes a difference. Upon return
what causes queue-head to be evaluated again? The call passes
queue-head by value and removes it from queue. Upon return nothing
causes queue-head to be evaluated again thus wouldn't we enter the loop
the second time with the same address?
This would be a serious bug in the compiler. The function also gets the
queue pointer, which means the compiler should not consider queue-head
was unchanged.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: reject negative values for scaled integer
On Fri, May 22, 2015 at 01:07:31PM -0600, Eric Blake wrote: On 05/22/2015 08:01 AM, Pavel Hrdina wrote: Some virsh commands have a size parameter, which is handled as scaled integer. We don't have any *feature* that would allow to use '-1' as maximum size, so it's safe to reject any negative values for those commands. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1159171 Signed-off-by: Pavel Hrdina phrd...@redhat.com --- tools/virsh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK, particularly since it gets hard to justify whether -1M and -1G would mean the same maximum size. Thanks, pushed now. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/3] conf: Resolve Coverity NEGATIVE_RETURNS
On 05/18/2015 09:21 AM, John Ferlan wrote: Commit id '73eda710' added virDomainKeyWrapDefParseXML which uses virXPathNodeSet, but does not handle a -1 return thus causing a possible loop condition exit problem later when the return value is used. Change the logic to return the value from virXPathNodeSet if = 0 Signed-off-by: John Ferlan jfer...@redhat.com --- 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 e2b1194..a97e640 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -940,8 +940,8 @@ virDomainKeyWrapDefParseXML(virDomainDefPtr def, xmlXPathContextPtr ctxt) xmlNodePtr *nodes = NULL; int n; -if (!(n = virXPathNodeSet(./keywrap/cipher, ctxt, nodes))) -return 0; +if ((n = virXPathNodeSet(./keywrap/cipher, ctxt, nodes)) 0) +return n; Seemed a bit strange at first (since the n == 0 case now continues instead of returning immediately), but in the end it makes sense - if n is 0, you end up allocating keywrap, never going through the loop, then freeing keywrap and returning 0, to the result is the same as before. ACK. if (VIR_ALLOC(def-keywrap) 0) goto cleanup; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/3] qemu: Resolve Coverity RESOURCE_LEAK
On 05/18/2015 09:21 AM, John Ferlan wrote:
Recent changes to the -M/--machine processing code in qemuParseCommandLine
caused Coverity to determine there was a possible resource leak with how
the 'list' is managed. Rather than try to add virStringFreeList calls
everywhere - just promote list to the top of the variables and free it
within the error processing code. Also required a couple of other tweaks
in order to avoid double free's.
ACK.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/qemu/qemu_command.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3ccd35d..411b7a4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -12382,6 +12382,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
size_t i;
bool nographics = false;
bool fullscreen = false;
+char **list = NULL;
char *path;
size_t nnics = 0;
const char **nics = NULL;
@@ -12849,7 +12850,6 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
VIR_FREE(def-name);
} else if (STREQ(arg, -M) ||
STREQ(arg, -machine)) {
-char **list;
char *param;
size_t j = 0;
@@ -12864,10 +12864,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
if (STRPREFIX(param, type=))
param += strlen(type=);
if (!strchr(param, '=')) {
-if (VIR_STRDUP(def-os.machine, param) 0) {
-virStringFreeList(list);
+if (VIR_STRDUP(def-os.machine, param) 0)
goto error;
-}
j++;
}
@@ -12912,6 +12910,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
}
}
virStringFreeList(list);
+list = NULL;
} else if (STREQ(arg, -serial)) {
WANT_VALUE();
if (STRNEQ(val, none)) {
@@ -13385,6 +13384,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
virDomainDiskDefFree(disk);
qemuDomainCmdlineDefFree(cmd);
virDomainDefFree(def);
+virStringFreeList(list);
VIR_FREE(nics);
if (monConfig) {
virDomainChrSourceDefFree(*monConfig);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/10] util: Introduce thread queues
On Fri, May 22, 2015 at 14:28:05 +0200, Jiri Denemark wrote: On Fri, May 22, 2015 at 13:09:17 +0100, Daniel P. Berrange wrote: On Fri, May 22, 2015 at 12:42:34AM +0200, Jiri Denemark wrote: Our usage of pthread conditions does not allow a single thread to wait for several events from different sources. This is because the condition is bound to the source of the event. We can invert the usage by giving each thread its own condition and providing APIs for registering this thread condition with several sources. Each of the sources can then signal the thread condition. Thread queues also support several threads to be registered with a single event source, which can either wakeup all waiting threads or just the first one. Signed-off-by: Jiri Denemark jdene...@redhat.com I'm not really convinced this abstraction is neccessary / appropriate. I can see what you mean about the problems with migration having access to several different virCond's that it needs to wait on, but AFAICT, all the condition variables are ultimately associated with a single guest domain. So it seems the problem could have been solved much more simply by just having a single virCond in the qemuDomainObjPrivate struct. Moving to a centralized single condition var per thread feels like it is really breaking encapsulation of the APIs across the codebase. Because we may want to wake up a thread which we know nothing about, that is, we have no idea what job (if any) or even on which domain it is doing. Currently, you can't gracefully kill libvirtd when it is, e.g., migrating a domain to another host. Apart from a bug which stops the main event loop first (which I already solved in another branch of mine), libvirtd would only stop once migration completes or is aborted manually. You have to force kill it if you don't want to wait. Using a thread local condition allows us to wake up any thread and ask it to finish the job asap, possibly canceling it. Actually, thinking about this a bit more, I could implement it with per-domain condition. Any thread working on a domain would just register the domain's condition with the thread's pool in case the pool wants to wake up its threads. I think this would even be a bit nicer than the approach I implemented. Although, I'd go with a condition stored directly in virDomainObj rather than inside qemuDomainObjPrivate since this is something all driver could (and should) use if they want to use conditions. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Only issue the rtc-reset-reinjection QMP command on x86.
The command is only defined in QEMU for TARGET_I386, so issuing it on
any other architecture can't possibly work.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1211938
---
src/qemu/qemu_driver.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..743ca6e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18945,7 +18945,10 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}
-if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) {
+/* The rtc-reset-reinjection QMP command is only available on x86 */
+if (ARCH_IS_X86(vm-def-os.arch)
+!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION))
+{
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(cannot set time: qemu doesn't support
rtc-reset-reinjection command));
@@ -18968,13 +18971,16 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}
-qemuDomainObjEnterMonitor(driver, vm);
-rv = qemuMonitorRTCResetReinjection(priv-mon);
-if (qemuDomainObjExitMonitor(driver, vm) 0)
-goto endjob;
+/* The rtc-reset-reinjection QMP command is only available on x86 */
+if (ARCH_IS_X86(vm-def-os.arch)) {
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv-mon);
+if (qemuDomainObjExitMonitor(driver, vm) 0)
+goto endjob;
-if (rv 0)
-goto endjob;
+if (rv 0)
+goto endjob;
+}
ret = 0;
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
On Mon, May 25, 2015 at 17:35:19 +0200, Michal Privoznik wrote:
On 25.05.2015 17:18, Peter Krempa wrote:
Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
via a pointer rather than in the return value. The change triggered
problems with platforms where the compiler decides to use a data type of
size different than integer at the point where we typecast it.
Work around the issue by using an intermediate variable of the correct
type that gets casted back by the default typecasting rules.
---
src/qemu/qemu_driver.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..2b9f125 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
virMemoryParameterPtr param = params[i];
switch (i) {
-case 0: /* fill numa mode here */
+case 0: { /* fill numa mode here */
+virDomainNumatuneMemMode tmp;
+virDomainNumatuneGetMode(def-numa, -1, tmp);
+
if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
-VIR_TYPED_PARAM_INT, 0) 0)
+VIR_TYPED_PARAM_INT, tmp) 0)
goto cleanup;
-virDomainNumatuneGetMode(def-numa, -1,
- (virDomainNumatuneMemMode *)
param-value.i);
break;
+}
case 1: /* fill numa nodeset here */
nodeset = virDomainNumatuneFormatNodeset(def-numa, NULL, -1);
I guess we saw the same coverity report since you're fixing the code I'm
just looking at. But I think what is coverity trying to tell us is that
in all other places virDomainNumatuneGetMode() is checked for return
value, here it's not. Or did you really see problem you're describing
in the commit message?
The problem I'm trying to solve is failure to compile libvirt on CentOS
5. The compilation error can be seen at:
https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=libvirt-centos-5/259/console
if you scroll down enough.
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] qemu: Fix compilation error when enum variable size differs from 'int'
On 25.05.2015 17:47, Peter Krempa wrote:
On Mon, May 25, 2015 at 17:35:19 +0200, Michal Privoznik wrote:
On 25.05.2015 17:18, Peter Krempa wrote:
Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
via a pointer rather than in the return value. The change triggered
problems with platforms where the compiler decides to use a data type of
size different than integer at the point where we typecast it.
Work around the issue by using an intermediate variable of the correct
type that gets casted back by the default typecasting rules.
---
src/qemu/qemu_driver.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..2b9f125 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
virMemoryParameterPtr param = params[i];
switch (i) {
-case 0: /* fill numa mode here */
+case 0: { /* fill numa mode here */
+virDomainNumatuneMemMode tmp;
+virDomainNumatuneGetMode(def-numa, -1, tmp);
+
if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
-VIR_TYPED_PARAM_INT, 0) 0)
+VIR_TYPED_PARAM_INT, tmp) 0)
goto cleanup;
-virDomainNumatuneGetMode(def-numa, -1,
- (virDomainNumatuneMemMode *)
param-value.i);
break;
+}
case 1: /* fill numa nodeset here */
nodeset = virDomainNumatuneFormatNodeset(def-numa, NULL, -1);
I guess we saw the same coverity report since you're fixing the code I'm
just looking at. But I think what is coverity trying to tell us is that
in all other places virDomainNumatuneGetMode() is checked for return
value, here it's not. Or did you really see problem you're describing
in the commit message?
The problem I'm trying to solve is failure to compile libvirt on CentOS
5. The compilation error can be seen at:
https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=libvirt-centos-5/259/console
if you scroll down enough.
Ah, okay; So ACK. Lets see what will Coverity think after you push this.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
via a pointer rather than in the return value. The change triggered
problems with platforms where the compiler decides to use a data type of
size different than integer at the point where we typecast it.
Work around the issue by using an intermediate variable of the correct
type that gets casted back by the default typecasting rules.
---
src/qemu/qemu_driver.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..2b9f125 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
virMemoryParameterPtr param = params[i];
switch (i) {
-case 0: /* fill numa mode here */
+case 0: { /* fill numa mode here */
+virDomainNumatuneMemMode tmp;
+virDomainNumatuneGetMode(def-numa, -1, tmp);
+
if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
-VIR_TYPED_PARAM_INT, 0) 0)
+VIR_TYPED_PARAM_INT, tmp) 0)
goto cleanup;
-virDomainNumatuneGetMode(def-numa, -1,
- (virDomainNumatuneMemMode *)
param-value.i);
break;
+}
case 1: /* fill numa nodeset here */
nodeset = virDomainNumatuneFormatNodeset(def-numa, NULL, -1);
--
2.4.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Only issue the rtc-reset-reinjection QMP command on x86.
On Mon, May 25, 2015 at 16:59:08 +0200, Andrea Bolognani wrote:
The command is only defined in QEMU for TARGET_I386, so issuing it on
any other architecture can't possibly work.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1211938
---
src/qemu/qemu_driver.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..743ca6e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18945,7 +18945,10 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}
-if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) {
+/* The rtc-reset-reinjection QMP command is only available on x86 */
Since we properly track support for this command via the capabilities
and qemu does not expose it:
+if (ARCH_IS_X86(vm-def-os.arch)
+!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION))
+{
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(cannot set time: qemu doesn't support
rtc-reset-reinjection command));
I'd simply remove this error message since it is semantically wrong once
PPC does not require to reset the RTC reinjection.
@@ -18968,13 +18971,16 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}
-qemuDomainObjEnterMonitor(driver, vm);
-rv = qemuMonitorRTCResetReinjection(priv-mon);
-if (qemuDomainObjExitMonitor(driver, vm) 0)
-goto endjob;
+/* The rtc-reset-reinjection QMP command is only available on x86 */
+if (ARCH_IS_X86(vm-def-os.arch)) {
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv-mon);
And just conditionally call this when the
QEMU_CAPS_RTC_RESET_REINJECTION is present and not in an architecture
specific way.
By this you get rid of the arch specific hackery.
+if (qemuDomainObjExitMonitor(driver, vm) 0)
+goto endjob;
-if (rv 0)
-goto endjob;
+if (rv 0)
+goto endjob;
+}
ret = 0;
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] qemu: Fix compilation error when enum variable size differs from 'int'
On 25.05.2015 17:18, Peter Krempa wrote:
Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
via a pointer rather than in the return value. The change triggered
problems with platforms where the compiler decides to use a data type of
size different than integer at the point where we typecast it.
Work around the issue by using an intermediate variable of the correct
type that gets casted back by the default typecasting rules.
---
src/qemu/qemu_driver.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..2b9f125 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
virMemoryParameterPtr param = params[i];
switch (i) {
-case 0: /* fill numa mode here */
+case 0: { /* fill numa mode here */
+virDomainNumatuneMemMode tmp;
+virDomainNumatuneGetMode(def-numa, -1, tmp);
+
if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
-VIR_TYPED_PARAM_INT, 0) 0)
+VIR_TYPED_PARAM_INT, tmp) 0)
goto cleanup;
-virDomainNumatuneGetMode(def-numa, -1,
- (virDomainNumatuneMemMode *)
param-value.i);
break;
+}
case 1: /* fill numa nodeset here */
nodeset = virDomainNumatuneFormatNodeset(def-numa, NULL, -1);
I guess we saw the same coverity report since you're fixing the code I'm
just looking at. But I think what is coverity trying to tell us is that
in all other places virDomainNumatuneGetMode() is checked for return
value, here it's not. Or did you really see problem you're describing
in the commit message?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: add testing of RPC JSON (de)serialization
On Fri, May 22, 2015 at 01:36:14PM +0100, Daniel P. Berrange wrote:
The virNetServer class has the ability to serialize its state
to a JSON file, and then re-load that data after an in-place
execve() call to re-connect to active file handles. This data
format is critical ABI that must have compatibility across
releases, so it should be tested...
---
src/libvirt_remote.syms| 1 +
src/rpc/virnetserver.c | 4 +-
src/rpc/virnetserver.h | 3 +
src/rpc/virnetserverclient.c | 13 +-
src/rpc/virnetserverservice.c | 6 +-
tests/Makefile.am | 7 +
tests/virnetserverdata/README | 14 +
.../virnetserverdata/input-data-anon-clients.json | 63 +
tests/virnetserverdata/input-data-initial.json | 62 +
.../virnetserverdata/output-data-anon-clients.json | 63 +
tests/virnetserverdata/output-data-initial.json| 63 +
tests/virnetservertest.c | 290 +
12 files changed, 579 insertions(+), 10 deletions(-)
create mode 100644 tests/virnetserverdata/README
create mode 100644 tests/virnetserverdata/input-data-anon-clients.json
create mode 100644 tests/virnetserverdata/input-data-initial.json
create mode 100644 tests/virnetserverdata/output-data-anon-clients.json
create mode 100644 tests/virnetserverdata/output-data-initial.json
create mode 100644 tests/virnetservertest.c
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 950e56e..bdd68f6 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -77,6 +77,7 @@ xdr_virNetMessageError;
# rpc/virnetserver.h
+virNetServerAddClient;
virNetServerAddProgram;
virNetServerAddService;
virNetServerAddShutdownInhibition;
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 42427dc..091a1dc 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -259,8 +259,8 @@ static int
virNetServerDispatchNewMessage(virNetServerClientPtr client,
}
-static int virNetServerAddClient(virNetServerPtr srv,
- virNetServerClientPtr client)
+int virNetServerAddClient(virNetServerPtr srv,
+ virNetServerClientPtr client)
{
virObjectLock(srv);
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 8c5ae07..4b452be 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -79,6 +79,9 @@ int virNetServerAddService(virNetServerPtr srv,
virNetServerServicePtr svc,
const char *mdnsEntryName);
+int virNetServerAddClient(virNetServerPtr srv,
+ virNetServerClientPtr client);
+
int virNetServerAddProgram(virNetServerPtr srv,
virNetServerProgramPtr prog);
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 34c1994..0e3a71f 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -536,13 +536,14 @@ virJSONValuePtr
virNetServerClientPreExecRestart(virNetServerClientPtr client)
goto error;
}
-if (client-privateData client-privateDataPreExecRestart
-!(child = client-privateDataPreExecRestart(client,
client-privateData)))
-goto error;
+if (client-privateData client-privateDataPreExecRestart) {
+if (!(child = client-privateDataPreExecRestart(client,
client-privateData)))
+goto error;
-if (virJSONValueObjectAppend(object, privateData, child) 0) {
-virJSONValueFree(child);
-goto error;
+if (virJSONValueObjectAppend(object, privateData, child) 0) {
+virJSONValueFree(child);
+goto error;
+}
}
virObjectUnlock(client);
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index d3cf31a..4df26cb 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -303,12 +303,15 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,
/* IO callback is initially disabled, until we're ready
* to deal with incoming clients */
+virObjectRef(svc);
if (virNetSocketAddIOCallback(svc-socks[i],
0,
virNetServerServiceAccept,
svc,
- virObjectFreeCallback) 0)
+ virObjectFreeCallback) 0) {
+virObjectUnref(svc);
goto error;
+}
}
@@ -388,7 +391,6 @@ virNetServerServicePtr
virNetServerServiceNewPostExecRestart(virJSONValuePtr obj
svc,
virObjectFreeCallback) 0) {
virObjectUnref(svc);
-virObjectUnref(sock);
goto error;
}
}
diff --git a/tests/Makefile.am
Re: [libvirt] [PATCH v2 1/6] util: multi-value virTypedParameter
On 21.05.2015 13:07, Pavel Boldin wrote:
The `virTypedParamsValidate' function now can be instructed to allow
multiple entries for some of the keys. For this flag the type with
the `VIR_TYPED_PARAM_MULTIPLE' flag.
Add unit tests for this new behaviour.
Signed-off-by: Pavel Boldin pbol...@mirantis.com
---
src/util/virtypedparam.c | 108 +++---
src/util/virtypedparam.h | 10 +++
tests/Makefile.am | 6 ++
tests/virtypedparamtest.c | 167
++
4 files changed, 252 insertions(+), 39 deletions(-)
create mode 100644 tests/virtypedparamtest.c
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index de2d447..43e49ca 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -47,11 +47,18 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST,
* internal utility functions (those in libvirt_private.syms) may
* report errors that the caller will dispatch. */
+static int virTypedParamsSortName(const void *left, const void *right)
+{
+const virTypedParameter *param_left = left, *param_right = right;
+return strcmp(param_left-field, param_right-field);
+}
+
/* Validate that PARAMS contains only recognized parameter names with
- * correct types, and with no duplicates. Pass in as many name/type
- * pairs as appropriate, and pass NULL to end the list of accepted
- * parameters. Return 0 on success, -1 on failure with error message
- * already issued. */
+ * correct types, and with no duplicates except for parameters
+ * specified with VIR_TYPED_PARAM_MULTIPLE flag in type.
+ * Pass in as many name/type pairs as appropriate, and pass NULL to end
+ * the list of accepted parameters. Return 0 on success, -1 on failure
+ * with error message already issued. */
int
virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...)
{
@@ -60,60 +67,83 @@ virTypedParamsValidate(virTypedParameterPtr params, int
nparams, ...)
size_t i, j;
const char *name;
int type;
+size_t nkeys = 0, nkeysmax = 0;
s/nkeysmax/nkeysalloc/ because the variable holds count of items allocated.
+virTypedParameterPtr sorted = NULL, keys = NULL;
va_start(ap, nparams);
-/* Yes, this is quadratic, but since we reject duplicates and
- * unknowns, it is constrained by the number of var-args passed
- * in, which is expected to be small enough to not be
- * noticeable. */
-for (i = 0; i nparams; i++) {
-va_end(ap);
-va_start(ap, nparams);
+if (VIR_ALLOC_N(sorted, nparams) 0)
+goto cleanup;
-name = va_arg(ap, const char *);
-while (name) {
-type = va_arg(ap, int);
-if (STREQ(params[i].field, name)) {
-if (params[i].type != type) {
-const char *badtype;
-
-badtype = virTypedParameterTypeToString(params[i].type);
-if (!badtype)
-badtype = virTypedParameterTypeToString(0);
-virReportError(VIR_ERR_INVALID_ARG,
- _(invalid type '%s' for parameter '%s',
- expected '%s'),
- badtype, params[i].field,
- virTypedParameterTypeToString(type));
-}
-break;
-}
-name = va_arg(ap, const char *);
-}
-if (!name) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
- _(parameter '%s' not supported),
- params[i].field);
+/* Here we intentionally don't copy values */
+memcpy(sorted, params, sizeof(*params) * nparams);
+qsort(sorted, nparams, sizeof(*sorted), virTypedParamsSortName);
+
+name = va_arg(ap, const char *);
+while (name) {
+type = va_arg(ap, int);
+if (VIR_RESIZE_N(keys, nkeysmax, nkeys, 1) 0)
+goto cleanup;
+
+if (virStrcpyStatic(keys[nkeys].field, name) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Field name '%s' too long), name);
goto cleanup;
}
-for (j = 0; j i; j++) {
-if (STREQ(params[i].field, params[j].field)) {
+
+keys[nkeys].type = type ~VIR_TYPED_PARAM_MULTIPLE;
+/* Value is not used anyway */
+keys[nkeys].value.i = type VIR_TYPED_PARAM_MULTIPLE;
+
+nkeys++;
+name = va_arg(ap, const char *);
+}
+
+qsort(keys, nkeys, sizeof(*keys), virTypedParamsSortName);
+
+for (i = 0, j = 0; i nparams j nkeys;) {
+if (STRNEQ(sorted[i].field, keys[j].field)) {
+j++;
+} else {
+if (i j !(keys[j].value.i VIR_TYPED_PARAM_MULTIPLE)) {
Re: [libvirt] [PATCH v2 4/6] util: add virTypedParamsAddStringList
On 21.05.2015 13:07, Pavel Boldin wrote: The `virTypedParamsAddStringList' function provides interface to add a NULL-terminated array of string values as a multi-value to the params. Signed-off-by: Pavel Boldin pbol...@mirantis.com --- include/libvirt/libvirt-host.h | 6 ++ src/libvirt_public.syms| 1 + src/util/virtypedparam.c | 36 tests/virtypedparamtest.c | 28 4 files changed, 71 insertions(+) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix compilation error when enum variable size differs from 'int'
On Mon, May 25, 2015 at 05:35:19PM +0200, Michal Privoznik wrote:
On 25.05.2015 17:18, Peter Krempa wrote:
Since commit bcd9a564b631aa virDomainNumatuneGetMode returns the value
via a pointer rather than in the return value. The change triggered
problems with platforms where the compiler decides to use a data type of
size different than integer at the point where we typecast it.
Work around the issue by using an intermediate variable of the correct
type that gets casted back by the default typecasting rules.
---
src/qemu/qemu_driver.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..2b9f125 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10566,14 +10566,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
virMemoryParameterPtr param = params[i];
switch (i) {
-case 0: /* fill numa mode here */
+case 0: { /* fill numa mode here */
+virDomainNumatuneMemMode tmp;
+virDomainNumatuneGetMode(def-numa, -1, tmp);
+
if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
-VIR_TYPED_PARAM_INT, 0) 0)
+VIR_TYPED_PARAM_INT, tmp) 0)
goto cleanup;
-virDomainNumatuneGetMode(def-numa, -1,
- (virDomainNumatuneMemMode *)
param-value.i);
break;
+}
case 1: /* fill numa nodeset here */
nodeset = virDomainNumatuneFormatNodeset(def-numa, NULL, -1);
I guess we saw the same coverity report since you're fixing the code I'm
just looking at. But I think what is coverity trying to tell us is that
in all other places virDomainNumatuneGetMode() is checked for return
value, here it's not. Or did you really see problem you're describing
in the commit message?
I think Peter is trying to fix the problem of compilation on centos 5:
https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=libvirt-centos-5/259/console
And I already had similar patch prepared, I only declared the tmp as
memMode for the whole function because switch cases with curly
brackets -- Yuck!
I'd say ACK, but I'm not sure whether Michal was trying to point out
something more that I misunderstood, so I leave it up to him.
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 6/6] virsh: selective block device migration
On 21.05.2015 13:07, Pavel Boldin wrote:
Add `virsh migrate' option `--migratedisks' that allows CLI user to
explicitly specify block devices to migrate.
Signed-off-by: Pavel Boldin pbol...@mirantis.com
---
tools/virsh-domain.c | 23 +++
tools/virsh.pod | 5 -
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 20f8c75..47a24ab 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9784,6 +9784,10 @@ static const vshCmdOptDef opts_migrate[] = {
.type = VSH_OT_STRING,
.help = N_(filename containing updated XML for the target)
},
+{.name = migratedisks,
+ .type = VSH_OT_STRING,
+ .help = N_(comma separated list of disks to be migrated)
+},
{.name = NULL}
};
@@ -9843,6 +9847,25 @@ doMigrate(void *opaque)
VIR_MIGRATE_PARAM_DEST_NAME, opt) 0)
goto save_error;
+if (vshCommandOptStringReq(ctl, cmd, migratedisks, opt) 0)
+goto out;
+if (opt) {
+char **val = NULL;
+
+val = virStringSplit(opt, ,, 0);
+
+if (virTypedParamsAddStringList(params,
+nparams,
+maxparams,
+VIR_MIGRATE_PARAM_MIGRATE_DISKS,
+(const char **)val) 0) {
+VIR_FREE(val);
+goto save_error;
+}
+
+VIR_FREE(val);
+}
+
if (vshCommandOptStringReq(ctl, cmd, xml, opt) 0)
goto out;
if (opt) {
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1bb655b..aad0f3b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1521,6 +1521,7 @@ to the Iuri namespace is displayed instead of being
modified.
[I--compressed] [I--abort-on-error] [I--auto-converge]
Idomain Idesturi [Imigrateuri] [Igraphicsuri] [Ilisten-address]
[Idname] [I--timeout Bseconds] [I--xml Bfile]
+[I--migratedisks Bcomma-separated-list]
Migrate domain to another host. Add I--live for live migration; --p2p
for peer-2-peer migration; I--direct for direct migration; or
I--tunnelled
@@ -1536,7 +1537,9 @@ with incremental copy (same base image shared between
source and destination).
In both cases the disk images have to exist on destination host, the
I--copy-storage-... options only tell libvirt to transfer data from the
images on source host to the images found at the same place on the
destination
-host. I--change-protection enforces that no incompatible configuration
+host. By default only non-shared non-readonly images are transfered. Use
+I--migratedisks to explicitly specify a list of images to transfer.
+I--change-protection enforces that no incompatible configuration
changes will be made to the domain while the migration is underway; this flag
is implicitly enabled when supported by the hypervisor, but can be explicitly
used to reject the migration if the hypervisor lacks change protection
We tend to keep info on argument format in a description rather than
command definition.
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/6] util: virTypedParams{Filter, PickStrings}
On 21.05.2015 13:07, Pavel Boldin wrote:
Add multikey API:
* virTypedParamsFilter that returns all the parameters with specified name.
* virTypedParamsPickStrings that returns a NULL-terminated `const char**'
list with all the values for specified name and string type.
Signed-off-by: Pavel Boldin pbol...@mirantis.com
---
include/libvirt/libvirt-host.h | 10
src/libvirt_public.syms| 6 +++
src/util/virtypedparam.c | 107
+
tests/virtypedparamtest.c | 96
4 files changed, 219 insertions(+)
diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
index 070550b..36267fc 100644
--- a/include/libvirt/libvirt-host.h
+++ b/include/libvirt/libvirt-host.h
@@ -249,6 +249,11 @@ virTypedParamsGet (virTypedParameterPtr params,
int nparams,
const char *name);
int
+virTypedParamsFilter(virTypedParameterPtr params,
+ int nparams,
+ const char *name,
+ virTypedParameterPtr **ret);
+int
virTypedParamsGetInt(virTypedParameterPtr params,
int nparams,
const char *name,
@@ -283,6 +288,11 @@ virTypedParamsGetString (virTypedParameterPtr params,
int nparams,
const char *name,
const char **value);
+const char **
+virTypedParamsPickStrings(virTypedParameterPtr params,
+ int nparams,
+ const char *name,
+ size_t *picked);
int
virTypedParamsAddInt(virTypedParameterPtr *params,
int *nparams,
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index ef3d2f0..8fc8c42 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -710,4 +710,10 @@ LIBVIRT_1.2.15 {
virDomainDelIOThread;
} LIBVIRT_1.2.14;
+LIBVIRT_1.2.16 {
+global:
+virTypedParamsFilter;
+virTypedParamsPickStrings;
+} LIBVIRT_1.2.15;
+
# define new API here using predicted next version number
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index ec20b74..a3d959e 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -481,6 +481,58 @@ virTypedParamsGet(virTypedParameterPtr params,
}
+/**
+ * virTypedParamsFilter:
+ * @params: array of typed parameters
+ * @nparams: number of parameters in the @params array
+ * @name: name of the parameter to find
+ * @ret: pointer to the returned array
+ *
+ *
+ * Filters @params retaining only the parameters named @name in the
+ * resulting array @ret. Caller should free the @ret array but no the
s/no/not/
+ * items since they are pointing to the @params elements.
+ *
+ * Returns amount of elements in @ret on success, -1 on error.
+ */
+int
+virTypedParamsFilter(virTypedParameterPtr params,
+ int nparams,
+ const char *name,
+ virTypedParameterPtr **ret)
+{
+size_t i, max = 0, n = 0;
s/max/alloc/
+
+virResetLastError();
+
+if (!params || !name || !ret) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(Required argument is missing: %s),
+ !params ? params : !name ? name : ret);
+goto error;
We have a macro for that: virCheckNonNullArgGoto()
+}
+
+for (i = 0; i nparams; i++) {
+if (STREQ(params[i].field, name)) {
+if (VIR_RESIZE_N(*ret, max, n, 1) 0)
+goto error;
+
+(*ret)[n] = params[i];
+
+n++;
+}
+}
So if there's no match, @ret is untouched. This is not cool, consider
the following code:
virTypedParameterPtr *ret;
if (virTypedParamsFilter(.., ret) 0) {
error;
} else {
success;
free(ret);
}
The free() may end up freeing a random pointer.
+
+return n;
+
+ error:
+if (ret)
+VIR_FREE(*ret);
+virDispatchError(NULL);
+return -1;
+}
+
+
#define VIR_TYPED_PARAM_CHECK_TYPE(check_type) \
do { if (param-type != check_type) { \
virReportError(VIR_ERR_INVALID_ARG, \
@@ -749,6 +801,61 @@ virTypedParamsGetString(virTypedParameterPtr params,
/**
+ * virTypedParamsPickStrings:
+ * @params: array of typed parameters
+ * @nparams: number of parameters in the @params array
+ * @name: name of the parameter to find
+ * @picked: pointer to the amount of picked strings.
+ *
+ *
+ * Finds typed parameters called @name.
+ *
+ * Returns a string list, which is a NULL terminated array of pointers to
+ * strings. Since a NULL is a valid parameter string value caller can ask
+ *
Re: [libvirt] [PATCHv2 0/6] Selective block device migration implementation
On 21.05.2015 13:07, Pavel Boldin wrote:
The patchset represented in the mail thread implements the selective block
device migration for the QEMU driver. This closes the referenced bug [1].
First the supplementary API implemented allowing for multiple key-values pair
in the virTypedParameter arrays. This is used to pass the list of block
devices to migrate in the following patches. Unit tests for this are added as
well. This is the subject of the first four patches.
Fifth patch is adding the necessary parameter `migrate_disks' and passes it
through the QEMU driver call stack. The introduced `qemuMigrateDisk' function
is then checks that the disk is to be migrated because it is in the list. If
there is no list specified then the legacy check is used: the device is
migrate
if it is not shared, not readonly and has a source.
Sixth and the last patch is adding the necessary code to the `virsh' utility
making it able for user to specify a comma-separated list of the block device
names that are to be migrated.
The implemented Python bindings patch is to be presented.
Changes in v2:
* Addressed review comments
* Removed all the duplicates check in the commit 'multi-value parameters in
virTypedParamsAdd*'
* reimplemented virTypedParamsPick as virTypedParamsFilter
* renamed virTypedParamsPackStrings to virTypedParamsAddStringList
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1203032
Pavel Boldin (6):
util: multi-value virTypedParameter
util: multi-value parameters in virTypedParamsAdd*
util: virTypedParams{Filter,PickStrings}
util: add virTypedParamsAddStringList
qemu: migration: selective block device migration
virsh: selective block device migration
include/libvirt/libvirt-domain.h | 9 ++
include/libvirt/libvirt-host.h | 16 +++
src/libvirt_public.syms | 7 +
src/qemu/qemu_driver.c | 66 ++---
src/qemu/qemu_migration.c| 174 +++
src/qemu/qemu_migration.h| 23 ++--
src/util/virtypedparam.c | 263 ---
src/util/virtypedparam.h | 10 ++
tests/Makefile.am| 6 +
tests/virtypedparamtest.c| 291
+++
tools/virsh-domain.c | 23
tools/virsh.pod | 5 +-
12 files changed, 750 insertions(+), 143 deletions(-)
create mode 100644 tests/virtypedparamtest.c
So, I think we are almost there. I've pointed out a few things.
Moreover, as I've been reviewing I keep fixing the nits I'm raising. So
instead of me giving you headache of posting v3, I'll do that.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/6] util: multi-value parameters in virTypedParamsAdd*
On 21.05.2015 13:07, Pavel Boldin wrote: Allow multi-value parameters to be build using virTypedParamsAdd* functions by removing check for duplicates. Signed-off-by: Pavel Boldin pbol...@mirantis.com --- src/util/virtypedparam.c | 16 1 file changed, 16 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/6] qemu: migration: selective block device migration
On 21.05.2015 13:07, Pavel Boldin wrote:
Implement a `migrate_disks' parameters for the QEMU driver. This multi-
value parameter can be used to explicitly specify what block devices
are to be migrated using the NBD server. Tunnelled migration using NBD
is to be done.
Signed-off-by: Pavel Boldin pbol...@mirantis.com
---
include/libvirt/libvirt-domain.h | 9 ++
src/qemu/qemu_driver.c | 66 ++-
src/qemu/qemu_migration.c| 174
+--
src/qemu/qemu_migration.h| 23 --
4 files changed, 183 insertions(+), 89 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h
b/include/libvirt/libvirt-domain.h
index 0f465b9..6b48923 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -748,6 +748,15 @@ typedef enum {
*/
# define VIR_MIGRATE_PARAM_LISTEN_ADDRESSlisten_address
+/**
+ * VIR_MIGRATE_PARAM_MIGRATE_DISKS:
+ *
+ * virDomainMigrate* params multiple field: The multiple values that list
+ * the block devices to be migrated. At the moment this is only supported
+ * by the QEMU driver but not for the tunnelled migration.
+ */
+# define VIR_MIGRATE_PARAM_MIGRATE_DISKSmigrate_disks
+
/* Domain migration. */
virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn,
unsigned long flags, const char *dname,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2668011..77b7d9d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12472,7 +12472,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn,
ret = qemuMigrationPrepareDirect(driver, dconn,
NULL, 0, NULL, NULL, /* No cookies */
uri_in, uri_out,
- def, origname, NULL, flags);
+ def, origname, NULL, flags, NULL);
cleanup:
VIR_FREE(origname);
@@ -12525,7 +12525,7 @@ qemuDomainMigratePerform(virDomainPtr dom,
* Consume any cookie we were able to decode though
*/
ret = qemuMigrationPerform(driver, dom-conn, vm,
- NULL, dconnuri, uri, NULL, NULL,
+ NULL, dconnuri, uri, NULL, NULL, NULL,
cookie, cookielen,
NULL, NULL, /* No output cookies in v2 */
flags, dname, resource, false);
@@ -12602,7 +12602,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
}
return qemuMigrationBegin(domain-conn, vm, xmlin, dname,
- cookieout, cookieoutlen, flags);
+ cookieout, cookieoutlen, flags, NULL);
}
static char *
@@ -12615,11 +12615,13 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain,
{
const char *xmlin = NULL;
const char *dname = NULL;
+const char **migrate_disks = NULL;
+char *ret = NULL;
virDomainObjPtr vm;
virCheckFlags(QEMU_MIGRATION_FLAGS, NULL);
if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS)
0)
-return NULL;
+goto cleanup;
if (virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_XML,
@@ -12627,18 +12629,26 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain,
virTypedParamsGetString(params, nparams,
VIR_MIGRATE_PARAM_DEST_NAME,
dname) 0)
-return NULL;
+goto cleanup;
+
+migrate_disks = virTypedParamsPickStrings(params, nparams,
+
VIR_MIGRATE_PARAM_MIGRATE_DISKS,
+ NULL);
Since this function looks slightly different now, this patch needs to be
changed too. Mostly, where @migrate_disks is parsed, new variable (say
@nmigrate_disks) describing the array length needs to be passed too.
if (!(vm = qemuDomObjFromDomain(domain)))
-return NULL;
+goto cleanup;
if (virDomainMigrateBegin3ParamsEnsureACL(domain-conn, vm-def) 0) {
virDomainObjEndAPI(vm);
-return NULL;
+goto cleanup;
}
-return qemuMigrationBegin(domain-conn, vm, xmlin, dname,
- cookieout, cookieoutlen, flags);
+ret = qemuMigrationBegin(domain-conn, vm, xmlin, dname,
+ cookieout, cookieoutlen, flags, migrate_disks);
+
+ cleanup:
+VIR_FREE(migrate_disks);
+return ret;
}
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8fe1cfb..708d1aa 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1579,12 +1579,32 @@ qemuMigrationPrecreateDisk(virConnectPtr conn,
return ret;
}
+static int
This should return bool
Re: [libvirt] [PATCH] qemu: Only issue the rtc-reset-reinjection QMP command on x86.
On Mon, May 25, 2015 at 05:26:01PM +0200, Peter Krempa wrote:
On Mon, May 25, 2015 at 16:59:08 +0200, Andrea Bolognani wrote:
The command is only defined in QEMU for TARGET_I386, so issuing it on
any other architecture can't possibly work.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1211938
---
src/qemu/qemu_driver.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aa0acde..743ca6e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18945,7 +18945,10 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}
-if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION)) {
+/* The rtc-reset-reinjection QMP command is only available on x86 */
Since we properly track support for this command via the capabilities
and qemu does not expose it:
+if (ARCH_IS_X86(vm-def-os.arch)
+!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_RTC_RESET_REINJECTION))
+{
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(cannot set time: qemu doesn't support
rtc-reset-reinjection command));
I'd simply remove this error message since it is semantically wrong once
PPC does not require to reset the RTC reinjection.
@@ -18968,13 +18971,16 @@ qemuDomainSetTime(virDomainPtr dom,
goto endjob;
}
-qemuDomainObjEnterMonitor(driver, vm);
-rv = qemuMonitorRTCResetReinjection(priv-mon);
-if (qemuDomainObjExitMonitor(driver, vm) 0)
-goto endjob;
+/* The rtc-reset-reinjection QMP command is only available on x86 */
+if (ARCH_IS_X86(vm-def-os.arch)) {
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv-mon);
And just conditionally call this when the
QEMU_CAPS_RTC_RESET_REINJECTION is present and not in an architecture
specific way.
By this you get rid of the arch specific hackery.
But on x86 we don't even want to call the SetTime command when we
cannot reset the rtc reinjection. On ppc there is no reinjection
being done, hence nothing to reset.
+if (qemuDomainObjExitMonitor(driver, vm) 0)
+goto endjob;
-if (rv 0)
-goto endjob;
+if (rv 0)
+goto endjob;
+}
ret = 0;
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/8] parallels: add vz driver
On 05/22/2015 09:18 PM, Maxim Nestratov wrote: This new driver 'vz' will be used as a substitution for parallels driver. New domains will be created with 'vz' domain type. Old 'parallels' domains remain supported. Connection to the driver can be made either way: vz:///system or parallels:///system. New URI works fine (together with the old one). But the driver returns 'vz' as domain type in xml and doesn't allow parallels in input xml for *DefineXML function. [root@dmserv libvirt]# virsh -c parallels+unix:///system dumpxml instance-0007 | head domain type='vz' nameinstance-0007/name uuid3967ae06-9dc9-4717-990a-414f695c0734/uuid memory unit='KiB'524288/memory currentMemory unit='KiB'524288/currentMemory vcpu placement='static'1/vcpu os type arch='i686'hvm/type /os clock offset='utc'/ Could, you, please, fix it? Daniel told we have to continue returning parallels. Maxim Nestratov (8): parallels: introduce vz driver constant and string parallels: use newly introduced VIR_DOMAIN_VIRT_VZ parallels: add new guest capabilities assigned to vz driver parallels: accept vz as a driver uri and name parallels: add a new vz connection driver and hypervisor structures parallels: increment the number of connection drivers parallels: recommend to connect to vz:///system when connection fails parallels: set VIR_DOMAIN_VIRT_VZ virtType for new domains src/conf/domain_conf.c| 19 +- src/conf/domain_conf.h| 1 + src/libvirt.c | 2 +- src/parallels/parallels_driver.c | 52 ++- src/parallels/parallels_network.c | 3 ++- src/parallels/parallels_sdk.c | 2 +- src/parallels/parallels_storage.c | 3 ++- 7 files changed, 66 insertions(+), 16 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
On 05/25/2015 08:43 PM, Eric Blake wrote: On 05/25/2015 08:28 PM, zhang bo wrote: IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) Thanks; but this is not the right place for translation bugs. See https://www.redhat.com/archives/libvir-list/2015-March/msg00302.html for instructions on submitting translations to zanata. And we should really make that more prominent in HACKING; patches welcome... -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Allow PCI virtio on ARM virt machine
On 05/24/2015 09:38 AM, Pavel Fedin wrote:
Virt machine in qemu has PCI generic host controller, and can use PCI
devices. This
provides performance improvement as well as vhost-net with irqfd support for
virtio-net.
However libvirt still insists on virtio devices attached to Virt machine to
have MMIO
bindings.
This patch allows to use both. If the user doesn't specify address
type='virtio-mmio',
PCI will be used by default.
This patch breaks make check, since there's test cases that depend on the
virtio-mmio behavior. I think you'll need to add a new qemu capabilities flag
like QEMU_CAPS_AARCH64_VIRT_PCI that's enabled for qemu 2.3.0 or later... if
it's available, then we default to PCI.
After that we will definitely want to add test cases to exercise this new
behavior.
Another thing is we probably need to extend the XML parser to add a
controller type='pci' automatically like we do for x86.
I can help with some of this if you want, just let me know.
virt- prefix is intentionally ignored in third chunk because it is a
temporary thing
and qemu developers agreed to use gic version option, for which there is
already support
in libvirt.
I don't follow this, can you expand a bit? Maybe Michal can explain too since
he did the libvirt gic patches
Thanks,
Cole
---
src/qemu/qemu_command.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 81e89fc..0580a37 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -457,7 +457,7 @@ qemuDomainSupportsNicdev(virDomainDefPtr def,
/* non-virtio ARM nics require legacy -net nic */
if (((def-os.arch == VIR_ARCH_ARMV7L) ||
(def-os.arch == VIR_ARCH_AARCH64))
-net-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
+strcmp(net-model, virtio))
return false;
return true;
@@ -1374,9 +1374,7 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr
def,
{
if (((def-os.arch == VIR_ARCH_ARMV7L) ||
(def-os.arch == VIR_ARCH_AARCH64))
-(STRPREFIX(def-os.machine, vexpress-) ||
-STREQ(def-os.machine, virt) ||
-STRPREFIX(def-os.machine, virt-))
+STRPREFIX(def-os.machine, vexpress-)
virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MMIO)) {
qemuDomainPrimeVirtioDeviceAddresses(
def, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO);
@@ -2501,6 +2499,12 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
continue;
+ if (((def-os.arch == VIR_ARCH_ARMV7L) ||
+ (def-os.arch == VIR_ARCH_AARCH64))
+ STREQ(def-os.machine, virt)
+ def-disks[i]-info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
+ continue;
+
if (def-disks[i]-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
{
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(virtio disk cannot have an address of type
'%s'),
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/6] Selective block device migration implementation
Thank you.
I will go through your comments just in case I'm going to push any other
changes.
Sorry for the inconvenience from my patches :-/.
Pavel
On Mon, May 25, 2015 at 5:58 PM, Michal Privoznik mpriv...@redhat.com
wrote:
On 21.05.2015 13:07, Pavel Boldin wrote:
The patchset represented in the mail thread implements the selective
block
device migration for the QEMU driver. This closes the referenced bug [1].
First the supplementary API implemented allowing for multiple key-values
pair
in the virTypedParameter arrays. This is used to pass the list of block
devices to migrate in the following patches. Unit tests for this are
added as
well. This is the subject of the first four patches.
Fifth patch is adding the necessary parameter `migrate_disks' and passes
it
through the QEMU driver call stack. The introduced `qemuMigrateDisk'
function
is then checks that the disk is to be migrated because it is in the
list. If
there is no list specified then the legacy check is used: the device is
migrate
if it is not shared, not readonly and has a source.
Sixth and the last patch is adding the necessary code to the `virsh'
utility
making it able for user to specify a comma-separated list of the block
device
names that are to be migrated.
The implemented Python bindings patch is to be presented.
Changes in v2:
* Addressed review comments
* Removed all the duplicates check in the commit 'multi-value
parameters in
virTypedParamsAdd*'
* reimplemented virTypedParamsPick as virTypedParamsFilter
* renamed virTypedParamsPackStrings to virTypedParamsAddStringList
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1203032
Pavel Boldin (6):
util: multi-value virTypedParameter
util: multi-value parameters in virTypedParamsAdd*
util: virTypedParams{Filter,PickStrings}
util: add virTypedParamsAddStringList
qemu: migration: selective block device migration
virsh: selective block device migration
include/libvirt/libvirt-domain.h | 9 ++
include/libvirt/libvirt-host.h | 16 +++
src/libvirt_public.syms | 7 +
src/qemu/qemu_driver.c | 66 ++---
src/qemu/qemu_migration.c| 174 +++
src/qemu/qemu_migration.h| 23 ++--
src/util/virtypedparam.c | 263
---
src/util/virtypedparam.h | 10 ++
tests/Makefile.am| 6 +
tests/virtypedparamtest.c| 291
+++
tools/virsh-domain.c | 23
tools/virsh.pod | 5 +-
12 files changed, 750 insertions(+), 143 deletions(-)
create mode 100644 tests/virtypedparamtest.c
So, I think we are almost there. I've pointed out a few things.
Moreover, as I've been reviewing I keep fixing the nits I'm raising. So
instead of me giving you headache of posting v3, I'll do that.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/4] docs: formatstorage: Update permissions docs
On 05/24/2015 07:20 AM, John Ferlan wrote: On 05/21/2015 04:03 PM, Cole Robinson wrote: - Don't redocument the permissions fields for backingstore, just point to the volume docs. - Clarify that owner/group are inherited from the parent directory at volume create/pool build time. - Clarify that permissions fields report runtime values too --- v3: New patch docs/formatstorage.html.in | 36 ++-- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 474abd6..f07bb5d 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -405,11 +405,17 @@ pools, which are mapped as a directory into the local filesystem namespace. It provides information about the permissions to use for the final directory when the pool is built. The -codemode/code element contains the octal permission set. The -codeowner/code element contains the numeric user ID. The codegroup/code -element contains the numeric group ID. The codelabel/code element -contains the MAC (eg SELinux) label string. s/.$/. There are 4 child elements. Fixed. +codemode/code element contains the octal permission set. +The codeowner/code element contains the numeric user ID. +The codegroup/code element contains the numeric group ID. +If codeowner/code or codegroup/code aren't specified when +creating a directory, the values are inherited from the parent +directory. The codelabel/code element contains the MAC (eg SELinux) +label string. span class=sinceSince 0.4.1/span +For running directory or filesystem based pools, these fields +will be filled with the values used by the existing directory. +span class=sinceSince 1.2.16/span /dd dtcodetimestamps/code/dt ddProvides timing information about the volume. Up to four @@ -583,15 +589,20 @@ volume format type value and the default pool format will be used. span class=sinceSince 0.4.1/span/dd dtcodepermissions/code/dt - ddProvides information about the default permissions to use + ddProvides information about the permissions to use when creating volumes. This is currently only useful for directory or filesystem based pools, where the volumes allocated are simple files. For pools where the volumes are device nodes, the hotplug scripts determine permissions. It contains 4 child elements. The s/It contains /There are/ Fixed. -codemode/code element contains the octal permission set. The -codeowner/code element contains the numeric user ID. The codegroup/code -element contains the numeric group ID. The codelabel/code element -contains the MAC (eg SELinux) label string. +codemode/code element contains the octal permission set. +The codeowner/code element contains the numeric user ID. +The codegroup/code element contains the numeric group ID. +If codeowner/code or codegroup/code aren't specified when +creating a supported volume, the values are inherited from the parent +directory. The codelabel/code element contains the MAC (eg SELinux) +label string. +For existing directory or filesystem based volumes, these fields +will be filled with the values used by the existing file. ^^^ the span used above for 1.2.16 ^^^ the span used above for 1.2.16 This was intentional, the pool permission syncing pre-dates 1.2.16, my patches only added it for volumes. I tried a git log grep to try and figure out when it was added but gave up after a couple minutes. So I left this as is and pushed. Thanks, Cole span class=sinceSince 0.4.1/span /dd dtcodecompat/code/dt @@ -659,11 +670,8 @@ span class=sinceSince 0.6.0/span/dd dtcodepermissions/code/dt ddProvides information about the permissions of the backing file. -It contains 4 child elements. The -codemode/code element contains the octal permission set. The -codeowner/code element contains the numeric user ID. The codegroup/code -element contains the numeric group ID. The codelabel/code element -contains the MAC (eg SELinux) label string. + See volume codepermissions/code documentation for explanation + of individual fields. span class=sinceSince 0.6.0/span /dd /dl -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] parallels: fix possible crash in case of errors in prlsdkLoadDomain
On 05/21/2015 04:49 PM, Maxim Nestratov wrote:
Cleanup code in prlsdkLoadDomain doesn't take into account the fact
if private domain structure along with freeing function is assigned
or not. In case it is, we shouldn't call it manually because
virDomainObjListRemove calls it and frees pdom.
Also, allocated def structure should be freed only if it's not
assigned to domain. Otherwise it will be called twice: one time by
virDomainObjListRemove and the second by prlsdkLoadDomain itself.
OK, pushed, thanks.
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
---
src/parallels/parallels_sdk.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 4d4582f..c4ad4eb 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1379,10 +1379,21 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
return dom;
error:
-if (dom !olddom)
+if (dom !olddom) {
+/* Domain isn't persistent means that we haven't yet set
+ * prlsdkDomObjFreePrivate and should call it manually
+ */
+if (!dom-persistent)
+prlsdkDomObjFreePrivate(pdom);
+
virDomainObjListRemove(privconn-domains, dom);
-virDomainDefFree(def);
-prlsdkDomObjFreePrivate(pdom);
+}
+/* Delete newly allocated def only if we haven't assigned it to domain
+ * Otherwise we will end up with domain having invalid def within it
+ */
+if (!dom)
+virDomainDefFree(def);
+
return NULL;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
On 05/25/2015 08:28 PM, zhang bo wrote: IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) Thanks; but this is not the right place for translation bugs. See https://www.redhat.com/archives/libvir-list/2015-March/msg00302.html for instructions on submitting translations to zanata. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
On 2015/5/26 10:48, Eric Blake wrote: On 05/25/2015 08:43 PM, Eric Blake wrote: On 05/25/2015 08:28 PM, zhang bo wrote: IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) Thanks; but this is not the right place for translation bugs. See https://www.redhat.com/archives/libvir-list/2015-March/msg00302.html for instructions on submitting translations to zanata. And we should really make that more prominent in HACKING; patches welcome... thank you Eric, I'm looking into zanata now and patches will be pushed later. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/4] conf: storage: Don't emit empty permissions block
On 05/24/2015 07:38 AM, John Ferlan wrote: On 05/21/2015 04:03 PM, Cole Robinson wrote: --- v3: New patch src/conf/storage_conf.c| 42 +- tests/storagepoolxml2xmlout/pool-netfs-gluster.xml | 2 -- tests/storagevolxml2xmlout/vol-gluster-dir.xml | 2 -- tests/storagevolxml2xmlout/vol-sheepdog.xml| 2 -- 4 files changed, 26 insertions(+), 22 deletions(-) ACK - although I suppose it could be combined with 2/4, but I'm OK with it being separate. I intentionally kept them separate since I didn't want to mix up functional changes with a cosmetic output change. Pushed this and patch #4 as is. Thanks for the review! - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] storage: conf: Don't set any default mode in the XML
On 05/24/2015 07:37 AM, John Ferlan wrote: On 05/21/2015 04:03 PM, Cole Robinson wrote: The XML parser sets a default mode if none is explicitly passed in. This is then used at pool/vol creation time, and unconditionally reported in the XML. The problem with this approach is that it's impossible for other code to determine if the user explicitly requested a storage mode. There are some cases where we want to make this distinction, but we currently can't. Handle mode parsing like we handle owner/group: if no value is passed in, set it to -1, and adjust the internal consumers to handle it. --- v3: Drop needless test churn Add docs about default mode docs/formatstorage.html.in | 4 +++ docs/schemas/storagecommon.rng | 8 +++-- src/conf/storage_conf.c| 34 +- src/storage/storage_backend.c | 20 + src/storage/storage_backend.h | 3 ++ src/storage/storage_backend_fs.c | 9 -- src/storage/storage_backend_logical.c | 4 ++- tests/storagepoolxml2xmlout/pool-netfs-gluster.xml | 1 - tests/storagevolxml2xmlout/vol-gluster-dir.xml | 1 - tests/storagevolxml2xmlout/vol-sheepdog.xml| 1 - 10 files changed, 51 insertions(+), 34 deletions(-) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index f07bb5d..ccde978 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -406,6 +406,8 @@ namespace. It provides information about the permissions to use for the final directory when the pool is built. The codemode/code element contains the octal permission set. +If codemode/code is unset when creating a directory, +the value 0755 is used. Consider The mode defaults to 0755 when not provided. The verbiage is unset reads strangly Thanks, fixed. The codeowner/code element contains the numeric user ID. The codegroup/code element contains the numeric group ID. If codeowner/code or codegroup/code aren't specified when @@ -595,6 +597,8 @@ files. For pools where the volumes are device nodes, the hotplug scripts determine permissions. It contains 4 child elements. The codemode/code element contains the octal permission set. +If codemode/code is unset when creating a supported volume, +the value 0600 is used. ditto but 0600 Also, wherever you point the backing store elements permissions to should be whichever default is correct for the backing store. that is from patch 1, you indicated that the elements are the same as one of these two elements, so to be technically correct be sure to redirect to either the 0755 or 0600 for which the backing store element would default to if not provided. It's already saying 'see volume docs' essentially, so this should be fine. Pushed with the change mentioned above. Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/po/zh_CN.po b/po/zh_CN.po index f5c7cf1..38cf6a9 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -20800,12 +20800,12 @@ msgstr 瀹夊叏 doi 瓒呰繃鏈€澶ч暱搴︼細%zu #: src/remote/remote_driver.c:2639 msgid caller ignores cookie or cookielen -msgstr 璋冨害绋嬪簭蹇界暐 cookie 鎴栬€?cookielen +msgstr 璋冪敤绋嬪簭蹇界暐 cookie 鎴栬€?cookielen #: src/remote/remote_driver.c:2648 src/remote/remote_driver.c:6106 #: src/remote/remote_driver.c:7136 msgid caller ignores uri_out -msgstr 璋冨害绋嬪簭蹇界暐 uri_out +msgstr 璋冪敤绋嬪簭蹇界暐 uri_out #: src/remote/remote_driver.c:2781 #, c-format @@ -20892,7 +20892,7 @@ msgstr 鏃?internalFlags 鏀寔 #: src/remote/remote_driver.c:7127 src/remote/remote_driver.c:7225 #: src/remote/remote_driver.c:7297 src/remote/remote_driver.c:7370 msgid caller ignores cookieout or cookieoutlen -msgstr 鍡茬敤绋嬪簭蹇界暐 cookieout 鎴栬€?cookieoutlen +msgstr 璋冪敤绋嬪簭蹇界暐 cookieout 鎴栬€?cookieoutlen #: src/remote/remote_driver.c:6386 #, c-format @@ -25090,7 +25090,7 @@ msgstr 鍦?Win32 骞冲彴涓儴鏀寔鎵ц鏂拌繘绋? #: src/util/vircommand.c:2224 msgid cannot mix caller fds with blocking execution -msgstr 鏃犳硶灏嗚皟搴︾▼搴?fd 涓庨樆姝㈡墽琛屾贩鍚? +msgstr 鏃犳硶灏嗚皟鐢ㄧ▼搴?fd 涓庨樆姝㈡墽琛屾贩鍚? #: src/util/vircommand.c:2230 msgid cannot mix string I/O with daemon -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] parallels: move up updating parameter in prlsdkLoadDomain
On 05/21/2015 04:49 PM, Maxim Nestratov wrote:
It is better to get all necessary parameters and check them on newly
created configuration before actually creating a domain with them or
applying them to an existing domain.
Signed-off-by: Maxim Nestratov mnestra...@parallels.com
Thanks, ACKED and pushed.
---
src/parallels/parallels_sdk.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 786e0ad..4d4582f 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1312,6 +1312,12 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
*s = '\0';
}
+pret = PrlVmCfg_GetAutoStart(sdkdom, autostart);
+prlsdkCheckRetGoto(pret, error);
+
+if (prlsdkGetDomainState(sdkdom, domainState) 0)
+goto error;
+
if (virDomainDefAddImplicitControllers(def) 0)
goto error;
@@ -1349,15 +1355,6 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
dom-privateDataFreeFunc = prlsdkDomObjFreePrivate;
dom-persistent = 1;
-if (prlsdkGetDomainState(sdkdom, domainState) 0)
-goto error;
-
-if (prlsdkConvertDomainState(domainState, envId, dom) 0)
-goto error;
-
-pret = PrlVmCfg_GetAutoStart(sdkdom, autostart);
-prlsdkCheckRetGoto(pret, error);
-
switch (autostart) {
case PAO_VM_START_ON_LOAD:
dom-autostart = 1;
@@ -1371,6 +1368,9 @@ prlsdkLoadDomain(parallelsConnPtr privconn,
goto error;
}
+if (prlsdkConvertDomainState(domainState, envId, dom) 0)
+goto error;
+
if (!pdom-sdkdom) {
pret = PrlHandle_AddRef(sdkdom);
prlsdkCheckRetGoto(pret, error);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] po: fix translation error for caller in zh_CN.po
I don't know why the codes in chinese become messy-code in this patch for me. I'm using thunderBird on *Windows*. The patch looked fine on my *Linux* machine. How does it appear at your side? If there's more work to do to make it look normal, please let me know. BTW: shall I change the headers of zh_CN.po at the meanwhile? eg. POT-Creation-Date, PO-Revision-Date, Last-Translator. On 2015/5/26 10:28, zhang bo wrote: IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit caller should be translated to 璋冪敤绋嬪簭 rather than 璋冨害绋嬪簭, which in fact refers to scheduler in English. --- po/zh_CN.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/po/zh_CN.po b/po/zh_CN.po index f5c7cf1..38cf6a9 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -20800,12 +20800,12 @@ msgstr 瀹夊叏 doi 瓒呰繃鏈€澶ч暱搴︼細%zu #: src/remote/remote_driver.c:2639 msgid caller ignores cookie or cookielen -msgstr 璋冨害绋嬪簭蹇界暐 cookie 鎴栬€?cookielen +msgstr 璋冪敤绋嬪簭蹇界暐 cookie 鎴栬€?cookielen #: src/remote/remote_driver.c:2648 src/remote/remote_driver.c:6106 #: src/remote/remote_driver.c:7136 msgid caller ignores uri_out -msgstr 璋冨害绋嬪簭蹇界暐 uri_out +msgstr 璋冪敤绋嬪簭蹇界暐 uri_out #: src/remote/remote_driver.c:2781 #, c-format @@ -20892,7 +20892,7 @@ msgstr 鏃?internalFlags 鏀寔 #: src/remote/remote_driver.c:7127 src/remote/remote_driver.c:7225 #: src/remote/remote_driver.c:7297 src/remote/remote_driver.c:7370 msgid caller ignores cookieout or cookieoutlen -msgstr 鍡茬敤绋嬪簭蹇界暐 cookieout 鎴栬€?cookieoutlen +msgstr 璋冪敤绋嬪簭蹇界暐 cookieout 鎴栬€?cookieoutlen #: src/remote/remote_driver.c:6386 #, c-format @@ -25090,7 +25090,7 @@ msgstr 鍦?Win32 骞冲彴涓儴鏀寔鎵ц鏂拌繘绋? #: src/util/vircommand.c:2224 msgid cannot mix caller fds with blocking execution -msgstr 鏃犳硶灏嗚皟搴︾▼搴?fd 涓庨樆姝㈡墽琛屾贩鍚? +msgstr 鏃犳硶灏嗚皟鐢ㄧ▼搴?fd 涓庨樆姝㈡墽琛屾贩鍚? #: src/util/vircommand.c:2230 msgid cannot mix string I/O with daemon -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] zfs: fix storagepoolxml2xml test
Commit 7c2d65d dropped setting default mode. Update zfs tests accordingly. --- tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml | 3 --- tests/storagepoolxml2xmlout/pool-zfs.xml | 3 --- 2 files changed, 6 deletions(-) diff --git a/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml b/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml index 89dcdbf..8d9be73 100644 --- a/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml +++ b/tests/storagepoolxml2xmlout/pool-zfs-sourcedev.xml @@ -10,8 +10,5 @@ /source target path/dev/zvol/testpool/path -permissions - mode0755/mode -/permissions /target /pool diff --git a/tests/storagepoolxml2xmlout/pool-zfs.xml b/tests/storagepoolxml2xmlout/pool-zfs.xml index c9e5df9..1a8de4f 100644 --- a/tests/storagepoolxml2xmlout/pool-zfs.xml +++ b/tests/storagepoolxml2xmlout/pool-zfs.xml @@ -9,8 +9,5 @@ /source target path/dev/zvol/testpool/path -permissions - mode0755/mode -/permissions /target /pool -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
