[libvirt] [PATCH 1/3] Implementation deficiency in virInitctlSetRunLevel v2
Hello, list.
Refuse following symlinks in virInitctlSetRunLevel.
A reasonable fallback for the next two patches, which apply fork-setns
technique recommended on this list.
---
src/util/virinitctl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virinitctl.c b/src/util/virinitctl.c
index 64bc23a..5cea992 100644
--- a/src/util/virinitctl.c
+++ b/src/util/virinitctl.c
@@ -139,7 +139,7 @@ int virInitctlSetRunLevel(virInitctlRunLevel level,
return -1;
}
-if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) 0)
{
+if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW))
0) {
if (errno == ENOENT) {
ret = 0;
goto cleanup;
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] Implementation deficiency in virInitctlSetRunLevel v2
Implement fork setns for lxcDomainShutdownFlags
---
src/lxc/lxc_driver.c | 45 +
1 file changed, 45 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c499182..9d200b2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2619,6 +2619,50 @@ lxcDomainShutdownFlags(virDomainPtr dom,
goto cleanup;
}
+#ifdef HAVE_SETNS
+if (flags == 0 || flags VIR_DOMAIN_SHUTDOWN_INITCTL) {
+int pid = -1;
+int status = -1;
+int mfd = -1;
+
+if (virAsprintf(vroot, /proc/%llu/ns/mnt,
+(unsigned long long)priv-initpid) 0) {
+goto cleanup;
+}
+
+if ((mfd = open(vroot, O_RDONLY)) 0) {
+virReportSystemError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(Kernel does not provide mount namespace));
+ goto cleanup;
+}
+
+switch (pid = fork()) {
+case 0:
+if (setns(mfd, 0) == -1) {
+exit(-1);
+}
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
+exit(rc);
+break;
+case -1:
+virReportSystemError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Fork failed));
+goto cleanup;
+default:
+if (waitpid(pid, status, 0) 0 || status 0) {
+virReportSystemError(errno,
+ _(Sending shutdown failed with status
%i),
+ status);
+rc = 0;
+} else {
+rc = status;
+}
+close(mfd);
+}
+} else {
+rc = 0;
+}
+#else
if (virAsprintf(vroot, /proc/%llu/root,
(unsigned long long)priv-initpid) 0)
goto cleanup;
@@ -2638,6 +2682,7 @@ lxcDomainShutdownFlags(virDomainPtr dom,
} else {
rc = 0;
}
+#endif
if (rc == 0
(flags == 0 ||
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] Implementation deficiency in virInitctlSetRunLevel v2
Implement fork setns for lxcDomainShutdownFlags
---
src/lxc/lxc_driver.c | 45 +
1 file changed, 45 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 9d200b2..1227736 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2744,6 +2744,50 @@ lxcDomainReboot(virDomainPtr dom,
goto cleanup;
}
+#ifdef HAVE_SETNS
+if (flags == 0 || flags VIR_DOMAIN_REBOOT_INITCTL) {
+int pid = -1;
+int status = -1;
+int mfd = -1;
+
+if (virAsprintf(vroot, /proc/%llu/ns/mnt,
+(unsigned long long)priv-initpid) 0) {
+goto cleanup;
+}
+
+if ((mfd = open(vroot, O_RDONLY)) 0) {
+virReportSystemError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(Kernel does not provide mount namespace));
+ goto cleanup;
+}
+
+switch (pid = fork()) {
+case 0:
+if (setns(mfd, 0) == -1) {
+exit(-1);
+}
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT, NULL);
+exit(rc);
+break;
+case -1:
+virReportSystemError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Fork failed));
+goto cleanup;
+default:
+if (waitpid(pid, status, 0) 0 || status 0) {
+virReportSystemError(errno,
+ _(Sending reboot failed with status %i),
+ status);
+rc = 0;
+} else {
+rc = status;
+}
+close(mfd);
+}
+} else {
+rc = 0;
+}
+#else
if (virAsprintf(vroot, /proc/%llu/root,
(unsigned long long)priv-initpid) 0)
goto cleanup;
@@ -2763,6 +2807,7 @@ lxcDomainReboot(virDomainPtr dom,
} else {
rc = 0;
}
+#endif
if (rc == 0
(flags == 0 ||
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Implementation deficiency in virInitctlSetRunLevel v2
Hello, list.
Refuse following symlinks in virInitctlSetRunLevel.
A reasonable fallback for the next two patches, which apply fork-setns
technique recommended on this list.
---
src/util/virinitctl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virinitctl.c b/src/util/virinitctl.c
index 64bc23a..5cea992 100644
--- a/src/util/virinitctl.c
+++ b/src/util/virinitctl.c
@@ -139,7 +139,7 @@ int virInitctlSetRunLevel(virInitctlRunLevel level,
return -1;
}
-if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) 0)
{
+if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW))
0) {
if (errno == ENOENT) {
ret = 0;
goto cleanup;
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Fill qemucapabilitiesdata with some data
On 10/03/2013 07:25 AM, Michal Privoznik wrote: The actual patches are accessible at: git://gitorious.org/libvirt/michal-staging.git branch test_qemu_capabilities_data I'm not sending the actual patches as it's big junk of JSON qemu replies. The patches has from 35KiB to 59KiB. I don't want to overload the list. While trying to backport this series, I noticed that: tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 114 + tests/qemucapabilitiesdata/caps_1.2.2-1.replies | 1543 + these .replies files are very hard to modify, when using a newer or older version of libvirt that issues more or fewer JSON commands (that is, the replies file is hardcoded to giving answers in the same order as libvirt asks questions, but if the set or order of questions changes, it's hard to see which answer(s) needs changing). I would suggest that we update the files to add a comment describing the JSON command issued prior to each reply; the test should filter out the comments. Doing this will make it easier to modify the .replies files in the future if we tweak qemu_capabilities.c to trigger more JSON commands even for older qemu responses. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]
On gio, 2013-12-19 at 11:46 +, Ian Jackson wrote: Dario Faggioli writes (Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]): [Moving this to libvir, libvir-users in Bcc. Also, added xen-devel] 4.2.1 is lacking libxl: fix stale timeout event callback race libxl: fix stale fd event callback race These are both in stable-4.2 as 6f0f339dd4378d062a211969f45cd23af12bf386 a87ef897295ec17788e41e9a8f4c0ada7a5a45f8 I see. Thanks Ian. Around the time I was developing those in xen-unstable.git there were some libvirt patches which were necessary too. That should be ok, as he's using the very latest libvirt. For Xen, cooldharma06, you really should at least upgrade to 4.2.3, considering it is the latest 4.2.x release, which is really what ought to be used. Regards, Dario -- This happens because I choose it to happen! (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems RD Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] lvm storage pool destroy problem
On Thu, Dec 19, 2013 at 01:54:19AM +0500, Umar Draz wrote: Hi I am trying to stop the storage pool through php-libvirt api which is a LVM volume group. when I tried to stop it I got error error: internal error Child process (/sbin/vgchange -aln mylvmgrp) unexpected exit status 5: Can't deactivate volume group mylvmgrp with 4 open logical volume(s) Then I tried throggh virsh command but the result remain same. *root@kvm02:~# virsh pool-destroy vms* error: Failed to destroy pool vms error: internal error Child process (/sbin/vgchange -aln mylvmgrp) unexpected exit status 5: Can't deactivate volume group mylvmgrp with 4 open logical volume(s) This indicates some process(s) are still using the logical volumes so it is preventingn you breaking those apps. This is expected behaviour. The same error also occured if I use direct vgchange -aln on my kvm host, but then I tried *vgchange -aly mylvmgrp* this time its worked That is a totally different operation -aly *activates* volumes which is the exact opposite of what 'pool-destroy' needs, which is to de-activate the volumes 14 logical volume(s) in volume group mylvmgrp now active 1) Now would you please help me? is this libvirt issue? or LVM? 2) Is this possible virsh pool-destroy pool also run the vgchange -aly instead of -aln AFAICT, there's no bug here. You just need to stop using the volumes before stopping the pool. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virnettlscontexttest fails with GNUTLS 3.0.28
Changed the constraints on gnutls to 3.1+ --- tests/virnettlscontexttest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index fc512fc..d9a4e9d 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -268,7 +268,8 @@ mymain(void) * be rejected. GNUTLS 3 does not reject it and * we don't anticipate them changing this behaviour */ -DO_CTX_TEST(true, cacert4req.filename, servercert4req.filename, GNUTLS_VERSION_MAJOR = 3); +DO_CTX_TEST(true, cacert4req.filename, servercert4req.filename, +GNUTLS_VERSION_MAJOR = 3 GNUTLS_VERSION_MINOR = 1); DO_CTX_TEST(true, cacert5req.filename, servercert5req.filename, true); DO_CTX_TEST(true, cacert6req.filename, servercert6req.filename, true); -- 1.8.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: fix bogus target in gluster volume xml
On 12/19/2013 02:18 AM, Peter Krempa wrote:
On 12/19/13 05:38, Eric Blake wrote:
Commit 6cd60b6 was flat out broken - it tried to print into the
wrong variable. My testing was obviously too cursory (did the
name get a slash added?); valgrind would have caught the error.
Thankfully it didn't hit any release.
... you still need to free state-uri-path before you overwrite it with
the previous value that is temporarily stored in 'tmp' a few lines below:
tmp = state-uri-path;
if (virAsprintf(vol-key, %s%s, state-uri-path, name) 0) {
state-uri-path = tmp;
goto cleanup;
}
if (!(vol-target.path = virURIFormat(state-uri))) {
state-uri-path = tmp;
goto cleanup;
}
state-uri-path = tmp; --- here
ACK with the memleak resolved.
A demonstration of how NOT to write patches late at night. Thanks for
catching that; pushing with this squashed in.
diff --git i/src/storage/storage_backend_gluster.c
w/src/storage/storage_backend_gluster.c
index 9aa692e..2ec2424 100644
--- i/src/storage/storage_backend_gluster.c
+++ w/src/storage/storage_backend_gluster.c
@@ -232,9 +232,11 @@
virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
goto cleanup;
}
if (!(vol-target.path = virURIFormat(state-uri))) {
+VIR_FREE(state-uri-path);
state-uri-path = tmp;
goto cleanup;
}
+VIR_FREE(state-uri-path);
state-uri-path = tmp;
if (S_ISDIR(st-st_mode)) {
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxcDomainShutdownFlags: Cleanup @flags usage
On Wed, Dec 18, 2013 at 07:19:31PM +0100, Michal Privoznik wrote:
Currently, the @flags usage is a bit unclear at first sight to say the
least. There's no need for such unclear code especially when we can
borrow the working code from qemuDomainShutdownFlags().
In addition, this fixes one bug too. If user requested both
VIR_DOMAIN_SHUTDOWN_INITCTL and VIR_DOMAIN_SHUTDOWN_SIGNAL at the same
time, he is basically saying: 'Use the force Luke! If initctl fails try
sending a signal.' But with the current code we don't do that. If
initctl fails for some reason (e.g. inability to write to /dev/initctl)
we don't try sending any signal but fail immediately. To make things
worse, making a domain shutdown with bare _SIGNAL was working by blind
chance of a @rc variable being placed at correct place on the stack so
its initial value was zero.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/lxc/lxc_driver.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c499182..a563b70 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2594,7 +2594,8 @@ lxcDomainShutdownFlags(virDomainPtr dom,
virDomainObjPtr vm;
char *vroot = NULL;
int ret = -1;
-int rc;
+int rc = 0;
+bool useInitctl = false, initctlRequested, signalRequested;
virCheckFlags(VIR_DOMAIN_SHUTDOWN_INITCTL |
VIR_DOMAIN_SHUTDOWN_SIGNAL, -1);
@@ -2623,25 +2624,24 @@ lxcDomainShutdownFlags(virDomainPtr dom,
(unsigned long long)priv-initpid) 0)
goto cleanup;
-if (flags == 0 ||
-(flags VIR_DOMAIN_SHUTDOWN_INITCTL)) {
-if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
-vroot)) 0) {
+initctlRequested = flags VIR_DOMAIN_SHUTDOWN_INITCTL;
+signalRequested = flags VIR_DOMAIN_SHUTDOWN_SIGNAL;
+
+if (initctlRequested || !flags)
+useInitctl = true;
I'm not sure this logic is right.
I think we want to do without useInitctl and have
initctlRequested = !flags || (flags VIR_DOMAIN_SHUTDOWN_INITCTL);
signalRequested = !flags || (flags VIR_DOMAIN_SHUTDOWN_SIGNAL);
because otherwise
+
+if (useInitctl) {
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, vroot);
+if (rc 0 !signalRequested)
goto cleanup;
-}
-if (rc == 0 flags != 0
-((flags ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
+if (rc == 0 !signalRequested) {
This code block won't fallback to trying the signal method
when flags == 0
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(Container does not provide an initctl pipe));
goto cleanup;
}
-} else {
-rc = 0;
}
-if (rc == 0
-(flags == 0 ||
- (flags VIR_DOMAIN_SHUTDOWN_SIGNAL))) {
+if (rc == 0 !useInitctl) {
if (kill(priv-initpid, SIGTERM) 0
errno != ESRCH) {
virReportSystemError(errno,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 RESEND 7/8] qemu: allow to setup throttle blkio cgroup through virsh
On Fri, Dec 13, 2013 at 11:08:10AM +0800, Gao feng wrote: With this patch, user can setup throttle blkio cgroup through virsh for qemu domain. Signed-off-by: Guan Qiang hzguanqi...@corp.netease.com Signed-off-by: Gao feng gaof...@cn.fujitsu.com --- src/qemu/qemu_driver.c | 442 - 1 file changed, 404 insertions(+), 38 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] maint: improve debug of libvirt-{qemu, lxc} apis
On 12/19/2013 08:17 AM, Daniel P. Berrange wrote:
On Thu, Dec 19, 2013 at 08:13:36AM -0700, Eric Blake wrote:
diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
index 115d8d1..b8c842d 100644
--- a/src/libvirt_internal.h
+++ b/src/libvirt_internal.h
@@ -27,6 +27,113 @@
# include internal.h
+#define VIR_DOMAIN_DEBUG(...) \
+VIR_DOMAIN_DEBUG_EXPAND(VIR_DOMAIN_DEBUG_, \
+VIR_HAS_COMMA(__VA_ARGS__), \
+__VA_ARGS__)
I'd suggest these go in datatypes.h
Good idea.
+
+/**
+ * VIR_UUID_DEBUG:
+ * @conn: connection
+ * @uuid: possibly null UUID array
+ */
+#define VIR_UUID_DEBUG(conn, uuid) \
+do {\
+if (uuid) { \
+char _uuidstr[VIR_UUID_STRING_BUFLEN]; \
+virUUIDFormat(uuid, _uuidstr); \
+VIR_DEBUG(conn=%p, uuid=%s, conn, _uuidstr); \
+} else {\
+VIR_DEBUG(conn=%p, uuid=(null), conn);\
+} \
+} while (0)
And this in viruuid.h
Sure, although VIR_DOMAIN_DEBUG requires the use of viruuid.h in the
first place.
+#define virLibDomainSnapshotError(code, ...) \
+virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, code, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
I'd venture to sugggest that these functions really have little
to no benefit / reason to exist. They aren't really simplifying
like, just making it different. They're historical baggage from
the old time when they were actual functions which did extra
sprintf formatting work. Could we just remove them ??
Perhaps, by using virReportError instead. I can give that a try; but
just looking at it, virReportError() hardcodes VIR_FROM_THIS as its
first argument, whereas virLib*Error() sets different VIR_FROM_*
categories. Worse, we use multiple calls from within a single function
(so continually redefining VIR_FROM_THIS would get painful) - for
example, virDomainSnapshotCreateXML() uses both virLibDomainError()
(VIR_FROM_DOMAIN) and virLibConnError() (VIR_FROM_NONE).
I'll definitely split into 2 patches; the VIR_DOMAIN_DEBUG motion in one
(since it was less controversial) and the potential virLib*Error cleanup
in the other. v2 coming up later today.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 RESEND 8/8] lxc: allow to setup throttle blkio cgroup through virsh
On Fri, Dec 13, 2013 at 11:09:01AM +0800, Gao feng wrote:
With this patch,user can set throttle blkio cgroup for
lxc domain through virsh tool.
Signed-off-by: Guan Qiang hzguanqi...@corp.netease.com
Signed-off-by: Gao feng gaof...@cn.fujitsu.com
---
src/lxc/lxc_driver.c | 671
+--
1 file changed, 646 insertions(+), 25 deletions(-)
ACK
@@ -4623,6 +5243,7 @@ static virDriver lxcDriver = {
.name = LXC_DRIVER_NAME,
.connectOpen = lxcConnectOpen, /* 0.4.2 */
.connectClose = lxcConnectClose, /* 0.4.2 */
+.connectSupportsFeature = lxcConnectSupportsFeature, /* 1.1.4 */
But change this to 1.2.1
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] maint: improve debug of libvirt-{qemu, lxc} apis
On Thu, Dec 19, 2013 at 09:35:23AM -0700, Eric Blake wrote:
On 12/19/2013 08:17 AM, Daniel P. Berrange wrote:
On Thu, Dec 19, 2013 at 08:13:36AM -0700, Eric Blake wrote:
diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
index 115d8d1..b8c842d 100644
--- a/src/libvirt_internal.h
+++ b/src/libvirt_internal.h
@@ -27,6 +27,113 @@
# include internal.h
+#define VIR_DOMAIN_DEBUG(...) \
+VIR_DOMAIN_DEBUG_EXPAND(VIR_DOMAIN_DEBUG_, \
+VIR_HAS_COMMA(__VA_ARGS__), \
+__VA_ARGS__)
I'd suggest these go in datatypes.h
Good idea.
+
+/**
+ * VIR_UUID_DEBUG:
+ * @conn: connection
+ * @uuid: possibly null UUID array
+ */
+#define VIR_UUID_DEBUG(conn, uuid) \
+do {\
+if (uuid) { \
+char _uuidstr[VIR_UUID_STRING_BUFLEN]; \
+virUUIDFormat(uuid, _uuidstr); \
+VIR_DEBUG(conn=%p, uuid=%s, conn, _uuidstr); \
+} else {\
+VIR_DEBUG(conn=%p, uuid=(null), conn);\
+} \
+} while (0)
And this in viruuid.h
Sure, although VIR_DOMAIN_DEBUG requires the use of viruuid.h in the
first place.
+#define virLibDomainSnapshotError(code, ...) \
+virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, code, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
I'd venture to sugggest that these functions really have little
to no benefit / reason to exist. They aren't really simplifying
like, just making it different. They're historical baggage from
the old time when they were actual functions which did extra
sprintf formatting work. Could we just remove them ??
Perhaps, by using virReportError instead. I can give that a try; but
just looking at it, virReportError() hardcodes VIR_FROM_THIS as its
first argument, whereas virLib*Error() sets different VIR_FROM_*
categories. Worse, we use multiple calls from within a single function
(so continually redefining VIR_FROM_THIS would get painful) - for
example, virDomainSnapshotCreateXML() uses both virLibDomainError()
(VIR_FROM_DOMAIN) and virLibConnError() (VIR_FROM_NONE).
Oh true I'd forgotten it relied on VIR_FROM_THIS.
How about just renaming them s/Lib/Report/
eg virLibDomainError - virReportDomainError and having
them in datatypes.h too.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Implementation deficiency in virInitctlSetRunLevel v2
On Thu, Dec 19, 2013 at 08:07:39PM +0400, Reco wrote:
Hello, list.
Refuse following symlinks in virInitctlSetRunLevel.
A reasonable fallback for the next two patches, which apply fork-setns
technique recommended on this list.
---
src/util/virinitctl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virinitctl.c b/src/util/virinitctl.c
index 64bc23a..5cea992 100644
--- a/src/util/virinitctl.c
+++ b/src/util/virinitctl.c
@@ -139,7 +139,7 @@ int virInitctlSetRunLevel(virInitctlRunLevel level,
return -1;
}
-if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) 0)
{
+if ((fd = open(path, O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW))
0) {
if (errno == ENOENT) {
ret = 0;
goto cleanup;
Unfortunately O_NOFOLLOW will still resolve a symlink for '/dev' itself.
AFAICT there is simply no safe way to open /proc/$pid/root/* files at
all if you don't trust the $pid.
Unless someone has bright ideas then I think we should just abandon any
and all use of /proc/$PID/root and mandate setns() for this. Yes it will
mean we require newer kernel for this functionality to work but that is
preferrable to an insecure impl I think.
NB, we are treating this issue as a public security flaw and will assign
a CVE for it soon.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix explicit usage of default video PCI slots
Don't set the PCI address of primary video to the default
if it's already occupied by another device.
Without this, a primary video card that doesn't have an
address gets the default one on the 1st pass of PCI address
allocation, even if it's occupied. This address gets picked
up by the second pass, and a conflict occurs:
XML error: Attempted double use of PCI slot :00:02.0
(may need multifunction='on' for device on function 0)
Also fix the test that was supposed to catch this.
---
src/qemu/qemu_command.c | 50 +---
tests/qemuxml2argvtest.c | 3 ++-
2 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d723dc8..2a64cd1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2379,7 +2379,6 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
size_t i;
virDevicePCIAddress tmp_addr;
bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
-virDevicePCIAddressPtr addrptr;
char *addrStr = NULL;
qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_HOTPLUGGABLE |
QEMU_PCI_CONNECT_TYPE_PCI;
@@ -2446,22 +2445,17 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
*/
virDomainVideoDefPtr primaryVideo = def-videos[0];
if (primaryVideo-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-primaryVideo-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-primaryVideo-info.addr.pci.domain = 0;
-primaryVideo-info.addr.pci.bus = 0;
-primaryVideo-info.addr.pci.slot = 2;
-primaryVideo-info.addr.pci.function = 0;
-addrptr = primaryVideo-info.addr.pci;
-
-if (!(addrStr = qemuDomainPCIAddressAsString(addrptr)))
+memset(tmp_addr, 0, sizeof(tmp_addr));
+tmp_addr.slot = 2;
+
+if (!(addrStr = qemuDomainPCIAddressAsString(tmp_addr)))
goto cleanup;
-if (!qemuDomainPCIAddressValidate(addrs, addrptr,
+if (!qemuDomainPCIAddressValidate(addrs, tmp_addr,
addrStr, flags, false))
goto cleanup;
-if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
+if (qemuDomainPCIAddressSlotInUse(addrs, tmp_addr)) {
if (qemuDeviceVideoUsable) {
-virResetLastError();
if (qemuDomainPCIAddressReserveNextSlot(addrs,
primaryVideo-info,
flags) 0)
@@ -2472,8 +2466,11 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
QEMU needs it for primary video));
goto cleanup;
}
-} else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr, flags)
0) {
-goto cleanup;
+} else {
+if (qemuDomainPCIAddressReserveSlot(addrs, tmp_addr, flags)
0)
+goto cleanup;
+primaryVideo-info.addr.pci = tmp_addr;
+primaryVideo-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
}
} else if (!qemuDeviceVideoUsable) {
if (primaryVideo-info.addr.pci.domain != 0 ||
@@ -2535,7 +2532,6 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
size_t i;
virDevicePCIAddress tmp_addr;
bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
-virDevicePCIAddressPtr addrptr;
char *addrStr = NULL;
qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCIE;
@@ -2619,22 +2615,17 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
*/
virDomainVideoDefPtr primaryVideo = def-videos[0];
if (primaryVideo-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
-primaryVideo-info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-primaryVideo-info.addr.pci.domain = 0;
-primaryVideo-info.addr.pci.bus = 0;
-primaryVideo-info.addr.pci.slot = 1;
-primaryVideo-info.addr.pci.function = 0;
-addrptr = primaryVideo-info.addr.pci;
-
-if (!(addrStr = qemuDomainPCIAddressAsString(addrptr)))
+memset(tmp_addr, 0, sizeof(tmp_addr));
+tmp_addr.slot = 1;
+
+if (!(addrStr = qemuDomainPCIAddressAsString(tmp_addr)))
goto cleanup;
-if (!qemuDomainPCIAddressValidate(addrs, addrptr,
+if (!qemuDomainPCIAddressValidate(addrs, tmp_addr,
addrStr, flags, false))
goto cleanup;
-if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
+if (qemuDomainPCIAddressSlotInUse(addrs, tmp_addr)) {
if (qemuDeviceVideoUsable) {
-
Re: [libvirt] [PATCH 2/3] Implementation deficiency in virInitctlSetRunLevel v2
On Thu, Dec 19, 2013 at 08:07:50PM +0400, Reco wrote:
Implement fork setns for lxcDomainShutdownFlags
---
src/lxc/lxc_driver.c | 45 +
1 file changed, 45 insertions(+)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c499182..9d200b2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2619,6 +2619,50 @@ lxcDomainShutdownFlags(virDomainPtr dom,
goto cleanup;
}
+#ifdef HAVE_SETNS
+if (flags == 0 || flags VIR_DOMAIN_SHUTDOWN_INITCTL) {
+int pid = -1;
+int status = -1;
+int mfd = -1;
+
+if (virAsprintf(vroot, /proc/%llu/ns/mnt,
+(unsigned long long)priv-initpid) 0) {
+goto cleanup;
+}
+
+if ((mfd = open(vroot, O_RDONLY)) 0) {
+virReportSystemError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(Kernel does not provide mount
namespace));
+ goto cleanup;
+}
+
+switch (pid = fork()) {
+case 0:
+if (setns(mfd, 0) == -1) {
+exit(-1);
+}
+rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF, NULL);
+exit(rc);
+break;
+case -1:
+virReportSystemError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Fork failed));
+goto cleanup;
+default:
+if (waitpid(pid, status, 0) 0 || status 0) {
+virReportSystemError(errno,
+ _(Sending shutdown failed with status
%i),
+ status);
+rc = 0;
+} else {
+rc = status;
+}
+close(mfd);
+}
+} else {
+rc = 0;
+}
Since this is a fairly tedious large piece of code I think it is
worth hiding it in a helper function. I'd suggest that in the
src/util/virprocess.{c,h} file we create a
typedef int (*virProcessNamespaceCallback)(pid_t pid, void *opaque);
virProcessRunInMountNamespace(pid_t pid,
virProcessNamespaceCallback cb,
void *opaque)
This lxcDomainShutdownFlags code would then use that and pass a
callback function which does the virInitctlSetRunLevel() API call.
That should reduce the code size significantly, since we also need
to fix all the other places in this file which use /proc/$PID/root.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] docs: improve event-related documentation
On 12/19/2013 08:18 AM, Daniel P. Berrange wrote: On Thu, Dec 19, 2013 at 08:13:35AM -0700, Eric Blake wrote: While looking at event code, I noticed that the documentation was trying to refer me to functions that don't exist. Also fix some typos and poor formatting. * src/libvirt.c (virConnectDomainEventDeregister) (virConnectDomainEventRegisterAny) (virConnectDomainEventDeregisterAny) (virConnectNetworkEventRegisterAny) (virConnectNetworkEventDeregisterAny): Link to correct function. * include/libvirt.h.in (VIR_DOMAIN_EVENT_CALLBACK) (VIR_NETWORK_EVENT_CALLBACK): Likewise. (virDomainEventID, virConnectDomainEventGenericCallback) (virNetworkEventID, virConnectNetworkEventGenericCallback): Improve docs. Signed-off-by: Eric Blake ebl...@redhat.com --- include/libvirt/libvirt.h.in | 36 src/libvirt.c| 24 2 files changed, 40 insertions(+), 20 deletions(-) ACK Thanks; pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] lvm storage pool destroy problem
Hi Daniel, my Virtual machines are running, so how I can stop the volumes? Br. Umar On Thu, Dec 19, 2013 at 9:21 PM, Daniel P. Berrange berra...@redhat.comwrote: On Thu, Dec 19, 2013 at 01:54:19AM +0500, Umar Draz wrote: Hi I am trying to stop the storage pool through php-libvirt api which is a LVM volume group. when I tried to stop it I got error error: internal error Child process (/sbin/vgchange -aln mylvmgrp) unexpected exit status 5: Can't deactivate volume group mylvmgrp with 4 open logical volume(s) Then I tried throggh virsh command but the result remain same. *root@kvm02:~# virsh pool-destroy vms* error: Failed to destroy pool vms error: internal error Child process (/sbin/vgchange -aln mylvmgrp) unexpected exit status 5: Can't deactivate volume group mylvmgrp with 4 open logical volume(s) This indicates some process(s) are still using the logical volumes so it is preventingn you breaking those apps. This is expected behaviour. The same error also occured if I use direct vgchange -aln on my kvm host, but then I tried *vgchange -aly mylvmgrp* this time its worked That is a totally different operation -aly *activates* volumes which is the exact opposite of what 'pool-destroy' needs, which is to de-activate the volumes 14 logical volume(s) in volume group mylvmgrp now active 1) Now would you please help me? is this libvirt issue? or LVM? 2) Is this possible virsh pool-destroy pool also run the vgchange -aly instead of -aln AFAICT, there's no bug here. You just need to stop using the volumes before stopping the pool. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/:| |: http://libvirt.org -o- http://virt-manager.org:| |: http://autobuild.org -o- http://search.cpan.org/~danberr/:| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc:| -- Umar Draz Network Architect -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] lvm storage pool destroy problem
On Thu, Dec 19, 2013 at 09:51:31PM +0500, Umar Draz wrote: Hi Daniel, my Virtual machines are running, so how I can stop the volumes? If you have VMs using volumes in a pool then you cannot stop that pool until the VMs are stopped. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] network: only prevent forwarding of DNS requests for unqualified names
On Mon, Dec 16, 2013 at 04:41:31PM +0200, Laine Stump wrote: On 12/10/2013 04:45 PM, Daniel P. Berrange wrote: So considering your example XML config above we're debating the behaviour of the following 5 possible DNS requests - myguest - myguest.example.com - notmyguest - notmyguest.example.com - google.com Originally - myguest - dnsmasq - myguest.example.com - dnsmasq - notmyguest - forwarded - notmyguest.example.com - forwarded - google.com - forwarded With the current GIT - myguest - dnsmasq - myguest.example.com - dnsmasq - notmyguest - error - notmyguest.example.com - error - google.com - forwarded With your proposal - myguest - dnsmasq - myguest.example.com - dnsmasq - notmyguest - error - notmyguest.example.com - fowarded - google.com - forwarded Nicely organized and diagrammed! This is really a case of accepting a patch without performing adequate due diligence on the ramifications of the change in behavior. The reason behind adding the patch seemed valid, but it had a wider effect than intended. Indeed. IMHO I tend to think that the original behaviour should not have been changed and is the right default. If we desired either of the other behaviours we should have a config option for them to force returning of errors instead of forwarding. A simple boolean wouldn't suffice since there are 3 possible valid behaviours here - so we'd need an enum attribute The first time I encountered this particular blowback of the original patch (it had also included --filterwin2k, which itself caused problems and was reverted), my response was to add the dns forwardPlainNames='yes'/ attribute - that restores (tries to anyway) the original behavior, but not by default. (My thinking was that the intent of the original patch was desirable, since it had been ACKed and pushed (and besides, forwarding of non-qualified names really is frowned on, at least by root dns servers). Too bad we didn't have this discussion back then :-/ That's the problem of having such a high volume of mail on the list - there's never enough time for everyone to read it all, so some discussions just never happen. So is your suggestion that we add to this patch another patch which changes the default for forwardPlainNames to no? Oh, I didn't realize we had that attribute already. Don't we need it to be a tri-state instead of just yes/no ? Going back to be list at the to. IIUC, the forwardPlainNames=no toggles between Originally - notmyguest - forwarded - notmyguest.example.com - forwarded With the current GIT - notmyguest - error - notmyguest.example.com - error But doesn't give us a way to handle this new use case: With your proposal - notmyguest - error - notmyguest.example.com - fowarded Or am I misunderstanding ? (I can also see how this could/should functionally fit together with a list of domains/dns servers, which we *also* kind of support via the forwarder element (which unfortunately only does things halfway - it allows specifying the IP address of a dns server, but not what domain it should be used for; strange, seeing that the dnsmasq option it is encapsulating does exactly that). I'm thinking there should be another patch which extends each forwarder to have an optional domain attribute to limit which domains a particular dns server is used for) Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] Setting devid for emulated NICs (Xen 4.3.1 / libvirt 1.2.0) using libxl driver
On 19.12.2013 11:19, Ian Campbell wrote:
On Wed, 2013-12-18 at 17:44 -0700, Jim Fehlig wrote:
Stefan Bader wrote:
On 18.12.2013 14:28, Ian Campbell wrote:
On Wed, 2013-12-18 at 14:12 +0100, Stefan Bader wrote:
On 18.12.2013 13:27, Ian Campbell wrote:
On Tue, 2013-12-17 at 18:32 +0100, Stefan Bader wrote:
Might this libxl fix be relevant:
commit 5420f26507fc5c9853eb1076401a8658d72669da
Author: Jim Fehlig jfeh...@suse.com
Date: Fri Jan 11 12:22:26 2013 +
libxl: Set vfb and vkb devid if not done so by the caller
Other devices set a sensible devid if the caller has not
done so.
Do the same for vfb and vkb. While at it, factor out the
common code
used to determine a sensible devid, so it can be used by
other
libxl__device_*_add functions.
Signed-off-by: Jim Fehlig jfeh...@suse.com
Acked-by: Ian Campbell ian.campb...@citrix.com
Committed-by: Ian Campbell ian.campb...@citrix.com
and a follow up in dfeccbeaa. Although the comment implies that nic's
were already correctly assigning a devid if the caller specified -1, so
I don't know why it doesn't work for you :-(
Ok, yes, the commit above indeed changes libxl__device_nic_add to call
libxl__device_nextid for the devid... Just how is this actually called.
Maybe not sufficient but git grep libxl__device_nic_add in the xen
code only
shows the definition and a declaration in libxl_internal.h to me...
I have a feeling a macro might be involved...
Here we go, look for DEFINE_DEVICE_REMOVE in libxl.c. We should really
add the eventual function names in comments to provide grep fodder
Oh duh, yeah. So in DEFINE_DEVICE_ADD a libxl_device_nic_add is created
which
calls to libxl__device_nic_add. When I look for the single _ version I
find a
call from xl_cmdimpl.c and its public declaration in libxl.h.
So I guess the bug is that libvirt in the libxl driver never seems to do
so
The macro creates libxl__add_nics which adds the nics from the
libxl_domain_config-nics array. I don't think libvirt needs to call
libxl_device_nic_add manually unless it is hotplugging a new nic at
runtime.
Hm, so I think this is the path:
libxl_domain_create_new
- do_domain_create
- initiate_domain_create
- libxl__bootloader_run (HVM domain, skipping bootloader)
- domcreate_bootloader_done
- domcreate_rebuild_done
- domcreate_launch_dm
- libxl__spawn_local_dm
- domcreate_devmodel_started
In libxl__spawn_local_dm, there is the following loop:
for (i = 0; i d_config-num_nics; i++) {
/* We have to init the nic here, because we still haven't
* called libxl_device_nic_add at this point, but qemu needs
* the nic information to be complete.
*/
ret = libxl__device_nic_setdefault(gc, d_config-nics[i], domid);
if (ret)
goto error_out;
}
So I think when starting the dm, the devid just is not set as setdefault
does
not seem to do so. I would be done in the later domcreate_devmodel_started
callback but that is too late for the generated qemu arguments.
Sorry for jumping in late...
I stumbled across this problem just before openSUSE13.1 released and did
a quick fix in libvirt
https://build.opensuse.org/package/view_file/Virtualization:openSUSE13.1/libvirt/libxl-hvm-nic.patch?expand=1
I removed setting the NIC devid in the libxl driver a while back to be
consistent with other devices
http://libvirt.org/git/?p=libvirt.git;a=commit;h=ba64b97134a6129a48684f22f31be92c3b6eef96
The quick fix was to essentially revert the above commit until I could
investigate further. Thank you for now having done that investigation
:). Can the devid assignment logic be moved from
libxl__device_nic_add() to libxl__device_nic_setdefault()?
It certainly seems like it would be more natural to do it there.
I suspect it might be done this way because at setdefault time you might
be walking a list of nics none of which have been created yet -- so
looking in xenstore would return devid zero is free for every one of
them?
How about we:
* move the init to setdefault to catch the single NIC added via
hotplug case
Init of devid? Hm, would that work as I am not sure there is a simple way of
differentiating between a NIC config for a single hotplug and one that is part
of a create-time array...
* we add somewhere early in the domain create path a call to a
function which assigns devids to an entire array of devices (and
do it for all the different device types). Perhaps in
initiate_domain_create() after the calls to
libxl__domain_create_info_setdefault and
Re: [libvirt] [PATCH] virkeycode: Allow ANSI_A
On Thu, Dec 19, 2013 at 16:46:50 +0100, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1044806
Currently, sending the ANSI_A keycode from os_x codepage doesn't work as
it has a special value of 0x0. Our internal code handles that no
different to other not defined keycodes. Hence, in order to allow it we
must change all the undefined keycodes from 0 to -1 and adapt some code
too.
# virsh send-key guestname --codeset os_x ANSI_A
error: invalid keycode: 'ANSI_A'
# virsh send-key guestname --codeset os_x ANSI_B
# virsh send-key guestname --codeset os_x ANSI_C
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virkeycode-mapgen.py | 4 ++--
src/util/virkeycode.c | 4 ++--
tools/virsh-domain.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/util/virkeycode-mapgen.py b/src/util/virkeycode-mapgen.py
index 22b21b4..8360bfe 100755
--- a/src/util/virkeycode-mapgen.py
+++ b/src/util/virkeycode-mapgen.py
@@ -86,12 +86,12 @@ for i in range(len(cols)):
if isname:
print const char *virKeymapNames_ + name + [] = {
else:
-print unsigned short virKeymapValues_ + name + [] = {
+print int virKeymapValues_ + name + [] = {
for entry in keycodes:
if isname:
print+ quotestring(entry[i] or NULL) + ,
else:
-print+ (entry[i] or 0) + ,
+print+ (entry[i] or -1) + ,
print };\n
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index 50594d6..7880a0a 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -50,7 +50,7 @@ static const char **virKeymapNames[] = {
};
verify(ARRAY_CARDINALITY(virKeymapNames) == VIR_KEYCODE_SET_LAST);
-static unsigned short *virKeymapValues[] = {
+static int *virKeymapValues[] = {
[VIR_KEYCODE_SET_LINUX] =
virKeymapValues_linux,
[VIR_KEYCODE_SET_XT] =
@@ -113,7 +113,7 @@ int virKeycodeValueTranslate(virKeycodeSet from_codeset,
{
size_t i;
-if (key_value = 0)
+if (key_value 0)
return -1;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 760dca5..b33b808 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7091,7 +7091,7 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd)
}
if ((keycode = vshKeyCodeGetInt(opt-data)) = 0) {
This = 0 above is suspicious too, isn't it?
-if ((keycode = virKeycodeValueFromString(codeset, opt-data)) =
0) {
+if ((keycode = virKeycodeValueFromString(codeset, opt-data))
0) {
vshError(ctl, _(invalid keycode: '%s'), opt-data);
goto cleanup;
}
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix explicit usage of default video PCI slots
On Thu, Dec 19, 2013 at 17:43:33 +0100, Jano Tomko wrote: Don't set the PCI address of primary video to the default if it's already occupied by another device. Without this, a primary video card that doesn't have an address gets the default one on the 1st pass of PCI address allocation, even if it's occupied. This address gets picked up by the second pass, and a conflict occurs: XML error: Attempted double use of PCI slot :00:02.0 (may need multifunction='on' for device on function 0) Also fix the test that was supposed to catch this. And does it actually work after this patch? IIRC, the primary video card is forced to be at :00:02.0 by QEMU. That is, if this address is occupied by another device, there should be no way to add a video card. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix explicit usage of default video PCI slots
On 12/19/2013 06:18 PM, Jiri Denemark wrote: On Thu, Dec 19, 2013 at 17:43:33 +0100, Jano Tomko wrote: Don't set the PCI address of primary video to the default if it's already occupied by another device. Without this, a primary video card that doesn't have an address gets the default one on the 1st pass of PCI address allocation, even if it's occupied. This address gets picked up by the second pass, and a conflict occurs: XML error: Attempted double use of PCI slot :00:02.0 (may need multifunction='on' for device on function 0) Also fix the test that was supposed to catch this. And does it actually work after this patch? IIRC, the primary video card is forced to be at :00:02.0 by QEMU. That is, if this address is occupied by another device, there should be no way to add a video card. It works without this patch too, if you explicitly specify the PCI address of the video card (and you have QEMU = 1.6, because that's when we use -device instead of -vga - (the QEMU_CAPS_DEVICE_VIDEO_PRIMARY capability)). 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] [Xen-devel] Setting devid for emulated NICs (Xen 4.3.1 / libvirt 1.2.0) using libxl driver
On Thu, 2013-12-19 at 18:06 +0100, Stefan Bader wrote:
How about we:
* move the init to setdefault to catch the single NIC added via
hotplug case
Init of devid?
Yes, sorry for not being clear.
Hm, would that work as I am not sure there is a simple way of
differentiating between a NIC config for a single hotplug and one that is part
of a create-time array...
The create time array would be initialised with devid's != -1 as part of
the steps outlined in the following bullets, so setdefault woudln't
touch it again.
* we add somewhere early in the domain create path a call to a
function which assigns devids to an entire array of devices (and
do it for all the different device types). Perhaps in
initiate_domain_create() after the calls to
libxl__domain_create_info_setdefault and
libxl__domain_build_info_setdefault but before the loop calling
libxl__device_disk_setdefault for the disks.
* perhaps that same function should call setdefault too, after
having assigned the device, rather than it being done later in
an adhoc way?
Does that sound at all plausible?
I wonder, well this won't help for any other device types (maybe not really
needed), what about just adding the following to the existing loop in
domcreate_launch_dm (just a brain dump, not even tried to compile):
for (i = 0; i d_config-num_nics; i++) {
/* We have to init the nic here, because we still haven't
* called libxl_device_nic_add at this point, but qemu needs
* the nic information to be complete.
*/
ret = libxl__device_nic_setdefault(gc, d_config-nics[i], domid);
if (ret)
goto error_out;
+ if (d_config-nics[i].devid 0)
+ d_config-nics[i].devid = i;
}
Of course this a gain won't work well if the caller had assigned some devids
but
not other.
Indeed.
Ok, maybe do the loop twice, first round sets default and picks the
highest pre-assigned devid and second round makes sure any still unassigned
ones
are set to ++that.
That would potentially leave holes, I don't know if that matters.
Oh, just while talking about setdefault. Jim, this is one of the odd things
when
moving from xm to xl stack from libvirt: libvirt defaults to the netfront NIC
when no model is specified and sets the type. The libxl setdefault function
sets
the model to rtl8139 but leaves the type untouched.
This sounds like a bug in libxl to me -- it should do something
consistent I think.
So setting no model in the
xml config creates a domain with no emulated NIC (this does not matter after
Linux is up because the emulated devices get unplugged). Just that PXE boot
will
not work. This gets odd because with the old xen (xm) driver, no model meant
rtl8139.
Sigh, and to hijack this thread even further I noticed a quite unexpected
behaviour when starting a domain trhough libvirt and then try to use xl list
-l to get config details. xl list shows all running domains but xl list
-l
produces something like you have to specify a domain name. I found the
origin
of this to be libxl_userdata_retrieve which takes a userdate_userid as an
argument. Libvirt uses libvirt-xml for that, while xl uses xl. This might
be
intentional and the bug is just that we need a better check for not finding
the
userdata and then skipping those domains. On the other hand ... its after all
in
both cases a domain created and started through libxl...
I think this was discussed a few weeks ago on the list, and there were
one or two separate bugs and short comings. I'm not sure which subset
were actually fixed.
One issue is that xl stored the guest config and then retrieves it for
use in xl list -l, but libvirt != xl and therefore has no config file to
save.
The solution is probably for the list stuff to be based on dynamically
gathering the domain info, instead of reparsing the config.
Ian.,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events
On 12/19/2013 08:23 AM, Daniel P. Berrange wrote:
On Thu, Dec 19, 2013 at 08:15:09AM -0700, Eric Blake wrote:
diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
index db52c65..849932d 100644
+if (dom
+!(VIR_IS_CONNECTED_DOMAIN(dom) dom-conn == conn)) {
+virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(conn);
+return -1;
+}
+virCheckNonNullArgGoto(cb, error)
I have a gut feeling that we should restrict use of this API to
authenticated users only. So add a check for a read-only connection
here
Hmm. It would match what we have for qemu-monitor-command; on the other
hand, it differs from what we have for normal events (libvirt events are
fine on a read-only connection). I guess an argument in favor of
requiring write privileges is that qemu may expose details in its events
that are better left internal to libvirt (that is, libvirt has a chance
to scrub details before handing information to read-only guests, but
with raw event handling, there is no scrubbing). And it's always easier
to relax things later than it is to add read-write restrictions later
and break existing callers that had grown used to read-only.
+if ((conn-driver)
(conn-driver-connectDomainQemuMonitorEventRegister)) {
This is excessively bracketed isn't it
Copy-and-paste, all the way! I'll clean it up.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] Setting devid for emulated NICs (Xen 4.3.1 / libvirt 1.2.0) using libxl driver
Stefan Bader wrote: Oh, just while talking about setdefault. Jim, this is one of the odd things when moving from xm to xl stack from libvirt: libvirt defaults to the netfront NIC when no model is specified and sets the type. The libxl setdefault function sets the model to rtl8139 but leaves the type untouched. The xend toolstack always creates both emulated and vif devices unless 'type=netfront' is explicitly specified. As you say, the guest gets to choose what to do with them. E.g. PXE boot using the emulated device, or have the driver for the PV device unplug the emulated one. I don't think libxl supports this right? So setting no model in the xml config creates a domain with no emulated NIC (this does not matter after Linux is up because the emulated devices get unplugged). Just that PXE boot will not work. This gets odd because with the old xen (xm) driver, no model meant rtl8139. Sigh, and to hijack this thread even further I noticed a quite unexpected behaviour when starting a domain trhough libvirt and then try to use xl list -l to get config details. xl list shows all running domains but xl list -l produces something like you have to specify a domain name. I found the origin of this to be libxl_userdata_retrieve which takes a userdate_userid as an argument. Libvirt uses libvirt-xml for that, while xl uses xl. This might be intentional and the bug is just that we need a better check for not finding the userdata and then skipping those domains. On the other hand ... its after all in both cases a domain created and started through libxl... As Ian mentioned, this has already been discussed http://lists.xen.org/archives/html/xen-devel/2013-10/msg01921.html But looks like that thread has lost some steam... Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events
On Thu, Dec 19, 2013 at 11:32:32AM -0700, Eric Blake wrote:
On 12/19/2013 08:23 AM, Daniel P. Berrange wrote:
On Thu, Dec 19, 2013 at 08:15:09AM -0700, Eric Blake wrote:
diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
index db52c65..849932d 100644
+if (dom
+!(VIR_IS_CONNECTED_DOMAIN(dom) dom-conn == conn)) {
+virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(conn);
+return -1;
+}
+virCheckNonNullArgGoto(cb, error)
I have a gut feeling that we should restrict use of this API to
authenticated users only. So add a check for a read-only connection
here
Hmm. It would match what we have for qemu-monitor-command; on the other
hand, it differs from what we have for normal events (libvirt events are
fine on a read-only connection). I guess an argument in favor of
requiring write privileges is that qemu may expose details in its events
that are better left internal to libvirt (that is, libvirt has a chance
to scrub details before handing information to read-only guests, but
with raw event handling, there is no scrubbing). And it's always easier
to relax things later than it is to add read-write restrictions later
and break existing callers that had grown used to read-only.
Yes, my concern is that there could one day be a QEMU event that includes
some sensitive information in its parameters. Normal users should never
need to use this API anyway, so restricting its access shouldn't really
hurt people.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]
Dario Faggioli wrote: On gio, 2013-12-19 at 11:46 +, Ian Jackson wrote: Dario Faggioli writes (Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]): [Moving this to libvir, libvir-users in Bcc. Also, added xen-devel] 4.2.1 is lacking libxl: fix stale timeout event callback race libxl: fix stale fd event callback race These are both in stable-4.2 as 6f0f339dd4378d062a211969f45cd23af12bf386 a87ef897295ec17788e41e9a8f4c0ada7a5a45f8 I see. Thanks Ian. Yep, both of those are definitely needed on the Xen side. Around the time I was developing those in xen-unstable.git there were some libvirt patches which were necessary too. That should be ok, as he's using the very latest libvirt. Correct, all of the libvirt-related patches have been in for a while, so he should have them. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix explicit usage of default video PCI slots
On Thu, Dec 19, 2013 at 18:41:25 +0100, Jano Tomko wrote: On 12/19/2013 06:18 PM, Jiri Denemark wrote: On Thu, Dec 19, 2013 at 17:43:33 +0100, Jano Tomko wrote: Don't set the PCI address of primary video to the default if it's already occupied by another device. Without this, a primary video card that doesn't have an address gets the default one on the 1st pass of PCI address allocation, even if it's occupied. This address gets picked up by the second pass, and a conflict occurs: XML error: Attempted double use of PCI slot :00:02.0 (may need multifunction='on' for device on function 0) Also fix the test that was supposed to catch this. And does it actually work after this patch? IIRC, the primary video card is forced to be at :00:02.0 by QEMU. That is, if this address is occupied by another device, there should be no way to add a video card. It works without this patch too, if you explicitly specify the PCI address of the video card (and you have QEMU = 1.6, because that's when we use -device instead of -vga - (the QEMU_CAPS_DEVICE_VIDEO_PRIMARY capability)). Oh so this is only changing the situation for QEMU = 1.6? This makes more sense now although it wasn't exactly obvious when looking at the patch without bigger context. It could be a good idea to explicitly mention this in the commit message. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events
On 12/19/2013 09:01 AM, Daniel P. Berrange wrote:
+typedef void (*virConnectDomainQemuMonitorEventCallback)(virConnectPtr
conn,
+ virDomainPtr dom,
+ const char *event,
+ const char
*details,
+ void *opaque);
So for instance on this event:
2013-12-19 15:55:05.575+: 18630: debug :
qemuMonitorJSONIOProcessLine:172 : QEMU_MONITOR_RECV_EVENT:
mon=0x7f8c80008910 event={timestamp: {seconds: 1387468505,
microseconds: 574652}, event: SPICE_INITIALIZED, data: {server:
{auth: none, port: 5900, family: ipv4, host: 127.0.0.1},
client: {port: 39285, family: ipv4, channel-type: 4,
connection-id: 375558326, host: 127.0.0.1, channel-id: 0, tls:
false}}}
the callback will be invoked with:
event=SPICE_INITIALIZED
details={server: {auth: }}?
Ooh, just noticed that the timestamp is not part of the event data;
probably worth adding another parameter to the callback function to list
the event timestamp (as knowing when qemu fired an event may indeed be
important to a developer using this interface for debugging). What type
would be best? Is it okay to tie our public interface to struct
timespec (which in turn risks problems if a compile-time switch can move
between 32- and 64-bit seconds since Epoch), or should I just open-code
it to two parameters: 'long long seconds, int microseconds'?
After all, I don't think we should do anything clever about it. Apps dealing
with monitor/agent code (e.g. command passthru) are dealing with JSON anyway.
Agreed, just sending out the JSON 'data' block as-is seems fine to me.
Sounds like I'm on the right track, then.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Implementation deficiency in virInitctlSetRunLevel v2
On 12/19/2013 09:07 AM, Reco wrote: Hello, list. Refuse following symlinks in virInitctlSetRunLevel. A reasonable fallback for the next two patches, which apply fork-setns technique recommended on this list. Your patches came through as independent top-level threads, rather than in-reply to a common cover letter. That makes it a bit harder to track things in mail clients, especially ones set to sort threads by most recent activity. A good rule of thumb when setting up 'git send-email' is to first send email to yourself to make sure it comes through properly threaded, before sending to the list. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virnettlscontexttest fails with GNUTLS 3.0.28
On 12/19/2013 09:23 AM, Cédric Bosdonnat wrote: Changed the constraints on gnutls to 3.1+ --- tests/virnettlscontexttest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index fc512fc..d9a4e9d 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -268,7 +268,8 @@ mymain(void) * be rejected. GNUTLS 3 does not reject it and * we don't anticipate them changing this behaviour */ -DO_CTX_TEST(true, cacert4req.filename, servercert4req.filename, GNUTLS_VERSION_MAJOR = 3); +DO_CTX_TEST(true, cacert4req.filename, servercert4req.filename, +GNUTLS_VERSION_MAJOR = 3 GNUTLS_VERSION_MINOR = 1); Not quite. This will reject gnutls 4.0. It has to be more like: (GNUTLS_VERSION_MAJOR == 3 GNUTLS_VERSION_MINOR = 1) || GNUTLS_VERSION_MAJOR 3 What distro were you on when you hit this failure? I'm a little bit reluctant to bump the minimum requirement without knowing a bit more about how common 3.0 is in practice. Adding more details in your commit log about why you needed it (not just what you changed) makes it easier to review. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] maint: improve debug of libvirt-{qemu, lxc} apis
On 12/19/2013 09:37 AM, Daniel P. Berrange wrote: +#define virLibDomainSnapshotError(code, ...) \ +virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) I'd venture to sugggest that these functions really have little to no benefit / reason to exist. They aren't really simplifying like, just making it different. They're historical baggage from the old time when they were actual functions which did extra sprintf formatting work. Could we just remove them ?? Perhaps, by using virReportError instead. I can give that a try; but just looking at it, virReportError() hardcodes VIR_FROM_THIS as its first argument, whereas virLib*Error() sets different VIR_FROM_* categories. Worse, we use multiple calls from within a single function (so continually redefining VIR_FROM_THIS would get painful) - for example, virDomainSnapshotCreateXML() uses both virLibDomainError() (VIR_FROM_DOMAIN) and virLibConnError() (VIR_FROM_NONE). Oh true I'd forgotten it relied on VIR_FROM_THIS. How about just renaming them s/Lib/Report/ eg virLibDomainError - virReportDomainError and having them in datatypes.h too. Sounds reasonable, but again, splitting into two patches to keep the review work easier. v2 coming up later today. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Block info query: Add flag to allow failure if not active
- Original Message - From: Daniel P. Berrange berra...@redhat.com To: John Ferlan jfer...@redhat.com Cc: libvir-list@redhat.com Sent: Wednesday, December 18, 2013 3:57:22 PM Subject: Re: [libvirt] [PATCH] Block info query: Add flag to allow failure if not active On Wed, Dec 18, 2013 at 09:49:46AM -0500, John Ferlan wrote: On 12/18/2013 09:35 AM, Daniel P. Berrange wrote: On Wed, Dec 18, 2013 at 06:58:10AM -0500, John Ferlan wrote: I'm not convinced this flag is desirable. Can't apps just check whether the guest is running themselves, or indeed something like RHEV surely knows what its own VM is doing already. I do agree with you and that's been my argument in the referenced BZ; however, as I understand it - they have a thread that continually polls for blockInfo information and a separate thread that handles the migration. The blockInfo thread doesn't have the state information. But it must have a virDomainPtr instance, so it can use virDomainGetState() or virDomainIsActive() if it cares about this. oVirt is using only transient domains, so it seems to me that the assumption here is that blockInfo should either return a value from a running qemu process or an error (because the domain doesn't exist anymore). Having a short moment where it's treated as a non-transient domain is confusing. It's fine by me if you want to maintain the current implementation for non-transient domains (and therefore the new flag is not needed). I contend it's just as simple to add a check about the domain state and to get/check the reason value as well. That is - I think the blockInfo thread should be more aware of state. Of course, the return argument is that libvirt shouldn't return different answers on consecutive fetches where the first fetch is when the guest is active and the second is when it's not. Sure, I agree that libvirt isn't ideal here - our hands are tied by the fact that QEMU doesn't make this data available to us offline. If we changed anything on the libvirt side, then I'd want to see us do a proper fix to get the data but that'd require qemu help; I added the support for this to qemu-img. It scans a qcow2 image and reports the highest allocated cluster. Sadly the only way to implement it was on the image check (takes a long time). -- Federico -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] maint: improve debug of libvirt-{qemu, lxc} apis
I noticed that the virDomainQemuMonitorCommand debug output wasn't
telling me the name of the domain it was working on. While it was
easy enough to determine which pointer matches the domain based on
other log messages, it is nicer to be consistent.
* src/util/viruuid.h (VIR_UUID_DEBUG): Moved here from...
* src/libvirt.c (VIR_UUID_DEBUG): ...here.
(VIR_ARG15, VIR_HAS_COMMA, VIR_DOMAIN_DEBUG_EXPAND)
(VIR_DOMAIN_DEBUG_PASTE, VIR_DOMAIN_DEBUG_0, VIR_DOMAIN_DEBUG_1)
(VIR_DOMAIN_DEBUG_2, VIR_DOMAIN_DEBUG): Move...
* src/datatypes.h: ...here.
* src/libvirt-qemu.c (virDomainQemuMonitorCommand)
(virDomainQemuAgentCommand): Better debug messages.
* src/libvirt-lxc.c (virDomainLxcOpenNamespace): Likewise.
Signed-off-by: Eric Blake ebl...@redhat.com
---
The simpler part of my changes; the work on cleaning up the
remaining virLib*Error() calls in libvirt.c is turning into
a bigger cleanup project.
src/datatypes.h| 60 -
src/libvirt-lxc.c | 4 +--
src/libvirt-qemu.c | 11 +
src/libvirt.c | 71 --
src/util/viruuid.h | 20 ++-
5 files changed, 86 insertions(+), 80 deletions(-)
diff --git a/src/datatypes.h b/src/datatypes.h
index 6026bb0..7f325b0 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -1,7 +1,7 @@
/*
* datatypes.h: management of structs for public data types
*
- * Copyright (C) 2006-2008, 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2008, 2010-2011, 2013 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
@@ -94,6 +94,64 @@ extern virClassPtr virStoragePoolClass;
(VIR_IS_SNAPSHOT(obj) VIR_IS_DOMAIN((obj)-domain))
+/* Helper macros to implement VIR_DOMAIN_DEBUG using just C99. This
+ * assumes you pass fewer than 15 arguments to VIR_DOMAIN_DEBUG, but
+ * can easily be expanded if needed.
+ *
+ * Note that gcc provides extensions of define a(b...) b or
+ * define a(b,...) b,##__VA_ARGS__ as a means of eliding a comma
+ * when no var-args are present, but we don't want to require gcc.
+ */
+# define VIR_ARG15(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, \
+ _11, _12, _13, _14, _15, ...) _15
+# define VIR_HAS_COMMA(...) VIR_ARG15(__VA_ARGS__, \
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0)
+
+/* Form the name VIR_DOMAIN_DEBUG_[01], then call that macro,
+ * according to how many arguments are present. Two-phase due to
+ * macro expansion rules. */
+# define VIR_DOMAIN_DEBUG_EXPAND(a, b, ...) \
+VIR_DOMAIN_DEBUG_PASTE(a, b, __VA_ARGS__)
+# define VIR_DOMAIN_DEBUG_PASTE(a, b, ...) \
+a##b(__VA_ARGS__)
+
+/* Internal use only, when VIR_DOMAIN_DEBUG has one argument. */
+# define VIR_DOMAIN_DEBUG_0(dom) \
+VIR_DOMAIN_DEBUG_2(dom, %s, )
+
+/* Internal use only, when VIR_DOMAIN_DEBUG has three or more arguments. */
+# define VIR_DOMAIN_DEBUG_1(dom, fmt, ...) \
+VIR_DOMAIN_DEBUG_2(dom, , fmt, __VA_ARGS__)
+
+/* Internal use only, with final format. */
+# define VIR_DOMAIN_DEBUG_2(dom, fmt, ...) \
+do {\
+char _uuidstr[VIR_UUID_STRING_BUFLEN]; \
+const char *_domname = NULL;\
+\
+if (!VIR_IS_DOMAIN(dom)) { \
+memset(_uuidstr, 0, sizeof(_uuidstr)); \
+} else {\
+virUUIDFormat((dom)-uuid, _uuidstr); \
+_domname = (dom)-name; \
+} \
+\
+VIR_DEBUG(dom=%p, (VM: name=%s, uuid=%s) fmt, \
+ dom, NULLSTR(_domname), _uuidstr, __VA_ARGS__); \
+} while (0)
+
+/**
+ * VIR_DOMAIN_DEBUG:
+ * @dom: domain
+ * @fmt: optional format for additional information
+ * @...: optional arguments corresponding to @fmt.
+ */
+# define VIR_DOMAIN_DEBUG(...) \
+VIR_DOMAIN_DEBUG_EXPAND(VIR_DOMAIN_DEBUG_, \
+VIR_HAS_COMMA(__VA_ARGS__), \
+__VA_ARGS__)
+
+
typedef struct _virConnectCloseCallbackData virConnectCloseCallbackData;
typedef virConnectCloseCallbackData *virConnectCloseCallbackDataPtr;
diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
index c8cdcea..5cbe8bf 100644
--- a/src/libvirt-lxc.c
+++ b/src/libvirt-lxc.c
@@ -28,6 +28,7 @@
#include virfile.h
#include virlog.h
#include virprocess.h
+#include viruuid.h
#include datatypes.h
#ifdef WITH_SELINUX
Re: [libvirt] [PATCH 1/3] libxl: implement virDomainGetVcpuPinInfo
Dario Faggioli wrote:
So that it is possible to query vcpu related information of
a persistent but not running domain, like it is for the QEMU
driver.
In fact, before this patch, we have:
# virsh list --all
IdName State
5 debian_32 running
- fedora20_64shut off
# virsh vcpuinfo fedora20_64
error: this function is not supported by the connection driver:
virDomainGetVcpuPinInfo
After (same situation as above, i.e., fedora20_64 not running):
# virsh vcpuinfo fedora20_64
VCPU: 0
CPU:N/A
State: N/A
CPU timeN/A
CPU Affinity:
VCPU: 1
CPU:N/A
State: N/A
CPU timeN/A
CPU Affinity:
Signed-off-by: Dario Faggioli dario.faggi...@citrix.com
Cc: Jim Fehlig jfeh...@suse.com
Cc: Ian Jackson ian.jack...@eu.citrix.com
---
src/libxl/libxl_driver.c | 83
++
1 file changed, 83 insertions(+)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 692c3b7..750d4ec 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2415,6 +2415,88 @@ cleanup:
return ret;
}
+static int
+libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps,
+ unsigned char *cpumaps, int maplen,
+ unsigned int flags)
+{
+libxlDriverPrivatePtr driver = dom-conn-privateData;
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+virDomainObjPtr vm = NULL;
+virDomainDefPtr targetDef = NULL;
+virDomainVcpuPinDefPtr *vcpupin_list;
+virBitmapPtr cpumask = NULL;
+int maxcpu, hostcpus, vcpu, pcpu, n, ret = -1;
+unsigned char *cpumap;
+bool pinned;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(vm = libxlDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetVcpuPinInfoEnsureACL(dom-conn, vm-def) 0)
+goto cleanup;
+
+if (virDomainLiveConfigHelperMethod(cfg-caps, driver-xmlopt, vm,
+flags, targetDef) 0)
+goto cleanup;
+
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+targetDef = vm-def;
+}
+
+sa_assert(targetDef);
I think we should add a comment stating this is needed to silence
Coverity, similar to the qemu driver.
+
+/* Clamp to actual number of vcpus */
+if (ncpumaps targetDef-vcpus)
+ncpumaps = targetDef-vcpus;
+
+if (!cpumaps || ncpumaps 1) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(cannot return affinity via a NULL pointer));
+goto cleanup;
+}
cpumaps is guaranteed to be non-NULL on entry and ncpumaps is guaranteed
to be 0 on entry, so the above check can be removed.
+
+/* we use cfg-ctx, as vm-privateData-ctx may be NULL if VM is down */
+if ((hostcpus = libxl_get_max_cpus(cfg-ctx)) 0)
+goto cleanup;
+
+maxcpu = maplen * 8;
+if (maxcpu hostcpus)
+maxcpu = hostcpus;
+
+/* initialize cpumaps */
+memset(cpumaps, 0xff, maplen * ncpumaps);
+if (maxcpu % 8) {
+for (vcpu = 0; vcpu ncpumaps; vcpu++) {
+cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu);
+cpumap[maplen - 1] = (1 maxcpu % 8) - 1;
+}
+}
+
+/* if vcpupin setting exists, there may be unused pcpus */
+for (n = 0; n targetDef-cputune.nvcpupin; n++) {
+vcpupin_list = targetDef-cputune.vcpupin;
+vcpu = vcpupin_list[n]-vcpuid;
+cpumask = vcpupin_list[n]-cpumask;
+cpumap = VIR_GET_CPUMAP(cpumaps, maplen, vcpu);
+for (pcpu = 0; pcpu maxcpu; pcpu++) {
+if (virBitmapGetBit(cpumask, pcpu, pinned) 0)
+goto cleanup;
+if (!pinned)
+VIR_UNUSE_CPU(cpumap, pcpu);
+}
+}
+ret = ncpumaps;
+
+cleanup:
+if (vm)
+virObjectUnlock(vm);
+virObjectUnref(cfg);
+return ret;
+}
static int
libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo,
@@ -4237,6 +4319,7 @@ static virDriver libxlDriver = {
.domainGetVcpusFlags = libxlDomainGetVcpusFlags, /* 0.9.0 */
.domainPinVcpu = libxlDomainPinVcpu, /* 0.9.0 */
.domainGetVcpus = libxlDomainGetVcpus, /* 0.9.0 */
+.domainGetVcpuPinInfo = libxlDomainGetVcpuPinInfo, /* 1.2.1 */
.domainGetXMLDesc = libxlDomainGetXMLDesc, /* 0.9.0 */
.connectDomainXMLFromNative = libxlConnectDomainXMLFromNative, /* 0.9.0
*/
.connectDomainXMLToNative = libxlConnectDomainXMLToNative, /* 0.9.0 */
Looks good otherwise. Thanks!
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libxl: fix segfault when domain create fail
there is a segfault in libxl logging in libxl_ctx_free when domain create fail. because the log output handler vmessage is freed by xtl_logger_destroy before libxl_ctx_free in virDomainObjListRemove. move xtl_logger_destroy after libxl_ctx_free could fix this bug. Signed-off-by: Bamvor Jian Zhang bjzh...@suse.com --- src/libxl/libxl_domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 68009db..e72c483 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -354,12 +354,11 @@ libxlDomainObjPrivateDispose(void *obj) libxl_evdisable_domain_death(priv-ctx, priv-deathW); virChrdevFree(priv-devs); - -xtl_logger_destroy(priv-logger); +libxl_ctx_free(priv-ctx); if (priv-logger_file) VIR_FORCE_FCLOSE(priv-logger_file); -libxl_ctx_free(priv-ctx); +xtl_logger_destroy(priv-logger); } static void -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Add helper program to create custom leases
Introduce helper program to catch events from dnsmasq and maintain a custom
lease file per network. It supports dhcpv4 and dhcpv6. The file is saved as
interface-name.status.
The format of each lease is:
expiry-time (epoch time) mac iaid ip-address hostname clientid
Example of custom leases file content:
1385245780 52:54:00:2f:ba:76 * 192.168.150.153 * *
1385245781 52:54:00:2f:ba:76 3127926 2001:db8:ca2:2:1::6c *
00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec
1385245964 52:54:00:44:7c:d7 * 192.168.150.219 iiit-ad885e4aa1
01:52:54:00:44:7c:d7
1385245964 52:54:00:44:7c:d7 * 192.168.150.219 * 01:52:54:00:44:7c:d7
1385246016 52:54:00:5d:99:92 * 192.168.150.212 iiit-ad885e4aa1
01:52:54:00:5d:99:92
1385246041 52:54:00:3b:16:e0 * 192.168.150.207 * *
1385246081 52:54:00:db:dd:98 * 192.168.150.234 * *
1385246088 52:54:00:db:dd:98 14409112 2001:db8:ca2:2:1::6d *
00:04:76:00:cf:ae:b3:0b:fc:cd:0e:22:2e:97:76:65:74:ec
---
src/util/leaseshelper.c | 232
1 file changed, 232 insertions(+)
create mode 100644 src/util/leaseshelper.c
diff --git a/src/util/leaseshelper.c b/src/util/leaseshelper.c
new file mode 100644
index 000..9ed22a6
--- /dev/null
+++ b/src/util/leaseshelper.c
@@ -0,0 +1,232 @@
+/*
+ * leasehelper.c: Helper program to create custom leases file
+ *
+ * Copyright (C) 2013 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/.
+ *
+ * Author: Nehal J Wani nehaljw.k...@gmail.com
+ *
+ */
+
+#include config.h
+
+#include stdio.h
+#include stdlib.h
+
+#include virutil.h
+#include virthread.h
+#include virfile.h
+#include virbuffer.h
+#include virstring.h
+#include virerror.h
+#include viralloc.h
+#include configmake.h
+
+#define VIR_FROM_THIS VIR_FROM_NETWORK
+
+/**
+ * VIR_NETWORK_DHCP_LEASE_FIELDS:
+ *
+ * Macro providing the maximum number of fields in an entry in
+ * the leases file
+ */
+#define VIR_NETWORK_DHCP_LEASE_FIELDS 6
+/**
+ * VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX:
+ *
+ * Macro providing the upper limit on the size of leases file
+ */
+#define VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX 2097152
+
+/*
+ * Use this when passing possibly-NULL strings to printf-a-likes.
+ */
+# define EMPTY_STR(s) ((s) ? (s) : *)
+
+int
+main(int argc, char **argv) {
+
+FILE *g = fopen(/tmp/wtf, a);
+int j;
+for (j = 0; j argc; j++)
+fprintf(g, called :: : %s, , argv[j]);
+fprintf(g, \n);
+fclose(g);
+
+/* Doesn't hurt to check */
+if (argc 4) {
+/* Refer man page of dnsmasq --dhcp-script for more details */
+fprintf(stderr, Usage: $program $action ${mac|clientid} $ip\n);
+return -1;
+}
+
+const char *program_name = argv[0];
+const char *action = argv[1];
+const char *interface = EMPTY_STR(virGetEnvAllowSUID(DNSMASQ_INTERFACE));
+const char *expirytime =
EMPTY_STR(virGetEnvAllowSUID(DNSMASQ_LEASE_EXPIRES));
+const char *mac = argv[2];
+const char *ip = argv[3];
+const char *iaid = EMPTY_STR(virGetEnvAllowSUID(DNSMASQ_IAID));
+const char *hostname =
EMPTY_STR(virGetEnvAllowSUID(DNSMASQ_SUPPLIED_HOSTNAME));
+const char *clientid = EMPTY_STR(virGetEnvAllowSUID(DNSMASQ_CLIENT_ID));
+const char *leases_str = NULL;
+char *lease_file = NULL;
+char *lease_entries = NULL;
+char *lease_entry = NULL;
+char **lease_fields = NULL;
+bool delete = false;
+bool add = false;
+int rv = -1;
+int lease_file_len = 0;
+FILE *fp = NULL;
+long long expirytime_tmp = 0;
+virBuffer buf_new_lease = VIR_BUFFER_INITIALIZER;
+virBuffer buf_all_leases = VIR_BUFFER_INITIALIZER;
+
+if (setlocale(LC_ALL, ) == NULL ||
+bindtextdomain(PACKAGE, LOCALEDIR) == NULL ||
+textdomain(PACKAGE) == NULL) {
+fprintf(stderr, _(%s: initialization failed\n), program_name);
+exit(EXIT_FAILURE);
+}
+
+if (virThreadInitialize() 0 ||
+virErrorInitialize() 0) {
+fprintf(stderr, _(%s: initialization failed\n), program_name);
+exit(EXIT_FAILURE);
+}
+
+if (virAsprintf(lease_file, %s/%s.status, LOCALSTATEDIR
+/lib/libvirt/dnsmasq/, interface) 0)
+goto cleanup;
+
+if (virGetEnvAllowSUID(DNSMASQ_IAID)) {
+mac = EMPTY_STR(virGetEnvAllowSUID(DNSMASQ_MAC));
+clientid = argv[2];
+}
+
+/* Make sure
Re: [libvirt] [PATCH] Fix segmentation fault when accessing default qemu machine type
Yudai, would you be comfortable with updating our test suite to test this corner case? Okay, I've taken look at the test suite and I can update it. However, my time is pretty limited right now so it may take a little while to get the update done. If you need this ASAP, maybe someone else should do this. If not, I'll do it in my spare time. Meanwhile, is it possible for you to have this patch merged? Best Regards, Yudai Yamagishi 2013/12/17 Yudai Yamagishi yu...@sfc.wide.ad.jp: I'm not familiar with the libvirt test suite but let me see if I can enhance it a little bit. Best Regards Yudai Yamagishi 2013/12/17 Jiri Denemark jdene...@redhat.com: On Tue, Dec 17, 2013 at 17:33:19 +0900, Yudai Yamagishi wrote: From: Yudai Yamagish yu...@sfc.wide.ad.jp This patch fixes a segmentation fault when creating new virtual machines using QEMU. The segmentation fault is caused by commit f41830680e40d3ec845cefd25419bd87414b9ccf and commit cbb6ec42e2447d7920b30d66923b2a2b2670133b. In virQEMUCapsProbeQMPMachineTypes, when copying machines to qemuCaps, none is skipped. Therefore, the value of i and qemuCaps-nmachineTypes - 1 do not always match. However, defIdx value (used to call virQEMUCapsSetDefaultMachine) is set using the value in i when the array elements are in qemuCaps-nmachineTypes - 1. So, when libvirt tries to create virtual machines using the default machine type, qemuCaps-machineTypes[defIdx] is accessed and since the defIdx is NULL, it results in segmentation fault. Signed-off-by: Yudai Yamagishi yu...@sfc.wide.ad.jp --- src/qemu/qemu_capabilities.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5e9c65e..5def55c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2150,7 +2150,7 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, machines[i]-name) 0) goto cleanup; if (machines[i]-isDefault) -defIdx = i; +defIdx = qemuCaps-nmachineTypes - 1; qemuCaps-machineMaxCpus[qemuCaps-nmachineTypes - 1] = machines[i]-maxCpus; } Oops, good catch. The reason we haven't seen it yet is the order of machine types returned by your QEMU is apparently different than the order in which anyone else gets them :-) If none comes after the default machine type, everything works as expected. Also if the default machine is not the last one, libvirt would just select a wrong machine type as the default one instead of segfaulting. Yudai, would you be comfortable with updating our test suite to test this corner case? We already have qemucapabilitiestest.c but it only tests QEMU capabilities even though the data files already contain machine types. So the test would need to be enhanced to actually check machine types decoded from the data files. And we would need a data file where none is not the last machine type in the list. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] virsh nodecpustats erroneously returns stats for offline cpus on Linux
On 12/19/2013 12:40 PM, Pradipta Kr. Banerjee wrote:
From: Pradipta Kr. Banerjee bpra...@in.ibm.com
virsh nodecpustats erroneously returns stats for offline cpus on Linux
To retrieve node cpu statistics on Linux system, the
linuxNodeGetCPUstats function performs a minimal match of the input
cpuid with the cpuid read from /proc/cpustat. On systems with larger
cpus this can result in erroneous data.
For eg if /proc/stat has similar data
cpu 3645648 485 361475 3691194868 176595 40645 69850 0 1526172 0
cpu0 494045 62 42763 138516619 16107 2339 2431 0 248345 0
cpu4 402646 59 34745 138617562 17738 1413 1196 0 193948 0
cpu8 318662 32 32119 138715131 7903 927 982 0 115429 0
cpu12 247004 33 30465 138791438 5251 759 913 0 51938 0
and cpu 1,2 are offline, then
'virsh nodecpustats 1' displays data for cpu12
whereas virsh nodecpustats 2 correctly throws the following error
error: Unable to get node cpu stats
error: Invalid cpuNum in linuxNodeGetCPUStats
This patch fixes the above mentioned problem
Signed-off-by: Pradipta Kr. Banerjee bpra...@in.ibm.com
---
src/nodeinfo.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 1838547..c9e5ff1 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -614,6 +614,8 @@ linuxNodeGetCPUStats(FILE *procstat,
unsigned long long usr, ni, sys, idle, iowait;
unsigned long long irq, softirq, steal, guest, guest_nice;
char cpu_header[3 + INT_BUFSIZE_BOUND(cpuNum)];
+char cpu_header_read[3 + INT_BUFSIZE_BOUND(cpuNum)];
+char cpu_header_fmt[8];
if ((*nparams) == 0) {
/* Current number of cpu stats supported by linux */
@@ -631,8 +633,11 @@ linuxNodeGetCPUStats(FILE *procstat,
if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) {
strcpy(cpu_header, cpu);
+snprintf(cpu_header_fmt, sizeof(cpu_header_fmt), %%%ds, 3);
} else {
snprintf(cpu_header, sizeof(cpu_header), cpu%d, cpuNum);
+snprintf(cpu_header_fmt, sizeof(cpu_header_fmt), %%%ds,
+ (int)(3 + INT_BUFSIZE_BOUND(cpuNum)));
}
while (fgets(line, sizeof(line), procstat) != NULL) {
@@ -649,6 +654,14 @@ linuxNodeGetCPUStats(FILE *procstat,
continue;
}
+/*
+ * Process with stats gathering only if the cpuid from /proc/stat
+ * matches exactly with the input cpuid
+*/
+sscanf(buf, cpu_header_fmt, cpu_header_read);
+if (STRNEQ(cpu_header, cpu_header_read))
+continue;
+
for (i = 0; i *nparams; i++) {
virNodeCPUStatsPtr param = params[i];
I think we can use strsplit() to handle this bug easily.
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 1838547..aa1ad81 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -637,8 +637,9 @@ linuxNodeGetCPUStats(FILE *procstat,
while (fgets(line, sizeof(line), procstat) != NULL) {
char *buf = line;
+char **buf_header = virStringSplit(buf, , 2);
-if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
+if (STREQ(buf_header[0], cpu_header)) { /* aka logical CPU time */
size_t i;
if (sscanf(buf,
@@ -697,6 +698,7 @@ linuxNodeGetCPUStats(FILE *procstat,
ret = 0;
goto cleanup;
}
+virStringFreeList(buf_header);
}
virReportInvalidArg(cpuNum,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Best Regards,
Bing Bu Cao
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] storage: fix bogus target in gluster volume xml
On 12/19/13 05:38, Eric Blake wrote:
Commit 6cd60b6 was flat out broken - it tried to print into the
wrong variable. My testing was obviously too cursory (did the
name get a slash added?); valgrind would have caught the error.
Thankfully it didn't hit any release.
Reported by Peter Krempa.
* src/storage/storage_backend_gluster.c
(virStorageBackendGlusterRefreshVol): Fix bogus code.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/storage/storage_backend_gluster.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 622526b..9aa692e 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -227,7 +227,7 @@
virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
goto cleanup;
tmp = state-uri-path;
-if (virAsprintf(vol-key, %s%s, state-uri-path, name) 0) {
+if (virAsprintf(state-uri-path, /%s, vol-key) 0) {
Ok, but ...
state-uri-path = tmp;
goto cleanup;
}
... you still need to free state-uri-path before you overwrite it with
the previous value that is temporarily stored in 'tmp' a few lines below:
tmp = state-uri-path;
if (virAsprintf(vol-key, %s%s, state-uri-path, name) 0) {
state-uri-path = tmp;
goto cleanup;
}
if (!(vol-target.path = virURIFormat(state-uri))) {
state-uri-path = tmp;
goto cleanup;
}
state-uri-path = tmp; --- here
ACK with the memleak resolved.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] Setting devid for emulated NICs (Xen 4.3.1 / libvirt 1.2.0) using libxl driver
On Wed, 2013-12-18 at 17:44 -0700, Jim Fehlig wrote:
Stefan Bader wrote:
On 18.12.2013 14:28, Ian Campbell wrote:
On Wed, 2013-12-18 at 14:12 +0100, Stefan Bader wrote:
On 18.12.2013 13:27, Ian Campbell wrote:
On Tue, 2013-12-17 at 18:32 +0100, Stefan Bader wrote:
Might this libxl fix be relevant:
commit 5420f26507fc5c9853eb1076401a8658d72669da
Author: Jim Fehlig jfeh...@suse.com
Date: Fri Jan 11 12:22:26 2013 +
libxl: Set vfb and vkb devid if not done so by the caller
Other devices set a sensible devid if the caller has not
done so.
Do the same for vfb and vkb. While at it, factor out the
common code
used to determine a sensible devid, so it can be used by
other
libxl__device_*_add functions.
Signed-off-by: Jim Fehlig jfeh...@suse.com
Acked-by: Ian Campbell ian.campb...@citrix.com
Committed-by: Ian Campbell ian.campb...@citrix.com
and a follow up in dfeccbeaa. Although the comment implies that nic's
were already correctly assigning a devid if the caller specified -1, so
I don't know why it doesn't work for you :-(
Ok, yes, the commit above indeed changes libxl__device_nic_add to call
libxl__device_nextid for the devid... Just how is this actually called.
Maybe not sufficient but git grep libxl__device_nic_add in the xen
code only
shows the definition and a declaration in libxl_internal.h to me...
I have a feeling a macro might be involved...
Here we go, look for DEFINE_DEVICE_REMOVE in libxl.c. We should really
add the eventual function names in comments to provide grep fodder
Oh duh, yeah. So in DEFINE_DEVICE_ADD a libxl_device_nic_add is created
which
calls to libxl__device_nic_add. When I look for the single _ version I
find a
call from xl_cmdimpl.c and its public declaration in libxl.h.
So I guess the bug is that libvirt in the libxl driver never seems to do
so
The macro creates libxl__add_nics which adds the nics from the
libxl_domain_config-nics array. I don't think libvirt needs to call
libxl_device_nic_add manually unless it is hotplugging a new nic at
runtime.
Hm, so I think this is the path:
libxl_domain_create_new
- do_domain_create
- initiate_domain_create
- libxl__bootloader_run (HVM domain, skipping bootloader)
- domcreate_bootloader_done
- domcreate_rebuild_done
- domcreate_launch_dm
- libxl__spawn_local_dm
- domcreate_devmodel_started
In libxl__spawn_local_dm, there is the following loop:
for (i = 0; i d_config-num_nics; i++) {
/* We have to init the nic here, because we still haven't
* called libxl_device_nic_add at this point, but qemu needs
* the nic information to be complete.
*/
ret = libxl__device_nic_setdefault(gc, d_config-nics[i], domid);
if (ret)
goto error_out;
}
So I think when starting the dm, the devid just is not set as setdefault
does
not seem to do so. I would be done in the later domcreate_devmodel_started
callback but that is too late for the generated qemu arguments.
Sorry for jumping in late...
I stumbled across this problem just before openSUSE13.1 released and did
a quick fix in libvirt
https://build.opensuse.org/package/view_file/Virtualization:openSUSE13.1/libvirt/libxl-hvm-nic.patch?expand=1
I removed setting the NIC devid in the libxl driver a while back to be
consistent with other devices
http://libvirt.org/git/?p=libvirt.git;a=commit;h=ba64b97134a6129a48684f22f31be92c3b6eef96
The quick fix was to essentially revert the above commit until I could
investigate further. Thank you for now having done that investigation
:). Can the devid assignment logic be moved from
libxl__device_nic_add() to libxl__device_nic_setdefault()?
It certainly seems like it would be more natural to do it there.
I suspect it might be done this way because at setdefault time you might
be walking a list of nics none of which have been created yet -- so
looking in xenstore would return devid zero is free for every one of
them?
How about we:
* move the init to setdefault to catch the single NIC added via
hotplug case
* we add somewhere early in the domain create path a call to a
function which assigns devids to an entire array of devices (and
do it for all the different device types). Perhaps in
initiate_domain_create() after the calls to
libxl__domain_create_info_setdefault and
libxl__domain_build_info_setdefault but before the loop calling
libxl__device_disk_setdefault for the disks.
* perhaps that same function should call setdefault too,
Re: [libvirt] [PATCH 1/1] virsh nodecpustats erroneously returns stats for offline cpus on Linux
On 12/19/2013 02:02 PM, Bing Bu Cao wrote:
On 12/19/2013 12:40 PM, Pradipta Kr. Banerjee wrote:
From: Pradipta Kr. Banerjee bpra...@in.ibm.com
virsh nodecpustats erroneously returns stats for offline cpus on Linux
To retrieve node cpu statistics on Linux system, the
linuxNodeGetCPUstats function performs a minimal match of the input
cpuid with the cpuid read from /proc/cpustat. On systems with larger
cpus this can result in erroneous data.
For eg if /proc/stat has similar data
cpu 3645648 485 361475 3691194868 176595 40645 69850 0 1526172 0
cpu0 494045 62 42763 138516619 16107 2339 2431 0 248345 0
cpu4 402646 59 34745 138617562 17738 1413 1196 0 193948 0
cpu8 318662 32 32119 138715131 7903 927 982 0 115429 0
cpu12 247004 33 30465 138791438 5251 759 913 0 51938 0
and cpu 1,2 are offline, then
'virsh nodecpustats 1' displays data for cpu12
whereas virsh nodecpustats 2 correctly throws the following error
error: Unable to get node cpu stats
error: Invalid cpuNum in linuxNodeGetCPUStats
This patch fixes the above mentioned problem
Signed-off-by: Pradipta Kr. Banerjee bpra...@in.ibm.com
---
src/nodeinfo.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 1838547..c9e5ff1 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -614,6 +614,8 @@ linuxNodeGetCPUStats(FILE *procstat,
unsigned long long usr, ni, sys, idle, iowait;
unsigned long long irq, softirq, steal, guest, guest_nice;
char cpu_header[3 + INT_BUFSIZE_BOUND(cpuNum)];
+char cpu_header_read[3 + INT_BUFSIZE_BOUND(cpuNum)];
+char cpu_header_fmt[8];
if ((*nparams) == 0) {
/* Current number of cpu stats supported by linux */
@@ -631,8 +633,11 @@ linuxNodeGetCPUStats(FILE *procstat,
if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) {
strcpy(cpu_header, cpu);
+snprintf(cpu_header_fmt, sizeof(cpu_header_fmt), %%%ds, 3);
} else {
snprintf(cpu_header, sizeof(cpu_header), cpu%d, cpuNum);
+snprintf(cpu_header_fmt, sizeof(cpu_header_fmt), %%%ds,
+ (int)(3 + INT_BUFSIZE_BOUND(cpuNum)));
}
while (fgets(line, sizeof(line), procstat) != NULL) {
@@ -649,6 +654,14 @@ linuxNodeGetCPUStats(FILE *procstat,
continue;
}
+/*
+ * Process with stats gathering only if the cpuid from
/proc/stat
+ * matches exactly with the input cpuid
+*/
+sscanf(buf, cpu_header_fmt, cpu_header_read);
+if (STRNEQ(cpu_header, cpu_header_read))
+continue;
+
for (i = 0; i *nparams; i++) {
virNodeCPUStatsPtr param = params[i];
What about this?
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 1838547..aa1ad81 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -637,8 +637,9 @@ linuxNodeGetCPUStats(FILE *procstat,
while (fgets(line, sizeof(line), procstat) != NULL) {
char *buf = line;
+char **buf_header = virStringSplit(buf, , 2);
-if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
+if (STREQ(buf_header[0], cpu_header)) { /* aka logical CPU time */
size_t i;
if (sscanf(buf,
@@ -697,6 +698,7 @@ linuxNodeGetCPUStats(FILE *procstat,
ret = 0;
goto cleanup;
}
+virStringFreeList(buf_header);
}
virReportInvalidArg(cpuNum,
This is definitely better and lesser amount of code..
Thanks
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Regards,
Pradipta
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 09/27] storage: Add gluster pool filter and fix virsh pool listing
On 12/19/13 05:54, Eric Blake wrote: On 12/16/2013 09:32 AM, Peter Krempa wrote: Recent adition of the gluster pool type omitted fixing the virsh and s/adition/addition/ virConnectListAllStoragePool filters. A typecast of the converting function in virsh showed that also the sheepdog pool was omitted in the command parser. This patch adds gluster pool filtering support and fixes virsh to properly convert all supported storage pool types. The added typecast should avoid doing such mistakes in the future. --- include/libvirt/libvirt.h.in | 1 + src/conf/storage_conf.c | 4 +++- tools/virsh-pool.c | 9 +++-- 3 files changed, 11 insertions(+), 3 deletions(-) Missing a doc change in virsh.pod, to mention that 'gluster' is also a valid type. Code is fine, so ACK if you fix the missing docs. I've fixed the man page and pushed the patch. Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]
[Moving this to libvir, libvir-users in Bcc. Also, added xen-devel] Jim, cooldharma06 reports having issues when destroying VMs with libvirt 1.2.0 and Xen 4.2.1 (is that the case, cooldharma06)? Look at the stack trace at the end of this message, or here: https://bugzilla.redhat.com/show_bug.cgi?id=1044838 Ian, since it looks like events are involved... Any idea? cooldharma06, about the Xen 4.2.1, thing, is there a specific reason why you use it instead, of, e.g., 4.3.x ? Since you are building from source anyway, could you at least try Xen 4.3, to see whether the issue could be a bug in 4.2's libxl? I'm asking because, with both Xen 4.3 and the curret git tip, I don't have issues destroying domains with virsh. Thanks and Regards, Dario On gio, 2013-12-19 at 15:42 +0530, cool dharma06 wrote: i did the debugging as you said. Kindly refer the following logs: (gdb) c Continuing. thread apply all bt [New Thread 0x7fc337c6b700 (LWP 29520)] Program received signal SIGSEGV, Segmentation fault. 0x in ?? () (gdb) thread apply all bt Thread 12 (Thread 0x7fc337c6b700 (LWP 29520)): #0 0x7fc33509f18d in read () from /lib/x86_64-linux-gnu/libpthread.so.0 #1 0x7fc32b1555c4 in read_all (fd=26, data=0x7fc3380e0a70, data@entry=0x20, len=len@entry=16, nonblocking=nonblocking@entry=0) at xs.c:365 #2 0x7fc32b1556d8 in read_message (h=h@entry=0x7fc324013ee0, nonblocking=nonblocking@entry=0) at xs.c:1071 #3 0x7fc32b156005 in read_thread (arg=0x7fc324013ee0) at xs.c:1137 #4 0x7fc335097b50 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #5 0x7fc3349daa3d in clone () from /lib/x86_64-linux-gnu/libc.so.6 #6 0x in ?? () Thread 11 (Thread 0x7fc330e7f700 (LWP 29170)): #0 0x7fc33509c2d4 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0 #1 0x7fc3371ae9ea in virCondWait (c=c@entry=0x7fc3380dbd80, m=m@entry=0x7fc3380dbd58) at util/virthreadpthread.c:117 #2 0x7fc3371af0cb in virThreadPoolWorker (opaque=opaque@entry=0x7fc3380db870) at util/virthreadpool.c:103 #3 0x7fc3371ae686 in virThreadHelper (data=optimized out) at util/virthreadpthread.c:161 #4 0x7fc335097b50 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #5 0x7fc3349daa3d in clone () from /lib/x86_64-linux-gnu/libc.so.6 #6 0x in ?? () Thread 10 (Thread 0x7fc33067e700 (LWP 29171)): #0 0x7fc33509c2d4 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0 #1 0x7fc3371ae9ea in virCondWait (c=c@entry=0x7fc3380dbd80, m=m@entry=0x7fc3380dbd58) at util/virthreadpthread.c:117 #2 0x7fc3371af0cb in virThreadPoolWorker (opaque=opaque@entry=0x7fc3380db640) at util/virthreadpool.c:103 #3 0x7fc3371ae686 in virThreadHelper (data=optimized out) at util/virthreadpthread.c:161 #4 0x7fc335097b50 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #5 0x7fc3349daa3d in clone () from /lib/x86_64-linux-gnu/libc.so.6 #6 0x in ?? () Thread 9 (Thread 0x7fc32fe7d700 (LWP 29172)): #0 0x7fc33509c2d4 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib/x86_64-linux-gnu/libpthread.so.0 #1 0x7fc3371ae9ea in virCondWait (c=c@entry=0x7fc3380dbd80, m=m@entry=0x7fc3380dbd58) at util/virthreadpthread.c:117 #2 0x7fc3371af0cb in virThreadPoolWorker (opaque=opaque@entry=0x7fc3380db870) at util/virthreadpool.c:103 #3 0x7fc3371ae686 in virThreadHelper (data=optimized out) at util/virthreadpthread.c:161 #4 0x7fc335097b50 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #5 0x7fc3349daa3d in clone () from /lib/x86_64-linux-gnu/libc.so.6 #6 0x in ?? () Thread 8 (Thread 0x7fc32f67c700 (LWP 29173)): #0 0x7fc33497cb9f in realloc () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x7fc3349eec85 in __vasprintf_chk () from /lib/x86_64-linux-gnu/libc.so.6 ---Type return to continue, or q return to quit--- #2 0x7fc3371aac0a in vasprintf (__ap=0x7fc32f67b6d0, __fmt=0x7fc337391017 %llu: %s : %s:%d : %s\n, __ptr=0x7fc32f67b830) at /usr/include/x86_64-linux-gnu/bits/stdio2.h:199 #3 virVasprintfInternal (report=report@entry=false, domcode=0, filename=0x0, funcname=0x0, linenr=0, strp=0x7fc32f67b830, fmt=fmt@entry=0x7fc337391017 %llu: %s : %s:%d : %s\n, list=list@entry=0x7fc32f67b6d0) at util/virstring.c:337 #4 0x7fc3371aad1b in virAsprintfInternal (report=report@entry=false, domcode=domcode@entry=0, filename=filename@entry=0x0, funcname=funcname@entry=0x0, linenr=linenr@entry=0, strp=strp@entry=0x7fc32f67b830, fmt=fmt@entry=0x7fc337391017 %llu: %s : %s:%d : %s\n) at util/virstring.c:358 #5 0x7fc33718cb59 in virLogFormatString (str=optimized out, priority=VIR_LOG_DEBUG, funcname=0x7fc337394734 virObjectRef, linenr=293, msg=0x7fc32f67b830) at util/virlog.c:719 #6 virLogVMessage
Re: [libvirt] [PATCH RFC 10/27] storage: Avoid forward declaration of virStorageVolDelete
On 12/19/13 05:56, Eric Blake wrote:
On 12/16/2013 09:32 AM, Peter Krempa wrote:
Move the code around so that the forward declaration isn't needed.
---
src/storage/storage_driver.c | 181
++-
1 file changed, 91 insertions(+), 90 deletions(-)
ACK; mechanical.
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index f08255e..8b1dcae 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1495,7 +1495,97 @@ cleanup:
return ret;
}
-static int storageVolDelete(virStorageVolPtr obj, unsigned int flags);
+
+static int
+storageVolDelete(virStorageVolPtr obj,
+ unsigned int flags) {
This { should be on its own line; but you were doing straight code
motion so it's okay if you don't clean it up until a later patch. (Or
you could clean it up here, as long as the commit message mentions that
you did minor tweaking to fix style issues as part of the code motion)
I've done as you've advised and pushed this one. Thanks.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]
Dario Faggioli writes (Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]): [Moving this to libvir, libvir-users in Bcc. Also, added xen-devel] 4.2.1 is lacking libxl: fix stale timeout event callback race libxl: fix stale fd event callback race These are both in stable-4.2 as 6f0f339dd4378d062a211969f45cd23af12bf386 a87ef897295ec17788e41e9a8f4c0ada7a5a45f8 Around the time I was developing those in xen-unstable.git there were some libvirt patches which were necessary too. Ian. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]
On gio, 2013-12-19 at 11:46 +, Ian Jackson wrote: Dario Faggioli writes (Segfault in libxl_osevent_occurred_timeout [Was: Re: [libvirt-users] About debugging of libvirt.]): [Moving this to libvir, libvir-users in Bcc. Also, added xen-devel] 4.2.1 is lacking libxl: fix stale timeout event callback race libxl: fix stale fd event callback race These are both in stable-4.2 as 6f0f339dd4378d062a211969f45cd23af12bf386 a87ef897295ec17788e41e9a8f4c0ada7a5a45f8 I see. Thanks Ian. Around the time I was developing those in xen-unstable.git there were some libvirt patches which were necessary too. That should be ok, as he's using the very latest libvirt. For Xen, cooldharma06, you really should at least upgrade to 4.2.3, which is the latest available 4.2.x. Regards, Dario -- This happens because I choose it to happen! (Raistlin Majere) - Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems RD Ltd., Cambridge (UK) signature.asc Description: This is a digitally signed message part -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: avoid duplicate security label restore on hostdev attach failure
This eliminates the misleading error message that was being logged when a vfio hostdev hotplug failed: error: unable to set user and group to '107:107' on '/dev/vfio/22': No such file or directory as documented in: https://bugzilla.redhat.com/show_bug.cgi?id=1035490 Commit ee414b5d (pushed as a fix for Bug 1016511 and part of Bug 1025108) replaced the single call to virSecurityManagerSetHostdevLabel() in qemuDomainAttachHostDevice() with individual calls to that same function in each device-type-specific attach function (for PCI, USB, and SCSI). It also added a corresponding call to virSecurityManagerRestoreHostdevLabel() in the error handling of the device-type-specific functions, but forgot to remove the common call to that from qemuDomainAttachHostDevice() - this resulted in a duplicate call to virSecurityManagerRestoreHostdevLabel(), with the second occurrence being after (e.g.) a PCI device has already been re-attached to the host driver, thus destroying some of the device nodes / links that we then attempted to re-label (e.f. /dev/vfio/22) and generating an error log that obscured the original error. --- src/qemu/qemu_hotplug.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4a2c5ce..7a8caf1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1666,9 +1666,6 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver, return 0; error: -if (virSecurityManagerRestoreHostdevLabel(driver-securityManager, - vm-def, hostdev, NULL) 0) -VIR_WARN(Unable to restore host device labelling on hotplug fail); return -1; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: re-add hostdev interfaces to hostdev array on libvirtd restart
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1045002
If a domain has an interface type='hostdev' or an interface
type='network' where the network itself is a pool of hostdev devices,
then libvirt will internally keep that device on both the interface
list *and* the hostdev list for the domain. One of the places this
comes in handy is when a new device is being added and libvirt wants
to find a unique alias name for it - it just scans through the
hostdev array and makes sure it picks a name that doesn't match the
alias of any device in that array.
However, when libvirtd was restarted, if there was an interface
type='network' with the network being a hostdev pool, the device
would not be added to the reconstructed internal hostdev array, so its
alias would not be found during a scan of the hostdev array, thus
attempts to add a new hostdev (or interface type='hostdev' or
interface type='network') would result in a message like this:
internal error: unable to execute QEMU command 'device_add':
Duplicate ID 'hostdev0' for device
This patch simply fixes the existing code in the domain XML parser
that fixes up the hostdev array in the case of interface
type='hostdev' to do the same thing in the case of interface
type='network' with a hostdev network.
This bug has existed since the very first addition of hostdev networks
to libvirt (0.10.0).
---
src/conf/domain_conf.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0079234..583ea02 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12101,9 +12101,12 @@ virDomainDefParseXML(xmlDocPtr xml,
def-nets[def-nnets++] = net;
-/* interface type='hostdev' must also be in the hostdevs array */
-if (net-type == VIR_DOMAIN_NET_TYPE_HOSTDEV
-virDomainHostdevInsert(def, net-data.hostdev.def) 0) {
+/* interface type='hostdev' (and interface type='net'
+ * where the actual network type is already known to be
+ * hostdev) must also be in the hostdevs array.
+ */
+if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV
+virDomainHostdevInsert(def, virDomainNetGetActualHostdev(net))
0) {
goto error;
}
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] PCI device assignment bugfixes
These were all found as a result of researching Bug 1035490. Patch 1 fixes the originally reported problem, Patch 2 fixes the cause of incorrect error reporting in a later comment of that bug, and Patch 3 fixes a behavior that I encountered while searching for the cause. Laine Stump (3): qemu: properly set MaxMemLock when hotplugging with VFIO qemu: avoid duplicate security label restore on hostdev attach failure qemu: re-add hostdev interfaces to hostdev array on libvirtd restart src/conf/domain_conf.c | 9 ++--- src/qemu/qemu_hotplug.c | 22 +++--- 2 files changed, 17 insertions(+), 14 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: properly set MaxMemLock when hotplugging with VFIO
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1035490 virProcessSetMaxMemLock() (which is a wrapper over prlimit(3)) expects the memory size in bytes, but libvirt's domain definition (which was being used by qemuDomainAttachHostPciDevice()) stores all memory tuning parameters in KiB. This was being accounted for when setting MaxMemLock at domain startup time (so cold-plugged devices would work), but not for hotplug. This patch simplifies the few lines that call virProcessSetMemMaxLock(), and multiply the amount * 1024 so that we're locking the correct amount of memory. What remains a mystery to me is why hot-plug of a managed='no' device would succeed (at least on my system) while managed='yes' would fail. I guess in one case the memory was coincidentally already resident and in the other it wasn't. --- src/qemu/qemu_hotplug.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a375b6c..4a2c5ce 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1155,6 +1155,7 @@ qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver, bool teardowncgroup = false; bool teardownlabel = false; int backend = hostdev-source.subsys.u.pci.backend; +unsigned long long memKB; if (VIR_REALLOC_N(vm-def-hostdevs, vm-def-nhostdevs + 1) 0) return -1; @@ -1172,16 +1173,18 @@ qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver, goto error; } -/* VFIO requires all of the guest's memory to be locked resident. - * In this case, the guest's memory may already be locked, but it +/* VFIO requires all of the guest's memory to be locked + * resident (plus an additional 1GiB to cover IO space). During + * hotplug, the guest's memory may already be locked, but it * doesn't hurt to change the limit to the same value. + * NB: the domain's memory tuning parameters are stored as + * Kibibytes, but virProcessSetMaxMemLock expects the value in + * bytes. */ -if (vm-def-mem.hard_limit) -virProcessSetMaxMemLock(vm-pid, vm-def-mem.hard_limit); -else -virProcessSetMaxMemLock(vm-pid, -vm-def-mem.max_balloon + (1024 * 1024)); - +memKB = vm-def-mem.hard_limit +? vm-def-mem.hard_limit +: vm-def-mem.max_balloon + (1024 * 1024); +virProcessSetMaxMemLock(vm-pid, memKB * 1024); break; case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] docs: improve event-related documentation
While looking at event code, I noticed that the documentation was
trying to refer me to functions that don't exist. Also fix some
typos and poor formatting.
* src/libvirt.c (virConnectDomainEventDeregister)
(virConnectDomainEventRegisterAny)
(virConnectDomainEventDeregisterAny)
(virConnectNetworkEventRegisterAny)
(virConnectNetworkEventDeregisterAny): Link to correct function.
* include/libvirt.h.in (VIR_DOMAIN_EVENT_CALLBACK)
(VIR_NETWORK_EVENT_CALLBACK): Likewise.
(virDomainEventID, virConnectDomainEventGenericCallback)
(virNetworkEventID, virConnectNetworkEventGenericCallback):
Improve docs.
Signed-off-by: Eric Blake ebl...@redhat.com
---
include/libvirt/libvirt.h.in | 36
src/libvirt.c| 24
2 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 6f79c49..b6c4cd0 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4507,14 +4507,17 @@ int virDomainSnapshotDelete(virDomainSnapshotPtr
snapshot,
int virDomainSnapshotRef(virDomainSnapshotPtr snapshot);
int virDomainSnapshotFree(virDomainSnapshotPtr snapshot);
-/*
+/**
* virConnectDomainEventGenericCallback:
* @conn: the connection pointer
* @dom: the domain pointer
* @opaque: application specified data
*
- * A generic domain event callback handler. Specific events usually
- * have a customization with extra parameters
+ * A generic domain event callback handler, for use with
+ * virConnectDomainEventRegisterAny(). Specific events usually
+ * have a customization with extra parameters, often with @opaque being
+ * passed in a different parameter position; use VIR_DOMAIN_EVENT_CALLBACK()
+ * when registering an appropriate handler.
*/
typedef void (*virConnectDomainEventGenericCallback)(virConnectPtr conn,
virDomainPtr dom,
@@ -4929,11 +4932,18 @@ typedef void
(*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
* VIR_DOMAIN_EVENT_CALLBACK:
*
* Used to cast the event specific callback into the generic one
- * for use for virDomainEventRegister
+ * for use for virConnectDomainEventRegisterAny()
*/
#define VIR_DOMAIN_EVENT_CALLBACK(cb)
((virConnectDomainEventGenericCallback)(cb))
+/**
+ * virDomainEventID:
+ *
+ * An enumeration of supported eventId parameters for
+ * virConnectDomainEventRegisterAny(). Each event id determines which
+ * signature of callback function will be used.
+ */
typedef enum {
VIR_DOMAIN_EVENT_ID_LIFECYCLE = 0, /* virConnectDomainEventCallback
*/
VIR_DOMAIN_EVENT_ID_REBOOT = 1, /*
virConnectDomainEventGenericCallback */
@@ -5014,10 +5024,17 @@ typedef void
(*virConnectNetworkEventLifecycleCallback)(virConnectPtr conn,
* VIR_NETWORK_EVENT_CALLBACK:
*
* Used to cast the event specific callback into the generic one
- * for use for virNetworkEventRegister
+ * for use for virConnectNetworkEventRegisterAny()
*/
#define VIR_NETWORK_EVENT_CALLBACK(cb)
((virConnectNetworkEventGenericCallback)(cb))
+/**
+ * virNetworkEventID:
+ *
+ * An enumeration of supported eventId parameters for
+ * virConnectNetworkEventRegisterAny(). Each event id determines which
+ * signature of callback function will be used.
+ */
typedef enum {
VIR_NETWORK_EVENT_ID_LIFECYCLE = 0, /*
virConnectNetworkEventLifecycleCallback */
@@ -5031,14 +5048,17 @@ typedef enum {
#endif
} virNetworkEventID;
-/*
+/**
* virConnectNetworkEventGenericCallback:
* @conn: the connection pointer
* @net: the network pointer
* @opaque: application specified data
*
- * A generic network event callback handler. Specific events usually
- * have a customization with extra parameters
+ * A generic network event callback handler, for use with
+ * virConnectNetworkEventRegisterAny(). Specific events usually
+ * have a customization with extra parameters, often with @opaque being
+ * passed in a different parameter position; use VIR_NETWORK_EVENT_CALLBACK()
+ * when registering an appropriate handler.
*/
typedef void (*virConnectNetworkEventGenericCallback)(virConnectPtr conn,
virNetworkPtr net,
diff --git a/src/libvirt.c b/src/libvirt.c
index d15d617..77f481e 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -16127,7 +16127,7 @@ error:
* occurring on a connection
*
* Use of this method is no longer recommended. Instead applications
- * should try virConnectDomainEventRegisterAny which has a more flexible
+ * should try virConnectDomainEventRegisterAny() which has a more flexible
* API contract
*
* The virDomainPtr object handle passed into the callback upon delivery
@@ -16178,7 +16178,7 @@ error:
* function.
*
* Use of this method is no longer recommended. Instead applications
- * should try virConnectDomainEventUnregisterAny which has a more flexible
+ * should try
[libvirt] [PATCH 2/2] maint: improve debug of libvirt-{qemu, lxc} apis
I noticed that the virDomainQemuMonitorCommand debug output wasn't
telling me the name of the domain it was working on. While it was
easy enough to determine which pointer matches the domain based on
other log messages, it is nicer to be consistent. Along the same
lines, having virLibDomainError() take two arguments in libvirt.c
but three arguments in libvirt-qemu.c is not friendly to code
copying between the two files.
* src/libvirt.c (VIR_ARG15, VIR_HAS_COMMA)
(VIR_DOMAIN_DEBUG*, VIR_UUID_DEBUG, virLib*Error): Move...
* src/libvirt_internal.h: ...here.
* src/libvirt-qemu.c (virDomainQemuMonitorCommand)
(virDomainQemuAttach, virDomainQemuAgentCommand): Better debug
messages.
* src/libvirt-lxc.c (virDomainLxcOpenNamespace): Likewise.
* src/datatypes.c (virLibConnError): Drop duplicate definition.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/datatypes.c| 6 +--
src/libvirt-lxc.c | 18 +++--
src/libvirt-qemu.c | 37 +++--
src/libvirt.c | 105
src/libvirt_internal.h | 107 +
5 files changed, 128 insertions(+), 145 deletions(-)
diff --git a/src/datatypes.c b/src/datatypes.c
index 161f1b0..f03f9b3 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -1,7 +1,7 @@
/*
* datatypes.h: management of structs for public data types
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 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
@@ -31,10 +31,6 @@
#define VIR_FROM_THIS VIR_FROM_NONE
-#define virLibConnError(code, ...)\
-virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
- __FUNCTION__, __LINE__, __VA_ARGS__)
-
virClassPtr virConnectClass;
virClassPtr virConnectCloseCallbackDataClass;
diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
index c8cdcea..19c6d66 100644
--- a/src/libvirt-lxc.c
+++ b/src/libvirt-lxc.c
@@ -28,6 +28,7 @@
#include virfile.h
#include virlog.h
#include virprocess.h
+#include viruuid.h
#include datatypes.h
#ifdef WITH_SELINUX
# include selinux/selinux.h
@@ -35,14 +36,6 @@
#define VIR_FROM_THIS VIR_FROM_NONE
-#define virLibConnError(conn, error, info) \
-virReportErrorHelper(VIR_FROM_NONE, error, __FILE__, __FUNCTION__, \
- __LINE__, info)
-
-#define virLibDomainError(domain, error, info) \
-virReportErrorHelper(VIR_FROM_DOM, error, __FILE__, __FUNCTION__, \
- __LINE__, info)
-
/**
* virDomainLxcOpenNamespace:
* @domain: a domain object
@@ -69,13 +62,12 @@ virDomainLxcOpenNamespace(virDomainPtr domain,
{
virConnectPtr conn;
-VIR_DEBUG(domain=%p, fdlist=%p flags=%x,
- domain, fdlist, flags);
+VIR_DOMAIN_DEBUG(domain, fdlist=%p flags=%x, fdlist, flags);
virResetLastError();
if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
-virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
@@ -85,7 +77,7 @@ virDomainLxcOpenNamespace(virDomainPtr domain,
virCheckNonNullArgGoto(fdlist, error);
if (conn-flags VIR_CONNECT_RO) {
-virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
goto error;
}
@@ -99,7 +91,7 @@ virDomainLxcOpenNamespace(virDomainPtr domain,
return ret;
}
-virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
error:
virDispatchError(conn);
diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
index 83fb3b3..db52c65 100644
--- a/src/libvirt-qemu.c
+++ b/src/libvirt-qemu.c
@@ -2,7 +2,7 @@
* libvirt-qemu.c: Interfaces for the libvirt library to handle qemu-specific
* APIs.
*
- * Copyright (C) 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2010-2013 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
@@ -25,18 +25,11 @@
#include virerror.h
#include virlog.h
+#include viruuid.h
#include datatypes.h
#define VIR_FROM_THIS VIR_FROM_NONE
-#define virLibConnError(conn, error, info) \
-virReportErrorHelper(VIR_FROM_NONE, error, __FILE__, __FUNCTION__, \
- __LINE__, info)
-
-#define virLibDomainError(domain, error, info) \
-virReportErrorHelper(VIR_FROM_DOM, error, __FILE__, __FUNCTION__, \
- __LINE__, info)
-
/**
* virDomainQemuMonitorCommand:
* @domain: a domain object
@@ -75,13 +68,13 @@ virDomainQemuMonitorCommand(virDomainPtr
[libvirt] [PATCH 0/2] event cleanup patches
Noticed these while working on another patch. Although it is mostly
cosmetic, it is not small enough to push under the trivial rule, so
I'll wait for a review.
Eric Blake (2):
docs: improve event-related documentation
maint: improve debug of libvirt-{qemu,lxc} apis
include/libvirt/libvirt.h.in | 36 +---
src/datatypes.c | 6 +-
src/libvirt-lxc.c| 18 ++
src/libvirt-qemu.c | 37 +
src/libvirt.c| 129 ---
src/libvirt_internal.h | 107 +++
6 files changed, 168 insertions(+), 165 deletions(-)
--
1.8.4.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events
Several times in the past, qemu has implemented a new event,
but libvirt has not yet caught up to reporting that event to
the user applications. While it is possible to track libvirt
logs to see that an unknown event was received and ignored,
it would be nicer to copy what 'virsh qemu-monitor-command'
does, and expose this information to the end developer as
one of our unsupported qemu-specific commands.
If you find yourself needing to use this API for more than
just development purposes, please ask on the libvirt list
for a supported counterpart event to be added in libvirt.so.
While the supported virConnectDomainEventRegisterAny() API
takes an id which determines the signature of the callback,
this version takes a string filter and always uses the same
signature. Furthermore, I chose to expose this as a new API
instead of trying to add a new eventID at the top level, in
part because the generic option lacks event name filtering,
and in part because the normal domain event namespace should
not be polluted by a qemu-only event. I also added a flags
argument; unused for now, but we might decide to use it to
allow a user to request event names by glob or regex instead
of literal match.
* include/libvirt/libvirt-qemu.h
(virConnectDomainQemuMonitorEventCallback)
(virConnectDomainQemuMonitorEventRegister)
(virConnectDomainQemuMonitorEventDeregister): New prototypes.
* src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister)
(virConnectDomainQemuMonitorEventDeregister): New functions.
* src/libvirt_qemu.syms (LIBVIRT_QEMU_1.2.1): Export them.
* src/driver.h (virDrvConnectDomainQemuMonitorEventRegister)
(virDrvConnectDomainQemuMonitorEventDeregister): New callbacks.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Before I go and implement the guts of this new API, I first
wanted to get approval from the list that I'm on the right track.
include/libvirt/libvirt-qemu.h | 31 ++-
src/driver.h | 15 +
src/libvirt-qemu.c | 123 +
src/libvirt_qemu.syms | 6 ++
4 files changed, 174 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 3e79a8c..5403093 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -4,7 +4,7 @@
* Description: Provides the interfaces of the libvirt library to handle
* qemu specific methods
*
- * Copyright (C) 2010, 2012 Red Hat, Inc.
+ * Copyright (C) 2010, 2012-2013 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
@@ -54,6 +54,35 @@ typedef enum {
char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
int timeout, unsigned int flags);
+/**
+ * virConnectDomainQemuMonitorEventCallback:
+ * @conn: the connection pointer
+ * @dom: the domain pointer
+ * @event: the name of the event
+ * @details: the JSON details of the event
+ * @opaque: application specified data
+ *
+ * The callback signature to use when registering for a qemu monitor
+ * event with virConnectDomainQemuMonitorEventRegister().
+ */
+typedef void (*virConnectDomainQemuMonitorEventCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *event,
+ const char *details,
+ void *opaque);
+
+int virConnectDomainQemuMonitorEventRegister(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *event,
+
virConnectDomainQemuMonitorEventCallback cb,
+ void *opaque,
+ virFreeCallback freecb,
+ unsigned int flags);
+
+int virConnectDomainQemuMonitorEventDeregister(virConnectPtr conn,
+ int callbackID);
+
+
# ifdef __cplusplus
}
# endif
diff --git a/src/driver.h b/src/driver.h
index b6927ea..e9bf5cb 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -841,6 +841,19 @@ typedef virDomainPtr
unsigned int flags);
typedef int
+(*virDrvConnectDomainQemuMonitorEventRegister)(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *event,
+
virConnectDomainQemuMonitorEventCallback cb,
+ void *opaque,
+ virFreeCallback freecb,
+ unsigned int flags);
+
+typedef int
Re: [libvirt] [PATCH 2/2] maint: improve debug of libvirt-{qemu, lxc} apis
On Thu, Dec 19, 2013 at 08:13:36AM -0700, Eric Blake wrote:
diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
index 115d8d1..b8c842d 100644
--- a/src/libvirt_internal.h
+++ b/src/libvirt_internal.h
@@ -27,6 +27,113 @@
# include internal.h
+/* Helper macros to implement VIR_DOMAIN_DEBUG using just C99. This
+ * assumes you pass fewer than 15 arguments to VIR_DOMAIN_DEBUG, but
+ * can easily be expanded if needed.
+ *
+ * Note that gcc provides extensions of define a(b...) b or
+ * define a(b,...) b,##__VA_ARGS__ as a means of eliding a comma
+ * when no var-args are present, but we don't want to require gcc.
+ */
+#define VIR_ARG15(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13,
_14, _15, ...) _15
+#define VIR_HAS_COMMA(...) VIR_ARG15(__VA_ARGS__, 1, 1, 1, 1, 1, 1, 1, 1, 1,
1, 1, 1, 1, 0)
+
+/* Form the name VIR_DOMAIN_DEBUG_[01], then call that macro,
+ * according to how many arguments are present. Two-phase due to
+ * macro expansion rules. */
+#define VIR_DOMAIN_DEBUG_EXPAND(a, b, ...) \
+VIR_DOMAIN_DEBUG_PASTE(a, b, __VA_ARGS__)
+#define VIR_DOMAIN_DEBUG_PASTE(a, b, ...) \
+a##b(__VA_ARGS__)
+
+/* Internal use only, when VIR_DOMAIN_DEBUG has one argument. */
+#define VIR_DOMAIN_DEBUG_0(dom) \
+VIR_DOMAIN_DEBUG_2(dom, %s, )
+
+/* Internal use only, when VIR_DOMAIN_DEBUG has three or more arguments. */
+#define VIR_DOMAIN_DEBUG_1(dom, fmt, ...) \
+VIR_DOMAIN_DEBUG_2(dom, , fmt, __VA_ARGS__)
+
+/* Internal use only, with final format. */
+#define VIR_DOMAIN_DEBUG_2(dom, fmt, ...) \
+do {\
+char _uuidstr[VIR_UUID_STRING_BUFLEN]; \
+const char *_domname = NULL;\
+\
+if (!VIR_IS_DOMAIN(dom)) { \
+memset(_uuidstr, 0, sizeof(_uuidstr)); \
+} else {\
+virUUIDFormat((dom)-uuid, _uuidstr); \
+_domname = (dom)-name; \
+} \
+\
+VIR_DEBUG(dom=%p, (VM: name=%s, uuid=%s) fmt, \
+ dom, NULLSTR(_domname), _uuidstr, __VA_ARGS__); \
+} while (0)
+
+/**
+ * VIR_DOMAIN_DEBUG:
+ * @dom: domain
+ * @fmt: optional format for additional information
+ * @...: optional arguments corresponding to @fmt.
+ */
+#define VIR_DOMAIN_DEBUG(...) \
+VIR_DOMAIN_DEBUG_EXPAND(VIR_DOMAIN_DEBUG_, \
+VIR_HAS_COMMA(__VA_ARGS__), \
+__VA_ARGS__)
I'd suggest these go in datatypes.h
+
+/**
+ * VIR_UUID_DEBUG:
+ * @conn: connection
+ * @uuid: possibly null UUID array
+ */
+#define VIR_UUID_DEBUG(conn, uuid) \
+do {\
+if (uuid) { \
+char _uuidstr[VIR_UUID_STRING_BUFLEN]; \
+virUUIDFormat(uuid, _uuidstr); \
+VIR_DEBUG(conn=%p, uuid=%s, conn, _uuidstr); \
+} else {\
+VIR_DEBUG(conn=%p, uuid=(null), conn);\
+} \
+} while (0)
And this in viruuid.h
So we avoid turning internal.h into even more of a dumping ground.
+
+
+#define virLibConnError(code, ...)\
+virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
+#define virLibDomainError(code, ...) \
+virReportErrorHelper(VIR_FROM_DOM, code, __FILE__,\
+ __FUNCTION__, __LINE__, __VA_ARGS__)
+#define virLibNetworkError(code, ...) \
+virReportErrorHelper(VIR_FROM_NETWORK, code, __FILE__,\
+ __FUNCTION__, __LINE__, __VA_ARGS__)
+#define virLibStoragePoolError(code, ...) \
+virReportErrorHelper(VIR_FROM_STORAGE, code, __FILE__,\
+ __FUNCTION__, __LINE__, __VA_ARGS__)
+#define virLibStorageVolError(code, ...) \
+virReportErrorHelper(VIR_FROM_STORAGE, code, __FILE__,\
+ __FUNCTION__, __LINE__, __VA_ARGS__)
+#define virLibInterfaceError(code, ...) \
+
Re: [libvirt] [PATCH 1/2] docs: improve event-related documentation
On Thu, Dec 19, 2013 at 08:13:35AM -0700, Eric Blake wrote: While looking at event code, I noticed that the documentation was trying to refer me to functions that don't exist. Also fix some typos and poor formatting. * src/libvirt.c (virConnectDomainEventDeregister) (virConnectDomainEventRegisterAny) (virConnectDomainEventDeregisterAny) (virConnectNetworkEventRegisterAny) (virConnectNetworkEventDeregisterAny): Link to correct function. * include/libvirt.h.in (VIR_DOMAIN_EVENT_CALLBACK) (VIR_NETWORK_EVENT_CALLBACK): Likewise. (virDomainEventID, virConnectDomainEventGenericCallback) (virNetworkEventID, virConnectNetworkEventGenericCallback): Improve docs. Signed-off-by: Eric Blake ebl...@redhat.com --- include/libvirt/libvirt.h.in | 36 src/libvirt.c| 24 2 files changed, 40 insertions(+), 20 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events
On Thu, Dec 19, 2013 at 08:15:09AM -0700, Eric Blake wrote:
diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
index db52c65..849932d 100644
--- a/src/libvirt-qemu.c
+++ b/src/libvirt-qemu.c
@@ -237,3 +237,126 @@ error:
virDispatchError(conn);
return NULL;
}
+
+
+/**
+ * virConnectDomainQemuMonitorEventRegister:
+ * @conn: pointer to the connection
+ * @dom: pointer to the domain, or NULL
+ * @event: name of the event, or NULL
+ * @cb: callback to the function handling monitor events
+ * @opaque: opaque data to pass on to the callback
+ * @freecb: optional function to deallocate opaque when not used anymore
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * This API is QEMU specific, so it will only work with hypervisor
+ * connections to the QEMU driver.
+ *
+ * Adds a callback to receive notifications of arbitrary qemu monitor events
+ * occurring on a domain. Many qemu monitor events also result in a libvirt
+ * event which can be delivered via virConnectDomainEventRegisterAny(); this
+ * command is primarily for testing new qemu events that have not yet been
+ * given a libvirt counterpart event.
+ *
+ * If @dom is NULL, then events will be monitored for any domain. If @dom
+ * is non-NULL, then only the specific domain will be monitored.
+ *
+ * If @event is NULL, then all monitor events will be reported. If @event is
+ * non-NULL, then only the specific monitor event will be reported. @flags
+ * is currently unused, but in the future may support a flag for passing
+ * @event as a glob instead of a literal name to match a category of events.
+ *
+ * The virDomainPtr object handle passed into the callback upon delivery
+ * of an event is only valid for the duration of execution of the callback.
+ * If the callback wishes to keep the domain object after the callback
returns,
+ * it shall take a reference to it, by calling virDomainRef().
+ * The reference can be released once the object is no longer required
+ * by calling virDomainFree().
+ *
+ * The return value from this method is a positive integer identifier
+ * for the callback. To unregister a callback, this callback ID should
+ * be passed to the virConnectDomainQemuMonitorEventDeregister() method.
+ *
+ * Returns a callback identifier on success, -1 on failure
+ */
+int
+virConnectDomainQemuMonitorEventRegister(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *event,
+
virConnectDomainQemuMonitorEventCallback cb,
+ void *opaque,
+ virFreeCallback freecb,
+ unsigned int flags)
+{
+VIR_DOMAIN_DEBUG(dom,
+ conn=%p, event=%s, cb=%p, opaque=%p, freecb=%p,
flags=%x,
+ conn, NULLSTR(event), cb, opaque, freecb, flags);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECT(conn)) {
+virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+if (dom
+!(VIR_IS_CONNECTED_DOMAIN(dom) dom-conn == conn)) {
+virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(conn);
+return -1;
+}
+virCheckNonNullArgGoto(cb, error)
I have a gut feeling that we should restrict use of this API to
authenticated users only. So add a check for a read-only connection
here
+if ((conn-driver)
(conn-driver-connectDomainQemuMonitorEventRegister)) {
This is excessively bracketed isn't it
+int ret;
+ret = conn-driver-connectDomainQemuMonitorEventRegister(conn, dom,
event, cb, opaque, freecb, flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+error:
+virDispatchError(conn);
+return -1;
+}
+
+/**
+ * virConnectDomainQemuMonitorEventDeregister:
+ * @conn: pointer to the connection
+ * @callbackID: the callback identifier
+ *
+ * Removes an event callback. The callbackID parameter should be the
+ * value obtained from a previous virConnectDomainQemuMonitorEventRegister()
+ * method.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virConnectDomainQemuMonitorEventDeregister(virConnectPtr conn,
+ int callbackID)
+{
+VIR_DEBUG(conn=%p, callbackID=%d, conn, callbackID);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECT(conn)) {
+virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+virDispatchError(NULL);
+return -1;
+}
+virCheckNonNegativeArgGoto(callbackID, error);
And add read-only check
+
+if ((conn-driver)
(conn-driver-connectDomainQemuMonitorEventDeregister)) {
To many brackets again
+
[libvirt] [PATCH] virkeycode: Allow ANSI_A
https://bugzilla.redhat.com/show_bug.cgi?id=1044806
Currently, sending the ANSI_A keycode from os_x codepage doesn't work as
it has a special value of 0x0. Our internal code handles that no
different to other not defined keycodes. Hence, in order to allow it we
must change all the undefined keycodes from 0 to -1 and adapt some code
too.
# virsh send-key guestname --codeset os_x ANSI_A
error: invalid keycode: 'ANSI_A'
# virsh send-key guestname --codeset os_x ANSI_B
# virsh send-key guestname --codeset os_x ANSI_C
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virkeycode-mapgen.py | 4 ++--
src/util/virkeycode.c | 4 ++--
tools/virsh-domain.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/util/virkeycode-mapgen.py b/src/util/virkeycode-mapgen.py
index 22b21b4..8360bfe 100755
--- a/src/util/virkeycode-mapgen.py
+++ b/src/util/virkeycode-mapgen.py
@@ -86,12 +86,12 @@ for i in range(len(cols)):
if isname:
print const char *virKeymapNames_ + name + [] = {
else:
-print unsigned short virKeymapValues_ + name + [] = {
+print int virKeymapValues_ + name + [] = {
for entry in keycodes:
if isname:
print+ quotestring(entry[i] or NULL) + ,
else:
-print+ (entry[i] or 0) + ,
+print+ (entry[i] or -1) + ,
print };\n
diff --git a/src/util/virkeycode.c b/src/util/virkeycode.c
index 50594d6..7880a0a 100644
--- a/src/util/virkeycode.c
+++ b/src/util/virkeycode.c
@@ -50,7 +50,7 @@ static const char **virKeymapNames[] = {
};
verify(ARRAY_CARDINALITY(virKeymapNames) == VIR_KEYCODE_SET_LAST);
-static unsigned short *virKeymapValues[] = {
+static int *virKeymapValues[] = {
[VIR_KEYCODE_SET_LINUX] =
virKeymapValues_linux,
[VIR_KEYCODE_SET_XT] =
@@ -113,7 +113,7 @@ int virKeycodeValueTranslate(virKeycodeSet from_codeset,
{
size_t i;
-if (key_value = 0)
+if (key_value 0)
return -1;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 760dca5..b33b808 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7091,7 +7091,7 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd)
}
if ((keycode = vshKeyCodeGetInt(opt-data)) = 0) {
-if ((keycode = virKeycodeValueFromString(codeset, opt-data)) =
0) {
+if ((keycode = virKeycodeValueFromString(codeset, opt-data)) 0)
{
vshError(ctl, _(invalid keycode: '%s'), opt-data);
goto cleanup;
}
--
1.8.5.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1] virsh nodecpustats erroneously returns stats for offline cpus on Linux
On 12/19/2013 12:40 PM, Pradipta Kr. Banerjee wrote:
From: Pradipta Kr. Banerjee bpra...@in.ibm.com
virsh nodecpustats erroneously returns stats for offline cpus on Linux
To retrieve node cpu statistics on Linux system, the
linuxNodeGetCPUstats function performs a minimal match of the input
cpuid with the cpuid read from /proc/cpustat. On systems with larger
cpus this can result in erroneous data.
For eg if /proc/stat has similar data
cpu 3645648 485 361475 3691194868 176595 40645 69850 0 1526172 0
cpu0 494045 62 42763 138516619 16107 2339 2431 0 248345 0
cpu4 402646 59 34745 138617562 17738 1413 1196 0 193948 0
cpu8 318662 32 32119 138715131 7903 927 982 0 115429 0
cpu12 247004 33 30465 138791438 5251 759 913 0 51938 0
and cpu 1,2 are offline, then
'virsh nodecpustats 1' displays data for cpu12
whereas virsh nodecpustats 2 correctly throws the following error
error: Unable to get node cpu stats
error: Invalid cpuNum in linuxNodeGetCPUStats
This patch fixes the above mentioned problem
Signed-off-by: Pradipta Kr. Banerjee bpra...@in.ibm.com
---
src/nodeinfo.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 1838547..c9e5ff1 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -614,6 +614,8 @@ linuxNodeGetCPUStats(FILE *procstat,
unsigned long long usr, ni, sys, idle, iowait;
unsigned long long irq, softirq, steal, guest, guest_nice;
char cpu_header[3 + INT_BUFSIZE_BOUND(cpuNum)];
+char cpu_header_read[3 + INT_BUFSIZE_BOUND(cpuNum)];
+char cpu_header_fmt[8];
if ((*nparams) == 0) {
/* Current number of cpu stats supported by linux */
@@ -631,8 +633,11 @@ linuxNodeGetCPUStats(FILE *procstat,
if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS) {
strcpy(cpu_header, cpu);
+snprintf(cpu_header_fmt, sizeof(cpu_header_fmt), %%%ds, 3);
} else {
snprintf(cpu_header, sizeof(cpu_header), cpu%d, cpuNum);
+snprintf(cpu_header_fmt, sizeof(cpu_header_fmt), %%%ds,
+ (int)(3 + INT_BUFSIZE_BOUND(cpuNum)));
}
while (fgets(line, sizeof(line), procstat) != NULL) {
@@ -649,6 +654,14 @@ linuxNodeGetCPUStats(FILE *procstat,
continue;
}
+/*
+ * Process with stats gathering only if the cpuid from /proc/stat
+ * matches exactly with the input cpuid
+*/
+sscanf(buf, cpu_header_fmt, cpu_header_read);
+if (STRNEQ(cpu_header, cpu_header_read))
+continue;
+
for (i = 0; i *nparams; i++) {
virNodeCPUStatsPtr param = params[i];
What about this?
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 1838547..aa1ad81 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -637,8 +637,9 @@ linuxNodeGetCPUStats(FILE *procstat,
while (fgets(line, sizeof(line), procstat) != NULL) {
char *buf = line;
+char **buf_header = virStringSplit(buf, , 2);
-if (STRPREFIX(buf, cpu_header)) { /* aka logical CPU time */
+if (STREQ(buf_header[0], cpu_header)) { /* aka logical CPU time */
size_t i;
if (sscanf(buf,
@@ -697,6 +698,7 @@ linuxNodeGetCPUStats(FILE *procstat,
ret = 0;
goto cleanup;
}
+virStringFreeList(buf_header);
}
virReportInvalidArg(cpuNum,
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] JNA Error Callback could cause core dump.
At Thu, 18 Oct 2012 15:48:22 +,
Benjamin Wang (gendwang) wrote:
Hi,
When I changed code as following:
public class Connect {
// Load the native part
static {
Libvirt.INSTANCE.virInitialize();
try {
ErrorHandler.processError(Libvirt.INSTANCE);
} catch (Exception e) {
e.printStackTrace();
}
+ Libvirt.INSTANCE.virSetErrorFunc(null, new ErrorCallback());
}
The server will generate the following core dump:
Program terminated with signal 6, Aborted.
#0 0x003f9b030265 in raise () from /lib64/libc.so.6
(gdb) where
#0 0x003f9b030265 in raise () from /lib64/libc.so.6
#1 0x003f9b031d10 in abort () from /lib64/libc.so.6
#2 0x003f9b06a84b in __libc_message () from /lib64/libc.so.6
#3 0x003f9b07230f in _int_free () from /lib64/libc.so.6
#4 0x003f9b07276b in free () from /lib64/libc.so.6
#5 0x2cf46868 in ?? ()
#6 0x in ?? ()
The problem was caused that when JNA call setErrorFunc, it will
create ErrorCallback object. But when GC is executed, the object is
GCed.
This should not lead to the crash above.
But even I change code as following. When GC is excuted, the
callback object will be moved. Then C can't find this object.
No, this cannot happen. The VM keeps track of its objects, the C code
never gets a direct pointer to an object but only a handle and JNA is
registering a trampoline function in native code which doesn't move.
Both of scenarios will cause core dump. It seems that JNA mustn't provide
ErrorCallback Class,
Because nobody can use this.
Please correct me.
Which version of JNA are you using?
Which OS and which machine type?
Are you running this on a virtual machine? If yes, which one?
Which JVM are you using?
There's a unit test which exercises the Connect.setErrorCallback
method (which in turn calls virSetErrorFunc). This test works for me
for any version of JNA from 3.3.0 to 4.0.0 using OpenJDK 1.6.0 64bit
on Linux. Did you try to run the unit tests?
Could you show us the Java crash dump?
Claudio
--
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:http://www.av-test.org
Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events
On 19.12.2013 16:15, Eric Blake wrote:
Several times in the past, qemu has implemented a new event,
but libvirt has not yet caught up to reporting that event to
the user applications. While it is possible to track libvirt
logs to see that an unknown event was received and ignored,
it would be nicer to copy what 'virsh qemu-monitor-command'
does, and expose this information to the end developer as
one of our unsupported qemu-specific commands.
If you find yourself needing to use this API for more than
just development purposes, please ask on the libvirt list
for a supported counterpart event to be added in libvirt.so.
While the supported virConnectDomainEventRegisterAny() API
takes an id which determines the signature of the callback,
this version takes a string filter and always uses the same
signature. Furthermore, I chose to expose this as a new API
instead of trying to add a new eventID at the top level, in
part because the generic option lacks event name filtering,
and in part because the normal domain event namespace should
not be polluted by a qemu-only event. I also added a flags
argument; unused for now, but we might decide to use it to
allow a user to request event names by glob or regex instead
of literal match.
* include/libvirt/libvirt-qemu.h
(virConnectDomainQemuMonitorEventCallback)
(virConnectDomainQemuMonitorEventRegister)
(virConnectDomainQemuMonitorEventDeregister): New prototypes.
* src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister)
(virConnectDomainQemuMonitorEventDeregister): New functions.
* src/libvirt_qemu.syms (LIBVIRT_QEMU_1.2.1): Export them.
* src/driver.h (virDrvConnectDomainQemuMonitorEventRegister)
(virDrvConnectDomainQemuMonitorEventDeregister): New callbacks.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Before I go and implement the guts of this new API, I first
wanted to get approval from the list that I'm on the right track.
include/libvirt/libvirt-qemu.h | 31 ++-
src/driver.h | 15 +
src/libvirt-qemu.c | 123
+
src/libvirt_qemu.syms | 6 ++
4 files changed, 174 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 3e79a8c..5403093 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -4,7 +4,7 @@
* Description: Provides the interfaces of the libvirt library to handle
* qemu specific methods
*
- * Copyright (C) 2010, 2012 Red Hat, Inc.
+ * Copyright (C) 2010, 2012-2013 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
@@ -54,6 +54,35 @@ typedef enum {
char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
int timeout, unsigned int flags);
+/**
+ * virConnectDomainQemuMonitorEventCallback:
+ * @conn: the connection pointer
+ * @dom: the domain pointer
+ * @event: the name of the event
+ * @details: the JSON details of the event
+ * @opaque: application specified data
+ *
+ * The callback signature to use when registering for a qemu monitor
+ * event with virConnectDomainQemuMonitorEventRegister().
+ */
+typedef void (*virConnectDomainQemuMonitorEventCallback)(virConnectPtr conn,
+ virDomainPtr dom,
+ const char *event,
+ const char *details,
+ void *opaque);
So for instance on this event:
2013-12-19 15:55:05.575+: 18630: debug : qemuMonitorJSONIOProcessLine:172 :
QEMU_MONITOR_RECV_EVENT: mon=0x7f8c80008910 event={timestamp: {seconds:
1387468505, microseconds: 574652}, event: SPICE_INITIALIZED, data:
{server: {auth: none, port: 5900, family: ipv4, host:
127.0.0.1}, client: {port: 39285, family: ipv4, channel-type: 4,
connection-id: 375558326, host: 127.0.0.1, channel-id: 0, tls:
false}}}
the callback will be invoked with:
event=SPICE_INITIALIZED
details={server: {auth: }}?
After all, I don't think we should do anything clever about it. Apps dealing
with monitor/agent code (e.g. command passthru) are dealing with JSON anyway.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virkeycode: Allow ANSI_A
On Thu, Dec 19, 2013 at 04:46:50PM +0100, Michal Privoznik wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1044806 Currently, sending the ANSI_A keycode from os_x codepage doesn't work as it has a special value of 0x0. Our internal code handles that no different to other not defined keycodes. Hence, in order to allow it we must change all the undefined keycodes from 0 to -1 and adapt some code too. # virsh send-key guestname --codeset os_x ANSI_A error: invalid keycode: 'ANSI_A' # virsh send-key guestname --codeset os_x ANSI_B # virsh send-key guestname --codeset os_x ANSI_C Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/util/virkeycode-mapgen.py | 4 ++-- src/util/virkeycode.c | 4 ++-- tools/virsh-domain.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) ACK, that's a fun bug :-) Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH] qemu: new API for tracking arbitrary monitor events
On Thu, Dec 19, 2013 at 04:59:28PM +0100, Michal Privoznik wrote:
On 19.12.2013 16:15, Eric Blake wrote:
Several times in the past, qemu has implemented a new event,
but libvirt has not yet caught up to reporting that event to
the user applications. While it is possible to track libvirt
logs to see that an unknown event was received and ignored,
it would be nicer to copy what 'virsh qemu-monitor-command'
does, and expose this information to the end developer as
one of our unsupported qemu-specific commands.
If you find yourself needing to use this API for more than
just development purposes, please ask on the libvirt list
for a supported counterpart event to be added in libvirt.so.
While the supported virConnectDomainEventRegisterAny() API
takes an id which determines the signature of the callback,
this version takes a string filter and always uses the same
signature. Furthermore, I chose to expose this as a new API
instead of trying to add a new eventID at the top level, in
part because the generic option lacks event name filtering,
and in part because the normal domain event namespace should
not be polluted by a qemu-only event. I also added a flags
argument; unused for now, but we might decide to use it to
allow a user to request event names by glob or regex instead
of literal match.
* include/libvirt/libvirt-qemu.h
(virConnectDomainQemuMonitorEventCallback)
(virConnectDomainQemuMonitorEventRegister)
(virConnectDomainQemuMonitorEventDeregister): New prototypes.
* src/libvirt-qemu.c (virConnectDomainQemuMonitorEventRegister)
(virConnectDomainQemuMonitorEventDeregister): New functions.
* src/libvirt_qemu.syms (LIBVIRT_QEMU_1.2.1): Export them.
* src/driver.h (virDrvConnectDomainQemuMonitorEventRegister)
(virDrvConnectDomainQemuMonitorEventDeregister): New callbacks.
Signed-off-by: Eric Blake ebl...@redhat.com
---
Before I go and implement the guts of this new API, I first
wanted to get approval from the list that I'm on the right track.
include/libvirt/libvirt-qemu.h | 31 ++-
src/driver.h | 15 +
src/libvirt-qemu.c | 123
+
src/libvirt_qemu.syms | 6 ++
4 files changed, 174 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
index 3e79a8c..5403093 100644
--- a/include/libvirt/libvirt-qemu.h
+++ b/include/libvirt/libvirt-qemu.h
@@ -4,7 +4,7 @@
* Description: Provides the interfaces of the libvirt library to handle
* qemu specific methods
*
- * Copyright (C) 2010, 2012 Red Hat, Inc.
+ * Copyright (C) 2010, 2012-2013 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
@@ -54,6 +54,35 @@ typedef enum {
char *virDomainQemuAgentCommand(virDomainPtr domain, const char *cmd,
int timeout, unsigned int flags);
+/**
+ * virConnectDomainQemuMonitorEventCallback:
+ * @conn: the connection pointer
+ * @dom: the domain pointer
+ * @event: the name of the event
+ * @details: the JSON details of the event
+ * @opaque: application specified data
+ *
+ * The callback signature to use when registering for a qemu monitor
+ * event with virConnectDomainQemuMonitorEventRegister().
+ */
+typedef void (*virConnectDomainQemuMonitorEventCallback)(virConnectPtr
conn,
+ virDomainPtr dom,
+ const char *event,
+ const char
*details,
+ void *opaque);
So for instance on this event:
2013-12-19 15:55:05.575+: 18630: debug : qemuMonitorJSONIOProcessLine:172
: QEMU_MONITOR_RECV_EVENT: mon=0x7f8c80008910 event={timestamp: {seconds:
1387468505, microseconds: 574652}, event: SPICE_INITIALIZED, data:
{server: {auth: none, port: 5900, family: ipv4, host:
127.0.0.1}, client: {port: 39285, family: ipv4, channel-type:
4, connection-id: 375558326, host: 127.0.0.1, channel-id: 0, tls:
false}}}
the callback will be invoked with:
event=SPICE_INITIALIZED
details={server: {auth: }}?
After all, I don't think we should do anything clever about it. Apps dealing
with monitor/agent code (e.g. command passthru) are dealing with JSON anyway.
Agreed, just sending out the JSON 'data' block as-is seems fine to me.
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
