Re: [libvirt] [PATCH v3 2/2] virsh: report only filled values in 'nodecpustats'
On 01/28/2014 06:49 PM, Roman Bogorodskiy wrote: A set of fields for CPU stats could vary on different platforms, for example, FreeBSD doesn't report 'iowait'. Make virsh print out only the fields that were actually filled. --- tools/virsh-host.c | 119 - 1 file changed, 72 insertions(+), 47 deletions(-) This looks okay, but I still don't like the hash tables. ACK unless my version I've just sent gets pushed. Jan 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 v3 1/3] utils: Introduce functions for modprobe
On 01/30/2014 06:50 PM, John Ferlan wrote:
This patch adds functions for various usages of modprobe
Signed-off-by: John Ferlan jfer...@redhat.com
---
configure.ac | 6 ++
src/Makefile.am | 1 +
src/libvirt_private.syms | 7 ++
src/util/virkmod.c | 182
+++
src/util/virkmod.h | 34 +
5 files changed, 230 insertions(+)
create mode 100644 src/util/virkmod.c
create mode 100644 src/util/virkmod.h
diff --git a/configure.ac b/configure.ac
index 1670a41..eb11e3b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -405,6 +405,8 @@ AC_PATH_PROG([UDEVSETTLE], [udevsettle], [],
[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
AC_PATH_PROG([MODPROBE], [modprobe], [modprobe],
[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
+AC_PATH_PROG([RMMOD], [rmmod], [rmmod],
+ [/sbin:/usr/sbin:/usr/local/sbin:$PATH])
AC_PATH_PROG([OVSVSCTL], [ovs-vsctl], [ovs-vsctl],
[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
AC_PATH_PROG([SCRUB], [scrub], [scrub],
@@ -433,6 +435,10 @@ if test -n $MODPROBE; then
AC_DEFINE_UNQUOTED([MODPROBE],[$MODPROBE],
[Location or name of the modprobe program])
fi
+if test -n $RMMOD; then
+ AC_DEFINE_UNQUOTED([RMMOD],[$RMMOD],
+[Location or name of the rmmod program])
+fi
AC_DEFINE_UNQUOTED([SCRUB],[$SCRUB],
[Location or name of the scrub program (for wiping algorithms)])
diff --git a/src/Makefile.am b/src/Makefile.am
index abe0a51..b704045 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -125,6 +125,7 @@ UTIL_SOURCES =
\
util/virnetdevvportprofile.h util/virnetdevvportprofile.c \
util/virnetlink.c util/virnetlink.h \
util/virnodesuspend.c util/virnodesuspend.h \
+ util/virkmod.c util/virkmod.h \
util/virnuma.c util/virnuma.h \
util/virobject.c util/virobject.h \
util/virpci.c util/virpci.h \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 45f3117..372231c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1382,6 +1382,13 @@ virKeyFileLoadFile;
virKeyFileNew;
+# util/virkmod.h
+virKModConfig;
Do we need to export KModConfig as well?
+virKModIsBlacklisted;
+virKModLoad;
+virKModUnload;
+
+
# util/virlockspace.h
virLockSpaceAcquireResource;
virLockSpaceCreateResource;
diff --git a/src/util/virkmod.c b/src/util/virkmod.c
new file mode 100644
index 000..d39a2e3
--- /dev/null
+++ b/src/util/virkmod.c
@@ -0,0 +1,182 @@
+/*
+ * virkmod.c: helper APIs for managing kernel modules
+ *
+ * Copyright (C) 2014 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
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ */
+
+#include config.h
+#include viralloc.h
+#include virkmod.h
+#include vircommand.h
+#include virstring.h
+
+static int
+doModprobe(const char *opts, const char *module, char **outbuf, char
**errbuf)
+{
+int ret = -1;
+virCommandPtr cmd = NULL;
+
+cmd = virCommandNew(MODPROBE);
+if (opts)
+virCommandAddArg(cmd, opts);
+if (module)
+virCommandAddArg(cmd, module);
+if (outbuf)
+virCommandSetOutputBuffer(cmd, outbuf);
+if (errbuf)
+virCommandSetErrorBuffer(cmd, errbuf);
+
+if (virCommandRun(cmd, NULL) 0)
+goto cleanup;
+
+ret = 0;
+
+cleanup:
+virCommandFree(cmd);
+return ret;
+}
+
+static int
+doRmmod(const char *module, char **errbuf)
+{
+int ret = -1;
+virCommandPtr cmd = NULL;
+
+cmd = virCommandNewArgList(RMMOD, module, NULL);
+virCommandSetErrorBuffer(cmd, errbuf);
+
+if (virCommandRun(cmd, NULL) 0)
+goto cleanup;
+
+ret = 0;
+
+cleanup:
+virCommandFree(cmd);
+return ret;
+}
+
+/**
+ * virKModConfig:
+ *
+ * Get the current kernel module configuration
+ *
+ * Returns NULL on failure or a pointer to the output which
+ * must be VIR_FREE()'d by the caller
+ */
+char *
+virKModConfig(void)
+{
+char *outbuf = NULL;
+
+if (doModprobe(-c, NULL, outbuf, NULL) 0)
+return NULL;
+
+return
Re: [libvirt] [PATCH v3 2/3] tests: Add test for new virkmod functions
On 01/30/2014 06:50 PM, John Ferlan wrote:
Adding tests for new virKMod{Config|Load|Unload}() API's.
A test for virKModIsBlacklisted() would require some setup which cannot
be assumed.
Signed-off-by: John Ferlan jfer...@redhat.com
---
.gitignore | 2 +
tests/Makefile.am | 5 ++
tests/virkmodtest.c | 183
3 files changed, 190 insertions(+)
create mode 100644 tests/virkmodtest.c
diff --git a/.gitignore b/.gitignore
index 96b7211..79437e9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -70,6 +70,7 @@
/docs/libvirt-refs.xml
/docs/search.php
/docs/todo.html.in
+/examples/domain-events/
/examples/object-events/event-test
/examples/dominfo/info1
/examples/domsuspend/suspend
Unrelated change.
ACK to the rest (with /sbin/modprobe changed to MODPROBE)
Jan
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 v3 3/3] Honor blacklist for modprobe command
On 01/30/2014 06:50 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1045124
When loading modules, libvirt does not honor the modprobe blacklist.
Use the new virKModLoad() API in order to attempt load with blacklist check.
Use the new virKModIsBlacklisted() API to check if the failure to load
was due to the blacklist
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/util/virpci.c | 28 +++-
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index e2d222e..41735eb 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -42,6 +42,7 @@
#include vircommand.h
#include virerror.h
#include virfile.h
+#include virkmod.h
#include virstring.h
#include virutil.h
@@ -990,18 +991,26 @@ recheck:
VIR_FREE(drvpath);
if (!probed) {
-const char *const probecmd[] = { MODPROBE, driver, NULL };
+char *errbuf = NULL;
probed = true;
-if (virRun(probecmd, NULL) 0) {
-char ebuf[1024];
-VIR_WARN(failed to load driver %s: %s, driver,
- virStrerror(errno, ebuf, sizeof(ebuf)));
-return -1;
+if ((errbuf = virKModLoad(driver, true))) {
+VIR_WARN(failed to load driver %s: %s, driver, errbuf);
+VIR_FREE(errbuf);
+goto cleanup;
}
goto recheck;
}
+/* If we know failure was because of blacklist, let's report that */
+if (virKModIsBlacklisted(driver)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to load PCI stub module %s:
+ administratively prohibited),
+ driver);
+}
+
+cleanup:
return -1;
}
@@ -1313,9 +1322,10 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
virPCIDeviceList *inactiveDevs)
{
if (virPCIProbeStubDriver(dev-stubDriver) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Failed to load PCI stub module %s),
- dev-stubDriver);
+if (virGetLastError() == NULL)
This seems to be the only caller of virPCIProbeStubDriver.
You could just report the error unconditionally inside it.
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to load PCI stub module %s),
+ dev-stubDriver);
return -1;
}
Jan
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] qemu: be sure we're using the updated value of backend during hotplug
On 02/04/2014 12:24 PM, Laine Stump wrote: Probable fix for https://bugzilla.redhat.com/show_bug.cgi?id=1056360 commit f094aaac changed qemuPrepareHostdevPCIDevices() such that it may modify the backend (vfio vs. legacy kvm) setting in the virHostdevDef. However, qemuDomainAttachHostPciDevice() (used by hotplug) copies the backend setting into a local *before* calling qemuPrepareHostdevPCIDevices(), and then later makes a decision based on that pre-change value. The result is that, if the backend had been set to default (i.e. not specified in the config) and was later updated to VFIO by qemuPrepareHostdevPCIDevices(), the qemu process' MacMemLock is not increased (as is required for VFIO device assignment). This patch delays making the local copy of backend until after its potential modification. --- src/qemu/qemu_hotplug.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) ACK Jan 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 v3 2/3] tests: Add test for new virkmod functions
On 02/04/2014 03:01 PM, John Ferlan wrote:
On 02/04/2014 06:06 AM, Ján Tomko wrote:
On 01/30/2014 06:50 PM, John Ferlan wrote:
Adding tests for new virKMod{Config|Load|Unload}() API's.
A test for virKModIsBlacklisted() would require some setup which cannot
be assumed.
Signed-off-by: John Ferlan jfer...@redhat.com
---
.gitignore | 2 +
tests/Makefile.am | 5 ++
tests/virkmodtest.c | 183
3 files changed, 190 insertions(+)
create mode 100644 tests/virkmodtest.c
diff --git a/.gitignore b/.gitignore
index 96b7211..79437e9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -70,6 +70,7 @@
/docs/libvirt-refs.xml
/docs/search.php
/docs/todo.html.in
+/examples/domain-events/
/examples/object-events/event-test
/examples/dominfo/info1
/examples/domsuspend/suspend
Unrelated change.
OK - I'll send this separately (trivially).
Actually, domain-events has been renamed to object-events by 950c2a5, it seems
there are some leftovers in your local git repository.
Jan
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 v3 3/3] Honor blacklist for modprobe command
On 02/04/2014 03:29 PM, John Ferlan wrote:
On 02/04/2014 06:06 AM, Ján Tomko wrote:
On 01/30/2014 06:50 PM, John Ferlan wrote:
+VIR_FREE(errbuf);
+goto cleanup;
}
goto recheck;
}
+/* If we know failure was because of blacklist, let's report that */
+if (virKModIsBlacklisted(driver)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to load PCI stub module %s:
+ administratively prohibited),
+ driver);
+}
+
+cleanup:
return -1;
}
@@ -1313,9 +1322,10 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
virPCIDeviceList *inactiveDevs)
{
if (virPCIProbeStubDriver(dev-stubDriver) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Failed to load PCI stub module %s),
- dev-stubDriver);
+if (virGetLastError() == NULL)
This seems to be the only caller of virPCIProbeStubDriver.
You could just report the error unconditionally inside it.
Attempting to make the differentiation between load failed load failed
because of administratively prohibited means an additional check or two
in the caller.
I meant that right now virPCIProbeStubDriver is only called from here and if
it did not report an error, we will report one here.
It seemed cleaner not to report an error here and make virPCIProbeStubDriver
report an error in all cases (not just when the module is blacklisted and/or
on OOM in virPCIDriverDir).
Furthermore if something that virPCIProbeStubDriver() called provided
some other error wouldn't it be better to not overwrite the message? If
the virAsprintf() called by virPCIDriverDir() failed because of memory
allocation, then which error message would be displayed without the
virGetLastError() check? I guess I'm not 100% clear in my mind which
error message would get displayed...
Only the last reported error gets displayed, but both will get logged.
Jan
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 v3 3/3] Honor blacklist for modprobe command
On 02/04/2014 04:29 PM, John Ferlan wrote:
On 02/04/2014 10:10 AM, Ján Tomko wrote:
On 02/04/2014 03:29 PM, John Ferlan wrote:
On 02/04/2014 06:06 AM, Ján Tomko wrote:
On 01/30/2014 06:50 PM, John Ferlan wrote:
+VIR_FREE(errbuf);
+goto cleanup;
}
goto recheck;
}
+/* If we know failure was because of blacklist, let's report that */
+if (virKModIsBlacklisted(driver)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to load PCI stub module %s:
+ administratively prohibited),
+ driver);
+}
+
+cleanup:
return -1;
}
@@ -1313,9 +1322,10 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
virPCIDeviceList *inactiveDevs)
{
if (virPCIProbeStubDriver(dev-stubDriver) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Failed to load PCI stub module %s),
- dev-stubDriver);
+if (virGetLastError() == NULL)
This seems to be the only caller of virPCIProbeStubDriver.
You could just report the error unconditionally inside it.
Attempting to make the differentiation between load failed load failed
because of administratively prohibited means an additional check or two
in the caller.
I meant that right now virPCIProbeStubDriver is only called from here and if
it did not report an error, we will report one here.
It seemed cleaner not to report an error here and make virPCIProbeStubDriver
report an error in all cases (not just when the module is blacklisted and/or
on OOM in virPCIDriverDir).
oh - ah... light dawns on marblehead.
Furthermore if something that virPCIProbeStubDriver() called provided
some other error wouldn't it be better to not overwrite the message? If
the virAsprintf() called by virPCIDriverDir() failed because of memory
allocation, then which error message would be displayed without the
virGetLastError() check? I guess I'm not 100% clear in my mind which
error message would get displayed...
Only the last reported error gets displayed, but both will get logged.
Jan
The last message is what was important to someone using virt-install
so a git diff master src/util/virpci.c then has the following...
Basically an adjustment cleanup: label in order to handle both
error conditions and removal of the error message from the caller
(if you want to see a v4 I'll send it, but hopefully this is OK):
diff --git a/src/util/virpci.c b/src/util/virpci.c
index e2d222e..c3d211f 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -42,6 +42,7 @@
#include vircommand.h
#include virerror.h
#include virfile.h
+#include virkmod.h
#include virstring.h
#include virutil.h
@@ -990,18 +991,32 @@ recheck:
VIR_FREE(drvpath);
if (!probed) {
-const char *const probecmd[] = { MODPROBE, driver, NULL };
+char *errbuf = NULL;
probed = true;
-if (virRun(probecmd, NULL) 0) {
-char ebuf[1024];
-VIR_WARN(failed to load driver %s: %s, driver,
- virStrerror(errno, ebuf, sizeof(ebuf)));
-return -1;
+if ((errbuf = virKModLoad(driver, true))) {
+VIR_WARN(failed to load driver %s: %s, driver, errbuf);
Can you do virReportError here with the contents of errbuf (and return instead
of jumping to cleanup)?
+VIR_FREE(errbuf);
+goto cleanup;
}
goto recheck;
}
+cleanup:
+/* If we know failure was because of blacklist, let's report that;
+ * otherwise, report a more generic failure message
+ */
+if (virKModIsBlacklisted(driver)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to load PCI stub module %s:
+ administratively prohibited),
+ driver);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Failed to load PCI stub module %s),
+ driver);
+}
+
return -1;
}
@@ -1312,12 +1327,8 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
virPCIDeviceList *activeDevs,
virPCIDeviceList *inactiveDevs)
{
-if (virPCIProbeStubDriver(dev-stubDriver) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(Failed to load PCI stub module %s),
- dev-stubDriver);
+if (virPCIProbeStubDriver(dev-stubDriver) 0)
return -1;
-}
if (activeDevs virPCIDeviceListFind(activeDevs, dev)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
ACK
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] 1.2.0 segfault on Centos 6
On 02/04/2014 11:23 PM, Jiri Denemark wrote:
On Tue, Feb 04, 2014 at 17:02:41 +0100, Franky Van Liedekerke wrote:
Hi,
using libvirt 1.2.0 on a up-to-date Centos6.5 machine leads to
occasional segmentation faults (see below).
Sometimes it runs for 5 minutes, sometimes for an hour, but after that
the result is always the same: segfault after some weird qom-list, that
apparently the qemu version on centos doesn't know. Has 1.2.1 a known
fix for this?
I believe the following patch should fix the crash. I'll do some testing
tomorrow and send it as a proper patch afterwards:
diff --git i/src/qemu/qemu_monitor.c w/src/qemu/qemu_monitor.c
index a968901..cdd817f 100644
--- i/src/qemu/qemu_monitor.c
+++ w/src/qemu/qemu_monitor.c
@@ -1019,7 +1019,9 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
virDomainObjPtr vm,
const char *curpath)
{
-size_t i, j, npaths = 0, nprops = 0;
+size_t i, j;
+int npaths = 0;
+int nprops = 0;
int ret = 0;
char *nextpath = NULL;
qemuMonitorJSONListPathPtr *paths = NULL;
@@ -1045,6 +1047,8 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
VIR_DEBUG(Searching for Balloon Object Path starting at %s, curpath);
npaths = qemuMonitorJSONGetObjectListPaths(mon, curpath, paths);
+if (npaths 0)
+return -1;
for (i = 0; i npaths ret == 0; i++) {
@@ -1061,6 +1065,11 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
* then this version of qemu/kvm does not support the feature.
*/
nprops = qemuMonitorJSONGetObjectListPaths(mon, nextpath,
bprops);
+if (nprops 0) {
+ret = -1;
+goto cleanup;
+}
+
for (j = 0; j nprops; j++) {
if (STREQ(bprops[j]-name, guest-stats-polling-interval)) {
VIR_DEBUG(Found Balloon Object Path %s, nextpath);
npaths and nprops are also compared against size_t in the cleanup section.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Generate a valid imagelabel even for type 'none'
Commit 2ce63c1 added imagelabel generation when relabeling is turned off. But we weren't filling out the sensitivity for type 'none' labels, resulting in an invalid label: $ virsh managedsave domain error: unable to set security context 'system_u:object_r:svirt_image_t' on fd 28: Invalid argument --- src/security/security_selinux.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index aa47667..448f686 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -670,7 +670,14 @@ virSecuritySELinuxGenSecurityLabel(virSecurityManagerPtr mgr, break; case VIR_DOMAIN_SECLABEL_NONE: -/* no op */ +if (virSecuritySELinuxMCSGetProcessRange(sens, + catMin, + catMax) 0) +goto cleanup; + +if (VIR_STRDUP(mcs, sens) 0) +goto cleanup; + break; default: -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Generate a valid imagelabel even for type 'none'
On 02/05/2014 07:54 PM, Eric Blake wrote: On 02/05/2014 11:47 AM, Ján Tomko wrote: Commit 2ce63c1 added imagelabel generation when relabeling is turned off. But we weren't filling out the sensitivity for type 'none' labels, resulting in an invalid label: $ virsh managedsave domain error: unable to set security context 'system_u:object_r:svirt_image_t' on fd 28: Invalid argument --- src/security/security_selinux.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) ACK. Thanks, pushed. Jan 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 v3 1/2] BSD: implement nodeGetCPUStats
On 02/04/2014 10:24 AM, Ján Tomko wrote: On 01/28/2014 06:49 PM, Roman Bogorodskiy wrote: Implementation obtains CPU usage information using kern.cp_time and kern.cp_times sysctl(8)s and reports CPU utilization. --- include/libvirt/libvirt.h.in | 8 src/nodeinfo.c | 104 +++ tools/virsh-host.c | 11 - 3 files changed, 121 insertions(+), 2 deletions(-) ACK Pushed now. Jan 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] virsh: only report filled values in nodecpustats
On 02/06/2014 01:14 PM, Michal Privoznik wrote: On 04.02.2014 10:22, Ján Tomko wrote: Rewrite the function to use an array instead of a struct, translating the field names to int via an enum. --- tools/virsh-host.c | 126 +++-- 1 file changed, 64 insertions(+), 62 deletions(-) ACK, I like this one more. Now pushed. Thank you for the reviews! Jan 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 v2] qemuxml2argvtest: Set timezone
On 02/06/2014 03:11 PM, Michal Privoznik wrote: With my recent work on the test, both time() and localtime() are used. While mocking the former one, we get predictable result for UTC. But since the latter function uses timezone to get local time, the result of localtime() is not so predictive. Therefore, we must set the TZ variable at the beginning of the test. To be able to catch some things that work just by a blind chance, I'm choosing a virtual timezone that (hopefully) no libvirt developer resides in. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- .../qemuxml2argv-clock-localtime-basis-localtime.args | 2 +- tests/qemuxml2argvtest.c | 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-) ACK, please push. Jan 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 1/2] qemuxml2argvmock: Mock time() on non-linux platforms too
On 02/06/2014 02:33 PM, Michal Privoznik wrote: The qemuxml2argvtest is run on more platforms than linux. For instance FreeBSD. On these platforms we are, however, not mocking time() which results in current time being fetched from system and hence tests number 32 and 33 failing. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/qemuxml2argvmock.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 1/2] Split out bind() from virPortAllocatorAcquire
---
src/util/virportallocator.c | 72 -
1 file changed, 45 insertions(+), 27 deletions(-)
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 694f191..5d51434 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -99,19 +99,59 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name,
return pa;
}
+static int virPortAllocatorBindToPort(bool *used,
+ unsigned short port)
+{
+struct sockaddr_in addr = {
+.sin_family = AF_INET,
+.sin_port = htons(port),
+.sin_addr.s_addr = htonl(INADDR_ANY)
+};
+int reuse = 1;
+int ret = -1;
+int fd = -1;
+
+*used = false;
+
+fd = socket(PF_INET, SOCK_STREAM, 0);
+if (fd 0) {
+virReportSystemError(errno, %s, _(Unable to open test socket));
+goto cleanup;
+}
+
+if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)reuse,
+ sizeof(reuse)) 0) {
+virReportSystemError(errno, %s,
+ _(Unable to set socket reuse addr flag));
+goto cleanup;
+}
+
+if (bind(fd, (struct sockaddr*)addr, sizeof(addr)) 0) {
+if (errno == EADDRINUSE) {
+*used = true;
+ret = 0;
+} else {
+virReportSystemError(errno, _(Unable to bind to port %d), port);
+}
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FORCE_CLOSE(fd);
+return ret;
+}
+
int virPortAllocatorAcquire(virPortAllocatorPtr pa,
unsigned short *port)
{
int ret = -1;
size_t i;
-int fd = -1;
*port = 0;
virObjectLock(pa);
for (i = pa-start; i = pa-end !*port; i++) {
-int reuse = 1;
-struct sockaddr_in addr;
bool used = false;
if (virBitmapGetBit(pa-bitmap,
@@ -124,31 +164,10 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
if (used)
continue;
-addr.sin_family = AF_INET;
-addr.sin_port = htons(i);
-addr.sin_addr.s_addr = htonl(INADDR_ANY);
-fd = socket(PF_INET, SOCK_STREAM, 0);
-if (fd 0) {
-virReportSystemError(errno, %s,
- _(Unable to open test socket));
+if (virPortAllocatorBindToPort(used, i) 0)
goto cleanup;
-}
-
-if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, (void*)reuse,
sizeof(reuse)) 0) {
-virReportSystemError(errno, %s,
- _(Unable to set socket reuse addr flag));
-goto cleanup;
-}
-if (bind(fd, (struct sockaddr*)addr, sizeof(addr)) 0) {
-if (errno != EADDRINUSE) {
-virReportSystemError(errno,
- _(Unable to bind to port %zu), i);
-goto cleanup;
-}
-/* In use, try next */
-VIR_FORCE_CLOSE(fd);
-} else {
+if (!used) {
/* Add port to bitmap of reserved ports */
if (virBitmapSetBit(pa-bitmap,
i - pa-start) 0) {
@@ -168,7 +187,6 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
}
cleanup:
virObjectUnlock(pa);
-VIR_FORCE_CLOSE(fd);
return ret;
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 0/2] Support IPv6 in port allocator
v1: Support IPv6 in port allocator https://www.redhat.com/archives/libvir-list/2013-October/msg7.html v2: https://www.redhat.com/archives/libvir-list/2013-October/msg01313.html bind to v4 and v6 separately v3: fix the embarrasing bug of hardcoding AF_INET anyway added a test that mocks a v4-only system even on systems with IPv6 compiled in Ján Tomko (2): Split out bind() from virPortAllocatorAcquire Support IPv6 in port allocator src/util/virportallocator.c | 106 +++ tests/virportallocatortest.c | 68 +-- 2 files changed, 143 insertions(+), 31 deletions(-) -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 2/2] Support IPv6 in port allocator
Also try to bind on IPv6 to check if the port is occupied.
Change the mocked bind in the test to return EADDRINUSE
for some ports only for the IPv4/IPv6 socket if we're testing
on a host with IPv6 compiled in.
Also mock socket() to make it fail with EAFNOTSUPPORTED
if LIBVIRT_TEST_IPV4ONLY is set in the environment, to
simulate a host without IPv6 support in the kernel. The
tests are repeated again with this variable set.
https://bugzilla.redhat.com/show_bug.cgi?id=1025407
---
src/util/virportallocator.c | 46 +-
tests/virportallocatortest.c | 68 ++--
2 files changed, 104 insertions(+), 10 deletions(-)
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 5d51434..06174b0 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -100,21 +100,45 @@ virPortAllocatorPtr virPortAllocatorNew(const char *name,
}
static int virPortAllocatorBindToPort(bool *used,
- unsigned short port)
+ unsigned short port,
+ int family)
{
-struct sockaddr_in addr = {
+struct sockaddr_in6 addr6 = {
+.sin6_family = AF_INET6,
+.sin6_port = htons(port),
+.sin6_addr = IN6ADDR_ANY_INIT,
+};
+struct sockaddr_in addr4 = {
.sin_family = AF_INET,
.sin_port = htons(port),
.sin_addr.s_addr = htonl(INADDR_ANY)
};
+struct sockaddr* addr;
+size_t addrlen;
+int v6only = 1;
int reuse = 1;
int ret = -1;
int fd = -1;
+bool ipv6 = false;
+
+if (family == AF_INET6) {
+addr = (struct sockaddr*)addr6;
+addrlen = sizeof(addr6);
+ipv6 = true;
+} else if (family == AF_INET) {
+addr = (struct sockaddr*)addr4;
+addrlen = sizeof(addr4);
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, _(Unknown family %d), family);
+return -1;
+}
*used = false;
-fd = socket(PF_INET, SOCK_STREAM, 0);
+fd = socket(family, SOCK_STREAM, 0);
if (fd 0) {
+if (errno == EAFNOSUPPORT)
+return 0;
virReportSystemError(errno, %s, _(Unable to open test socket));
goto cleanup;
}
@@ -126,7 +150,14 @@ static int virPortAllocatorBindToPort(bool *used,
goto cleanup;
}
-if (bind(fd, (struct sockaddr*)addr, sizeof(addr)) 0) {
+if (ipv6 setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)v6only,
+ sizeof(v6only)) 0) {
+virReportSystemError(errno, %s,
+ _(Unable to set IPV6_V6ONLY flag));
+goto cleanup;
+}
+
+if (bind(fd, addr, addrlen) 0) {
if (errno == EADDRINUSE) {
*used = true;
ret = 0;
@@ -152,7 +183,7 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
virObjectLock(pa);
for (i = pa-start; i = pa-end !*port; i++) {
-bool used = false;
+bool used = false, v6used = false;
if (virBitmapGetBit(pa-bitmap,
i - pa-start, used) 0) {
@@ -164,10 +195,11 @@ int virPortAllocatorAcquire(virPortAllocatorPtr pa,
if (used)
continue;
-if (virPortAllocatorBindToPort(used, i) 0)
+if (virPortAllocatorBindToPort(v6used, i, AF_INET6) 0 ||
+virPortAllocatorBindToPort(used, i, AF_INET) 0)
goto cleanup;
-if (!used) {
+if (!used !v6used) {
/* Add port to bitmap of reserved ports */
if (virBitmapSetBit(pa-bitmap,
i - pa-start) 0) {
diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
index 721356e..7fe18df 100644
--- a/tests/virportallocatortest.c
+++ b/tests/virportallocatortest.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Red Hat, Inc.
+ * Copyright (C) 2013-2014 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
@@ -19,6 +19,8 @@
*/
#include config.h
+#include stdlib.h
+#include virfile.h
#ifdef MOCK_HELPER
# include internal.h
@@ -26,6 +28,47 @@
# include errno.h
# include arpa/inet.h
# include netinet/in.h
+# include stdio.h
+# include dlfcn.h
+
+static bool host_has_ipv6 = false;
+static int (*realsocket)(int domain, int type, int protocol);
+
+static void init_syms(void)
+{
+int fd;
+
+if (realsocket)
+return;
+
+realsocket = dlsym(RTLD_NEXT, socket);
+
+if (!realsocket) {
+fprintf(stderr, Unable to find 'socket' symbol\n);
+abort();
+}
+
+fd = realsocket(AF_INET6, SOCK_STREAM, 0);
+if (fd 0)
+return;
+
+host_has_ipv6 = true;
+VIR_FORCE_CLOSE(fd);
+}
+
+int socket(int domain,
+ int type,
+ int protocol)
+{
+init_syms();
+
+if (getenv(LIBVIRT_TEST_IPV4ONLY) domain ==
Re: [libvirt] [PATCH] virpcitest: fix coverity issues
On 02/06/2014 05:36 PM, Pavel Hrdina wrote:
On 6.2.2014 16:48, Eric Blake wrote:
On 02/06/2014 08:18 AM, Pavel Hrdina wrote:
diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 994b300..8ff3b1d 100644
--- a/tests/virpcitest.c
+++ b/tests/virpcitest.c
@@ -248,6 +248,7 @@ testVirPCIDeviceDetachSingle(const void *opaque)
if (!dev)
goto cleanup;
+/* coverity[freed_arg] */
if (virPCIDeviceSetStubDriver(dev, pci-stub) 0 ||
virPCIDeviceDetach(dev, NULL, NULL) 0)
goto cleanup;
Is this something where an sa_assert() could do the trick in a more
tool-agnostic manner?
I'm looking at the code again and probably the issue found by coverity
could happen if the STRDUP fails inside of the
virPCIDeviceSetStubDriver.
If STRDUP fails, then SetStubDriver should return -1 and we wouldn't be
calling DeviceDetach.
It would be nice if we could 'fix' virPCIDeviceSetStubDriver in a way coverity
would understand (fixing both complaints, but still checking if we're not
calling virKModLoad with a NULL argument). Does this patch make it any happier?
Jan
diff --git a/src/util/virpci.c b/src/util/virpci.c
index c3d211f..88002c9 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1657,7 +1657,7 @@ int
virPCIDeviceSetStubDriver(virPCIDevicePtr dev, const char *driver)
{
VIR_FREE(dev-stubDriver);
-return driver ? VIR_STRDUP(dev-stubDriver, driver) : 0;
+return VIR_STRDUP(dev-stubDriver, driver);
}
const char *
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Rename 'index' in virCapabilitiesGetCpusForNode
This shadows the index function on some systems (RHEL-6.4, FreeBSD 9):
../../src/conf/capabilities.c: In function 'virCapabilitiesGetCpusForNode':
../../src/conf/capabilities.c:1005: warning: declaration of'index'
shadows a global declaration [-Wshadow]
/usr/include/strings.h:57: warning: shadowed declaration is here [-Wshadow]
---
src/conf/capabilities.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
Pushed as a build-breaker.
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 78f65cb..c1c4ab8 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -1002,11 +1002,11 @@ virCapabilitiesGetCpusForNode(virCapsPtr caps,
{
virCapsHostNUMACellPtr cell = NULL;
size_t cpu;
-size_t index;
+size_t i;
/* The numa node numbers can be non-contiguous. Ex: 0,1,16,17. */
-for (index = 0; index caps-host.nnumaCell; index++) {
-if (caps-host.numaCell[index]-num == node) {
-cell = caps-host.numaCell[index];
+for (i = 0; i caps-host.nnumaCell; i++) {
+if (caps-host.numaCell[i]-num == node) {
+cell = caps-host.numaCell[i];
break;
}
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix leaks in vircapstest
Coverity complains about cell_cpus being leaked on error
and valgrind shows 'caps' is leaked on success.
Introduced in eb64e87.
---
tests/vircapstest.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tests/vircapstest.c b/tests/vircapstest.c
index a40771d..4264e9e 100644
--- a/tests/vircapstest.c
+++ b/tests/vircapstest.c
@@ -40,7 +40,7 @@ static virCapsPtr
buildNUMATopology(int seq)
{
virCapsPtr caps;
-virCapsHostNUMACellCPUPtr cell_cpus;
+virCapsHostNUMACellCPUPtr cell_cpus = NULL;
int core_id, cell_id;
int id;
@@ -75,6 +75,8 @@ buildNUMATopology(int seq)
return caps;
error:
+virCapabilitiesClearHostNUMACellCPUTopology(cell_cpus, MAX_CPUS_IN_CELL);
+VIR_FREE(cell_cpus);
virObjectUnref(caps);
return NULL;
@@ -87,7 +89,7 @@ test_virCapabilitiesGetCpusForNodemask(const void *data
ATTRIBUTE_UNUSED)
const char *nodestr = 3,4,5,6;
virBitmapPtr nodemask = NULL;
virBitmapPtr cpumap = NULL;
-virCapsPtr caps;
+virCapsPtr caps = NULL;
int mask_size = 8;
int ret = -1;
@@ -107,6 +109,7 @@ test_virCapabilitiesGetCpusForNodemask(const void *data
ATTRIBUTE_UNUSED)
ret = 0;
error:
+virObjectUnref(caps);
virBitmapFree(nodemask);
virBitmapFree(cpumap);
return ret;
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/8] storage: lvm: Separate creating of the volume from building
On 02/12/2014 07:29 AM, Michael Chapman wrote:
On Thu, 16 Jan 2014, Peter Krempa wrote:
On 01/09/14 23:40, Eric Blake wrote:
On 01/06/2014 09:44 AM, Peter Krempa wrote:
Separate the steps to create libvirt's volume metadata from the actual
volume building process. This is already done for regular file based
pools to allow job support for storage APIs.
---
src/storage/storage_backend_logical.c | 60
+--
1 file changed, 37 insertions(+), 23 deletions(-)
ACK; I'm borderline whether this should wait for the release, though.
Now that 1.2.2 is out I've pushed this one and the rest with the same
complaint.
Hi Peter,
This breaks volume creation in an LVM pool:
# cat volume.xml
volume type='block'
nametest/name
capacity unit='bytes'10737418240/capacity
allocation unit='bytes'10737418240/allocation
/volume
# virsh vol-create vg volume.xml
error: Failed to create vol from volume.xml
error: key in virGetStorageVol must not be NULL
The problem is a storage backend's createVol function is expected to set an
appropriate key, but for an LVM volume this isn't done now until buildVol is
called. The LV's UUID is used as the volume's key.
For the disk backend, volume keys are also generated in buildVol after the
volume is created.
IMO we should revert commits:
commit af1fb38f55d4fb87e0fcaee1e973fa9c6713b1e6
storage: lvm: Separate creating of the volume from building
commit 67ccf91bf29488783bd1fda46b362450f71a2078
storage: disk: Separate creating of the volume from building
unless we have a really good reason not to.
vol-clone and vol-create-from are even worse: libvirtd crashes since the NULL
return from virGetStorageVol isn't handled in storageVolCreateXMLFrom. We
probably need:
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1828,6 +1828,10 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
pool-volumes.objs[pool-volumes.count++] = newvol;
volobj = virGetStorageVol(obj-conn, pool-def-name, newvol-name,
newvol-key, NULL, NULL);
+if (!volobj) {
+pool-volumes.count--;
+goto cleanup;
+}
/* Drop the pool lock during volume allocation */
pool-asyncjobs++;
ACK to this patch. No matter what way we fix the actual bug, virGetStorageVol
can still return NULL when we're out of memory. Would you like to write a
commit message and send it as a formal patch, or shall I do it?
Jan
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 v2 2/2] vircapstest: Introduce virCapabilitiesGetCpusForNodemask test
On 02/12/2014 12:43 PM, John Ferlan wrote: Coverity has complained this morning about this change... see below. Please send a patch to resolve. A patch has been sent this morning, please review: https://www.redhat.com/archives/libvir-list/2014-February/msg00660.html 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] Fix leaks in vircapstest
On 02/12/2014 02:32 PM, Eric Blake wrote: On 02/12/2014 02:47 AM, Ján Tomko wrote: Coverity complains about cell_cpus being leaked on error and valgrind shows 'caps' is leaked on success. Introduced in eb64e87. --- tests/vircapstest.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) ACK Thank you, now pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Partially revert separation of volume creating and building
The storage driver expects 'createVol' to fill out the volume key. For disk and logical backends, we generate the key only after the volume has been built. Revert the separation commits for these backends. Ján Tomko (2): Revert storage: lvm: Separate creating of the volume from building Revert storage: disk: Separate creating of the volume from building src/storage/storage_backend_disk.c| 44 - src/storage/storage_backend_logical.c | 60 ++- 2 files changed, 37 insertions(+), 67 deletions(-) -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Revert storage: disk: Separate creating of the volume from building
This reverts commit 67ccf91bf29488783bd1fda46b362450f71a2078.
We only generate the volume key after we've built it, but the storage
driver expects it to be filled after createVol finishes.
Squash the volume building back with creating to fulfill this
expectation.
---
src/storage/storage_backend_disk.c | 44 --
1 file changed, 14 insertions(+), 30 deletions(-)
diff --git a/src/storage/storage_backend_disk.c
b/src/storage/storage_backend_disk.c
index aa3b72f..a7a7d0e 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -617,43 +617,28 @@ virStorageBackendDiskPartBoundries(virStoragePoolObjPtr
pool,
static int
virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
-if (vol-target.encryption != NULL) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(storage pool does not support encrypted volumes));
-return -1;
-}
-
-vol-type = VIR_STORAGE_VOL_BLOCK;
-
-return 0;
-}
-
-
-static int
-virStorageBackendDiskBuildVol(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- unsigned int flags)
-{
int res = -1;
char *partFormat = NULL;
unsigned long long startOffset = 0, endOffset = 0;
-virCommandPtr cmd = NULL;
-
-virCheckFlags(0, -1);
-
-cmd = virCommandNewArgList(PARTED,
- pool-def-source.devices[0].path,
- mkpart,
- --script,
- NULL);
+virCommandPtr cmd = virCommandNewArgList(PARTED,
+ pool-def-source.devices[0].path,
+ mkpart,
+ --script,
+ NULL);
-if (virStorageBackendDiskPartFormat(pool, vol, partFormat) != 0)
+if (vol-target.encryption != NULL) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s, _(storage pool does not support encrypted
+ volumes));
goto cleanup;
+}
+if (virStorageBackendDiskPartFormat(pool, vol, partFormat) != 0) {
+goto cleanup;
+}
virCommandAddArg(cmd, partFormat);
if (virStorageBackendDiskPartBoundries(pool, startOffset,
@@ -783,6 +768,5 @@ virStorageBackend virStorageBackendDisk = {
.createVol = virStorageBackendDiskCreateVol,
.deleteVol = virStorageBackendDiskDeleteVol,
-.buildVol = virStorageBackendDiskBuildVol,
.buildVolFrom = virStorageBackendDiskBuildVolFrom,
};
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Revert storage: lvm: Separate creating of the volume from building
This reverts commit af1fb38f55d4fb87e0fcaee1e973fa9c6713b1e6.
With it, creating new logical volumes fails:
https://www.redhat.com/archives/libvir-list/2014-February/msg00658.html
In the storage driver, we expect CreateVol to fill out the volume key,
but the LVM backend fills the key with the uuid reported by lvs after the
logical volume is created.
---
src/storage/storage_backend_logical.c | 60 ++-
1 file changed, 23 insertions(+), 37 deletions(-)
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index 039d962..15b86dc 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -702,16 +702,32 @@ cleanup:
static int
-virStorageBackendLogicalBuildVol(virConnectPtr conn,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- unsigned int flags)
+virStorageBackendLogicalCreateVol(virConnectPtr conn,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol)
{
int fd = -1;
virCommandPtr cmd = NULL;
virErrorPtr err;
-virCheckFlags(0, -1);
+if (vol-target.encryption != NULL) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s, _(storage pool does not support encrypted
+ volumes));
+return -1;
+}
+
+vol-type = VIR_STORAGE_VOL_BLOCK;
+
+if (vol-target.path != NULL) {
+/* A target path passed to CreateVol has no meaning */
+VIR_FREE(vol-target.path);
+}
+
+if (virAsprintf(vol-target.path, %s/%s,
+pool-def-target.path,
+vol-name) == -1)
+return -1;
cmd = virCommandNewArgList(LVCREATE,
--name, vol-name,
@@ -770,7 +786,7 @@ virStorageBackendLogicalBuildVol(virConnectPtr conn,
return 0;
-error:
+ error:
err = virSaveLastError();
VIR_FORCE_CLOSE(fd);
virStorageBackendLogicalDeleteVol(conn, pool, vol, 0);
@@ -780,36 +796,6 @@ error:
return -1;
}
-
-static int
-virStorageBackendLogicalCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol)
-{
-if (vol-target.encryption != NULL) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
- _(storage pool does not support encrypted volumes));
-return -1;
-}
-
-vol-type = VIR_STORAGE_VOL_BLOCK;
-
-VIR_FREE(vol-target.path);
-if (virAsprintf(vol-target.path, %s/%s,
-pool-def-target.path,
-vol-name) == -1)
-return -1;
-
-if (virFileExists(vol-target.path)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
- _(volume target path '%s' already exists),
- vol-target.path);
-return -1;
-}
-
-return 0;
-}
-
static int
virStorageBackendLogicalBuildVolFrom(virConnectPtr conn,
virStoragePoolObjPtr pool,
@@ -837,7 +823,7 @@ virStorageBackend virStorageBackendLogical = {
.refreshPool = virStorageBackendLogicalRefreshPool,
.stopPool = virStorageBackendLogicalStopPool,
.deletePool = virStorageBackendLogicalDeletePool,
-.buildVol = virStorageBackendLogicalBuildVol,
+.buildVol = NULL,
.buildVolFrom = virStorageBackendLogicalBuildVolFrom,
.createVol = virStorageBackendLogicalCreateVol,
.deleteVol = virStorageBackendLogicalDeleteVol,
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: handle NULL return from virGetStorageVol
On 02/12/2014 12:05 PM, Michael Chapman wrote: virGetStorageVol can return NULL on out-of-memory. If it does, cleanly abort the volume clone operation. Signed-off-by: Michael Chapman m...@very.puzzling.org --- src/storage/storage_driver.c | 4 1 file changed, 4 insertions(+) ACK and pushed. Thank you! Jan 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 0/2] Partially revert separation of volume creating and building
On 02/12/2014 03:15 PM, Peter Krempa wrote: On 02/12/14 15:03, Ján Tomko wrote: The storage driver expects 'createVol' to fill out the volume key. For disk and logical backends, we generate the key only after the volume has been built. Revert the separation commits for these backends. Ján Tomko (2): Revert storage: lvm: Separate creating of the volume from building Revert storage: disk: Separate creating of the volume from building src/storage/storage_backend_disk.c| 44 - src/storage/storage_backend_logical.c | 60 ++- 2 files changed, 37 insertions(+), 67 deletions(-) ACK series, these changes are not needed in the end for my gluster series. Now pushed. Jan 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 v6 4/6] qemu_cap: Add USB keyboard capability
On 02/13/2014 09:48 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
QEMU can support USB keyboard but libvirt haven't supportted it yet.
This patch is to add USB keyboard capabilities and test cases.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
tests/qemuhelptest.c | 8
9 files changed, 18 insertions(+)
ACK
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e7d953a..0433607 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -249,6 +249,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
enable-fips,
spice-file-xfer-disable,
spiceport,
+
+ usb-kbd, /*165*/
There should be spaces around the comment: /* 165 */
);
struct _virQEMUCaps {
@@ -1403,6 +1405,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[]
= {
{ virtio-mmio, QEMU_CAPS_DEVICE_VIRTIO_MMIO },
{ ich9-intel-hda, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
{ pvpanic, QEMU_CAPS_DEVICE_PANIC },
+{ usb-kbd, QEMU_CAPS_DEVICE_USB_KBD },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index a4eecb6..dbc4c9a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -203,6 +203,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_ENABLE_FIPS= 162, /* -enable-fips */
QEMU_CAPS_SPICE_FILE_XFER_DISABLE = 163, /* -spice
disable-agent-file-xfer */
QEMU_CAPS_CHARDEV_SPICEPORT = 164, /* -chardev spiceport */
+QEMU_CAPS_DEVICE_USB_KBD = 165, /*-device usb-kbd*/
Same here.
QEMU_CAPS_LAST, /* this must always be the last item */
};
Jan
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 v6 1/6] conf: Add a keyboard input device type
On 02/13/2014 09:48 AM, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com There is no keyboard for non-x86 platforms when graphics are enabled. It's preferred to add one USB keyboard. This patch is to add keyboard input device type. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 20 +--- src/conf/domain_conf.h| 1 + 3 files changed, 15 insertions(+), 7 deletions(-) ACK Jan 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 v6 3/6] conf: Remove the implicit PS2 mouse for non-X86 platforms and add an implicit PS2 keyboard device for X86 platforms.
On 02/13/2014 09:48 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
PS2 device only works for X86 platform, other platforms may need
USB mouse. Athough it doesn't influence the QEMU command line, but
it's not right to add one PS2 mouse for non-X86 platform.
What's more, PS2 keyboard can be supported for X86.
So, this patch is to remove PS2 mouse for non-x86 platforms and also add
an implicit PS2 keyboard device for X86.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
src/conf/domain_conf.c | 70
+++---
src/util/virarch.h | 2 +
.../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 1 -
3 files changed, 37 insertions(+), 36 deletions(-)
@@ -17525,16 +17524,17 @@ virDomainDefFormatInternal(virDomainDefPtr def,
}
if (def-ngraphics 0) {
-/* If graphics is enabled, add the implicit mouse */
-virDomainInputDef autoInput = {
-VIR_DOMAIN_INPUT_TYPE_MOUSE,
-STREQ(def-os.type, hvm) ?
-VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
-{ .alias = NULL },
-};
-
-if (virDomainInputDefFormat(buf, autoInput, flags) 0)
-goto error;
+/* If graphics is enabled, add the implicit mouse/keyboard */
Just mouse.
+if (ARCH_IS_X86(def-os.arch)) {
This fails 'make check' in 'sexpr2xmltest'. It seems arch is set to 'NONE'
when we convert xen s-expression to our XML.
I think we should add these for VIR_ARCH_NONE as well.
+virDomainInputDef autoInput = {
+VIR_DOMAIN_INPUT_TYPE_MOUSE,
+STREQ(def-os.type, hvm) ?
+VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
+{ .alias = NULL },
+};
+if (virDomainInputDefFormat(buf, autoInput, flags) 0)
+goto error;
+}
for (n = 0; n def-ngraphics; n++)
if (virDomainGraphicsDefFormat(buf, def-graphics[n], flags) 0)
diff --git a/src/util/virarch.h b/src/util/virarch.h
index b180400..9b66e43 100644
--- a/src/util/virarch.h
+++ b/src/util/virarch.h
@@ -70,6 +70,8 @@ typedef enum {
VIR_ARCH_LAST,
} virArch;
+#define ARCH_IS_X86(arch) ((arch) == VIR_ARCH_X86_64 ||\
+(arch) == VIR_ARCH_I686)
This fails 'make syntax-check':
cppi: src/util/virarch.h: line 73: not properly indented
There should be a space between # and define, see:
http://libvirt.org/hacking.html#preprocessor
Other than that, this patch looks okay to me.
Jan
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 v6 5/6] qemu: parse qemu command line for USB keyboard
On 02/13/2014 09:48 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
This patch is to format qemu command line and xen driver for USB keyboard
and add test cases for it.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
src/qemu/qemu_command.c| 41
--
src/xenxs/xen_sxpr.c | 27 +-
src/xenxs/xen_xm.c | 30 +++-
.../qemuxml2argv-pseries-usb-kbd.args | 9 +
.../qemuxml2argv-pseries-usb-kbd.xml | 19 ++
tests/qemuxml2argvtest.c | 3 ++
6 files changed, 103 insertions(+), 26 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-kbd.xml
I'd split the patch in two - one for qemu, one for xen.
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index d514725..79cc20e 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -724,21 +724,23 @@ xenParseSxprUSB(virDomainDefPtr def,
tmp = sexpr_node(node, usbdevice);
if (tmp *tmp) {
if (STREQ(tmp, tablet) ||
-STREQ(tmp, mouse)) {
+STREQ(tmp, mouse) ||
+STREQ(tmp, keyboard)) {
virDomainInputDefPtr input;
if (VIR_ALLOC(input) 0)
goto error;
input-bus = VIR_DOMAIN_INPUT_BUS_USB;
if (STREQ(tmp, tablet))
input-type = VIR_DOMAIN_INPUT_TYPE_TABLET;
-else
+else if (STREQ(tmp, mouse))
input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
+else
+input-type = VIR_DOMAIN_INPUT_TYPE_KBD;
-if (VIR_REALLOC_N(def-inputs, def-ninputs+1) 0) {
+if (VIR_APPEND_ELEMENT(def-inputs, def-ninputs, input)
0) {
VIR_FREE(input);
goto error;
}
-def-inputs[def-ninputs++] = input;
} else {
/* XXX Handle other non-input USB devices later */
}
@@ -2144,15 +2146,24 @@ xenFormatSxprInput(virDomainInputDefPtr input,
return 0;
if (input-type != VIR_DOMAIN_INPUT_TYPE_MOUSE
-input-type != VIR_DOMAIN_INPUT_TYPE_TABLET) {
+input-type != VIR_DOMAIN_INPUT_TYPE_TABLET
+input-type != VIR_DOMAIN_INPUT_TYPE_KBD) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(unexpected input type %d), input-type);
return -1;
}
-virBufferAsprintf(buf, (usbdevice %s),
- input-type == VIR_DOMAIN_INPUT_TYPE_MOUSE ?
- mouse : tablet);
+switch (input-type) {
+case VIR_DOMAIN_INPUT_TYPE_MOUSE:
+virBufferAsprintf(buf, (usbdevice %s),mouse)
+break;
+case VIR_DOMAIN_INPUT_TYPE_TABLET:
+virBufferAsprintf(buf, (usbdevice %s),tablet)
+break;
+case VIR_DOMAIN_INPUT_TYPE_KBD:
+virBufferAsprintf(buf, (usbdevice %s),keyboard)
+break;
+}
In all three cases there is a space missing between arguments and a semicolon
missing at the end.
return 0;
}
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 5e89876..b448e99 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -886,14 +886,18 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
if (str
(STREQ(str, tablet) ||
- STREQ(str, mouse))) {
+ STREQ(str, mouse) ||
+ STREQ(str, keyboard))) {
virDomainInputDefPtr input;
if (VIR_ALLOC(input) 0)
goto cleanup;
input-bus = VIR_DOMAIN_INPUT_BUS_USB;
-input-type = STREQ(str, tablet) ?
-VIR_DOMAIN_INPUT_TYPE_TABLET :
-VIR_DOMAIN_INPUT_TYPE_MOUSE;
+if (STREQ(str, mouse))
+input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
+else if (STREQ(str, tablet))
+input-type = VIR_DOMAIN_INPUT_TYPE_TABLET;
+else (STREQ(str, keyboard))
s/else/else if/
+input-type = VIR_DOMAIN_INPUT_TYPE_KBD;
if (VIR_ALLOC_N(def-inputs, 1) 0) {
virDomainInputDefFree(input);
goto cleanup;
ACK to the QEMU part. The xen part looks fine to me.
Jan
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 v6 6/6] Add a default USB keyboard and USB mouse for PPC64
On 02/13/2014 09:48 AM, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com There is no keyboard working on PPC64 and PS2 mouse is only for PPC64 when graphics are enabled. It needs to add a USB keyboard and USB mouse for it. This patch is to add a USB keyboard and USB mouse when graphics are enabled. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- src/qemu/qemu_domain.c | 23 +- .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 ++ 2 files changed, 24 insertions(+), 1 deletion(-) ACK Jan 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 v6 2/6] conf: Add one interface to add default input devices
On 02/13/2014 09:48 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
This patch is to add one new interface to add input devices.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
src/conf/domain_conf.c | 29 +
src/conf/domain_conf.h | 4
src/libvirt_private.syms | 1 +
3 files changed, 34 insertions(+)
ACK.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9d344bc..786f9d1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11043,6 +11043,35 @@ virDomainDefMaybeAddController(virDomainDefPtr def,
return 0;
}
+int
+virDomainDefMaybeAddInput(virDomainDefPtr def,
+ int type,
+ int bus)
+{
+size_t i;
+virDomainInputDefPtr input;
+
+for (i = 0; i def-ninputs; i++) {
+if (def-inputs[i]-type == type
+def-inputs[i]-bus == bus)
+return 0;
+}
+
+if (VIR_ALLOC(input) 0)
+return -1;
+
+input-type = type;
+input-bus = bus;
+
+if (VIR_APPEND_ELEMENT(def-inputs, def-ninputs, input) 0) {
+VIR_FREE(input);
+return -1;
+}
+
+return 0;
+}
+
+
Nitpick: I'd move this extra line above the function, to have them separated
evenly by 2 lines.
/* Parse a memory element located at XPATH within CTXT, and store the
* result into MEM. If REQUIRED, then the value must exist;
Jan
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 v1 1/4] Introduce virDomain{Get,Set}Time APIs
On 02/13/2014 07:51 PM, Michal Privoznik wrote:
These APIs allow users to get or set time in a domain, which may come
handy if the domain has been resumed just recently and NTP is not
configured or hasn't kicked in yet and the guest is running
something time critical. In addition, NTP may refuse to re-set the clock
if the skew is too big.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
include/libvirt/libvirt.h.in | 13 +++
src/driver.h | 13 +++
src/libvirt.c| 91
src/libvirt_public.syms | 6 +++
4 files changed, 123 insertions(+)
+int virDomainGetTime(virDomainPtr dom,
+ long long *time,
+ unsigned int flags);
+
+typedef enum {
+VIR_DOMAIN_TIME_SYNC = (1 0), /* Re-sync domain time from domain's
RTC */
+} virDomainSetTimeFlags;
+
+int virDomainSetTime(virDomainPtr dom,
+ long long time,
+ const char *timezone,
Both 'time' and 'timezone' generate a warning about shadowed global
declaration with older GCC.
+ unsigned int flags);
+
/**
* virSchedParameterType:
*
Jan
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 v3 00/21] LXC configuration conversion
On 02/05/2014 03:09 PM, Cédric Bosdonnat wrote: Here is an updated version of the patch set fixing comments from Daniel. It also adds 3 commits: * One adding conversion for the newly supported blkio throttle tune in lxc driver. * One actually using the state of the veth network device in lxc driver. * One adding the ability to give major:minor numbers instead of a path for blkio tune devices. The last one is a way to address Daniel's comment on the /dev/block/... paths. Cédric Bosdonnat (21): Improve virConf parse to handle LXC config format LXC driver: started implementing connectDomainXMLFromNative LXC from native: import rootfs LXC from native: migrate fstab and lxc.mount.entry LXC from native: implement no network conversion LXC from native: migrate veth network configuration LXC from native: convert phys network types to net hostdev devices LXC from native: convert lxc.tty to console devices LXC from native: convert macvlan network configuration LXC from native: convert lxc.id_map into idmap LXC from native: migrate memory tuning LXC from native: map lxc.cgroup.cpu.* LXC from native: map lxc.cgroup.cpuset.* LXC from native: add lxc.cgroup.blkio.* mapping LXC from native: map lxc.arch to /domain/os/type@arch LXC from native: map block filesystems LXC from native: map vlan network type LXC: added some doc on domxml-from-native with mention of limitations LXC from native: convert blkio throttle config lxc: honor link state=up for veth interfaces blkiotune: allow node major='' minor=''/ in place of path ... src/lxc/lxc_native.c | 952 + Hi, The use of 'link' as a function parameter breaks the build on RHEL-6.4: cc1: warnings being treated as errors ../../src/lxc/lxc_native.c: In function 'lxcCreateNetDef': ../../src/lxc/lxc_native.c:337: error: declaration of 'link' shadows a global declaration [-Wshadow] /usr/include/unistd.h:809: error: shadowed declaration is here [-Wshadow] ../../src/lxc/lxc_native.c: In function 'lxcAddNetworkDefinition': ../../src/lxc/lxc_native.c:414: error: declaration of 'link' shadows a global declaration [-Wshadow] /usr/include/unistd.h:809: error: shadowed declaration is here [-Wshadow] make[3]: *** [lxc/libvirt_driver_lxc_impl_la-lxc_native.lo] Error 1 Jan 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 v6 1/6] conf: Add a keyboard input device type
On 02/14/2014 10:02 AM, Li Zhang wrote:
On 2014年02月14日 00:39, Daniel P. Berrange wrote:
On Thu, Feb 13, 2014 at 04:48:21PM +0800, Li Zhang wrote:
@@ -12422,10 +12426,12 @@ virDomainDefParseXML(xmlDocPtr xml,
* XXX will this be true for other virt types ? */
if ((STREQ(def-os.type, hvm)
input-bus == VIR_DOMAIN_INPUT_BUS_PS2
- input-type == VIR_DOMAIN_INPUT_TYPE_MOUSE) ||
+ (input-type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
+ input-type == VIR_DOMAIN_INPUT_TYPE_KBD)) ||
(STRNEQ(def-os.type, hvm)
input-bus == VIR_DOMAIN_INPUT_BUS_XEN
- input-type == VIR_DOMAIN_INPUT_TYPE_MOUSE)) {
+ (input-type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
+ input-type == VIR_DOMAIN_INPUT_TYPE_KBD))) {
virDomainInputDefFree(input);
continue;
Later on in this function there is
/* If graphics are enabled, there's an implicit PS2 mouse */
if (def-ngraphics 0) {
virDomainInputDefPtr input;
if (VIR_ALLOC(input) 0) {
goto error;
}
if (STREQ(def-os.type, hvm)) {
input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
input-bus = VIR_DOMAIN_INPUT_BUS_PS2;
} else {
input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
input-bus = VIR_DOMAIN_INPUT_BUS_XEN;
}
if (VIR_REALLOC_N(def-inputs, def-ninputs + 1) 0) {
virDomainInputDefFree(input);
goto error;
}
def-inputs[def-ninputs] = input;
def-ninputs++;
}
which needs to take care of keyboards too now.
And some more similar logic in virDomainDefFormatInternal
which needs updating
I have add the keyboard device in this function in this version [3/6].
But Jan suggested to remove it from virDomainDefFormatInternal in v5.
Because it would make the XML unreadable by older libvirtd that didn't know
the keyboard input type.
Sorry about that,
it seems we only care about XML to be readable by older libvirt if
virDomainDefFormat* is called with the
VIR_DOMAIN_XML_MIGRATABLE flag, so we can safely format it when this flag is
not set.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 0/2] Support IPv6 in port allocator
On 02/06/2014 05:43 PM, Ján Tomko wrote: v1: Support IPv6 in port allocator https://www.redhat.com/archives/libvir-list/2013-October/msg7.html v2: https://www.redhat.com/archives/libvir-list/2013-October/msg01313.html bind to v4 and v6 separately v3: fix the embarrasing bug of hardcoding AF_INET anyway added a test that mocks a v4-only system even on systems with IPv6 compiled in Ján Tomko (2): Split out bind() from virPortAllocatorAcquire Support IPv6 in port allocator src/util/virportallocator.c | 106 +++ tests/virportallocatortest.c | 68 +-- 2 files changed, 143 insertions(+), 31 deletions(-) I've pushed the series, thank you for the reviews! Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Test secret XML parsing and formatting
Ján Tomko (2): docs: remove auth from secret XML format Add tests for secret XML parsing docs/formatsecret.html.in| 2 - tests/Makefile.am| 12 ++- tests/secretschematest | 10 +++ tests/secretxml2xmlin/ephemeral-usage-volume.xml | 7 ++ tests/secretxml2xmlin/usage-ceph.xml | 7 ++ tests/secretxml2xmlin/usage-iscsi.xml| 7 ++ tests/secretxml2xmlin/usage-volume.xml | 7 ++ tests/secretxml2xmltest.c| 98 8 files changed, 147 insertions(+), 3 deletions(-) create mode 100755 tests/secretschematest create mode 100644 tests/secretxml2xmlin/ephemeral-usage-volume.xml create mode 100644 tests/secretxml2xmlin/usage-ceph.xml create mode 100644 tests/secretxml2xmlin/usage-iscsi.xml create mode 100644 tests/secretxml2xmlin/usage-volume.xml create mode 100644 tests/secretxml2xmltest.c -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Forgot to add lxcconf2xmldata to dist.
On 02/14/2014 04:06 PM, Cédric Bosdonnat wrote: --- tests/Makefile.am | 1 + 1 file changed, 1 insertion(+) ACK and pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Add tests for secret XML parsing
also validate it against the RNG schema.
---
tests/Makefile.am| 12 ++-
tests/secretschematest | 10 +++
tests/secretxml2xmlin/ephemeral-usage-volume.xml | 7 ++
tests/secretxml2xmlin/usage-ceph.xml | 7 ++
tests/secretxml2xmlin/usage-iscsi.xml| 7 ++
tests/secretxml2xmlin/usage-volume.xml | 7 ++
tests/secretxml2xmltest.c| 98
7 files changed, 147 insertions(+), 1 deletion(-)
create mode 100755 tests/secretschematest
create mode 100644 tests/secretxml2xmlin/ephemeral-usage-volume.xml
create mode 100644 tests/secretxml2xmlin/usage-ceph.xml
create mode 100644 tests/secretxml2xmlin/usage-iscsi.xml
create mode 100644 tests/secretxml2xmlin/usage-volume.xml
create mode 100644 tests/secretxml2xmltest.c
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0718a69..404c17d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -94,6 +94,7 @@ EXTRA_DIST = \
qemuxml2argvdata \
qemuxml2xmloutdata \
qemuxmlnsdata \
+ secretxml2xmlin \
securityselinuxlabeldata \
schematestutils.sh \
sexpr2xmldata \
@@ -257,6 +258,8 @@ test_programs += cputest
test_programs += metadatatest
+test_programs += secretxml2xmltest
+
test_scripts = \
capabilityschematest \
interfaceschematest \
@@ -266,7 +269,8 @@ test_scripts = \
domainschematest \
nodedevschematest \
nwfilterschematest \
- domainsnapshotschematest
+ domainsnapshotschematest \
+ secretschematest
if WITH_LIBVIRTD
test_scripts +=\
@@ -611,6 +615,12 @@ nwfilterxml2xmltest_SOURCES = \
testutils.c testutils.h
nwfilterxml2xmltest_LDADD = $(LDADDS)
+secretxml2xmltest_SOURCES = \
+ secretxml2xmltest.c \
+ testutils.c testutils.h
+secretxml2xmltest_LDADD = $(LDADDS)
+
+
if WITH_STORAGE
storagevolxml2argvtest_SOURCES = \
storagevolxml2argvtest.c \
diff --git a/tests/secretschematest b/tests/secretschematest
new file mode 100755
index 000..f64d1a3
--- /dev/null
+++ b/tests/secretschematest
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+: ${srcdir=.}
+. $srcdir/test-lib.sh
+. $abs_srcdir/schematestutils.sh
+
+DIRS=secretxml2xmlin
+SCHEMA=secret.rng
+
+check_schema $DIRS $SCHEMA
diff --git a/tests/secretxml2xmlin/ephemeral-usage-volume.xml
b/tests/secretxml2xmlin/ephemeral-usage-volume.xml
new file mode 100644
index 000..e273c57
--- /dev/null
+++ b/tests/secretxml2xmlin/ephemeral-usage-volume.xml
@@ -0,0 +1,7 @@
+secret ephemeral='yes' private='yes'
+ uuid22e1353d-c27e-4d6d-bf15-465053e6ba0b/uuid
+ descriptionEphemeral Private Secret/description
+ usage type='volume'
+volume/var/lib/libvirt/images/image.img/volume
+ /usage
+/secret
diff --git a/tests/secretxml2xmlin/usage-ceph.xml
b/tests/secretxml2xmlin/usage-ceph.xml
new file mode 100644
index 000..e880293
--- /dev/null
+++ b/tests/secretxml2xmlin/usage-ceph.xml
@@ -0,0 +1,7 @@
+secret ephemeral='no' private='yes'
+ uuidf52a81b2-424e-490c-823d-6bd4235bc572/uuid
+ descriptionCeph secret/description
+ usage type='ceph'
+nameCephCephCephCeph/name
+ /usage
+/secret
diff --git a/tests/secretxml2xmlin/usage-iscsi.xml
b/tests/secretxml2xmlin/usage-iscsi.xml
new file mode 100644
index 000..bfc9472
--- /dev/null
+++ b/tests/secretxml2xmlin/usage-iscsi.xml
@@ -0,0 +1,7 @@
+secret ephemeral='no' private='yes'
+ uuid27f25d34-aea6-4e2a-be85-fa2c18380be8/uuid
+ descriptioniSCSI secret/description
+ usage type='iscsi'
+targetiscsitarget/target
+ /usage
+/secret
diff --git a/tests/secretxml2xmlin/usage-volume.xml
b/tests/secretxml2xmlin/usage-volume.xml
new file mode 100644
index 000..e273c57
--- /dev/null
+++ b/tests/secretxml2xmlin/usage-volume.xml
@@ -0,0 +1,7 @@
+secret ephemeral='yes' private='yes'
+ uuid22e1353d-c27e-4d6d-bf15-465053e6ba0b/uuid
+ descriptionEphemeral Private Secret/description
+ usage type='volume'
+volume/var/lib/libvirt/images/image.img/volume
+ /usage
+/secret
diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c
new file mode 100644
index 000..be9ef64
--- /dev/null
+++ b/tests/secretxml2xmltest.c
@@ -0,0 +1,98 @@
+#include config.h
+
+#include stdlib.h
+
+#include internal.h
+#include testutils.h
+#include secret_conf.h
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+static int
+testCompareXMLToXMLFiles(const char *inxml, const char *outxml)
+{
+char *inXmlData = NULL;
+char *outXmlData = NULL;
+char *actual = NULL;
+int ret = -1;
+virSecretDefPtr secret = NULL;
+
+if (virtTestLoadFile(inxml, inXmlData) 0)
+goto fail;
+if (virtTestLoadFile(outxml, outXmlData) 0)
+goto fail;
+
+if (!(secret = virSecretDefParseString(inXmlData)))
+goto fail;
+
+if (!(actual = virSecretDefFormat(secret)))
+goto fail;
+
+if (STRNEQ(outXmlData, actual)) {
+
Re: [libvirt] [PATCH 0/2] Test secret XML parsing and formatting
On 02/14/2014 04:44 PM, Eric Blake wrote: On 02/14/2014 08:08 AM, Ján Tomko wrote: Ján Tomko (2): docs: remove auth from secret XML format Add tests for secret XML parsing ACK series. Wow - we've gone that long without testing secret XML? Thanks, pushed. Jan 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 v8 3/7] conf: Remove the implicit PS2 mouse for non-X86 platforms and add an implicit PS2 keyboard device for X86 platforms.
On 02/17/2014 08:33 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
PS2 device only works for X86 platform, other platforms may need
USB mouse. Athough it doesn't influence the QEMU command line, but
it's not right to add one PS2 mouse for non-X86 platform.
What's more, PS2 keyboard can be supported for X86.
So, this patch is to remove PS2 mouse for non-x86 platforms and also add
an implicit PS2 keyboard device for X86.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3301398..10c753c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12495,29 +12496,27 @@ virDomainDefParseXML(xmlDocPtr xml,
VIR_FREE(nodes);
/* If graphics are enabled, there's an implicit PS2 mouse */
-if (def-ngraphics 0) {
-virDomainInputDefPtr input;
+if (def-ngraphics 0
+ARCH_IS_X86(def-os.arch)) {
+int input_bus = VIR_DOMAIN_INPUT_BUS_XEN;
-if (VIR_ALLOC(input) 0) {
-goto error;
-}
-if (STREQ(def-os.type, hvm)) {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_PS2;
-} else {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_XEN;
-}
+if (STREQ(def-os.type, hvm))
+input_bus = VIR_DOMAIN_INPUT_BUS_PS2;
-if (VIR_REALLOC_N(def-inputs, def-ninputs + 1) 0) {
-virDomainInputDefFree(input);
+if (virDomainDefMaybeAddInput(def,
+ VIR_DOMAIN_INPUT_TYPE_MOUSE,
+ input_bus) 0)
goto error;
+
+/*Ignore keyboard for XEN, only add a PS2 keyboard device for hvm*/
+if (STREQ(def-os.type, hvm)) {
+if (virDomainDefMaybeAddInput(def,
+ VIR_DOMAIN_INPUT_TYPE_KBD,
+ input_bus) 0)
+goto error;
}
Adding the default keyboard should be in the patch that adds it to the parser
according to
https://www.redhat.com/archives/libvir-list/2014-February/msg00889.html
-def-inputs[def-ninputs] = input;
-def-ninputs++;
}
-
/* analysis of the sound devices */
if ((n = virXPathNodeSet(./devices/sound, ctxt, nodes)) 0) {
goto error;
@@ -17533,16 +17532,24 @@ virDomainDefFormatInternal(virDomainDefPtr def,
}
if (def-ngraphics 0) {
-/* If graphics is enabled, add the implicit mouse */
-virDomainInputDef autoInput = {
-VIR_DOMAIN_INPUT_TYPE_MOUSE,
-STREQ(def-os.type, hvm) ?
-VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
-{ .alias = NULL },
-};
+/* If graphics is enabled, add the implicit mouse/keyboard */
+if ((ARCH_IS_X86(def-os.arch)) ||
+def-os.arch == VIR_ARCH_NONE) {
+virDomainInputDef autoInput = {
+VIR_DOMAIN_INPUT_TYPE_MOUSE,
+STREQ(def-os.type, hvm) ?
+VIR_DOMAIN_INPUT_BUS_PS2 : VIR_DOMAIN_INPUT_BUS_XEN,
+{ .alias = NULL },
+};
+if (virDomainInputDefFormat(buf, autoInput, flags) 0)
+goto error;
-if (virDomainInputDefFormat(buf, autoInput, flags) 0)
-goto error;
+if (flags ~VIR_DOMAIN_XML_MIGRATABLE) {
This condition is true, if any other flag than MIGRATABLE is true.
It should be if (!(flags VIR_DOMAIN_XML_MIGRATABLE)) and also a part fo the
patch adding 'keyboard' to the parser.
+autoInput.type = VIR_DOMAIN_INPUT_TYPE_KBD;
+if (virDomainInputDefFormat(buf, autoInput, flags) 0)
+goto error;
+}
+}
for (n = 0; n def-ngraphics; n++)
if (virDomainGraphicsDefFormat(buf, def-graphics[n], flags) 0)
Jan
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 v8 3/7] conf: Remove the implicit PS2 mouse for non-X86 platforms and add an implicit PS2 keyboard device for X86 platforms.
On 02/17/2014 10:04 AM, Li Zhang wrote:
On 2014年02月17日 16:48, Ján Tomko wrote:
On 02/17/2014 08:33 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
PS2 device only works for X86 platform, other platforms may need
USB mouse. Athough it doesn't influence the QEMU command line, but
it's not right to add one PS2 mouse for non-X86 platform.
What's more, PS2 keyboard can be supported for X86.
So, this patch is to remove PS2 mouse for non-x86 platforms and also add
an implicit PS2 keyboard device for X86.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3301398..10c753c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12495,29 +12496,27 @@ virDomainDefParseXML(xmlDocPtr xml,
VIR_FREE(nodes);
/* If graphics are enabled, there's an implicit PS2 mouse */
-if (def-ngraphics 0) {
-virDomainInputDefPtr input;
+if (def-ngraphics 0
+ARCH_IS_X86(def-os.arch)) {
+int input_bus = VIR_DOMAIN_INPUT_BUS_XEN;
-if (VIR_ALLOC(input) 0) {
-goto error;
-}
-if (STREQ(def-os.type, hvm)) {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_PS2;
-} else {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_XEN;
-}
+if (STREQ(def-os.type, hvm))
+input_bus = VIR_DOMAIN_INPUT_BUS_PS2;
-if (VIR_REALLOC_N(def-inputs, def-ninputs + 1) 0) {
-virDomainInputDefFree(input);
+if (virDomainDefMaybeAddInput(def,
+ VIR_DOMAIN_INPUT_TYPE_MOUSE,
+ input_bus) 0)
goto error;
+
+/*Ignore keyboard for XEN, only add a PS2 keyboard device for hvm*/
+if (STREQ(def-os.type, hvm)) {
+if (virDomainDefMaybeAddInput(def,
+ VIR_DOMAIN_INPUT_TYPE_KBD,
+ input_bus) 0)
+goto error;
}
Adding the default keyboard should be in the patch that adds it to the parser
according to
https://www.redhat.com/archives/libvir-list/2014-February/msg00889.html
Do you mean merge this patch to the first patch?
I mean merging the part dealing with keyboards with the first patch and only
touch the implicit mouse in this patch.
Jan
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 v9 2/7] conf: Add a keyboard input device type
On 02/17/2014 11:17 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
There is no keyboard support currently in libvirt .
For some platforms, it needs to add a USB keyboard when graphics are enabled.
This patch is to add keyboard input device type.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c | 70
--
src/conf/domain_conf.h | 1 +
...
tests/vmx2xmldata/vmx2xml-graphics-vnc.xml | 1 +
27 files changed, 66 insertions(+), 31 deletions(-)
This fails 'make check' for me:
FAIL: sexpr2xmltest
FAIL: xmconfigtest
@@ -7805,9 +7806,10 @@ virDomainInputDefParseXML(const char *ostype,
goto error;
}
-if (STREQ(ostype, hvm)) {
-if (def-bus == VIR_DOMAIN_INPUT_BUS_PS2 /* Only allow mouse
for ps2 */
-def-type != VIR_DOMAIN_INPUT_TYPE_MOUSE) {
+if (STREQ(dom-os.type, hvm)) {
+if (def-bus == VIR_DOMAIN_INPUT_BUS_PS2 /* PS2 can be mouse
or keyboard */
+!(def-type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
+def-type == VIR_DOMAIN_INPUT_TYPE_KBD)) {
This is easier to read as:
def-type != MOUSE def-type != KBD
virReportError(VIR_ERR_INTERNAL_ERROR,
_(ps2 bus does not support %s input device),
type);
@@ -7833,8 +7836,9 @@ virDomainInputDefParseXML(const char *ostype,
}
}
} else {
-if (STREQ(ostype, hvm)) {
-if (def-type == VIR_DOMAIN_INPUT_TYPE_MOUSE)
+if (STREQ(dom-os.type, hvm)) {
+if ((def-type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
+def-type == VIR_DOMAIN_INPUT_TYPE_KBD))
Trailing space breaks 'make syntax-check'
def-bus = VIR_DOMAIN_INPUT_BUS_PS2;
else
def-bus = VIR_DOMAIN_INPUT_BUS_USB;
@@ -12491,29 +12497,26 @@ virDomainDefParseXML(xmlDocPtr xml,
VIR_FREE(nodes);
/* If graphics are enabled, there's an implicit PS2 mouse */
-if (def-ngraphics 0) {
-virDomainInputDefPtr input;
+if (def-ngraphics 0) {
Another trailing space.
+int input_bus = VIR_DOMAIN_INPUT_BUS_XEN;
-if (VIR_ALLOC(input) 0) {
-goto error;
-}
-if (STREQ(def-os.type, hvm)) {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_PS2;
-} else {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_XEN;
-}
+if (STREQ(def-os.type, hvm))
+input_bus = VIR_DOMAIN_INPUT_BUS_PS2;
-if (VIR_REALLOC_N(def-inputs, def-ninputs + 1) 0) {
-virDomainInputDefFree(input);
+if (virDomainDefMaybeAddInput(def,
+ VIR_DOMAIN_INPUT_TYPE_MOUSE,
+ input_bus) 0)
goto error;
+
+/*Ignore keyboard for XEN, only add a PS2 keyboard device for hvm*/
+if (STREQ(def-os.type, hvm)) {
Xen has hvm domains too, and since we were adding the mouse for non-hvm
domains, I think we should get rid of this condition and add keyboards too (I
think this is different from what I said earlier)
+if (virDomainDefMaybeAddInput(def,
+ VIR_DOMAIN_INPUT_TYPE_KBD,
+ input_bus) 0)
+goto error;
}
-def-inputs[def-ninputs] = input;
-def-ninputs++;
}
-
/* analysis of the sound devices */
if ((n = virXPathNodeSet(./devices/sound, ctxt, nodes)) 0) {
goto error;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml
b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml
index f9fdf37..341322c 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml
@@ -71,7 +71,9 @@
/console
input type='tablet' bus='usb'/
input type='mouse' bus='ps2'/
+input type='keyboard' bus='ps2'/
graphics type='spice' port='5900' autoport='no' passwd='sercet'
passwdValidTo='2011-05-31T16:11:22' connected='disconnect'/
+input type='keyboard' bus='ps2'/
sound model='ac97'
address type='pci' domain='0x' bus='0x00' slot='0x03'
function='0x0'/
/sound
Double keyboard entry.
ACK with this squashed in:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5031441..75b87d1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7808,9 +7808,9 @@ virDomainInputDefParseXML(const virDomainDef *dom,
}
if (STREQ(dom-os.type, hvm)) {
-
Re: [libvirt] [PATCH v9 1/7] conf: Add one interface to add default input devices
On 02/17/2014 11:17 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
This patch is to add one new interface to add input devices.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
src/conf/domain_conf.c | 29 +
src/conf/domain_conf.h | 4
src/libvirt_private.syms | 1 +
3 files changed, 34 insertions(+)
ACK with the following whitespace changes and with the hunk using this
function moved from 2/7 here:
Jan
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 794153d..939b423 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11051,11 +11051,10 @@ virDomainDefMaybeAddController(virDomainDefPtr def,
}
-
int
virDomainDefMaybeAddInput(virDomainDefPtr def,
- int type,
- int bus)
+ int type,
+ int bus)
{
size_t i;
virDomainInputDefPtr input;
@@ -11080,6 +11079,7 @@ virDomainDefMaybeAddInput(virDomainDefPtr def,
return 0;
}
+
/* Parse a memory element located at XPATH within CTXT, and store the
* result into MEM. If REQUIRED, then the value must exist;
* otherwise, the value is optional. The value is in blocks of 1024.
@@ -12493,25 +12493,15 @@ virDomainDefParseXML(xmlDocPtr xml,
/* If graphics are enabled, there's an implicit PS2 mouse */
if (def-ngraphics 0) {
-virDomainInputDefPtr input;
+int input_bus = VIR_DOMAIN_INPUT_BUS_XEN;
-if (VIR_ALLOC(input) 0) {
-goto error;
-}
-if (STREQ(def-os.type, hvm)) {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_PS2;
-} else {
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-input-bus = VIR_DOMAIN_INPUT_BUS_XEN;
-}
+if (STREQ(def-os.type, hvm))
+input_bus = VIR_DOMAIN_INPUT_BUS_PS2;
-if (VIR_REALLOC_N(def-inputs, def-ninputs + 1) 0) {
-virDomainInputDefFree(input);
+if (virDomainDefMaybeAddInput(def,
+ VIR_DOMAIN_INPUT_TYPE_MOUSE,
+ input_bus) 0)
goto error;
-}
-def-inputs[def-ninputs] = input;
-def-ninputs++;
}
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 v9 6/7] xen: format xen command line for USB keyboard
On 02/17/2014 11:17 AM, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com This patch is to format xen command line for USB keyboard s/xen command line/Xen XM and S-Expr config/ both here and in the subject. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- src/xenxs/xen_sxpr.c | 27 +++ src/xenxs/xen_xm.c | 30 ++ 2 files changed, 41 insertions(+), 16 deletions(-) ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v9 3/7] conf: Remove the implicit PS2 devices for non-X86 platforms
On 02/17/2014 11:17 AM, Li Zhang wrote:
From: Li Zhang zhlci...@linux.vnet.ibm.com
PS2 device only works for X86 platform, other platforms may need
USB devices instead. Athough it doesn't influence the QEMU command line,
but it's not right to add PS2 mouse/keyboard for non-X86 platform.
So, this patch is to remove PS2 devices for non-x86 platforms.
Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com
---
src/conf/domain_conf.c | 31
+-
src/util/virarch.h | 2 ++
.../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 --
3 files changed, 20 insertions(+), 15 deletions(-)
ACK with this squashed in:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 86ac997..2826847 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7840,10 +7840,11 @@ virDomainInputDefParseXML(const virDomainDef *dom,
if (STREQ(dom-os.type, hvm)) {
if ((def-type == VIR_DOMAIN_INPUT_TYPE_MOUSE ||
def-type == VIR_DOMAIN_INPUT_TYPE_KBD)
-ARCH_IS_X86(dom-os.arch))
+(ARCH_IS_X86(dom-os.arch) || dom-os.arch == VIR_ARCH_NONE)) {
def-bus = VIR_DOMAIN_INPUT_BUS_PS2;
-else
+} else {
def-bus = VIR_DOMAIN_INPUT_BUS_USB;
+}
} else {
def-bus = VIR_DOMAIN_INPUT_BUS_XEN;
}
@@ -12500,7 +12501,7 @@ virDomainDefParseXML(xmlDocPtr xml,
/* If graphics are enabled, there's an implicit PS2 mouse */
if (def-ngraphics 0
-ARCH_IS_X86(def-os.arch)) {
+(ARCH_IS_X86(def-os.arch) || def-os.arch == VIR_ARCH_NONE)) {
int input_bus = VIR_DOMAIN_INPUT_BUS_XEN;
if (STREQ(def-os.type, hvm))
@@ -17533,8 +17534,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
if (def-ngraphics 0) {
/* If graphics is enabled, add the implicit mouse/keyboard */
-if ((ARCH_IS_X86(def-os.arch)) ||
-def-os.arch == VIR_ARCH_NONE) {
+if ((ARCH_IS_X86(def-os.arch)) || def-os.arch == VIR_ARCH_NONE) {
virDomainInputDef autoInput = {
VIR_DOMAIN_INPUT_TYPE_MOUSE,
STREQ(def-os.type, hvm) ?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v9 0/7] Support keyboard device
On 02/18/2014 09:58 AM, Li Zhang wrote: Hi Jan and Daniel, Would you like to accept this patchset? Thanks. Hi, it looks good to me and I'd like to push it this week with the changes I pointed out, unless someone has a different opinion. Jan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix conflicting types of virInitctlSetRunLevel
aebbcdd didn't change the non-linux definition of the function,
breaking the build on FreeBSD:
../../src/util/virinitctl.c:164: error: conflicting types for
'virInitctlSetRunLevel'
../../src/util/virinitctl.h:40: error: previous declaration of
'virInitctlSetRunLevel' was here
---
Pushed as trivial.
src/util/virinitctl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virinitctl.c b/src/util/virinitctl.c
index 579268f..012200c 100644
--- a/src/util/virinitctl.c
+++ b/src/util/virinitctl.c
@@ -159,8 +159,7 @@ cleanup:
return ret;
}
#else
-int virInitctlSetRunLevel(virInitctlRunLevel level ATTRIBUTE_UNUSED,
- const char *vroot ATTRIBUTE_UNUSED)
+int virInitctlSetRunLevel(virInitctlRunLevel level ATTRIBUTE_UNUSED)
{
virReportUnsupportedError();
return -1;
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v9 0/7] Support keyboard device
On 02/19/2014 07:17 AM, Li Zhang wrote: On 2014年02月18日 20:52, Ján Tomko wrote: On 02/18/2014 09:58 AM, Li Zhang wrote: Hi Jan and Daniel, Would you like to accept this patchset? Thanks. Hi, it looks good to me and I'd like to push it this week with the changes I pointed out, unless someone has a different opinion. Thanks a lot. I've pushed it now. Jan 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 v9 2/7] conf: Add a keyboard input device type
On 02/19/2014 07:22 AM, Li Zhang wrote: On 2014年02月18日 20:51, Ján Tomko wrote: On 02/17/2014 11:17 AM, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com There is no keyboard support currently in libvirt . For some platforms, it needs to add a USB keyboard when graphics are enabled. This patch is to add keyboard input device type. Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 70 -- src/conf/domain_conf.h | 1 + ... tests/vmx2xmldata/vmx2xml-graphics-vnc.xml | 1 + 27 files changed, 66 insertions(+), 31 deletions(-) This fails 'make check' for me: FAIL: sexpr2xmltest FAIL: xmconfigtest Ah, I can't get any errors when doing 'make check' on my side. :( Libvirt needs to be built --with-xen for those tests to work. You probably don't have the xen library headers on your system. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix build of portallocator on mingw
IN6ADDR_ANY_INIT does not seem to be working as expected on MinGW:
error: missing braces around initializer [-Werror=missing-braces]
.sin6_addr = IN6ADDR_ANY_INIT,
Use the in6addr_any variable instead.
Reported by Daniel P. Berrange.
---
Pushed as a build breaker.
src/util/virportallocator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c
index 06174b0..22cdc37 100644
--- a/src/util/virportallocator.c
+++ b/src/util/virportallocator.c
@@ -106,7 +106,7 @@ static int virPortAllocatorBindToPort(bool *used,
struct sockaddr_in6 addr6 = {
.sin6_family = AF_INET6,
.sin6_port = htons(port),
-.sin6_addr = IN6ADDR_ANY_INIT,
+.sin6_addr = in6addr_any
};
struct sockaddr_in addr4 = {
.sin_family = AF_INET,
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Add virStringSearch method for regex matching
On 02/19/2014 09:36 PM, Daniel P. Berrange wrote:
From: Manuel VIVES manuel.vi...@diateam.net
Add a virStringSearch method to virstring.{c,h} which performs
a regex match against a string and returns the matching substrings.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
po/POTFILES.in | 1 +
src/libvirt_private.syms | 1 +
src/util/virstring.c | 102
+++
src/util/virstring.h | 7
tests/virstringtest.c| 100 ++
5 files changed, 211 insertions(+)
@@ -645,3 +647,103 @@ int virStringSortRevCompare(const void *a, const void
*b)
return strcmp(*sb, *sa);
}
+
+/**
+ * virStringSearch:
+ * @str: string to search
+ * @regexp: POSIX Extended regular expression pattern used for matching
+ * @max_matches: maximum number of substrings to return
+ * @result: pointer to an array to be filled with NULL terminated list of
matches
+ *
+ * Performs a POSIX extended regex search against a string and return all
matching substrings.
+ * The @result value should be freed with virStringFreeList() when no longer
Double space before 'virStringFreeList'.
+ * required.
+ *
+ * @code
+ * char *source = 6853a496-1c10-472e-867a-8244937bd6f0
+ * 773ab075-4cd7-4fc2-8b6e-21c84e9cb391
+ * bbb3c75c-d60f-43b0-b802-fd56b84a4222
+ * 60c04aa1-0375-4654-8d9f-e149d9885273
+ * 4548d465-9891-4c34-a184-3b1c34a26aa8;
+ * char **matches = NULL;
+ * virSearchRegex(source,
The function is now renamed.
+ *
([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}),
+ * 3,
+ * matches);
+ *
+ * // matches[0] == 4548d465-9891-4c34-a184-3b1c34a26aa8;
+ * // matches[1] == 6853a496-1c10-472e-867a-8244937bd6f0;
+ * // matches[2] == 773ab075-4cd7-4fc2-8b6e-21c84e9cb391
+ * // matches[3] == NULL;
And these matches are wrong.
+ *
+ * virStringFreeList(matches);
+ * @endcode
+ *
+ * Returns: -1 on error, or number of matches
+ */
+ssize_t
+virStringSearch(const char *str,
+const char *regexp,
+size_t max_matches,
+char ***matches)
+{
+regex_t re;
+regmatch_t rem;
+size_t nmatches = 0;
+ssize_t ret = -1;
+int rv = -1;
+
+*matches = NULL;
+
+VIR_DEBUG(search '%s' for '%s', str, regexp);
+
+if ((rv = regcomp(re, regexp, REG_EXTENDED)) != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Error while compiling regular expression '%s':
%d),
+ regexp, rv);
Using 'regerror' instead of a numeric error code would be nicer.
+return -1;
+}
+
+if (re.re_nsub != 1) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Regular expression '%s' must have exactly 1 match
group, not %zu),
+ regexp, re.re_nsub);
+goto cleanup;
+}
+
+/* '*matches' must always be NULL terminated in every iteration
+ * of the loop, so start by allocating 1 element
+ */
+if (VIR_EXPAND_N(*matches, nmatches, 1) 0)
+goto cleanup;
+
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index c6adf9f..b8c6115 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -274,6 +274,70 @@ testStringSortCompare(const void *opaque
ATTRIBUTE_UNUSED)
}
+struct stringSearchData {
+const char *str;
+const char *regexp;
+size_t maxMatches;
+size_t expectNMatches;
+const char **expectMatches;
+bool expectError;
+};
+
+static int
+testStringSearch(const void *opaque ATTRIBUTE_UNUSED)
+{
+const struct stringSearchData *data = opaque;
+char **matches = NULL;
+ssize_t nmatches;
+int ret;
ret = -1;
ACK with or without regerror.
Jan
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 2/2] Add virStringReplace method for substring replacement
On 02/19/2014 09:36 PM, Daniel P. Berrange wrote:
Add a virStringReplace method to virstring.{h,c} to perform
substring matching and replacement
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/virstring.c | 44 +++-
src/util/virstring.h | 5
tests/virstringtest.c| 59
4 files changed, 108 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f26190d..f7379a2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1794,6 +1794,7 @@ virStringArrayHasString;
virStringFreeList;
virStringJoin;
virStringListLength;
+virStringReplace;
virStringSearch;
virStringSortCompare;
virStringSortRevCompare;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 67a87d3..3e42b06 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -619,7 +619,6 @@ size_t virStringListLength(char **strings)
return i;
}
-
/**
* virStringSortCompare:
*
Unrelated whitespace change.
@@ -747,3 +746,46 @@ cleanup:
}
return ret;
}
+
+/**
+ * virStringReplace:
+ * @haystack: the source string to process
+ * @oldneedle: the substring to locate
+ * @newneedle: the substring to insert
+ *
+ * Search @haystack and replace all occurences of @oldneedle with @newneedle.
+ *
+ * Returns: a new string with all the replacements, or NULL on error
+ */
+char *
+virStringReplace(const char *haystack,
+ const char *oldneedle,
+ const char *newneedle)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+const char *tmp1, *tmp2;
+size_t oldneedlelen = strlen(oldneedle);
+size_t newneedlelen = strlen(newneedle);
+
+tmp1 = haystack;
+tmp2 = NULL;
+
+while (tmp1) {
+tmp2 = strstr(tmp1, oldneedle);
+
+if (tmp2) {
+virBufferAdd(buf, tmp1, (tmp2 - tmp1));
+virBufferAdd(buf, newneedle, newneedlelen);
+tmp2 += oldneedlelen;
+} else {
+virBufferAdd(buf, tmp1, -1);
+}
+
+tmp1 = tmp2;
+}
+
+if (virBufferError(buf))
virBufferError doesn't report an error, but I think this function should.
+return NULL;
+
+return virBufferContentAndReset(buf);
+}
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index b8c6115..43023d5 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -428,6 +460,33 @@ mymain(void)
const char *matches3[] = { foo, bar };
TEST_SEARCH(1foo2bar3eek, ([a-z]+), 2, 2, matches3, false);
+#define TEST_REPLACE(h, o, n, r)\
+do {\
+struct stringReplaceData data = { \
+.haystack = h, \
+.oldneedle = o, \
+.newneedle = n, \
+.result = r \
+}; \
+if (virtTestRun(virStringReplace h, testStringReplace, data)
0) \
+ret = -1; \
+} while (0)
+
+/* no matches */
+TEST_REPLACE(foo, bar, eek, foo);
+
+/* complete match */
+TEST_REPLACE(foo, foo, bar, bar);
+
+/* middle match */
+TEST_REPLACE(foobarwizz, bar, eek, fooeekwizz);
+
+/* many matches */
+TEST_REPLACE(foofoofoofoo, foo, bar, barbarbarbar);
+
+/* many matches */
+TEST_REPLACE(fof, foo, bar, barooobaroo);
+
return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
A test with different length of old and new needles would be nice.
ACK with error reporting.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add a stub for virCgroupGetDomainTotalCpuStats
Commit 6515889 broke the build on FreeBSD:
In function `qemuDomainGetCPUStats':
/../../src/qemu/qemu_driver.c:16102:
undefined reference to `virCgroupGetDomainTotalCpuStats'
---
Pushed as a build-breaker.
src/util/vircgroup.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 268a4ae..2dba10c 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -4261,6 +4261,17 @@ virCgroupGetCpuacctStat(virCgroupPtr group
ATTRIBUTE_UNUSED,
int
+virCgroupGetDomainTotalCpuStats(virCgroupPtr group ATTRIBUTE_UNUSED,
+virTypedParameterPtr params ATTRIBUTE_UNUSED,
+int nparams ATTRIBUTE_UNUSED)
+{
+virReportSystemError(ENOSYS, %s,
+ _(Control groups not supported on this platform));
+return -1;
+}
+
+
+int
virCgroupSetFreezerState(virCgroupPtr group ATTRIBUTE_UNUSED,
const char *state ATTRIBUTE_UNUSED)
{
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Ignore additional fields in iscsiadm output
There has been a new field introduced in iscsiadm --mode session
output [1], but our regex only expects four fields. This breaks
startup of iscsi pools:
error: Failed to start pool iscsi
error: internal error: cannot find session
Fix this by ignoring anything after the fourth field.
https://bugzilla.redhat.com/show_bug.cgi?id=1067173
[1] https://github.com/mikechristie/open-iscsi/commit/181af9a
---
src/storage/storage_backend_iscsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index 556c2cc..1149e43 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -102,7 +102,7 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool,
* Pull out 2nd and 4th fields
*/
const char *regexes[] = {
-^tcp:\\s+\\[(\\S+)\\]\\s+\\S+\\s+(\\S+)\\s*$
+^tcp:\\s+\\[(\\S+)\\]\\s+\\S+\\s+(\\S+).*$
};
int vars[] = {
2,
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ignore additional fields in iscsiadm output
On 02/21/2014 11:08 AM, Daniel P. Berrange wrote: On Fri, Feb 21, 2014 at 11:01:17AM +0100, Ján Tomko wrote: There has been a new field introduced in iscsiadm --mode session output [1], but our regex only expects four fields. This breaks startup of iscsi pools: error: Failed to start pool iscsi error: internal error: cannot find session Fix this by ignoring anything after the fourth field. https://bugzilla.redhat.com/show_bug.cgi?id=1067173 [1] https://github.com/mikechristie/open-iscsi/commit/181af9a --- src/storage/storage_backend_iscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK Daniel Thanks, pushed now. Jan 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] XML: Escape double-hyphens in XML comment
On 02/21/2014 10:01 AM, Philipp Hahn wrote: To quote http://www.w3.org/TR/REC-xml/#sec-comments: For compatibility, the string -- (double-hyphen) must not occur within comments. For example this breaks creating snapshots: $ virsh snapshot-create-as $VM comment--bug $ xmllint --noout /var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml /var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml:4: parser error : Comment not terminated !-- WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES virsh snapshot-edit $VM comment--bug $ /etc/init.d/libvirt-bin restart error : qemuDomainSnapshotLoad:367 : Failed to parse snapshot XML from file '/var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml' $ virsh snapshot-list ucs32-33787-performance Name Creation Time State Also applies to QEMU domains, where the name contains double-hyphens. This no longer reproduces with current master, it has been fixed by either of: commit 0b121614a2086a8e66ae1f004fe912ba7c1d8a75 Author: Ján Tomko jto...@redhat.com CommitDate: 2012-10-29 14:38:43 +0100 xml: print uuids in the warning commit 9b704ab8235af010b1fda4886201aab02098b969 Author: Ján Tomko jto...@redhat.com CommitDate: 2012-10-23 14:24:31 +0200 xml: omit domain name from comment if it contains double hyphen Jan 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 2/3] libvirt-guests: Wait for libirtd to initialize
s/libirtd/libvirtd/ in the subject Jan 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 2/3] rbd: Simplify opening RADOS IoCTX
On 02/12/2014 03:11 PM, Wido den Hollander wrote:
Reduces code and brings logging back to one function.
---
src/storage/storage_backend_rbd.c | 43
++---
1 file changed, 16 insertions(+), 27 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c
b/src/storage/storage_backend_rbd.c
index bd21873..8d1e320 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -221,6 +221,17 @@ cleanup:
return ret;
}
+static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr,
virStoragePoolObjPtr pool)
+{
+int r = rados_ioctx_create(ptr-cluster, pool-def-source.name,
ptr-ioctx);
+if (r 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(failed to create the RBD IoCTX. Does the pool '%s'
exist?: %d),
+ pool-def-source.name, r);
Looking at the documentation [1], librados returns -errno.
It would be more user-friendly to report it as a string instead of a number.
I'd suggest using:
virReportSystemError(-r,
_(failed to create the RBD IoCTX.
Does the pool '%s' exist?'),
pool-def-source.name);
Also, putting this patch first would reduce the number of error messages that
need to be changed.
Jan
[1] http://ceph.com/docs/master/rados/api/librados/
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Document the keyboard as a valid input type
Commit bc18373 added a new input type, but didn't change the documentation. --- docs/formatdomain.html.in | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ea1a97b..400de07 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3847,15 +3847,16 @@ qemu-kvm -net nic,model=? /dev/null ... lt;devicesgt; lt;input type='mouse' bus='usb'/gt; +lt;input type='keyboard' bus='usb'/gt; lt;/devicesgt; .../pre dl dtcodeinput/code/dt ddThe codeinput/code element has one mandatory attribute, -the codetype/code whose value can be either 'mouse' or -'tablet'. The latter provides absolute -cursor movement, while the former uses relative movement. The optional +the codetype/code whose value can be 'mouse', 'tablet' or +'keyboard'. The tablet provides absolute cursor movement, +while the mouse uses relative movement. The optional codebus/code attribute can be used to refine the exact device type. It takes values xen (paravirtualized), ps2 and usb./dd /dl -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Don't leak buffer if GetFDs fails in cmdCreate
Change the logic of the function to return false by default
and move the freeing of the buffer to the cleanup section.
https://bugzilla.redhat.com/show_bug.cgi?id=1067338
---
tools/virsh-domain.c | 29 -
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 00ace11..59e843f 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6518,7 +6518,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
const char *from = NULL;
-bool ret = true;
+bool ret = false;
char *buffer;
#ifndef WIN32
bool console = vshCommandOptBool(cmd, console);
@@ -6534,7 +6534,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
return false;
if (cmdStartGetFDs(ctl, cmd, nfds, fds) 0)
-return false;
+goto cleanup;
if (vshCommandOptBool(cmd, paused))
flags |= VIR_DOMAIN_START_PAUSED;
@@ -6545,20 +6545,23 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd)
dom = virDomainCreateXMLWithFiles(ctl-conn, buffer, nfds, fds, flags);
else
dom = virDomainCreateXML(ctl-conn, buffer, flags);
-VIR_FREE(buffer);
-if (dom != NULL) {
-vshPrint(ctl, _(Domain %s created from %s\n),
- virDomainGetName(dom), from);
-#ifndef WIN32
-if (console)
-cmdRunConsole(ctl, dom, NULL, 0);
-#endif
-virDomainFree(dom);
-} else {
+if (!dom) {
vshError(ctl, _(Failed to create domain from %s), from);
-ret = false;
+goto cleanup;
}
+
+vshPrint(ctl, _(Domain %s created from %s\n),
+ virDomainGetName(dom), from);
+#ifndef WIN32
+if (console)
+cmdRunConsole(ctl, dom, NULL, 0);
+#endif
+virDomainFree(dom);
+ret = true;
+
+cleanup:
+VIR_FREE(buffer);
VIR_FREE(fds);
return ret;
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Don't leak buffer if GetFDs fails in cmdCreate
On 02/24/2014 05:49 PM, Eric Blake wrote: On 02/24/2014 06:29 AM, Ján Tomko wrote: Change the logic of the function to return false by default and move the freeing of the buffer to the cleanup section. https://bugzilla.redhat.com/show_bug.cgi?id=1067338 --- tools/virsh-domain.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) ACK Thank you, now pushed. Jan 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] Document the keyboard as a valid input type
On 02/24/2014 05:39 PM, Eric Blake wrote: On 02/24/2014 02:55 AM, Ján Tomko wrote: Commit bc18373 added a new input type, but didn't change the documentation. --- docs/formatdomain.html.in | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ea1a97b..400de07 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3847,15 +3847,16 @@ qemu-kvm -net nic,model=? /dev/null ... lt;devicesgt; lt;input type='mouse' bus='usb'/gt; +lt;input type='keyboard' bus='usb'/gt; lt;/devicesgt; .../pre dl dtcodeinput/code/dt ddThe codeinput/code element has one mandatory attribute, -the codetype/code whose value can be either 'mouse' or -'tablet'. The latter provides absolute -cursor movement, while the former uses relative movement. The optional +the codetype/code whose value can be 'mouse', 'tablet' or +'keyboard'. The tablet provides absolute cursor movement, ACK if you also add a span for a 'since 1.2.2' designation on keyboard. Thanks, pushed now. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: mark CPU usage field names as translatable
My commit ac75801 removed the translation markers when
moving the field names into an array.
---
tools/virsh-host.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 502203b..6a04f9d 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -352,12 +352,12 @@ VIR_ENUM_IMPL(vshCPUStats, VSH_CPU_LAST,
VIR_NODE_CPU_STATS_UTILIZATION);
const char *vshCPUOutput[] = {
-user:,
-system:,
-idle:,
-iowait:,
-intr:,
-usage:
+N_(user:),
+N_(system:),
+N_(idle:),
+N_(iowait:),
+N_(intr:),
+N_(usage:)
};
static bool
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: initialize str to NULL for solving a build issue
On 02/25/2014 07:28 AM, Chen Hanxiao wrote: Fix a -Werror=maybe-uninitialized issue. Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com --- tools/virsh-domain-monitor.c | 2 +- tools/virsh-domain.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) ACK and pushed. Jan 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] virsh: mark CPU usage field names as translatable
On 02/24/2014 07:48 PM, Eric Blake wrote: On 02/24/2014 11:11 AM, Ján Tomko wrote: My commit ac75801 removed the translation markers when moving the field names into an array. --- tools/virsh-host.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) ACK. Thanks, pushed. Jan 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 0/2] Improve RBD storage pool backend
On 02/25/2014 10:50 AM, Wido den Hollander wrote: The first patch improves the logging for the user. Currently the return codes from librados are not written to any logfile, which might leave a user clueless to why the storage pool is not working. The second patch sets three timeout options in librados. These are useful for when the Ceph cluster is down. Librados will then not block for ever, but return a -ETIMEDOUT so libvirt does not block for ever. Wido den Hollander (2): rbd: Include return statusses from librados/librbd in logging s/statusses/statuses/ rbd: Set timeout options for librados src/storage/storage_backend_rbd.c | 144 - 1 file changed, 78 insertions(+), 66 deletions(-) ACK and pushed. Jan 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] Fix memory leak in virSCSIDeviceListDel()
On 02/22/2014 09:00 PM, Nehal J Wani wrote: While running virscsitest, it was found that valgrind pointed out the following memory leak: ==320== 5 bytes in 1 blocks are definitely lost in loss record 4 of 37 ==320==at 0x4A069EE: malloc (vg_replace_malloc.c:270) ==320==by 0x3E6CE81171: strdup (strdup.c:43) ==320==by 0x4CB28DF: virStrdup (virstring.c:554) ==320==by 0x4CAC987: virSCSIDeviceSetUsedBy (virscsi.c:289) ==320==by 0x402321: test2 (virscsitest.c:100) ==320==by 0x403231: virtTestRun (testutils.c:199) ==320==by 0x402121: mymain (virscsitest.c:180) ==320==by 0x4039AD: virtTestMain (testutils.c:782) ==320==by 0x3E6CE1ED1C: (below main) (libc-start.c:226) ==320== --- src/util/virscsi.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) ACK and pushed. Jan 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] Fix memory leak in virDomainSnapshotDiskDefClear()
On 02/22/2014 08:37 PM, Nehal J Wani wrote:
While running domainsnapshotxml2xmltest, it was found that valgrind pointed
out
the following memory leaks:
==32176== 42 (32 direct, 10 indirect) bytes in 1 blocks are definitely lost
in loss record 42 of 66
==32176==at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==32176==by 0x4A06B62: realloc (vg_replace_malloc.c:662)
==32176==by 0x4C65A07: virReallocN (viralloc.c:243)
==32176==by 0x4C65B2E: virExpandN (viralloc.c:292)
==32176==by 0x4C65E30: virInsertElementsN (viralloc.c:434)
==32176==by 0x4CD71F3: virDomainDiskSourceDefParse (domain_conf.c:5078)
==32176==by 0x4CF6EF4: virDomainSnapshotDefParseNode (snapshot_conf.c:151)
==32176==by 0x4CF7314: virDomainSnapshotDefParseString
(snapshot_conf.c:410)
==32176==by 0x41FB8D: testCompareXMLToXMLHelper
(domainsnapshotxml2xmltest.c:100)
==32176==by 0x420FD1: virtTestRun (testutils.c:199)
==32176==by 0x41F859: mymain (domainsnapshotxml2xmltest.c:222)
==32176==by 0x42174D: virtTestMain (testutils.c:782)
==32176==
==32176== 128 (96 direct, 32 indirect) bytes in 1 blocks are definitely lost
in loss record 51 of 66
==32176==at 0x4A06BE0: realloc (vg_replace_malloc.c:662)
==32176==by 0x4C65A07: virReallocN (viralloc.c:243)
==32176==by 0x4C65B2E: virExpandN (viralloc.c:292)
==32176==by 0x4C65E30: virInsertElementsN (viralloc.c:434)
==32176==by 0x4CD71F3: virDomainDiskSourceDefParse (domain_conf.c:5078)
==32176==by 0x4CF6EF4: virDomainSnapshotDefParseNode (snapshot_conf.c:151)
==32176==by 0x4CF7314: virDomainSnapshotDefParseString
(snapshot_conf.c:410)
==32176==by 0x41FB8D: testCompareXMLToXMLHelper
(domainsnapshotxml2xmltest.c:100)
==32176==by 0x420FD1: virtTestRun (testutils.c:199)
==32176==by 0x41F859: mymain (domainsnapshotxml2xmltest.c:222)
==32176==by 0x42174D: virtTestMain (testutils.c:782)
==32176==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==32176==
---
src/conf/snapshot_conf.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 12b0930..475525f 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -82,6 +82,7 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr
disk)
{
VIR_FREE(disk-name);
VIR_FREE(disk-file);
+virDomainDiskHostDefFree(disk-nhosts, disk-hosts);
This leaves nhosts and hosts at their original values, which seems to be OK
everywhere this function is called, but it might be a problem if someone tries
to reuse the disk definition instead of freeing it.
I'd rather write this as:
while (disk-nhosts)
virDomainDiskHostDefClear(disk-hosts[--def-nhosts])
VIR_FREE(disk-hosts)
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Add virusbtest
Test src/util/virusb.c. Ján Tomko (2): Add tests for virUSBDeviceFind functions Add a test for virUSBDeviceList functions .gitignore | 1 + cfg.mk | 3 +- tests/Makefile.am | 12 + tests/virusbtest.c | 343 + .../sys_bus_usb/devices/1-1.5.3.1/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3.1/idProduct| 1 + .../sys_bus_usb/devices/1-1.5.3.1/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3.1/serial | 1 + .../sys_bus_usb/devices/1-1.5.3.3/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3.3/idProduct| 1 + .../sys_bus_usb/devices/1-1.5.3.3/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3.3/serial | 1 + .../sys_bus_usb/devices/1-1.5.3/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.3/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3/serial | 1 + .../sys_bus_usb/devices/1-1.5.4/devnum | 1 + .../sys_bus_usb/devices/1-1.5.4/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.4/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.4/serial | 1 + .../sys_bus_usb/devices/1-1.5.5/devnum | 1 + .../sys_bus_usb/devices/1-1.5.5/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.5/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.5/serial | 1 + .../sys_bus_usb/devices/1-1.5.6/devnum | 1 + .../sys_bus_usb/devices/1-1.5.6/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.6/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.6/serial | 1 + .../sys_bus_usb/devices/1-1.5/devnum | 1 + .../sys_bus_usb/devices/1-1.5/idProduct| 1 + .../sys_bus_usb/devices/1-1.5/idVendor | 1 + .../sys_bus_usb/devices/1-1.5/serial | 1 + .../sys_bus_usb/devices/1-1.6/devnum | 1 + .../sys_bus_usb/devices/1-1.6/idProduct| 1 + .../sys_bus_usb/devices/1-1.6/idVendor | 1 + .../sys_bus_usb/devices/1-1.6/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/devnum | 1 + .../sys_bus_usb/devices/1-1/idProduct | 1 + .../sys_bus_usb/devices/1-1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/serial | 1 + .../sys_bus_usb/devices/2-1.2/devnum | 1 + .../sys_bus_usb/devices/2-1.2/idProduct| 1 + .../sys_bus_usb/devices/2-1.2/idVendor | 1 + .../sys_bus_usb/devices/2-1.2/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/devnum | 1 + .../sys_bus_usb/devices/2-1/idProduct | 1 + .../sys_bus_usb/devices/2-1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb1/devnum | 1 + .../sys_bus_usb/devices/usb1/idProduct | 1 + .../sys_bus_usb/devices/usb1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb1/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb2/devnum | 1 + .../sys_bus_usb/devices/usb2/idProduct | 1 + .../sys_bus_usb/devices/usb2/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb2/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb3/devnum | 1 + .../sys_bus_usb/devices/usb3/idProduct | 1 + .../sys_bus_usb/devices/usb3/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb3/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb4/devnum | 1 + .../sys_bus_usb/devices/usb4/idProduct | 1 + .../sys_bus_usb/devices/usb4/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb4/serial | 1 + 64 files changed, 418 insertions(+), 1 deletion(-) create mode 100644 tests/virusbtest.c create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idVendor create mode 100644 tests/virusbtestdata
[libvirt] [PATCH 1/2] Add tests for virUSBDeviceFind functions
Mock the /sys/bus/usb directory and test the finding (and not finding) of some USB devices. --- .gitignore | 1 + cfg.mk | 3 +- tests/Makefile.am | 12 + tests/virusbtest.c | 268 + .../sys_bus_usb/devices/1-1.5.3.1/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3.1/idProduct| 1 + .../sys_bus_usb/devices/1-1.5.3.1/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3.1/serial | 1 + .../sys_bus_usb/devices/1-1.5.3.3/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3.3/idProduct| 1 + .../sys_bus_usb/devices/1-1.5.3.3/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3.3/serial | 1 + .../sys_bus_usb/devices/1-1.5.3/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.3/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3/serial | 1 + .../sys_bus_usb/devices/1-1.5.4/devnum | 1 + .../sys_bus_usb/devices/1-1.5.4/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.4/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.4/serial | 1 + .../sys_bus_usb/devices/1-1.5.5/devnum | 1 + .../sys_bus_usb/devices/1-1.5.5/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.5/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.5/serial | 1 + .../sys_bus_usb/devices/1-1.5.6/devnum | 1 + .../sys_bus_usb/devices/1-1.5.6/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.6/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.6/serial | 1 + .../sys_bus_usb/devices/1-1.5/devnum | 1 + .../sys_bus_usb/devices/1-1.5/idProduct| 1 + .../sys_bus_usb/devices/1-1.5/idVendor | 1 + .../sys_bus_usb/devices/1-1.5/serial | 1 + .../sys_bus_usb/devices/1-1.6/devnum | 1 + .../sys_bus_usb/devices/1-1.6/idProduct| 1 + .../sys_bus_usb/devices/1-1.6/idVendor | 1 + .../sys_bus_usb/devices/1-1.6/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/devnum | 1 + .../sys_bus_usb/devices/1-1/idProduct | 1 + .../sys_bus_usb/devices/1-1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/serial | 1 + .../sys_bus_usb/devices/2-1.2/devnum | 1 + .../sys_bus_usb/devices/2-1.2/idProduct| 1 + .../sys_bus_usb/devices/2-1.2/idVendor | 1 + .../sys_bus_usb/devices/2-1.2/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/devnum | 1 + .../sys_bus_usb/devices/2-1/idProduct | 1 + .../sys_bus_usb/devices/2-1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb1/devnum | 1 + .../sys_bus_usb/devices/usb1/idProduct | 1 + .../sys_bus_usb/devices/usb1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb1/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb2/devnum | 1 + .../sys_bus_usb/devices/usb2/idProduct | 1 + .../sys_bus_usb/devices/usb2/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb2/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb3/devnum | 1 + .../sys_bus_usb/devices/usb3/idProduct | 1 + .../sys_bus_usb/devices/usb3/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb3/serial | 1 + .../virusbtestdata/sys_bus_usb/devices/usb4/devnum | 1 + .../sys_bus_usb/devices/usb4/idProduct | 1 + .../sys_bus_usb/devices/usb4/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb4/serial | 1 + 64 files changed, 343 insertions(+), 1 deletion(-) create mode 100644 tests/virusbtest.c create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/serial create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/serial create
[libvirt] [PATCH 2/2] Add a test for virUSBDeviceList functions
Most of them are already tested in a limited way
by testing virUSBDeviceFind.
---
tests/virusbtest.c | 75 ++
1 file changed, 75 insertions(+)
diff --git a/tests/virusbtest.c b/tests/virusbtest.c
index f9104bf..74b9a5e 100644
--- a/tests/virusbtest.c
+++ b/tests/virusbtest.c
@@ -204,6 +204,78 @@ cleanup:
static int
+testUSBList(const void *opaque ATTRIBUTE_UNUSED)
+{
+virUSBDeviceListPtr list = NULL;
+virUSBDeviceListPtr devlist = NULL;
+virUSBDevicePtr dev = NULL;
+int ret = -1;
+size_t i, ndevs;
+
+if (!(list = virUSBDeviceListNew()))
+goto cleanup;
+
+if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, true, devlist) 0)
+goto cleanup;
+
+ndevs = virUSBDeviceListCount(devlist);
+for (i = 0; i ndevs; i++) {
+dev = virUSBDeviceListGet(devlist, 0);
+dev = virUSBDeviceListSteal(devlist, dev);
+
+if (virUSBDeviceListAdd(list, dev) 0)
+goto cleanup;
+dev = NULL;
+}
+
+virObjectUnref(devlist);
+devlist = NULL;
+
+if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, true, devlist) 0)
+goto cleanup;
+
+ndevs = virUSBDeviceListCount(devlist);
+for (i = 0; i ndevs; i++) {
+dev = virUSBDeviceListGet(devlist, 0);
+dev = virUSBDeviceListSteal(devlist, dev);
+
+if (virUSBDeviceListAdd(list, dev) 0)
+goto cleanup;
+dev = NULL;
+}
+
+if (virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, NULL, true, dev) 0)
+goto cleanup;
+
+if (!virUSBDeviceListFind(list, dev)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Device '%s' not in list when it should be,
+ virUSBDeviceGetName(dev));
+goto cleanup;
+}
+
+virUSBDeviceListDel(list, dev);
+dev = NULL;
+
+if (virUSBDeviceListCount(list) != 5) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Wrong device count %zu expected %d,
+ virUSBDeviceListCount(list),
+ 5);
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+virObjectUnref(list);
+virObjectUnref(devlist);
+virUSBDeviceFree(dev);
+return ret;
+}
+
+
+static int
mymain(void)
{
int rv = 0;
@@ -251,6 +323,9 @@ mymain(void)
DO_TEST_FIND_BY_VENDOR_FAIL(Bogus vendor and product, 0xf00d, 0xbeef);
DO_TEST_FIND_BY_VENDOR_FAIL(Valid vendor, 0x1d6b, 0xbeef);
+if (virtTestRun(USB List test, testUSBList, NULL) 0)
+rv = -1;
+
if (rv 0)
return EXIT_FAILURE;
return EXIT_SUCCESS;
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] sanlock: Truncate domain names longer than SANLK_NAME_LEN
On 02/27/2014 09:43 AM, Jiri Denemark wrote: Libvirt uses a domain name to fill in owner_name in sanlock_options in virLockManagerSanlockAcquire. Unfortunately, owner_name is limited to SANLK_NAME_LEN characters (including trailing '\0'), which means domains with longer names fail to start when sanlock is enabled. However, we can truncate the name when setting owner_name as explained by sanlock's author: Setting sanlk_options or the owner_name is unnecessary, and has very little to no benefit. If you do provide something in owner_name, it can be anything, sanlock doesn't care or use it. If you run the command sanlock status, the output will display a list of clients connected to the sanlock daemon. This client list is displayed as pid owner_name if the client has provided an owner_name via sanlk_options. This debugging output is the only usage of owner_name, so its only benefit is to potentially provide a more human friendly output for debugging purposes. It might be nice to add a link the bug: https://bugzilla.redhat.com/show_bug.cgi?id=1060557 Jan 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 1/2] Add tests for virUSBDeviceFind functions
On 02/27/2014 02:27 PM, Michal Privoznik wrote:
On 26.02.2014 17:54, Ján Tomko wrote:
Mock the /sys/bus/usb directory and test the finding
(and not finding) of some USB devices.
---
@@ -815,6 +817,16 @@ virpcimock_la_LIBADD = $(GNULIB_LIBS) \
virpcimock_la_LDFLAGS = -module -avoid-version \
-rpath /evil/libtool/hack/to/force/shared/lib/creation
+virusbtest_SOURCES = \
+virusbtest.c testutils.h testutils.c
+virusbtest_LDADD = $(LDADDS)
+
+virusbmock_la_SOURCES = \
+virusbtest.c
+virusbmock_la_CFLAGS = $(AM_CFLAGS) -DMOCK_HELPER=1
+virusbmock_la_LDFLAGS = -module -avoid-version \
+-rpath /evil/libtool/hack/to/force/shared/lib/creation
Is there any specific reason why the mock functions are not in a separate
file? We have that pattern already.
This pattern is used by virportallocatortests, which I touched last :)
In both cases, I think the test should only be run on Linux, since Eric showed
that #if HAVE_DLFCN_H is not enough for Cygwin and most of the virusb.c file
is Linux-specific.
+
if WITH_DBUS
virdbustest_SOURCES = \
virdbustest.c testutils.h testutils.c
diff --git a/tests/virusbtest.c b/tests/virusbtest.c
new file mode 100644
index 000..f9104bf
--- /dev/null
+++ b/tests/virusbtest.c
+static int testDeviceFind(const void *opaque)
+{
+const struct findTestInfo *info = opaque;
+int ret = -1;
+virUSBDevicePtr dev = NULL;
+virUSBDeviceListPtr devs = NULL;
+int rv = 0;
+size_t i, ndevs = 0;
+
+switch (info-how) {
+case FIND_BY_ALL:
+rv = virUSBDeviceFind(info-vendor, info-product,
+ info-bus, info-devno,
+ info-vroot, info-mandatory, dev);
+break;
+case FIND_BY_VENDOR:
+rv = virUSBDeviceFindByVendor(info-vendor, info-product,
+ info-vroot, info-mandatory, devs);
+break;
+case FIND_BY_BUS:
+rv = virUSBDeviceFindByBus(info-bus, info-devno,
+ info-vroot, info-mandatory, dev);
+break;
+}
+
+if (info-expectFailure) {
+if (rv == 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ unexpected success);
+goto cleanup;
+}
Would it make sense to:
ret = 0;
goto cleanup;
since tesing either an empty device list or NULL dev doesn't make much sense?
I know you're calling FileIterate only when there's something to call it on,
but sill.
Yes, that might look nicer.
+} else if (rv 0) {
+goto cleanup;
+}
+
+if (dev virUSBDeviceFileIterate(dev, testDeviceFileActor, NULL) 0)
+goto cleanup;
+
+if (devs)
+ndevs = virUSBDeviceListCount(devs);
+
+for (i = 0; i ndevs; i++) {
+virUSBDevicePtr device = virUSBDeviceListGet(devs, i);
+if (virUSBDeviceFileIterate(device, testDeviceFileActor, NULL) 0)
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+virObjectUnref(devs);
+virUSBDeviceFree(dev);
+return ret;
+}
+
+++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial
@@ -0,0 +1 @@
+
Why are you creating these /serial files? They don't seem to be used anywhere,
moreover, they seem to not exists even in real sysfs (at least on my system):
# find /sys/bus/usb/ -name serial | wc -l
0
I was hoping to dig out the patch adding support for specifying USB devices by
serials someday, but they don't really belong in this patch.
Thanks,
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Check if systemd is the init before creating machines
If systemd is installed, but not the init system,
systemd-machined fails with an unhelpful error message:
Launch helper exited with unknown return code 1
Fall back to manual cgroup creation if systemd is installed,
but it's not PID 1.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=69962
---
(Yes, Gentoo.)
src/util/virsystemd.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 8adf209..0404a63 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -30,6 +30,7 @@
#include virstring.h
#include viralloc.h
#include virutil.h
+#include virfile.h
#include virlog.h
#include virerror.h
@@ -142,6 +143,28 @@ cleanup:
return machinename;
}
+/*
+ * Returns 0 if systemd is the init, -2 if not
+ * -1 on fatal error
+ */
+static int
+virSystemdIsInit(void)
+{
+char *buf = NULL;
+int ret = -2;
+
+if (virFileReadAll(/proc/1/comm, sizeof(systemd\n ), buf) 0)
+return -1;
+
+if (STREQ(buf, systemd\n))
+ret = 0;
+else
+VIR_DEBUG(systemd is not the init);
+
+VIR_FREE(buf);
+return ret;
+}
+
/**
* virSystemdCreateMachine:
* @name: driver unique name of the machine
@@ -173,6 +196,9 @@ int virSystemdCreateMachine(const char *name,
if (ret 0)
return ret;
+if ((ret = virSystemdIsInit()) 0)
+return ret;
+
if (!(conn = virDBusGetSystemBus()))
return -1;
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] Add virusbtest
v1: https://www.redhat.com/archives/libvir-list/2014-February/msg01620.html v2: Split the mocked part into a separate file and only build the test on Linux. Remove 'serial' files. Get out of testDeviceFind earlier if failure is expected and use a switch. Switches are nice. Check number of returned devices more often in testUSBList, not just at the end. Ján Tomko (2): Add tests for virUSBDeviceFind functions Add a test for virUSBDeviceList functions .gitignore | 1 + cfg.mk | 3 +- tests/Makefile.am | 22 ++ tests/virusbmock.c | 99 +++ tests/virusbtest.c | 292 + .../sys_bus_usb/devices/1-1.5.3.1/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3.1/idProduct| 1 + .../sys_bus_usb/devices/1-1.5.3.1/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3.3/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3.3/idProduct| 1 + .../sys_bus_usb/devices/1-1.5.3.3/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.3/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.4/devnum | 1 + .../sys_bus_usb/devices/1-1.5.4/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.4/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.5/devnum | 1 + .../sys_bus_usb/devices/1-1.5.5/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.5/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.6/devnum | 1 + .../sys_bus_usb/devices/1-1.5.6/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.6/idVendor | 1 + .../sys_bus_usb/devices/1-1.5/devnum | 1 + .../sys_bus_usb/devices/1-1.5/idProduct| 1 + .../sys_bus_usb/devices/1-1.5/idVendor | 1 + .../sys_bus_usb/devices/1-1.6/devnum | 1 + .../sys_bus_usb/devices/1-1.6/idProduct| 1 + .../sys_bus_usb/devices/1-1.6/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/devnum | 1 + .../sys_bus_usb/devices/1-1/idProduct | 1 + .../sys_bus_usb/devices/1-1/idVendor | 1 + .../sys_bus_usb/devices/2-1.2/devnum | 1 + .../sys_bus_usb/devices/2-1.2/idProduct| 1 + .../sys_bus_usb/devices/2-1.2/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/devnum | 1 + .../sys_bus_usb/devices/2-1/idProduct | 1 + .../sys_bus_usb/devices/2-1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb1/devnum | 1 + .../sys_bus_usb/devices/usb1/idProduct | 1 + .../sys_bus_usb/devices/usb1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb2/devnum | 1 + .../sys_bus_usb/devices/usb2/idProduct | 1 + .../sys_bus_usb/devices/usb2/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb3/devnum | 1 + .../sys_bus_usb/devices/usb3/idProduct | 1 + .../sys_bus_usb/devices/usb3/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb4/devnum | 1 + .../sys_bus_usb/devices/usb4/idProduct | 1 + .../sys_bus_usb/devices/usb4/idVendor | 1 + 50 files changed, 461 insertions(+), 1 deletion(-) create mode 100644 tests/virusbmock.c create mode 100644 tests/virusbtest.c create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/idProduct create mode 100644 tests/virusbtestdata
[libvirt] [PATCHv2 1/2] Add tests for virUSBDeviceFind functions
Mock the /sys/bus/usb directory and test the finding (and not finding) of some USB devices. --- .gitignore | 1 + cfg.mk | 3 +- tests/Makefile.am | 22 +++ tests/virusbmock.c | 99 +++ tests/virusbtest.c | 190 + .../sys_bus_usb/devices/1-1.5.3.1/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3.1/idProduct| 1 + .../sys_bus_usb/devices/1-1.5.3.1/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3.3/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3.3/idProduct| 1 + .../sys_bus_usb/devices/1-1.5.3.3/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.3/devnum | 1 + .../sys_bus_usb/devices/1-1.5.3/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.3/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.4/devnum | 1 + .../sys_bus_usb/devices/1-1.5.4/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.4/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.5/devnum | 1 + .../sys_bus_usb/devices/1-1.5.5/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.5/idVendor | 1 + .../sys_bus_usb/devices/1-1.5.6/devnum | 1 + .../sys_bus_usb/devices/1-1.5.6/idProduct | 1 + .../sys_bus_usb/devices/1-1.5.6/idVendor | 1 + .../sys_bus_usb/devices/1-1.5/devnum | 1 + .../sys_bus_usb/devices/1-1.5/idProduct| 1 + .../sys_bus_usb/devices/1-1.5/idVendor | 1 + .../sys_bus_usb/devices/1-1.6/devnum | 1 + .../sys_bus_usb/devices/1-1.6/idProduct| 1 + .../sys_bus_usb/devices/1-1.6/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/1-1/devnum | 1 + .../sys_bus_usb/devices/1-1/idProduct | 1 + .../sys_bus_usb/devices/1-1/idVendor | 1 + .../sys_bus_usb/devices/2-1.2/devnum | 1 + .../sys_bus_usb/devices/2-1.2/idProduct| 1 + .../sys_bus_usb/devices/2-1.2/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/2-1/devnum | 1 + .../sys_bus_usb/devices/2-1/idProduct | 1 + .../sys_bus_usb/devices/2-1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb1/devnum | 1 + .../sys_bus_usb/devices/usb1/idProduct | 1 + .../sys_bus_usb/devices/usb1/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb2/devnum | 1 + .../sys_bus_usb/devices/usb2/idProduct | 1 + .../sys_bus_usb/devices/usb2/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb3/devnum | 1 + .../sys_bus_usb/devices/usb3/idProduct | 1 + .../sys_bus_usb/devices/usb3/idVendor | 1 + .../virusbtestdata/sys_bus_usb/devices/usb4/devnum | 1 + .../sys_bus_usb/devices/usb4/idProduct | 1 + .../sys_bus_usb/devices/usb4/idVendor | 1 + 50 files changed, 359 insertions(+), 1 deletion(-) create mode 100644 tests/virusbmock.c create mode 100644 tests/virusbtest.c create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devnum create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/idProduct create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.5/idVendor create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devnum create mode 100644
[libvirt] [PATCHv2 2/2] Add a test for virUSBDeviceList functions
Most of them are already tested in a limited way
by testing virUSBDeviceFind.
---
tests/virusbtest.c | 102 +
1 file changed, 102 insertions(+)
diff --git a/tests/virusbtest.c b/tests/virusbtest.c
index 9eb9abe..bb849b5 100644
--- a/tests/virusbtest.c
+++ b/tests/virusbtest.c
@@ -135,6 +135,105 @@ cleanup:
static int
+testCheckNdevs(const char* occasion,
+ size_t got,
+ size_t expected) {
+if (got != expected) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ %s: got %zu devices, expected %zu,
+ occasion, got, expected);
+return -1;
+}
+return 0;
+}
+
+
+static int
+testUSBList(const void *opaque ATTRIBUTE_UNUSED)
+{
+virUSBDeviceListPtr list = NULL;
+virUSBDeviceListPtr devlist = NULL;
+virUSBDevicePtr dev = NULL;
+int ret = -1;
+size_t i, ndevs;
+
+if (!(list = virUSBDeviceListNew()))
+goto cleanup;
+
+#define EXPECTED_NDEVS_ONE 3
+if (virUSBDeviceFindByVendor(0x1d6b, 0x0002, NULL, true, devlist) 0)
+goto cleanup;
+
+ndevs = virUSBDeviceListCount(devlist);
+if (testCheckNdevs(After first find, ndevs, EXPECTED_NDEVS_ONE) 0)
+goto cleanup;
+
+for (i = 0; i ndevs; i++) {
+dev = virUSBDeviceListGet(devlist, 0);
+dev = virUSBDeviceListSteal(devlist, dev);
+
+if (virUSBDeviceListAdd(list, dev) 0)
+goto cleanup;
+dev = NULL;
+}
+
+virObjectUnref(devlist);
+devlist = NULL;
+
+ndevs = virUSBDeviceListCount(list);
+if (testCheckNdevs(After first loop, ndevs, EXPECTED_NDEVS_ONE) 0)
+goto cleanup;
+
+#define EXPECTED_NDEVS_TWO 3
+if (virUSBDeviceFindByVendor(0x18d1, 0x4e22, NULL, true, devlist) 0)
+goto cleanup;
+
+ndevs = virUSBDeviceListCount(devlist);
+if (testCheckNdevs(After second find, ndevs, EXPECTED_NDEVS_TWO) 0)
+goto cleanup;
+for (i = 0; i ndevs; i++) {
+dev = virUSBDeviceListGet(devlist, 0);
+dev = virUSBDeviceListSteal(devlist, dev);
+
+if (virUSBDeviceListAdd(list, dev) 0)
+goto cleanup;
+dev = NULL;
+}
+
+if (testCheckNdevs(After second loop,
+ virUSBDeviceListCount(list),
+ EXPECTED_NDEVS_ONE + EXPECTED_NDEVS_TWO) 0)
+goto cleanup;
+
+if (virUSBDeviceFind(0x18d1, 0x4e22, 1, 20, NULL, true, dev) 0)
+goto cleanup;
+
+if (!virUSBDeviceListFind(list, dev)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ Device '%s' not in list when it should be,
+ virUSBDeviceGetName(dev));
+goto cleanup;
+}
+
+virUSBDeviceListDel(list, dev);
+dev = NULL;
+
+if (testCheckNdevs(After deleting one,
+ virUSBDeviceListCount(list),
+ EXPECTED_NDEVS_ONE + EXPECTED_NDEVS_TWO - 1) 0)
+goto cleanup;
+
+ret = 0;
+
+cleanup:
+virObjectUnref(list);
+virObjectUnref(devlist);
+virUSBDeviceFree(dev);
+return ret;
+}
+
+
+static int
mymain(void)
{
int rv = 0;
@@ -182,6 +281,9 @@ mymain(void)
DO_TEST_FIND_BY_VENDOR_FAIL(Bogus vendor and product, 0xf00d, 0xbeef);
DO_TEST_FIND_BY_VENDOR_FAIL(Valid vendor, 0x1d6b, 0xbeef);
+if (virtTestRun(USB List test, testUSBList, NULL) 0)
+rv = -1;
+
if (rv 0)
return EXIT_FAILURE;
return EXIT_SUCCESS;
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Check if systemd is the init before creating machines
On 02/27/2014 09:29 PM, Ján Tomko wrote: If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1 Fall back to manual cgroup creation if systemd is installed, but it's not PID 1. [1] https://bugs.freedesktop.org/show_bug.cgi?id=69962 --- Hello, This fails virsystemdtest on machines where systemd is not the init. Jan 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] Check if systemd is the init before creating machines
On 02/28/2014 12:05 PM, Daniel P. Berrange wrote: On Thu, Feb 27, 2014 at 09:29:24PM +0100, Ján Tomko wrote: If systemd is installed, but not the init system, systemd-machined fails with an unhelpful error message: Launch helper exited with unknown return code 1 Fall back to manual cgroup creation if systemd is installed, but it's not PID 1. [1] https://bugs.freedesktop.org/show_bug.cgi?id=69962 --- Huh, that bug doesn't mention libvirt or systemd-machined at all ? Sorry, I must've been writing faster than thinking. Some libvirt users ([1][2]) have reported this error with installed but inactive systemd, where system dbus is running and the machine1 service shows up in ListActivatableNames output, but it's not actually usable. Perhaps https://bugs.gentoo.org/show_bug.cgi?id=493246 would be a better bug for the commit message, but it seems to track multiple issues and it only mentions the error in the comments. Jan [1] https://www.redhat.com/archives/libvirt-users/2014-January/msg00043.html [2] https://www.redhat.com/archives/libvirt-users/2014-February/msg00128.html 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] Bump version to 1.2.3 for new dev cycle
On 03/03/2014 09:44 AM, Michal Privoznik wrote: Signed-off-by: Michal Privoznik mpriv...@redhat.com --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK, trivial: https://www.redhat.com/archives/libvir-list/2014-January/msg00718.html Jan 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] Check if systemd is the init before creating machines
On 02/28/2014 06:36 PM, Daniel P. Berrange wrote:
On Thu, Feb 27, 2014 at 09:29:24PM +0100, Ján Tomko wrote:
If systemd is installed, but not the init system,
systemd-machined fails with an unhelpful error message:
Launch helper exited with unknown return code 1
Fall back to manual cgroup creation if systemd is installed,
but it's not PID 1.
[1] https://bugs.freedesktop.org/show_bug.cgi?id=69962
---
(Yes, Gentoo.)
src/util/virsystemd.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 8adf209..0404a63 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -30,6 +30,7 @@
#include virstring.h
#include viralloc.h
#include virutil.h
+#include virfile.h
#include virlog.h
#include virerror.h
@@ -142,6 +143,28 @@ cleanup:
return machinename;
}
+/*
+ * Returns 0 if systemd is the init, -2 if not
+ * -1 on fatal error
+ */
+static int
+virSystemdIsInit(void)
+{
+char *buf = NULL;
+int ret = -2;
+
+if (virFileReadAll(/proc/1/comm, sizeof(systemd\n ), buf) 0)
+return -1;
+
+if (STREQ(buf, systemd\n))
+ret = 0;
+else
+VIR_DEBUG(systemd is not the init);
+
+VIR_FREE(buf);
+return ret;
+}
+
/**
* virSystemdCreateMachine:
* @name: driver unique name of the machine
@@ -173,6 +196,9 @@ int virSystemdCreateMachine(const char *name,
if (ret 0)
return ret;
+if ((ret = virSystemdIsInit()) 0)
+return ret;
+
We already do a call
ret = virDBusIsServiceEnabled(org.freedesktop.machine1);
I'd suggest that we perhaps also call
ret = virDBusIsServiceEnabled(org.freedesktop.systemd1);
instead of looking in /proc/1/comm
systemd1 also shows up in the list of activatable names even if systemd is not
running.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Replace space with a tab in the Makefile
All the other test_programs in the section use tabs and virportallocatortest sticks out with tab width other than 8. --- Pushed as trivial. tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 1b752b2..cd91734 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -143,7 +143,7 @@ test_programs = virshtest sockettest \ virlockspacetest \ virlogtest \ virstringtest \ -virportallocatortest \ + virportallocatortest \ sysinfotest \ virstoragetest \ virnetdevbandwidthtest \ -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/2] Add virusbtest
On 02/28/2014 11:15 AM, Michal Privoznik wrote: On 28.02.2014 11:10, Ján Tomko wrote: v1: https://www.redhat.com/archives/libvir-list/2014-February/msg01620.html v2: Split the mocked part into a separate file and only build the test on Linux. Remove 'serial' files. Get out of testDeviceFind earlier if failure is expected and use a switch. Switches are nice. Check number of returned devices more often in testUSBList, not just at the end. Ján Tomko (2): Add tests for virUSBDeviceFind functions Add a test for virUSBDeviceList functions ACK series Thanks. I've pushed it - now that I can: https://www.redhat.com/archives/libvir-list/2014-March/msg00071.html Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] Split out most of virDBusIsServiceEnabled
Introduce virDBusIsServiceInList which can be used to call other
methods for listing services (ListNames), not just ListActivatableNames.
No functional change.
---
src/util/virdbus.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index a6232b7..3c9416e 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -1256,13 +1256,7 @@ int virDBusMessageRead(DBusMessage *msg,
return ret;
}
-/**
- * virDBusIsServiceEnabled:
- * @name: service name
- *
- * Retruns 0 if service is available, -1 on fatal error, or -2 if service is
not available
- */
-int virDBusIsServiceEnabled(const char *name)
+static int virDBusIsServiceInList(const char *listMethod, const char *name)
{
DBusConnection *conn;
DBusMessage *reply = NULL;
@@ -1280,7 +1274,7 @@ int virDBusIsServiceEnabled(const char *name)
org.freedesktop.DBus,
/org/freedesktop/DBus,
org.freedesktop.DBus,
- ListActivatableNames,
+ listMethod,
NULL) 0)
return ret;
@@ -1305,13 +1299,25 @@ int virDBusIsServiceEnabled(const char *name)
}
}
-VIR_DEBUG(Service %s is %s, name, ret ? unavailable : available);
-
cleanup:
dbus_message_unref(reply);
return ret;
}
+/**
+ * virDBusIsServiceEnabled:
+ * @name: service name
+ *
+ * Retruns 0 if service is available, -1 on fatal error, or -2 if service is
not available
+ */
+int virDBusIsServiceEnabled(const char *name)
+{
+int ret = virDBusIsServiceInList(ListActivatableNames, name);
+
+VIR_DEBUG(Service %s is %s, name, ret ? unavailable : available);
+
+return ret;
+}
#else /* ! WITH_DBUS */
void virDBusSetSharedBus(bool shared ATTRIBUTE_UNUSED)
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Don't always skip virportallocatortest
Include dlfcn.h before checking if RTLD_NEXT is defined
---
tests/virportallocatortest.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c
index 5a93dad..fd48c11 100644
--- a/tests/virportallocatortest.c
+++ b/tests/virportallocatortest.c
@@ -23,8 +23,11 @@
#include virfile.h
#include testutils.h
-#if HAVE_DLFCN_H defined(RTLD_NEXT)
+#if HAVE_DLFCN_H
+# include dlfcn.h
+#endif
+#if defined(RTLD_NEXT)
# ifdef MOCK_HELPER
# include internal.h
# include sys/socket.h
@@ -32,7 +35,6 @@
# include arpa/inet.h
# include netinet/in.h
# include stdio.h
-# include dlfcn.h
static bool host_has_ipv6 = false;
static int (*realsocket)(int domain, int type, int protocol);
@@ -265,7 +267,7 @@ mymain(void)
VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir
/.libs/libvirportallocatormock.so)
# endif
-#else /* ! HAVE_DLFCN_H */
+#else /* ! defined(RTLD_NEXT) */
int
main(void)
{
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] build: Include sys/wait.h in commandtest.c
Commit 631923e used a few macros from sys/wait.h without including it. On Linux, they were also defined in stdlib.h, but on FreeBSD the build failed: ../../tests/commandtest.c: In function 'test1': warning: implicit declaration of function 'WIFEXITED' warning: nested extern declaration of 'WIFEXITED' [-Wnested-externs] --- Pushed as a trivial build-breaker fix. tests/commandtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/commandtest.c b/tests/commandtest.c index cb78a3c..c8053ff 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -26,6 +26,7 @@ #include unistd.h #include signal.h #include sys/stat.h +#include sys/wait.h #include fcntl.h #include testutils.h -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Don't always skip virportallocatortest
On 03/03/2014 08:24 PM, Martin Kletzander wrote: On Mon, Mar 03, 2014 at 06:33:19PM +0100, Ján Tomko wrote: Include dlfcn.h before checking if RTLD_NEXT is defined --- tests/virportallocatortest.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) ACK Martin Thanks, pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/2] Check if systemd is running before creating machines
On 03/03/2014 08:57 PM, Eric Blake wrote:
On 03/03/2014 08:47 AM, Ján Tomko wrote:
If systemd is installed, but not the init system,
Took me a couple reads to understand this. It would be easier with
s/not/is not/
systemd-machined fails with an unhelpful error message:
Launch helper exited with unknown return code 1
Currently we only check if the machine1 service is
available (in ListActivatableNames).
Also check if systemd1 service is registered with DBus
(ListNames).
This fixes https://bugs.gentoo.org/show_bug.cgi?id=493246#c22
---
ACK.
+{
+int ret = virDBusIsServiceInList(ListNames, name);
+
+VIR_DEBUG(Service %s is %sregistered, name, ret ? not : );
Translation nightmare. But this string isn't marked for translation, so
it's okay to hard-code English grammar into the ?:.
I've changed it to:
VIR_DEBUG(Service %s is %s, name, ret ? not registered : registered);
to make it easier to read and pushed it with the amended commit message.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/2] Split out most of virDBusIsServiceEnabled
On 03/03/2014 08:55 PM, Eric Blake wrote: On 03/03/2014 08:47 AM, Ján Tomko wrote: Introduce virDBusIsServiceInList which can be used to call other methods for listing services (ListNames), not just ListActivatableNames. Nice refactoring. No functional change. --- src/util/virdbus.c | 26 -- 1 file changed, 16 insertions(+), 10 deletions(-) +/** + * virDBusIsServiceEnabled: + * @name: service name + * + * Retruns 0 if service is available, -1 on fatal error, or -2 if service is not available ...merely moved down. Please fix it while touching here :) I've fixed the typo... + */ +int virDBusIsServiceEnabled(const char *name) Two-line formatting might be nice. ... but kept the single-line formatting since most of the file uses it. My findings are cosmetic, so ACK whether you address them or not. Thanks, pushed. Jan 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] Fix memory leak in virDomainSnapshotDiskDefClear()
[cc-ing Peter as he invented the function]
On 03/03/2014 09:04 PM, Nehal J Wani wrote:
---
src/conf/snapshot_conf.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 12b0930..475525f 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -82,6 +82,7 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr
disk)
{
VIR_FREE(disk-name);
VIR_FREE(disk-file);
+virDomainDiskHostDefFree(disk-nhosts, disk-hosts);
This leaves nhosts and hosts at their original values, which seems to be OK
everywhere this function is called, but it might be a problem if someone
tries
to reuse the disk definition instead of freeing it.
I'd rather write this as:
while (disk-nhosts)
virDomainDiskHostDefClear(disk-hosts[--def-nhosts])
VIR_FREE(disk-hosts)
Jan
1. Since virDomainDiskHostDefFree() already calls VIR_FREE(hosts),
shouldn't the modified patch just be:
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 12b0930..a233e8e 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -82,6 +82,8 @@
virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk)
{
VIR_FREE(disk-name);
VIR_FREE(disk-file);
+virDomainDiskHostDefFree(disk-nhosts, disk-hosts);
+disk-nhosts = 0;
}
This leaves the pointer to the freed memory in disk-hosts.
void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
2. I have a question. Why don't we have disk-nhosts = 0; each time
someone calls virDomainDiskHostDefFree(disk-nhosts, disk-hosts) in
./src/conf/domain_conf.c, ./src/storage/storage_driver.c and
./src/qemu/qemu_conf.c?
In most cases either the whole def gets freed, or hosts and nhosts are
overwritten. I'm not sure if we overwrite it in all cases in
qemuTranslateDiskSourcePool or it needs to be added there too.
Or we can change the definition of virDomainDiskHostDefFree to
virDomainDiskHostDefFree(size_t *nhosts, virDomainDiskHostDefPtr hosts)
and make *nhosts = 0 inside the function itself and make the necessary
changes wherever this function is called?
That looks more awkward than the original function to me.
I think it should be called virDomainDiskHostDefsFree instead, since it's
different from all the other DefFree functions, which only take one DefPtr
argument. The callers should make sure that the leftover values in
nhosts/hosts aren't reused.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Treat zero cpu shares as a valid value
Currently, cputuneshares0/shares/cputune is treated
as if it were not specified.
Treat is as a valid value if it was explicitly specified
and write it to the cgroups.
---
src/conf/domain_conf.c | 12 +++-
src/conf/domain_conf.h | 1 +
src/lxc/lxc_cgroup.c | 2 +-
src/lxc/lxc_driver.c | 2 ++
src/lxc/lxc_native.c | 8 +---
src/qemu/qemu_cgroup.c | 2 +-
src/qemu/qemu_driver.c | 6 +-
src/vmx/vmx.c | 1 +
8 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1d5cc14..b5f4e67 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11460,11 +11460,13 @@ virDomainDefParseXML(xmlDocPtr xml,
}
/* Extract cpu tunables. */
-if (virXPathULong(string(./cputune/shares[1]), ctxt,
- def-cputune.shares) -1) {
+if ((n = virXPathULong(string(./cputune/shares[1]), ctxt,
+ def-cputune.shares)) -1) {
virReportError(VIR_ERR_XML_ERROR, %s,
_(can't parse cputune shares value));
goto error;
+} else if (n == 0) {
+def-cputune.sharesSpecified = true;
}
if (virXPathULongLong(string(./cputune/period[1]), ctxt,
@@ -17134,14 +17136,14 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf, current='%u', def-vcpus);
virBufferAsprintf(buf, %u/vcpu\n, def-maxvcpus);
-if (def-cputune.shares ||
+if (def-cputune.sharesSpecified ||
(def-cputune.nvcpupin !virDomainIsAllVcpupinInherited(def)) ||
def-cputune.period || def-cputune.quota ||
def-cputune.emulatorpin ||
def-cputune.emulator_period || def-cputune.emulator_quota)
virBufferAddLit(buf, cputune\n);
-if (def-cputune.shares)
+if (def-cputune.sharesSpecified)
virBufferAsprintf(buf, shares%lu/shares\n,
def-cputune.shares);
if (def-cputune.period)
@@ -17195,7 +17197,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf, cpuset='%s'/\n, cpumask);
VIR_FREE(cpumask);
}
-if (def-cputune.shares ||
+if (def-cputune.sharesSpecified ||
(def-cputune.nvcpupin !virDomainIsAllVcpupinInherited(def)) ||
def-cputune.period || def-cputune.quota ||
def-cputune.emulatorpin ||
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2467f65..fed1608 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1999,6 +1999,7 @@ struct _virDomainDef {
struct {
unsigned long shares;
+bool sharesSpecified;
unsigned long long period;
long long quota;
unsigned long long emulator_period;
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 39d955c..876c32e 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -36,7 +36,7 @@ static int virLXCCgroupSetupCpuTune(virDomainDefPtr def,
virCgroupPtr cgroup)
{
int ret = -1;
-if (def-cputune.shares != 0
+if (def-cputune.sharesSpecified
virCgroupSetCpuShares(cgroup, def-cputune.shares) 0)
goto cleanup;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 10e0fbb..26333a7 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1891,10 +1891,12 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
goto cleanup;
vm-def-cputune.shares = params[i].value.ul;
+vm-def-cputune.sharesSpecified = true;
}
if (flags VIR_DOMAIN_AFFECT_CONFIG) {
vmdef-cputune.shares = params[i].value.ul;
+vmdef-cputune.sharesSpecified = true;
}
} else if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) {
if (flags VIR_DOMAIN_AFFECT_LIVE) {
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 663e29c..fc19378 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -677,9 +677,11 @@ lxcSetCpuTune(virDomainDefPtr def, virConfPtr properties)
virConfValuePtr value;
if ((value = virConfGetValue(properties, lxc.cgroup.cpu.shares))
-value-str virStrToLong_ul(value-str, NULL, 10,
- def-cputune.shares) 0)
-goto error;
+value-str) {
+if (virStrToLong_ul(value-str, NULL, 10, def-cputune.shares) 0)
+goto error;
+def-cputune.sharesSpecified = true;
+}
if ((value = virConfGetValue(properties,
lxc.cgroup.cpu.cfs_quota_us))
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index a97f184..57c7302 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -653,7 +653,7 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
}
}
-if (vm-def-cputune.shares
+if (vm-def-cputune.sharesSpecified
[libvirt] [RFC][PATCH 0/2] Fix zero cpu shares handling
Hello, This series tries to fix the behavior of cputuneshares0/shares/cputune (means default shares of 1024) and virsh schedinfo --set cpu_shares=0 (means minimum of 2) If 0 was explicitly specified, treat it as a valid value (which will get converted to 2 by the kernel) Re-read the shares value to reflect the cpu shares in live XML. In v1, I've tried to change the schedinfo behavior instead, but I have not found a way to reset the value to the kernel default (which is currently 1024 - see ROOT_TASK_GROUP_LOAD in kernel/sched/sched.h in Linux source) other than writing it to the cgroup fs. Partially fixes https://bugzilla.redhat.com/show_bug.cgi?id=998431 Ján Tomko (2): Treat zero cpu shares as a valid value Show the real cpu shares value in live xml src/conf/domain_conf.c | 12 +++- src/conf/domain_conf.h | 1 + src/lxc/lxc_cgroup.c | 13 ++--- src/lxc/lxc_driver.c | 8 +++- src/lxc/lxc_native.c | 8 +--- src/qemu/qemu_cgroup.c | 12 +--- src/qemu/qemu_driver.c | 13 +++-- src/vmx/vmx.c | 1 + 8 files changed, 51 insertions(+), 17 deletions(-) -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] Show the real cpu shares value in live xml
---
src/lxc/lxc_cgroup.c | 13 ++---
src/lxc/lxc_driver.c | 6 +-
src/qemu/qemu_cgroup.c | 12 +---
src/qemu/qemu_driver.c | 7 ++-
4 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 876c32e..5887b24 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -36,9 +36,16 @@ static int virLXCCgroupSetupCpuTune(virDomainDefPtr def,
virCgroupPtr cgroup)
{
int ret = -1;
-if (def-cputune.sharesSpecified
-virCgroupSetCpuShares(cgroup, def-cputune.shares) 0)
-goto cleanup;
+
+if (def-cputune.sharesSpecified) {
+unsigned long long val;
+if (virCgroupSetCpuShares(cgroup, def-cputune.shares) 0)
+goto cleanup;
+
+if (virCgroupGetCpuShares(cgroup, val) 0)
+goto cleanup;
+def-cputune.shares = val;
+}
if (def-cputune.quota != 0
virCgroupSetCpuCfsQuota(cgroup, def-cputune.quota) 0)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 26333a7..aafb81a 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1887,10 +1887,14 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) {
if (flags VIR_DOMAIN_AFFECT_LIVE) {
+unsigned long long val;
if (virCgroupSetCpuShares(priv-cgroup, params[i].value.ul)
0)
goto cleanup;
-vm-def-cputune.shares = params[i].value.ul;
+if (virCgroupGetCpuShares(priv-cgroup, val) 0)
+goto cleanup;
+
+vm-def-cputune.shares = val;
vm-def-cputune.sharesSpecified = true;
}
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 57c7302..a13f085 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -653,9 +653,15 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
}
}
-if (vm-def-cputune.sharesSpecified
-virCgroupSetCpuShares(priv-cgroup, vm-def-cputune.shares) 0)
-return -1;
+if (vm-def-cputune.sharesSpecified) {
+unsigned long long val;
+if (virCgroupSetCpuShares(priv-cgroup, vm-def-cputune.shares) 0)
+return -1;
+
+if (virCgroupGetCpuShares(priv-cgroup, val) 0)
+return -1;
+vm-def-cputune.shares = val;
+}
return 0;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3c9ddb6..d082a6b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9004,9 +9004,14 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
if (STREQ(param-field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) {
if (flags VIR_DOMAIN_AFFECT_LIVE) {
+unsigned long long val;
if (virCgroupSetCpuShares(priv-cgroup, value_ul) 0)
goto cleanup;
-vm-def-cputune.shares = value_ul;
+
+if (virCgroupGetCpuShares(priv-cgroup, val) 0)
+goto cleanup;
+
+vm-def-cputune.shares = val;
vm-def-cputune.sharesSpecified = true;
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add PCI-multibus support for ppc
On 02/28/2014 10:10 AM, hong-hua@freescale.com wrote:
Hi Daniel,
Refer to libvirt/src/qemu/qemu_capabilities.c, only x86_64/i686 support
PCI_MULTIBUS.
/* Currently only x86_64 and i686 support PCI-multibus. */
if (qemuCaps-arch == VIR_ARCH_X86_64 ||
qemuCaps-arch == VIR_ARCH_I686) {
virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS);
libvirt/src/qemu/qemu_capabilities.h:
QEMU_CAPS_PCI_MULTIBUS = 52, /* bus=pci.0 vs bus=pci */
Exactly there're several PowerPC platforms also use pci.0 as PCI bus name.
$ grep -r pci\.0 qemu/hw/
qemu/hw/ppc/prep.c:pci_bus = (PCIBus *)qdev_get_child_bus(dev, pci.0);
qemu/hw/ppc/e500.c:pci_bus = (PCIBus *)qdev_get_child_bus(dev, pci.0);
qemu/hw/ppc/ppc440_bamboo.c:pcibus = (PCIBus *)qdev_get_child_bus(dev,
pci.0);
Looking at 'info qtree' output it seems pseries is the only PPC machine type
using pci (out of those I managed to run):
ppce500
bus: pci.0
mac99
bus: pci.0
g3beige
bus: pci.0
mpc8544ds
bus: pci.0
ref405ep
qemu-system-ppc64: Could not load PowerPC BIOS 'ppc405_rom.bin'
taihu
qemu-system-ppc64: Could not load PowerPC BIOS 'ppc405_rom.bin'
bamboo
bus: pci.0
prep
bus: pci.0
pseries
Warning: Disabling some instructions which are not emulated by TCG (0x0, 0x4)
bus: pci
virtex-ml507
There's no much platform specific code in libvirt.
How can we get the exact PCI bus name and make pci.0 work on the above
PowerPC platforms?
I think we need to check the def-os.machine string on PPC, since we only
probe a particular QEMU binary, not every machine type it supports.
Jan
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 4/7] Turn virLogSource into a struct instead of an enum
On 03/03/2014 08:18 PM, Daniel P. Berrange wrote:
As part of the goal to get away from doing string matching on
filenames when deciding whether to emit a log message, turn
the virLogSource enum into a struct which contains a log
name. There will eventually be one virLogSource instance
statically declared per source file. To minimise churn in this
commit though, a single global instance is used.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
src/libvirt_private.syms | 1 +
src/node_device/node_device_udev.c | 2 +-
src/qemu/qemu_capabilities.c | 2 +-
src/util/viraudit.c| 7 ---
src/util/viraudit.h| 10 ++
src/util/virerror.c| 2 +-
src/util/virlog.c | 30 --
src/util/virlog.h | 33 -
src/util/virprobe.h| 4 ++--
tests/testutils.c | 2 +-
10 files changed, 45 insertions(+), 48 deletions(-)
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 6ba2daa..cb27725 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -68,7 +67,7 @@ typedef enum {
*
* Do nothing but eat parameters.
*/
-static inline void virLogEatParams(virLogSource unused, ...)
+static inline void virLogEatParams(virLog unused, ...)
s/virLog /virLogSourcePtr /
{
/* Silence gcc */
unused = unused;
Jan
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 1/2] Treat zero cpu shares as a valid value
On 03/04/2014 03:30 PM, Martin Kletzander wrote: On Tue, Mar 04, 2014 at 02:13:14PM +0100, Ján Tomko wrote: Currently, cputuneshares0/shares/cputune is treated as if it were not specified. s/were/was/ in this particular case, I guess. I think this particular case is a perfectly cromulent use of the past subjunctive to express a counterfactual condition. ACK, Martin Thank you for the review, I will take another look at the series before pushing, since this was an RFC. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
