Re: [libvirt] PING: [RFC PATCH 0/3] Implement mockup capabilities cache in QEMU tests
On 08/27/2015 02:37 AM, Pavel Fedin wrote: Hello! I remember you were worrying about a temporary hack in qemu's postparse, because test suite could not generate proper capability cache. I promised to solve this, and here is (almost) a solution, i've outlined one small problem to solve together in the cover message. Since then, no response... Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia Sorry about all the delays, I'll be more responsive going forward. For my part I've been offline for 2 weeks on my honeymoon, just came back today... it's been a hectic 2 months for me between moving across the state, planning a wedding, then travelling out of the country for 2 weeks. I'm back now so I'll check the patches when I get caught up on other stuff. Most other people on this list were preparing for and attending KVM forum/Linuxcon the past couple weeks. So it's just bad timing in this case - Cole -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Pavel Fedin Sent: Tuesday, August 18, 2015 12:40 PM To: libvir-list@redhat.com Cc: Martin Kletzander Subject: [libvirt] [RFC PATCH 0/3] Implement mockup capabilities cache in QEMU tests Since commit e8d55172544c1fafe31a9e09346bdebca4f0d6f9 qemu driver checks emulator capabilities during domain XML post-parse. However, test suite does not initialize it, therefore a condition to skip all checks if there is no cache supplied was added. This is actually a hack, whose sole purpose is to make existing test suite working. Additionally, it prevents from writing new tests for this particular functionality. This series attempts to solve this problem by implementing proper cache mockup in test suite. The main idea is to create a cache in standard way and put there a pre-defined capabilities set (which tests already have). The main problem here is to know emulator binary name, which is contained in the source XML. However, we have to create our cache before reading the XML. The simplest way to resolve this is to assume particular binary name from test name. Currently tests which assume cross-architecture binary are all prefixed with the architecture name (with one exception of keywrap tests which all assume /usr/bin/qemu-system-s390x and do not have s390- prefix in their name). This scheme works fine, unless we use native emulator binary. Here we have a mess. Most newer tests use /usr/bin/qemu, however there is a large number of tests which use /usr/libexec/qemu-kvm or /usr/bin/kvm (i guess these are leftovers from the epoch when qemu-kvm was a separate fork of qemu). This is currently not handled in any way, and these tests may report errors due to missing binaries (because virQEMUCapsCacheLookup() attempts to populate the cache automatically by querying the binary if not already known). There are several possible ways to resolve this: a) Add all possible names as aliases for /usr/bin/qemu b) Forbid to use oldstyle names at all in these tests c) Declare some prefix like kvm- for those tests who want to use /usr/libexec/qemu-kvm. Again, this would ban /usr/bin/kvm and /usr/bin/qemu-kvm (if not using aliases like in (b) d) Hardcode (optional) emulator name per test. IMHO a bad idea because number of tests is huge. e) Do some preparsing of the XML and extract binary name from it. Again, i disliked it for not being simple enough. I also thought about an alternate implementation which would patch postParseCallback and insert own function there which builds a cache. At this point binary name is already known from the XML. However, such a design looks like an ugly hack by itself, so i stopped going in this direction. Comments and opinions are welcome. Pavel Fedin (3): Implement virQEMUCapsCache mockup Use mockup cache Removed unneeded check src/qemu/qemu_capabilities.c | 10 +- src/qemu/qemu_capspriv.h | 36 +++ src/qemu/qemu_domain.c | 5 + tests/qemuagenttest.c| 9 - tests/qemuargv2xmltest.c | 5 + tests/qemuhotplugtest.c | 23 ++ tests/qemuxml2argvtest.c | 5 + tests/qemuxml2xmltest.c | 6 ++ tests/qemuxmlnstest.c| 5 + tests/testutilsqemu.c| 45 tests/testutilsqemu.h| 3 +++ 11 files changed, 130 insertions(+), 22 deletions(-) create mode 100644 src/qemu/qemu_capspriv.h mode change 100644 = 100755 tests/qemuagenttest.c mode change 100644 = 100755 tests/qemuhotplugtest.c mode change 100644 = 100755 tests/qemuxml2xmltest.c mode change 100644 = 100755 tests/testutilsqemu.c -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com
Re: [libvirt] PING: [RFC PATCH 0/3] Implement mockup capabilities cache in QEMU tests
Hello! Sorry about all the delays, I'll be more responsive going forward. For my part I've been offline for 2 weeks on my honeymoon, just came back today... Wow, congratulations then! I didn't know, of course, this is very important thing. And of course i know about LinuxCon too, unfortunately i was unable to get a US visa in time, so my attendance failed. :( Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2 (v3)] util: fallback to ioctl(SIOCBRDELBR) if netlink RTM_DELLINK fails
On 08/27/2015 06:07 AM, Martin Kletzander wrote:
On Wed, Aug 26, 2015 at 03:05:25PM -0400, Laine Stump wrote:
commit 09778e09 switched from using ioctl(SIOCBRDELBR) for bridge
device deletion to using a netlink RTM_DELLINK message, which is the
more modern way to delete a bridge (and also doesn't require the
bridge to be ~IFF_UP to succeed). However, although older kernels
(e.g. 2.6.32, in RHEL6/CentOS6) support deleting *some* link types
with RTM_NEWLINK, they don't support deleting bridges, and there is no
compile-time way to figure this out.
This patch moves the body of the SIOCBRDELBR version of
virNetDevBridgeDelete() into a static function, calls the new function
from the original, and also calls the new function from the
RTM_DELLINK version if the RTM_DELLINK message generates an EOPNOTSUPP
error. Since RTM_DELLINK is done from the subordinate function
virNetlinkDelLink, which is also called for other purposes (deleting a
macvtap interface), a function pointer called fallback has been
added to the arglist of virNetlinkDelLink() - if that arg != NULL, the
provided function will be called when (and only when) RTM_DELLINK
fails with EOPNOTSUPP.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1252780 (part 2)
---
Another idea for doing this I came up with during review of the
others...
src/util/virnetdevbridge.c | 41
++---
src/util/virnetdevmacvlan.c | 2 +-
src/util/virnetlink.c | 13 +++--
src/util/virnetlink.h | 5 -
4 files changed, 46 insertions(+), 15 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index ae38901..3b06829 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -558,20 +558,15 @@ int virNetDevBridgeCreate(const char *brname)
*
* Returns 0 in case of success or an errno code in case of failure.
*/
-#if defined(__linux__) defined(HAVE_LIBNL)
-int virNetDevBridgeDelete(const char *brname)
-{
-/* If netlink is available, use it, as it is successful at
- * deleting a bridge even if it is currently IFF_UP.
- */
-return virNetlinkDelLink(brname);
-}
-#elif defined(HAVE_STRUCT_IFREQ) defined(SIOCBRDELBR)
-int virNetDevBridgeDelete(const char *brname)
+#if defined(HAVE_STRUCT_IFREQ) defined(SIOCBRDELBR)
+static int
+virNetDevBridgeDeleteWithIoctl(const char *brname)
{
int fd = -1;
int ret = -1;
+ignore_value(virNetDevSetOnline(brname, false));
+
if ((fd = virNetDevSetupControl(NULL, NULL)) 0)
return -1;
@@ -587,8 +582,32 @@ int virNetDevBridgeDelete(const char *brname)
VIR_FORCE_CLOSE(fd);
return ret;
}
+#endif
+
+
+#if defined(__linux__) defined(HAVE_LIBNL)
+int
+virNetDevBridgeDelete(const char *brname)
+{
+/* If netlink is available, use it, as it is successful at
+ * deleting a bridge even if it is currently IFF_UP. fallback to
+ * using ioctl(SIOCBRDELBR) if netlink fails with EOPNOTSUPP.
+ */
+return virNetlinkDelLink(brname, virNetDevBridgeDeleteWithIoctl);
You're passing DeleteWithIoctl here, but that was defined only if
defined(HAVE_STRUCT_IFREQ) defined(SIOCBRDELBR), does it mean that
if you have libnl on linux, you also have those two? If not, then I
suggest adding check for HAVE_STRUCT_IFREQ SIOCBRDELBR definitions
atop this function definition as well, just to make sure we don't run
into similar problems later on with some weird distribution/custom
builds/versions. No need to resend, this is just a hint before
pushing.
Yeah, thanks for pointing that out. I had that check in v2, but forgot
it here.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 1/3] libxl: fix ref counting of libxlMigrationDstArgs
Michal Privoznik wrote:
On 07.08.2015 19:53, Jim Fehlig wrote:
This patch fixes some flawed logic around ref counting the
libxlMigrationDstArgs object.
First, when adding sockets to the event loop with
virNetSocketAddIOCallback(), the generic virObjectFreeCallback()
was registered as a free function, with libxlMigrationDstArgs as
its parameter. A reference was also taken on
libxlMigrationDstArgs for each successful call to
virNetSocketAddIOCallback(). The rational behind this logic was
that the libxlMigrationDstArgs object had to out-live the socket
objects. But virNetSocketAddIOCallback() already takes a
reference on socket objects, ensuring their life until removed
from the event loop and unref'ed in virNetSocketEventFree(). We
only need to ensure libxlMigrationDstArgs lives until
libxlDoMigrateReceive() finishes, which can be done by simply
unref'ing libxlMigrationDstArgs at the end of
libxlDoMigrateReceive().
The second flaw was unref'ing the sockets in the failure path of
libxlMigrateReceive() and at the end of libxlDoMigrateReceive().
As mentioned above, the sockets are already unref'ed by
virNetSocketEventFree() when removed from the event loop.
Attempting to unref the socket a second time resulted in a
libvirtd crash since the socket was previously unref'ed and
disposed.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
V2: Initialize args in libxlDomainMigrationPrepare
src/libxl/libxl_migration.c | 20 ++--
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index aa9547b..f9673c8 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -112,11 +112,11 @@ libxlDoMigrateReceive(void *opaque)
virNetSocketUpdateIOCallback(socks[i], 0);
This is pre-existing, but since the socket callback is removed right
after, it does not make much sense to update its events to listen for.
Right. Should be removed since I'm touching this code.
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
-virObjectUnref(socks[i]);
This will leak the socks[i] object, wouldn't it?
Yes, you are right. I verified the virNetSocket was not disposed with
the missing unref.
I mean, in
libxlDomainMigrationPrepare() on line 392 virNetSocketNewListenTCP() is
called. This initialize the array with object pointers. Then
virNetSocketAddIOCallback() + virNetSocketRemoveIOCallback() pair keep
the ref counter consistent. This makes me think you should not remove
this line.
socks[i] = NULL;
}
args-nsocks = 0;
VIR_FORCE_CLOSE(recvfd);
+virObjectUnref(args);
}
@@ -164,11 +164,11 @@ libxlMigrateReceive(virNetSocketPtr sock,
virNetSocketUpdateIOCallback(socks[i], 0);
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
-virObjectUnref(socks[i]);
socks[i] = NULL;
}
args-nsocks = 0;
VIR_FORCE_CLOSE(recvfd);
+virObjectUnref(args);
}
static int
@@ -318,7 +318,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
virNetSocketPtr *socks = NULL;
size_t nsocks = 0;
int nsocks_listen = 0;
-libxlMigrationDstArgs *args;
+libxlMigrationDstArgs *args = NULL;
size_t i;
int ret = -1;
@@ -420,22 +420,12 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
VIR_EVENT_HANDLE_READABLE,
libxlMigrateReceive,
args,
- virObjectFreeCallback) 0)
+ NULL) 0)
continue;
-/*
- * Successfully added sock to event loop. Take a ref on args to
- * ensure it is not freed until sock is removed from the event loop.
- * Ref is dropped in virObjectFreeCallback after being removed
- * from the event loop.
- */
-virObjectRef(args);
nsocks_listen++;
}
-/* Done with args in this function, drop reference */
-virObjectUnref(args);
-
if (!nsocks_listen)
goto error;
@@ -448,6 +438,8 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
virObjectUnref(socks[i]);
}
VIR_FREE(socks);
+virObjectUnref(args);
+
/* Remove virDomainObj from domain list */
if (vm) {
virDomainObjListRemove(driver-domains, vm);
Otherwise looking good.
How does it look with the following squashed in?
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 8db3aea..9609e06 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -112,9 +112,9 @@ libxlDoMigrateReceive(void *opaque)
/* Remove all listen socks from event handler, and close them. */
for (i = 0; i nsocks;
Re: [libvirt] [PATCH] Revert LXC: show used memory as 0 when domain is not active
Daniel P. Berrange wrote: On Tue, Aug 11, 2015 at 10:25:19AM +0200, Peter Krempa wrote: On Tue, Aug 11, 2015 at 09:18:23 +0100, Daniel Berrange wrote: On Mon, Aug 10, 2015 at 12:49:55PM -0600, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. Conflicts: src/qemu/qemu_driver.c Signed-off-by: Jim Fehlig jfeh...@suse.com --- src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) ACK I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info-memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too. Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches? Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Revert LXC: show used memory as 0 when domain is not active
On 08/27/2015 04:38 PM, Jim Fehlig wrote: This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e, which introduced a significant semantic change to the virDomainGetInfo() API. Additionally, the change was only made to 2 of the 15 virt drivers. I guess we should probably do this in the stable branch too, since I think it made it into the most recent release. Erm, the change is out for a while now: git desc 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e v1.2.6-225-g1ce7c1d Oh yes, so it is. I'm still inclined to say we should be reverting it as I think it is wrong. The change was based on the misleading field name shown by virsh. The info-memory field shows the current balloon target, and conceptually this should be equal to the max memory if the ballooon driver is not active. As such I think it should be equal to max memory if shutoff too. Yikes, I forgot to commit this before leaving for travel/vacation. Is it still ok to commit to master? And just to be clear, then cherry pick to 1.2.7-1.2.18 maint branches? Yes, that sounds like the best way to handle it. -- 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] Entering freeze for libvirt-1.2.19
On 27.08.2015 11:41, Martin Kletzander wrote:
On Thu, Aug 27, 2015 at 09:52:53AM +0800, Daniel Veillard wrote:
So I have tagged rc1 in git and pushed signed tarball and rpms to
the usual place:
ftp://libvirt.org/libvirt/
The release works just fine in my limited testing, but others should
try it out to check portability and more advanced functionalities.
I will likely push rc2 at the end of the week-end and the final release
on Tuesday or Wednesday, assuming no big issue is raised in the meantime.
thanks for giving it a try.
I also note that we have some CI test failing, not for core libvirt
but perl bindings, virt-manager, ... it would be good to investigate
before
we push the release:
https://ci.centos.org/view/libvirt-project/
Having a quick look it seems there's still the rename API missing in
libvirt-python and that's why all the stuff on top of python is
failing. I'll see what I can do, even though I'm out till end of the
week.
I don't know what you mean. I can see this in build/libvirt.py after I build
libvirt-python:
class virDomain(object):
def __init__(self, conn, _obj=None):
self._conn = conn
self._o = _obj
...
def rename(self, new_name, flags=0):
Rename a domain. New domain name is specified in the second
argument. Depending on each driver implementation it may be
required that domain is in a specific state.
There might be some attributes and/or elements in domain XML that if no
value provided at XML defining time, libvirt will derive their value
from
the domain name. These are not updated by this API. Users are strongly
advised to change these after the rename was successful.
ret = libvirtmod.virDomainRename(self._o, new_name, flags)
if ret == -1: raise libvirtError ('virDomainRename() failed', dom=self)
return ret
What's failing anyway?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] util: fallback to ioctl(SIOCBRADDBR) if netlink RTM_NEWLINK fails
On 26.08.2015 05:47, Laine Stump wrote: commit fc7b23db switched from using ioctl(SIOCBRADDBR) to using a netlink RTM_NEWLINK message with an IFLA_INFO_KIND of bridge, which is the more modern way to create a bridge. However, although older kernels (e.g. 2.6.32, in RHEL6/CentOS6) support creating *some* link types with RTM_NEWLINK, they don't support creating bridges, and there is no compile-time way to figure this out (since the type isn't an enum, but rather a character string). This patch moves the body of the SIOCBRADDBR version of virNetDevBridgeCreate() into a static function, calls the new function from the original, and also calls the new function from the RTM_NEWLINK version if the RTM_NEWLINK message generates an EOPNOTSUPP error. This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1252780 --- src/util/virnetdevbridge.c | 64 -- 1 file changed, 45 insertions(+), 19 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] migration: remove direct migration dependency on version1 of driver
On 27.08.2015 12:23, Nikolay Shirokovskiy wrote:
From: Nikolay Shirokovskiy Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
Direct migration should work if *perform3 is present but *perform
is not. This is situation when driver migration is implemented
after new version of driver function is introduced. We should not
be forced to support old version too as its parameter space is
subspace of newer one.
Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
---
src/libvirt-domain.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 6ab50ba..c89775b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3427,7 +3427,8 @@ virDomainMigrateDirect(virDomainPtr domain,
NULLSTR(xmlin), flags, NULLSTR(dname),
NULLSTR(dconnuri),
NULLSTR(miguri), bandwidth);
-if (!domain-conn-driver-domainMigratePerform) {
+if (!domain-conn-driver-domainMigratePerform
+!domain-conn-driver-domainMigratePerform3) {
virReportUnsupportedError();
return -1;
}
Hm.. domainMigratePerform3 will be used iff connection driver has
VIR_DRV_FEATURE_MIGRATION_V3 feature. But this check will require that
regardless. What if we check the presence of implementation with respect
to that?
Moreover, can you please send patches rebased to current HEAD?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Please give me a wiki account
I saw a page I'd like to improve a bit. wiki username IanKelling. - Ian Kelling -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Entering freeze for libvirt-1.2.19
On Thu, Aug 27, 2015 at 09:52:53AM +0800, Daniel Veillard wrote: So I have tagged rc1 in git and pushed signed tarball and rpms to the usual place: ftp://libvirt.org/libvirt/ The release works just fine in my limited testing, but others should try it out to check portability and more advanced functionalities. I will likely push rc2 at the end of the week-end and the final release on Tuesday or Wednesday, assuming no big issue is raised in the meantime. thanks for giving it a try. I also note that we have some CI test failing, not for core libvirt but perl bindings, virt-manager, ... it would be good to investigate before we push the release: https://ci.centos.org/view/libvirt-project/ Having a quick look it seems there's still the rename API missing in libvirt-python and that's why all the stuff on top of python is failing. I'll see what I can do, even though I'm out till end of the week. thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virt-aa-helper: Improve valid_path
On Thu, Aug 27, 2015 at 03:04:12AM +0200, Michal Privoznik wrote:
So, after some movement in virt-aa-helper, I've noticed the
virt-aa-helper-test failing. I've ran gdb (it took me a while to
realize how to do that) and this showed up immediately:
What was the hard part of running gdb? Making the virt-aa-helper run
under it during test suite or anything else?
Program received signal SIGSEGV, Segmentation fault.
strlen () at ../sysdeps/x86_64/strlen.S:106
106 ../sysdeps/x86_64/strlen.S: No such file or directory.
(gdb) bt
#0 strlen () at ../sysdeps/x86_64/strlen.S:106
#1 0x55561a13 in array_starts_with (str=0x557ce910
/tmp/tmp.6nI2Fkv0KL/1.img, arr=0x7fffd160, size=-1540438016) at
security/virt-aa-helper.c:525
#2 0x55561d49 in valid_path (path=0x557ce910
/tmp/tmp.6nI2Fkv0KL/1.img, readonly=false) at security/virt-aa-helper.c:617
#3 0x55562506 in vah_add_path (buf=0x7fffd3e0, path=0x557cb910
/tmp/tmp.6nI2Fkv0KL/1.img, perms=0x55581585 rw, recursive=false) at
security/virt-aa-helper.c:823
#4 0x55562693 in vah_add_file (buf=0x7fffd3e0, path=0x557cb910
/tmp/tmp.6nI2Fkv0KL/1.img, perms=0x55581585 rw) at
security/virt-aa-helper.c:854
#5 0x55562918 in add_file_path (disk=0x557d4440, path=0x557cb910
/tmp/tmp.6nI2Fkv0KL/1.img, depth=0, opaque=0x7fffd3e0) at
security/virt-aa-helper.c:931
#6 0x778f18b1 in virDomainDiskDefForeachPath (disk=0x557d4440,
ignoreOpenFailure=true, iter=0x555628a6 add_file_path,
opaque=0x7fffd3e0) at conf/domain_conf.c:23286
#7 0x55562b5f in get_files (ctl=0x7fffd670) at
security/virt-aa-helper.c:982
#8 0x55564100 in vahParseArgv (ctl=0x7fffd670, argc=5,
argv=0x7fffd7e8) at security/virt-aa-helper.c:1277
#9 0x555643d6 in main (argc=5, argv=0x7fffd7e8) at
security/virt-aa-helper.c:1332
So I've taken look at valid_path() because it is obviously
calling array_starts_with() with malformed @size. And here's the
result: there are two variables to hold the size of three arrays
and their value is recalculated before each call of
array_starts_with(). What if we just use three variables,
initialize them and do not touch them afterwards?
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/security/virt-aa-helper.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index a78c4c8..b4a8f27 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -546,9 +546,6 @@ array_starts_with(const char *str, const char * const *arr,
const long size)
static int
valid_path(const char *path, const bool readonly)
{
-int npaths;
-int nropaths;
-
const char * const restricted[] = {
/bin/,
/etc/,
@@ -581,6 +578,10 @@ valid_path(const char *path, const bool readonly)
/etc/libvirt-sandbox/services/ /* for virt-sandbox service config */
};
+const int nropaths = ARRAY_CARDINALITY(restricted);
+const int nrwpaths = ARRAY_CARDINALITY(restricted_rw);
+const int nopaths = ARRAY_CARDINALITY(override);
+
if (path == NULL) {
vah_error(NULL, 0, _(bad pathname));
return -1;
@@ -600,21 +601,18 @@ valid_path(const char *path, const bool readonly)
vah_warning(_(path does not exist, skipping file type checks));
/* overrides are always allowed */
-npaths = sizeof(override)/sizeof(*(override));
-if (array_starts_with(path, override, npaths) == 0)
+if (array_starts_with(path, override, nopaths) == 0)
return 0;
/* allow read only paths upfront */
if (readonly) {
-nropaths = sizeof(restricted_rw)/sizeof(*(restricted_rw));
-if (array_starts_with(path, restricted_rw, nropaths) == 0)
+if (array_starts_with(path, restricted_rw, nrwpaths) == 0)
So if it wasn't readonly, the nropaths was not calculated, ...
return 0;
}
/* disallow RW acess to all paths in restricted and restriced_rw */
-npaths = sizeof(restricted)/sizeof(*(restricted));
-if ((array_starts_with(path, restricted, npaths) == 0
-|| array_starts_with(path, restricted_rw, nropaths) == 0))
...but it was used here. I remember that when reviewing I spotted
something like that, but when writing the review I missed it, so I
thought the first look fooled me. Well, it wasn't the case and this
is of course cleaner.
ACK and safe for freeze.
+if ((array_starts_with(path, restricted, nropaths) == 0 ||
+ array_starts_with(path, restricted_rw, nrwpaths) == 0))
return 1;
return 0;
--
2.4.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2 (v3)] util: fallback to ioctl(SIOCBRDELBR) if netlink RTM_DELLINK fails
On Wed, Aug 26, 2015 at 03:05:25PM -0400, Laine Stump wrote:
commit 09778e09 switched from using ioctl(SIOCBRDELBR) for bridge
device deletion to using a netlink RTM_DELLINK message, which is the
more modern way to delete a bridge (and also doesn't require the
bridge to be ~IFF_UP to succeed). However, although older kernels
(e.g. 2.6.32, in RHEL6/CentOS6) support deleting *some* link types
with RTM_NEWLINK, they don't support deleting bridges, and there is no
compile-time way to figure this out.
This patch moves the body of the SIOCBRDELBR version of
virNetDevBridgeDelete() into a static function, calls the new function
from the original, and also calls the new function from the
RTM_DELLINK version if the RTM_DELLINK message generates an EOPNOTSUPP
error. Since RTM_DELLINK is done from the subordinate function
virNetlinkDelLink, which is also called for other purposes (deleting a
macvtap interface), a function pointer called fallback has been
added to the arglist of virNetlinkDelLink() - if that arg != NULL, the
provided function will be called when (and only when) RTM_DELLINK
fails with EOPNOTSUPP.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1252780 (part 2)
---
Another idea for doing this I came up with during review of the others...
src/util/virnetdevbridge.c | 41 ++---
src/util/virnetdevmacvlan.c | 2 +-
src/util/virnetlink.c | 13 +++--
src/util/virnetlink.h | 5 -
4 files changed, 46 insertions(+), 15 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index ae38901..3b06829 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -558,20 +558,15 @@ int virNetDevBridgeCreate(const char *brname)
*
* Returns 0 in case of success or an errno code in case of failure.
*/
-#if defined(__linux__) defined(HAVE_LIBNL)
-int virNetDevBridgeDelete(const char *brname)
-{
-/* If netlink is available, use it, as it is successful at
- * deleting a bridge even if it is currently IFF_UP.
- */
-return virNetlinkDelLink(brname);
-}
-#elif defined(HAVE_STRUCT_IFREQ) defined(SIOCBRDELBR)
-int virNetDevBridgeDelete(const char *brname)
+#if defined(HAVE_STRUCT_IFREQ) defined(SIOCBRDELBR)
+static int
+virNetDevBridgeDeleteWithIoctl(const char *brname)
{
int fd = -1;
int ret = -1;
+ignore_value(virNetDevSetOnline(brname, false));
+
if ((fd = virNetDevSetupControl(NULL, NULL)) 0)
return -1;
@@ -587,8 +582,32 @@ int virNetDevBridgeDelete(const char *brname)
VIR_FORCE_CLOSE(fd);
return ret;
}
+#endif
+
+
+#if defined(__linux__) defined(HAVE_LIBNL)
+int
+virNetDevBridgeDelete(const char *brname)
+{
+/* If netlink is available, use it, as it is successful at
+ * deleting a bridge even if it is currently IFF_UP. fallback to
+ * using ioctl(SIOCBRDELBR) if netlink fails with EOPNOTSUPP.
+ */
+return virNetlinkDelLink(brname, virNetDevBridgeDeleteWithIoctl);
You're passing DeleteWithIoctl here, but that was defined only if
defined(HAVE_STRUCT_IFREQ) defined(SIOCBRDELBR), does it mean that
if you have libnl on linux, you also have those two? If not, then I
suggest adding check for HAVE_STRUCT_IFREQ SIOCBRDELBR definitions
atop this function definition as well, just to make sure we don't run
into similar problems later on with some weird distribution/custom
builds/versions. No need to resend, this is just a hint before
pushing.
Martin
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] libvirt_lxc: Claim success for --help
On Thu, Aug 27, 2015 at 03:06:52AM +0200, Michal Privoznik wrote: So far, if libvirt_lxc binary (usually to be found under /usr/libexec/) is run with --help, due to a missing line and our usual functions pattern, an 'uknown' error is returned. Yeah, the help is printed out, but we should not claim error. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/lxc/lxc_controller.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index a94e819..d36bc9b 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2613,6 +2613,7 @@ int main(int argc, char *argv[]) fprintf(stderr, -U FD, --share-uts FD\n); fprintf(stderr, -h, --help\n); fprintf(stderr, \n); +rc = 0; goto cleanup; } } ACK 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
Re: [libvirt] [PATCH 3/4] lxc_container: Turn lxcAttachNS into calling virProcessSetNamespaces
On Thu, Aug 27, 2015 at 03:06:53AM +0200, Michal Privoznik wrote:
Now that virProcessSetNamespaces() does accept FD list in the
correct format, we can simply turn lxcAttachNS into calling
virProcessSetNamespaces().
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/lxc/lxc_container.c | 22 +++---
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index feb8fad..eb7cad6 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2184,25 +2184,9 @@ static int
lxcContainerDropCapabilities(virDomainDefPtr def ATTRIBUTE_UNUSED,
*/
static int lxcAttachNS(int *ns_fd)
{
-size_t i;
-if (ns_fd)
-for (i = 0; i VIR_LXC_DOMAIN_NAMESPACE_LAST; i++) {
-if (ns_fd[i] 0)
-continue;
-VIR_DEBUG(Setting into namespace\n);
-/* We get EINVAL if new NS is same as the current
- * NS, or if the fd namespace doesn't match the
- * type passed to setns()'s second param. Since we
- * pass 0, we know the EINVAL is harmless
- */
-if (setns(ns_fd[i], 0) 0
-errno != EINVAL) {
-virReportSystemError(errno, _(failed to set namespace
'%s'),
- virLXCDomainNamespaceTypeToString(i));
-return -1;
-}
-VIR_FORCE_CLOSE(ns_fd[i]);
-}
+if (ns_fd
+virProcessSetNamespaces(VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd) 0)
+return -1;
return 0;
}
ACK, or we could just inline this call and get rid fo the
lxcAttachNS method entirely
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
Re: [libvirt] [PATCH 0/4] Couple of LXC fixes
On Thu, Aug 27, 2015 at 03:06:50AM +0200, Michal Privoznik wrote: After Dan's eff95ac8fce in which he's fixing the build on RHEL-6, I've realized we can do better. We don't need to copy our code around. Yep, nicely done. Safe for freeze too IMHO Michal Privoznik (4): util: Allow virProcessSetNamespaces() to have sparse FD list libvirt_lxc: Claim success for --help lxc_container: Turn lxcAttachNS into calling virProcessSetNamespaces Revert lxc: ensure setns() syscall is defined src/lxc/lxc_container.c | 56 +++- src/lxc/lxc_controller.c | 1 + src/util/virprocess.c| 3 +++ 3 files changed, 7 insertions(+), 53 deletions(-) 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
Re: [libvirt] [PATCH 4/4] Revert lxc: ensure setns() syscall is defined
On Thu, Aug 27, 2015 at 03:06:54AM +0200, Michal Privoznik wrote: After my previous commit this commit is no longer needed. This reverts commit eff95ac8fce8af47c0948a1c8a654b210633a350. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/lxc/lxc_container.c | 34 -- 1 file changed, 34 deletions(-) ACK 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
Re: [libvirt] [PATCH 1/4] util: Allow virProcessSetNamespaces() to have sparse FD list
On Thu, Aug 27, 2015 at 03:06:51AM +0200, Michal Privoznik wrote:
So far, the virProcessSetNamespaces() takes an array of FDs that
it tries to set namespace on. However, in the very next commit
this array may be sparse, having some -1's in it. Teach the
function to cope with that.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/util/virprocess.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 77a038a..e6b78ef 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -705,6 +705,9 @@ int virProcessSetNamespaces(size_t nfdlist,
return -1;
}
for (i = 0; i nfdlist; i++) {
+if (fdlist[i] 0)
+continue;
+
/* We get EINVAL if new NS is same as the current
* NS, or if the fd namespace doesn't match the
* type passed to setns()'s second param. Since we
ACK
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
Re: [libvirt] [PATCH v3 2/5] vz: add migration backbone code
On Tue, Aug 25, 2015 at 12:04:14PM +0300, nshirokovs...@virtuozzo.com wrote:
From: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
This patch makes basic vz migration possible. For example by virsh:
virsh -c vz:///system migrate --direct $NAME $STUB vz+ssh://$DST/system
$STUB could be anything as it is required virsh argument but it is not
used in direct migration.
Vz migration is implemented as direct migration. The reason is that vz sdk do
all the job. Prepare phase function is used to pass session uuid from
destination to source so we don't introduce new rpc call.
Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
---
src/libvirt-domain.c |3 +-
src/vz/vz_driver.c | 193
++
src/vz/vz_sdk.c | 33 +
src/vz/vz_sdk.h |2 +
4 files changed, 230 insertions(+), 1 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index cbf08fc..8577edd 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3425,7 +3425,8 @@ virDomainMigrateDirect(virDomainPtr domain,
NULLSTR(xmlin), flags, NULLSTR(dname), NULLSTR(uri),
bandwidth);
-if (!domain-conn-driver-domainMigratePerform) {
+if (!domain-conn-driver-domainMigratePerform
+!domain-conn-driver-domainMigratePerform3) {
virReportUnsupportedError();
return -1;
}
Can you send just this bit of code as a separate patch, as it is a
trivial unrelated fix we can merge now.
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
Re: [libvirt] [PATCH 1/2] util: fallback to ioctl(SIOCBRADDBR) if netlink RTM_NEWLINK fails
On Tue, Aug 25, 2015 at 11:47:54PM -0400, Laine Stump wrote:
commit fc7b23db switched from using ioctl(SIOCBRADDBR) to using a
netlink RTM_NEWLINK message with an IFLA_INFO_KIND of bridge, which
is the more modern way to create a bridge. However, although older
kernels (e.g. 2.6.32, in RHEL6/CentOS6) support creating *some* link
types with RTM_NEWLINK, they don't support creating bridges, and there
is no compile-time way to figure this out (since the type isn't an
enum, but rather a character string).
This patch moves the body of the SIOCBRADDBR version of
virNetDevBridgeCreate() into a static function, calls the new function
from the original, and also calls the new function from the
RTM_NEWLINK version if the RTM_NEWLINK message generates an EOPNOTSUPP
error.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1252780
---
src/util/virnetdevbridge.c | 64 --
1 file changed, 45 insertions(+), 19 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index aa255d6..ae38901 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -394,8 +394,33 @@ virNetDevBridgePortSetUnicastFlood(const char *brname
ATTRIBUTE_UNUSED,
*
* Returns 0 in case of success or -1 on failure
*/
+#if defined(HAVE_STRUCT_IFREQ) defined(SIOCBRADDBR)
+static int
+virNetDevBridgeCreateWithIoctl(const char *brname)
+{
+int fd = -1;
+int ret = -1;
+
+if ((fd = virNetDevSetupControl(NULL, NULL)) 0)
+return -1;
+
+if (ioctl(fd, SIOCBRADDBR, brname) 0) {
+virReportSystemError(errno,
+ _(Unable to create bridge %s), brname);
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+VIR_FORCE_CLOSE(fd);
+return ret;
+}
+#endif
+
#if defined(__linux__) defined(HAVE_LIBNL)
-int virNetDevBridgeCreate(const char *brname)
+int
+virNetDevBridgeCreate(const char *brname)
{
/* use a netlink RTM_NEWLINK message to create the bridge */
const char *type = bridge;
@@ -441,6 +466,17 @@ int virNetDevBridgeCreate(const char *brname)
switch (err-error) {
case 0:
break;
+case -EOPNOTSUPP:
+# if defined(HAVE_STRUCT_IFREQ) defined(SIOCBRADDBR)
The problem mentioned in te 2/2 (v3) doesn't exist here, so I think
the second patch should really be changed. Anyway, would it make
sense to also allow fallback to the version with SIOCIFCREATE2?
ACK if the answer to the question is No.
Martin
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PING: [RFC PATCH 0/3] Implement mockup capabilities cache in QEMU tests
Hello! I remember you were worrying about a temporary hack in qemu's postparse, because test suite could not generate proper capability cache. I promised to solve this, and here is (almost) a solution, i've outlined one small problem to solve together in the cover message. Since then, no response... Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia -Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Pavel Fedin Sent: Tuesday, August 18, 2015 12:40 PM To: libvir-list@redhat.com Cc: Martin Kletzander Subject: [libvirt] [RFC PATCH 0/3] Implement mockup capabilities cache in QEMU tests Since commit e8d55172544c1fafe31a9e09346bdebca4f0d6f9 qemu driver checks emulator capabilities during domain XML post-parse. However, test suite does not initialize it, therefore a condition to skip all checks if there is no cache supplied was added. This is actually a hack, whose sole purpose is to make existing test suite working. Additionally, it prevents from writing new tests for this particular functionality. This series attempts to solve this problem by implementing proper cache mockup in test suite. The main idea is to create a cache in standard way and put there a pre-defined capabilities set (which tests already have). The main problem here is to know emulator binary name, which is contained in the source XML. However, we have to create our cache before reading the XML. The simplest way to resolve this is to assume particular binary name from test name. Currently tests which assume cross-architecture binary are all prefixed with the architecture name (with one exception of keywrap tests which all assume /usr/bin/qemu-system-s390x and do not have s390- prefix in their name). This scheme works fine, unless we use native emulator binary. Here we have a mess. Most newer tests use /usr/bin/qemu, however there is a large number of tests which use /usr/libexec/qemu-kvm or /usr/bin/kvm (i guess these are leftovers from the epoch when qemu-kvm was a separate fork of qemu). This is currently not handled in any way, and these tests may report errors due to missing binaries (because virQEMUCapsCacheLookup() attempts to populate the cache automatically by querying the binary if not already known). There are several possible ways to resolve this: a) Add all possible names as aliases for /usr/bin/qemu b) Forbid to use oldstyle names at all in these tests c) Declare some prefix like kvm- for those tests who want to use /usr/libexec/qemu-kvm. Again, this would ban /usr/bin/kvm and /usr/bin/qemu-kvm (if not using aliases like in (b) d) Hardcode (optional) emulator name per test. IMHO a bad idea because number of tests is huge. e) Do some preparsing of the XML and extract binary name from it. Again, i disliked it for not being simple enough. I also thought about an alternate implementation which would patch postParseCallback and insert own function there which builds a cache. At this point binary name is already known from the XML. However, such a design looks like an ugly hack by itself, so i stopped going in this direction. Comments and opinions are welcome. Pavel Fedin (3): Implement virQEMUCapsCache mockup Use mockup cache Removed unneeded check src/qemu/qemu_capabilities.c | 10 +- src/qemu/qemu_capspriv.h | 36 +++ src/qemu/qemu_domain.c | 5 + tests/qemuagenttest.c| 9 - tests/qemuargv2xmltest.c | 5 + tests/qemuhotplugtest.c | 23 ++ tests/qemuxml2argvtest.c | 5 + tests/qemuxml2xmltest.c | 6 ++ tests/qemuxmlnstest.c| 5 + tests/testutilsqemu.c| 45 tests/testutilsqemu.h| 3 +++ 11 files changed, 130 insertions(+), 22 deletions(-) create mode 100644 src/qemu/qemu_capspriv.h mode change 100644 = 100755 tests/qemuagenttest.c mode change 100644 = 100755 tests/qemuhotplugtest.c mode change 100644 = 100755 tests/qemuxml2xmltest.c mode change 100644 = 100755 tests/testutilsqemu.c -- 2.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] migration: remove direct migration dependency on version1 of driver
From: Nikolay Shirokovskiy Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
Direct migration should work if *perform3 is present but *perform
is not. This is situation when driver migration is implemented
after new version of driver function is introduced. We should not
be forced to support old version too as its parameter space is
subspace of newer one.
Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
---
src/libvirt-domain.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 6ab50ba..c89775b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -3427,7 +3427,8 @@ virDomainMigrateDirect(virDomainPtr domain,
NULLSTR(xmlin), flags, NULLSTR(dname), NULLSTR(dconnuri),
NULLSTR(miguri), bandwidth);
-if (!domain-conn-driver-domainMigratePerform) {
+if (!domain-conn-driver-domainMigratePerform
+!domain-conn-driver-domainMigratePerform3) {
virReportUnsupportedError();
return -1;
}
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/5] vz: add migration backbone code
On Tue, Aug 25, 2015 at 12:04:14PM +0300, nshirokovs...@virtuozzo.com wrote:
From: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
This patch makes basic vz migration possible. For example by virsh:
virsh -c vz:///system migrate --direct $NAME $STUB vz+ssh://$DST/system
$STUB could be anything as it is required virsh argument but it is not
used in direct migration.
Vz migration is implemented as direct migration. The reason is that vz sdk do
all the job. Prepare phase function is used to pass session uuid from
destination to source so we don't introduce new rpc call.
Looking more closely at migration again, the scenario you have is pretty
much identical to the Xen scenario, in that the hypervisor actually
manages the migration, but you still need a connection to dest libvirtd
to fetch some initialization data.
You have claimed you are implementing, what we describe as direct, unmanaged
migration on this page:
http://libvirt.org/migration.html
But based on the fact that you need to talk to dest libvirtd, you should
in fact implement 'direct, managed' migration - this name is slightly
misleading as the VZ SDK is still actually managing it.
Since you don't need to have the begin/confirm phases, you also don't
need to implement the V3 migration protocol - it is sufficient to just
use V1.
This doesn't need many changes in your patch fortunately.
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 8fa7957..f82fff8 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1343,6 +1343,196 @@ vzDomainMemoryStats(virDomainPtr domain,
return ret;
}
+static char*
+vzFormatCookie(const unsigned char *session_uuid)
+{
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAddLit(buf, vz-migration1\n);
+virUUIDFormat(session_uuid, uuidstr);
+virBufferAsprintf(buf, session_uuid%s/session_uuid\n, uuidstr);
+virBufferAddLit(buf, /vz-migration1\n);
+
+if (virBufferCheckError(buf) 0)
+return NULL;
+
+return virBufferContentAndReset(buf);
+}
+
+static int
+vzParseCookie(const char *xml, unsigned char *session_uuid)
+{
+xmlDocPtr doc = NULL;
+xmlXPathContextPtr ctx = NULL;
+char *tmp = NULL;
+int ret = -1;
+
+if (!(doc = virXMLParseStringCtxt(xml, _((_migration_cookie)), ctx)))
+goto cleanup;
+
+if (!(tmp = virXPathString(string(./session_uuid[1]), ctx))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ %s, _(missing session_uuid element in migration
data));
+goto cleanup;
+}
+if (virUUIDParse(tmp, session_uuid) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ %s, _(malformed session_uuid element in migration
data));
+goto cleanup;
+}
+ret = 0;
+
+ cleanup:
+xmlXPathFreeContext(ctx);
+xmlFreeDoc(doc);
+VIR_FREE(tmp);
+
+return ret;
+}
+
+static int
+vzDomainMigratePrepare3(virConnectPtr conn,
+const char *cookiein ATTRIBUTE_UNUSED,
+int cookieinlen ATTRIBUTE_UNUSED,
+char **cookieout,
+int *cookieoutlen,
+const char *uri_in ATTRIBUTE_UNUSED,
+char **uri_out ATTRIBUTE_UNUSED,
+unsigned long flags,
+const char *dname ATTRIBUTE_UNUSED,
+unsigned long resource ATTRIBUTE_UNUSED,
+const char *dom_xml ATTRIBUTE_UNUSED)
Switch to implement domainMigratePrepare, instead of the v3
method, since you don't need anything except the cookieout
parameters
+{
+vzConnPtr privconn = conn-privateData;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(*cookieout = vzFormatCookie(privconn-session_uuid)))
+goto cleanup;
+*cookieoutlen = strlen(*cookieout) + 1;
+ret = 0;
Also fill in the uri_out parameter, so that the user does
not need to provide a URI explicitly.
+
+ cleanup:
+if (ret != 0) {
+VIR_FREE(*cookieout);
+*cookieoutlen = 0;
+}
+
+return ret;
+}
+
+static int
+vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature)
+{
+switch (feature) {
+case VIR_DRV_FEATURE_MIGRATION_V3:
+case VIR_DRV_FEATURE_MIGRATION_DIRECT:
+return 1;
+default:
+return 0;
+}
+}
You can drop this since you don't need V3 or DIRECT features.
+static virURIPtr
+vzMakeVzUri(const char *connuri_str)
+{
+virURIPtr connuri = NULL;
+virURIPtr vzuri = NULL;
+int ret = -1;
+
+if (!(connuri = virURIParse(connuri_str)))
+goto cleanup;
+
+if (VIR_ALLOC(vzuri) 0)
+goto cleanup;
+memset(vzuri, 0, sizeof(*vzuri));
+
+if (VIR_STRDUP(vzuri-server, connuri-server) 0)
+goto cleanup;
+vzuri-port =
Re: [libvirt] [PATCH v3] lxc: Inherit namespace feature
On 08/14/2015 08:09 AM, Daniel P. Berrange wrote:
From: Imran Khan ik.n...@gmail.com
This patch adds feature for lxc containers to inherit namespaces.
This is very similar to what lxc-tools or docker provides. Look
for man lxc-start and you will find that you can pass command
args as [ --share-[net|ipc|uts] name|pid ]. Or check out docker
networking option in which you can give --net=container:NAME_or_ID
as an option for sharing +namespace.
From this patch you can add extra libvirt option to share
namespace in following way.
lxc:namespace
lxc:sharenet type='netns' value='red'/
lxc:shareipc type='pid' value='12345'/
lxc:shareuts type='name' value='container1'/
/lxc:namespace
The netns option is specific to sharenet. It can be used to
inherit from existing network namespace.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
docs/drvlxc.html.in | 21 ++
docs/schemas/domaincommon.rng | 42
po/POTFILES.in| 1 +
src/Makefile.am | 6 +-
src/lxc/lxc_conf.c| 2 +-
src/lxc/lxc_container.c | 71 ++--
src/lxc/lxc_container.h | 2 +
src/lxc/lxc_controller.c | 45 -
src/lxc/lxc_domain.c | 149
++
src/lxc/lxc_domain.h | 26
src/lxc/lxc_process.c | 149
++
tests/lxcxml2xmltest.c| 1 +
12 files changed, 506 insertions(+), 9 deletions(-)
...
Coverity found a resource leak...
@@ -2342,6 +2378,7 @@ int lxcContainerStart(virDomainDefPtr def,
int *passFDs,
int control,
int handshakefd,
+ int *nsInheritFDs,
size_t nttyPaths,
char **ttyPaths)
{
@@ -2359,7 +2396,8 @@ int lxcContainerStart(virDomainDefPtr def,
.monitor = control,
.nttyPaths = nttyPaths,
.ttyPaths = ttyPaths,
-.handshakefd = handshakefd
+.handshakefd = handshakefd,
+.nsInheritFDs = nsInheritFDs,
};
/* allocate a stack for the container */
@@ -2368,7 +2406,7 @@ int lxcContainerStart(virDomainDefPtr def,
stacktop = stack + stacksize;
-cflags = CLONE_NEWPID|CLONE_NEWNS|CLONE_NEWUTS|CLONE_NEWIPC|SIGCHLD;
+cflags = CLONE_NEWPID|CLONE_NEWNS|SIGCHLD;
if (userns_required(def)) {
if (userns_supported()) {
@@ -2381,10 +2419,31 @@ int lxcContainerStart(virDomainDefPtr def,
return -1;
}
}
+if (nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_SHARENET] == -1) {
+if (lxcNeedNetworkNamespace(def)) {
+VIR_DEBUG(Enable network namespaces);
+cflags |= CLONE_NEWNET;
+}
+} else {
+if (lxcNeedNetworkNamespace(def)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(Config askes for inherit net namespace
+ as well as private network interfaces));
+return -1;
This leaks 'stack'...
Sending a patch shortly.
John
+}
+VIR_DEBUG(Inheriting a net namespace);
+}
-if (lxcNeedNetworkNamespace(def)) {
-VIR_DEBUG(Enable network namespaces);
-cflags |= CLONE_NEWNET;
+if (nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_SHAREIPC] == -1) {
+cflags |= CLONE_NEWIPC;
+} else {
+VIR_DEBUG(Inheriting an IPC namespace);
+}
+
+if (nsInheritFDs[VIR_LXC_DOMAIN_NAMESPACE_SHAREUTS] == -1) {
+cflags |= CLONE_NEWUTS;
+} else {
+VIR_DEBUG(Inheriting a UTS namespace);
}
VIR_DEBUG(Cloning container init process);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] lxc: Resolve Coverity RESOURCE_LEAK
Commit id 'c27553b6e' added a return -1 in a failure path without also VIR_FREE(stack) Signed-off-by: John Ferlan jfer...@redhat.com --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index feb8fad..125e1c8 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2463,6 +2463,7 @@ int lxcContainerStart(virDomainDefPtr def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Config askes for inherit net namespace as well as private network interfaces)); +VIR_FREE(stack); return -1; } VIR_DEBUG(Inheriting a net namespace); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] lxc: Resolve Coverity RESOURCE_LEAK
On Thu, Aug 27, 2015 at 07:08:24AM -0400, John Ferlan wrote: Commit id 'c27553b6e' added a return -1 in a failure path without also VIR_FREE(stack) Signed-off-by: John Ferlan jfer...@redhat.com --- src/lxc/lxc_container.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index feb8fad..125e1c8 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2463,6 +2463,7 @@ int lxcContainerStart(virDomainDefPtr def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, _(Config askes for inherit net namespace as well as private network interfaces)); +VIR_FREE(stack); return -1; } VIR_DEBUG(Inheriting a net namespace); ACK 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] [PATCHv2 1/5] conf: introduce seclabels in shmem device element
Introduce a new element in shmem device element, this
could help users to change the shm label to a specified
label.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
docs/formatdomain.html.in | 7 ++
docs/schemas/domaincommon.rng | 3 +
src/conf/domain_conf.c | 97 +++---
src/conf/domain_conf.h | 5 ++
.../qemuxml2argv-shmem-seclabel.xml| 55
tests/qemuxml2xmltest.c| 4 +
6 files changed, 141 insertions(+), 30 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-seclabel.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 5ca8ede..f2ac5fb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6195,6 +6195,13 @@ qemu-kvm -net nic,model=? /dev/null
vectors. The codeioeventd/code attribute enables/disables (values
on/off, respectively) ioeventfd.
/dd
+dtcodeseclabel/code/dt
+dd
+ The element may contain an optional codeseclabel/code to override the
+ way that labelling is done on the shm object path or shm server path.
If this
+ element is not present, the a href=#seclabelsecurity label is
inherited
+ from the per-domain setting/a.
+/dd
/dl
h4a name=elementsMemoryMemory devices/a/h4
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ccc74cc..f13f566 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3371,6 +3371,9 @@
/optional
/element
/optional
+zeroOrMore
+ ref name='devSeclabel'/
+/zeroOrMore
optional
ref name=address/
/optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c5e9653..ece9f2d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11515,6 +11515,8 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
static virDomainShmemDefPtr
virDomainShmemDefParseXML(xmlNodePtr node,
xmlXPathContextPtr ctxt,
+ virSecurityLabelDefPtr* vmSeclabels,
+ int nvmSeclabels,
unsigned int flags)
{
char *tmp = NULL;
@@ -11586,6 +11588,10 @@ virDomainShmemDefParseXML(xmlNodePtr node,
if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0)
goto cleanup;
+if (virSecurityDeviceLabelDefParseXML(def-seclabels, def-nseclabels,
+ vmSeclabels, nvmSeclabels,
+ ctxt, flags) 0)
+goto cleanup;
ret = def;
def = NULL;
@@ -12708,7 +12714,11 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_SHMEM:
-if (!(dev-data.shmem = virDomainShmemDefParseXML(node, ctxt, flags)))
+if (!(dev-data.shmem = virDomainShmemDefParseXML(node,
+ ctxt,
+ def-seclabels,
+ def-nseclabels,
+ flags)))
goto error;
break;
case VIR_DOMAIN_DEVICE_TPM:
@@ -16383,7 +16393,8 @@ virDomainDefParseXML(xmlDocPtr xml,
for (i = 0; i n; i++) {
virDomainShmemDefPtr shmem;
ctxt-node = nodes[i];
-shmem = virDomainShmemDefParseXML(nodes[i], ctxt, flags);
+shmem = virDomainShmemDefParseXML(nodes[i], ctxt, def-seclabels,
+ def-nseclabels, flags);
if (!shmem)
goto error;
@@ -20594,45 +20605,52 @@ virDomainShmemDefFormat(virBufferPtr buf,
virDomainShmemDefPtr def,
unsigned int flags)
{
-virBufferEscapeString(buf, shmem name='%s', def-name);
+virBuffer childrenBuf = VIR_BUFFER_INITIALIZER;
+int indent = virBufferGetIndent(buf, false);
+size_t n;
-if (!def-size
-!def-server.enabled
-!def-msi.enabled
-!virDomainDeviceInfoNeedsFormat(def-info, flags)) {
-virBufferAddLit(buf, /\n);
-return 0;
-} else {
-virBufferAddLit(buf, \n);
-}
+virBufferEscapeString(buf, shmem name='%s', def-name);
-virBufferAdjustIndent(buf, 2);
+virBufferAdjustIndent(childrenBuf, indent + 2);
if (def-size)
-virBufferAsprintf(buf, size unit='M'%llu/size\n, def-size
20);
+virBufferAsprintf(childrenBuf, size unit='M'%llu/size\n,
+ def-size 20);
if (def-server.enabled) {
-virBufferAddLit(buf, server);
-virBufferEscapeString(buf, path='%s',
def-server.chr.data.nix.path);
-virBufferAddLit(buf, /\n);
+virBufferAddLit(childrenBuf,
[libvirt] [PATCHv2 3/5] util: introduce new helpers to manage shmem device
Signed-off-by: Luyao Huang lhu...@redhat.com --- configure.ac | 10 + po/POTFILES.in | 3 +- src/Makefile.am | 7 +- src/libvirt_private.syms | 24 ++ src/util/virshm.c| 739 +++ src/util/virshm.h| 92 ++ 6 files changed, 872 insertions(+), 3 deletions(-) create mode 100644 src/util/virshm.c create mode 100644 src/util/virshm.h diff --git a/configure.ac b/configure.ac index 08a0f93..ce4908d 100644 --- a/configure.ac +++ b/configure.ac @@ -1176,6 +1176,16 @@ if test $with_linux = yes; then ]]) fi +dnl +dnl check for POSIX share memory functions +dnl +LIBRT_LIBS= +AC_CHECK_LIB([rt],[shm_open],[LIBRT_LIBS=-lrt]) +old_libs=$LIBS +LIBS=$old_libs $LIBRT_LIBS +AC_CHECK_FUNCS([shm_open]) +LIBS=$old_libs +AC_SUBST([LIBRT_LIBS]) dnl Need to test if pkg-config exists PKG_PROG_PKG_CONFIG diff --git a/po/POTFILES.in b/po/POTFILES.in index 46220f7..189699d 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -216,8 +216,9 @@ src/util/virpolkit.c src/util/virportallocator.c src/util/virprocess.c src/util/virrandom.c -src/util/virsexpr.c src/util/virscsi.c +src/util/virsexpr.c +src/util/virshm.c src/util/virsocketaddr.c src/util/virstats.c src/util/virstorageencryption.c diff --git a/src/Makefile.am b/src/Makefile.am index 57a06e8..3158b32 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -152,6 +152,7 @@ UTIL_SOURCES = \ util/virscsi.c util/virscsi.h \ util/virseclabel.c util/virseclabel.h \ util/virsexpr.c util/virsexpr.h \ + util/virshm.c util/virshm.h \ util/virsocketaddr.h util/virsocketaddr.c \ util/virstats.c util/virstats.h \ util/virstorageencryption.c util/virstorageencryption.h \ @@ -1049,7 +1050,7 @@ libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \ $(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(SYSTEMD_DAEMON_LIBS) \ - $(POLKIT_LIBS) + $(POLKIT_LIBS) $(LIBRT_LIBS) noinst_LTLIBRARIES += libvirt_conf.la @@ -1282,9 +1283,10 @@ libvirt_driver_qemu_impl_la_CFLAGS = \ $(AM_CFLAGS) libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \ -$(GNUTLS_LIBS) \ + $(GNUTLS_LIBS) \ $(LIBNL_LIBS) \ $(LIBXML_LIBS) \ + $(LIBRT_LIBS) \ $(NULL) libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES) @@ -2286,6 +2288,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS =\ $(AM_LDFLAGS) \ $(LIBXML_LIBS) \ $(SECDRIVER_LIBS) \ + $(LIBRT_LIBS) \ $(NULL) libvirt_setuid_rpc_client_la_CFLAGS = \ -DLIBVIRT_SETUID_RPC_CLIENT \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d57bf5b..e25ef6b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2089,6 +2089,30 @@ sexpr_u64; string2sexpr; +# util/virshm.h +virShmBuildPath; +virShmCreate; +virShmObjectFindByName; +virShmObjectFree; +virShmObjectGetName; +virShmObjectGetOtherCreate; +virShmObjectGetShareable; +virShmObjectGetSize; +virShmObjectGetType; +virShmObjectGetUsedDomainNumber; +virShmObjectGetUsedDriverName; +virShmObjectListAdd; +virShmObjectListDel; +virShmObjectListGetDefault; +virShmObjectListGetStateFilePath; +virShmObjectNew; +virShmObjectRemoveStateFile; +virShmObjectSaveState; +virShmOpen; +virShmRemoveUsedDomain; +virShmSetUsedDomain; +virShmUnlink; + # util/virsocketaddr.h virSocketAddrBroadcast; virSocketAddrBroadcastByPrefix; diff --git a/src/util/virshm.c b/src/util/virshm.c new file mode 100644 index 000..bec9dd2 --- /dev/null +++ b/src/util/virshm.c @@ -0,0 +1,739 @@ +/* + * virshm.c: helper API for POSIX share memory + * + * Copyright (C) 2015 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * 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
[libvirt] [PATCHv2 4/5] security: add security part for shmem device
A new api to help set/restore the shmem device
label.
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/libvirt_private.syms| 2 +
src/security/security_dac.c | 100
src/security/security_driver.h | 9
src/security/security_manager.c | 35 ++
src/security/security_manager.h | 6 +++
src/security/security_nop.c | 19
src/security/security_selinux.c | 97 ++
src/security/security_stack.c | 39
8 files changed, 307 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e25ef6b..620bd17 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1046,6 +1046,7 @@ virSecurityManagerRestoreDiskLabel;
virSecurityManagerRestoreHostdevLabel;
virSecurityManagerRestoreImageLabel;
virSecurityManagerRestoreSavedStateLabel;
+virSecurityManagerRestoreShmemLabel;
virSecurityManagerSetAllLabel;
virSecurityManagerSetChildProcessLabel;
virSecurityManagerSetDaemonSocketLabel;
@@ -1056,6 +1057,7 @@ virSecurityManagerSetImageFDLabel;
virSecurityManagerSetImageLabel;
virSecurityManagerSetProcessLabel;
virSecurityManagerSetSavedStateLabel;
+virSecurityManagerSetShmemLabel;
virSecurityManagerSetSocketLabel;
virSecurityManagerSetTapFDLabel;
virSecurityManagerStackAddNested;
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 864d75b..a6b4035 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -39,6 +39,7 @@
#include virstoragefile.h
#include virstring.h
#include virutil.h
+#include virshm.h
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -930,6 +931,94 @@
virSecurityDACRestoreSecurityTPMFileLabel(virSecurityManagerPtr mgr,
static int
+virSecurityDACSetShmemLabel(virSecurityManagerPtr mgr,
+virDomainDefPtr def,
+virDomainShmemDefPtr shmem)
+{
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityLabelDefPtr seclabel = NULL;
+virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+char *tmppath = NULL;
+uid_t user;
+gid_t group;
+
+if (!priv-dynamicOwnership)
+return 0;
+
+seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+if (seclabel !seclabel-relabel)
+return 0;
+
+shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
+ SECURITY_DAC_NAME);
+if (shmem_seclabel !shmem_seclabel-relabel)
+return 0;
+
+if (shmem_seclabel shmem_seclabel-label) {
+if (virParseOwnershipIds(shmem_seclabel-label, user, group) 0)
+return -1;
+} else {
+if (virSecurityDACGetIds(seclabel, priv, user, group, NULL, NULL)
0)
+return -1;
+}
+
+if (shmem-server.enabled)
+return virSecurityDACSetOwnership(shmem-server.chr.data.nix.path,
+ user, group);
+
+if (virShmBuildPath(shmem-name, tmppath) 0)
+return -1;
+
+if (virSecurityDACSetOwnership(tmppath, user, group) 0) {
+VIR_FREE(tmppath);
+return -1;
+}
+VIR_FREE(tmppath);
+return 0;
+}
+
+
+static int
+virSecurityDACRestoreShmemLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr shmem)
+{
+virSecurityDeviceLabelDefPtr shmem_seclabel = NULL;
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityLabelDefPtr secdef = NULL;
+char *tmppath = NULL;
+
+if (!priv-dynamicOwnership)
+return 0;
+
+if (shmem-shareable)
+return 0;
+
+secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+if (secdef !secdef-relabel)
+return 0;
+
+shmem_seclabel = virDomainShmemDefGetSecurityLabelDef(shmem,
SECURITY_DAC_NAME);
+
+if (shmem_seclabel !shmem_seclabel-relabel)
+return 0;
+
+if (shmem-server.enabled)
+return virSecurityDACRestoreChardevLabel(mgr, def, NULL,
shmem-server.chr);
+
+if (virShmBuildPath(shmem-name, tmppath) 0)
+return -1;
+
+if (virSecurityDACRestoreSecurityFileLabel(tmppath) 0) {
+VIR_FREE(tmppath);
+return -1;
+}
+VIR_FREE(tmppath);
+return 0;
+}
+
+
+static int
virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
bool migrated)
@@ -961,6 +1050,10 @@
virSecurityDACRestoreSecurityAllLabel(virSecurityManagerPtr mgr,
migrated) 0)
rc = -1;
}
+for (i = 0; i def-nshmems; i++) {
+if (virSecurityDACRestoreShmemLabel(mgr, def, def-shmems[i]))
+rc = -1;
+}
if (virDomainChrDefForeach(def,
[libvirt] [PATCHv2 2/5] conf: introduce shareable in shmem device element
Signed-off-by: Luyao Huang lhu...@redhat.com
---
docs/formatdomain.html.in | 5 +++
docs/schemas/domaincommon.rng | 5 +++
src/conf/domain_conf.c | 6 +++
src/conf/domain_conf.h | 1 +
.../qemuxml2argv-shmem-shareable.xml | 43 ++
tests/qemuxml2xmltest.c| 1 +
6 files changed, 61 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-shareable.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index f2ac5fb..def76fe 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6195,6 +6195,11 @@ qemu-kvm -net nic,model=? /dev/null
vectors. The codeioeventd/code attribute enables/disables (values
on/off, respectively) ioeventfd.
/dd
+dtcodeshareable/code/dt
+dd
+ If present, this indicates the device is expected to be shared
+ between domains (assuming the hypervisor and OS support this).
+/dd
dtcodeseclabel/code/dt
dd
The element may contain an optional codeseclabel/code to override the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f13f566..350f1b3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3371,6 +3371,11 @@
/optional
/element
/optional
+optional
+ element name=shareable
+empty/
+ /element
+/optional
zeroOrMore
ref name='devSeclabel'/
/zeroOrMore
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ece9f2d..34cd93c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11585,6 +11585,9 @@ virDomainShmemDefParseXML(xmlNodePtr node,
goto cleanup;
}
+if (virXPathBoolean(boolean(./shareable), ctxt))
+def-shareable = true;
+
if (virDomainDeviceInfoParseXML(node, NULL, def-info, flags) 0)
goto cleanup;
@@ -20634,6 +20637,9 @@ virDomainShmemDefFormat(virBufferPtr buf,
virBufferAddLit(childrenBuf, /\n);
}
+if (def-shareable)
+virBufferAddLit(childrenBuf, shareable/\n);
+
for (n = 0; n def-nseclabels; n++)
virSecurityDeviceLabelDefFormat(childrenBuf, def-seclabels[n],
flags);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d53c36f..06c304d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1642,6 +1642,7 @@ struct _virDomainShmemDef {
unsigned vectors;
virTristateSwitch ioeventfd;
} msi;
+bool shareable;
size_t nseclabels;
virSecurityDeviceLabelDefPtr *seclabels;
virDomainDeviceInfo info;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-shareable.xml
b/tests/qemuxml2argvdata/qemuxml2argv-shmem-shareable.xml
new file mode 100644
index 000..c94102c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-shareable.xml
@@ -0,0 +1,43 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory unit='KiB'219136/memory
+ currentMemory unit='KiB'219136/currentMemory
+ vcpu placement='static'1/vcpu
+ os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+ /os
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashdestroy/on_crash
+ devices
+emulator/usr/bin/qemu/emulator
+controller type='usb' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='none'/
+shmem name='shmem0'
+ shareable/
+/shmem
+shmem name='shmem1'
+ size unit='M'128/size
+ shareable/
+/shmem
+shmem name='shmem2'
+ size unit='M'256/size
+ shareable/
+ address type='pci' domain='0x' bus='0x00' slot='0x04'
function='0x0'/
+/shmem
+shmem name='shmem3'
+ size unit='M'512/size
+ server/
+ shareable/
+/shmem
+shmem name='shmem4'
+ size unit='M'1024/size
+ server path='/tmp/shmem4-sock'/
+ shareable/
+/shmem
+ /devices
+/domain
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7361db5..76df46a 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -623,6 +623,7 @@ mymain(void)
DO_TEST(shmem);
DO_TEST(shmem-seclabel);
+DO_TEST(shmem-shareable);
DO_TEST(smbios);
DO_TEST(smbios-multiple-type2);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/5] manage the shmem device source
v1 link: https://www.redhat.com/archives/libvir-list/2015-July/msg00926.html v2 different: -Introduce new xml element shareable to indicate whether the device is intended to be shared across multiple guests. -Make virSecuritySELinuxRestoreSecurityAllLabel and virSecuritySELinuxSetSecurityAllLabel call set/restore label -Move struct _virShmObject and _virShmObjectList in virshm.c and introduce some functions to get the parameter. -Change the return value to -1 when the function are not supported on some platform. -add the code in src/security/security_nop.c -some small pieces fix Since there is a shmobj leak when let qemu create shmobj by themselves, also the label of shmobj/shmem-server socket is not right. Guest cannot direct use the shmem-server if users enabled selinux. So it will be better to manage it in libvirt. The way i chosed is region the shmem deivce in a list, and save it status to a local file to avoid losing it after restart libvirtd, and count the guest which use it, and let the callers know if there is no guest is using it (then we can relabel/cleanup some resource). About shmem-server we decided introduce some new selinux label to fix that, however ivshmem server still not finished right now (i mean patches were still being reviewed in qemu-devel), we could implement the set up ivshmem-server part later. BTW, during some research i noticed the vhost-user network should have the same problem like the ivshmem-server, since it use the same way to connect to guest (a host app connect to guest via socket), and after some test i found the guest cannot connect to the sockect which created by app if i enable the Selinux. So i think maybe we could fix these two issue together, if there is already have a way to make vhost-usr works well with SElinux, i think we could use that way in this place. Luyao Huang (5): conf: introduce seclabels in shmem device element conf: introduce shareable in shmem device element util: introduce new helpers to manage shmem device security: add security part for shmem device qemu: call the helpers in virshm.c to manage shmem device configure.ac | 10 + docs/formatdomain.html.in | 12 + docs/schemas/domaincommon.rng | 8 + po/POTFILES.in | 3 +- src/Makefile.am| 7 +- src/conf/domain_conf.c | 103 ++- src/conf/domain_conf.h | 6 + src/libvirt_private.syms | 26 + src/qemu/qemu_conf.h | 3 + src/qemu/qemu_driver.c | 4 + src/qemu/qemu_process.c| 157 + src/security/security_dac.c| 100 +++ src/security/security_driver.h | 9 + src/security/security_manager.c| 35 + src/security/security_manager.h| 6 + src/security/security_nop.c| 19 + src/security/security_selinux.c| 97 +++ src/security/security_stack.c | 39 ++ src/util/virshm.c | 739 + src/util/virshm.h | 92 +++ .../qemuxml2argv-shmem-seclabel.xml| 55 ++ .../qemuxml2argv-shmem-shareable.xml | 43 ++ tests/qemuxml2xmltest.c| 5 + 23 files changed, 1545 insertions(+), 33 deletions(-) create mode 100644 src/util/virshm.c create mode 100644 src/util/virshm.h create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-seclabel.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-shareable.xml -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 5/5] qemu: call the helpers in virshm.c to manage shmem device
Signed-off-by: Luyao Huang lhu...@redhat.com
---
src/qemu/qemu_conf.h| 3 +
src/qemu/qemu_driver.c | 4 ++
src/qemu/qemu_process.c | 157
3 files changed, 164 insertions(+)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index ed9cd46..67a5c61 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -46,6 +46,7 @@
# include virclosecallbacks.h
# include virhostdev.h
# include virfile.h
+# include virshm.h
# ifdef CPU_SETSIZE /* Linux */
# define QEMUD_CPUMASK_LEN CPU_SETSIZE
@@ -235,6 +236,8 @@ struct _virQEMUDriver {
/* Immutable pointer. Unsafe APIs. XXX */
virHashTablePtr sharedDevices;
+virShmObjectListPtr shmlist;
+
/* Immutable pointer, self-locking APIs */
virPortAllocatorPtr remotePorts;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b263ce0..f698ef8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -776,6 +776,9 @@ qemuStateInitialize(bool privileged,
if (qemuMigrationErrorInit(qemu_driver) 0)
goto error;
+if (!(qemu_driver-shmlist = virShmObjectListGetDefault()))
+goto error;
+
if (privileged) {
char *channeldir;
@@ -1085,6 +1088,7 @@ qemuStateCleanup(void)
virObjectUnref(qemu_driver-config);
virObjectUnref(qemu_driver-hostdevMgr);
virHashFree(qemu_driver-sharedDevices);
+virObjectUnref(qemu_driver-shmlist);
virObjectUnref(qemu_driver-caps);
virQEMUCapsCacheFree(qemu_driver-qemuCapsCache);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f7eb2b6..7b266e9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4323,6 +4323,153 @@ qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
}
+static int
+qemuPrepareShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+int ret = -1;
+virShmObjectPtr tmp;
+virShmObjectListPtr list = driver-shmlist;
+bool othercreate = false;
+char *path = NULL;
+bool teardownshm = false;
+int type, fd;
+
+virObjectLock(list);
+
+if ((tmp = virShmObjectFindByName(list, shmem-name))) {
+if (shmem-size != virShmObjectGetSize(tmp)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Shmem object %s is already exists and
+ size is not equal require size),
+ virShmObjectGetName(tmp));
+goto cleanup;
+}
+if (!shmem-shareable) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot use a already exist shmem
+ device in a private way));
+goto cleanup;
+} else if (!virShmObjectGetShareable(tmp)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot use a already exist private shmem
+ device in a shareable way));
+goto cleanup;
+}
+
+if (virShmSetUsedDomain(tmp, QEMU_DRIVER_NAME, vm-def-name) 0)
+goto cleanup;
+
+if (virShmObjectSaveState(tmp, virShmObjectListGetStateFilePath(list))
0)
+goto cleanup;
+
+virObjectUnlock(list);
+return 0;
+}
+
+if (!shmem-server.enabled) {
+if ((fd = virShmCreate(shmem-name, shmem-size,
+ false, othercreate, 0600)) 0)
+goto cleanup;
+VIR_FORCE_CLOSE(fd);
+
+if (virShmBuildPath(shmem-name, path) 0)
+goto cleanup;
+
+type = VIR_SHM_TYPE_SHM;
+} else {
+if (!virFileExists(shmem-server.chr.data.nix.path)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Shmem device server socket is not exist));
+goto cleanup;
+} else {
+othercreate = true;
+}
+type = VIR_SHM_TYPE_SERVER;
+}
+teardownshm = true;
+
+if (!(tmp = virShmObjectNew(shmem-name, shmem-size, path, type,
othercreate,
+shmem-shareable, QEMU_DRIVER_NAME,
vm-def-name)))
+goto cleanup;
+
+if (virShmObjectSaveState(tmp, virShmObjectListGetStateFilePath(list))
0) {
+virShmObjectFree(tmp);
+goto cleanup;
+}
+
+if (virShmObjectListAdd(list, tmp) 0) {
+virShmObjectFree(tmp);
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+if (ret 0) {
+if (teardownshm !shmem-server.enabled
+!othercreate virShmUnlink(shmem-name) 0)
+VIR_WARN(Unable to unlink shared memory object);
+}
+VIR_FREE(path);
+virObjectUnlock(list);
+return ret;
+}
+
+
+static int
+qemuCleanUpShmemDevice(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainShmemDefPtr shmem)
+{
+virShmObjectPtr tmp;
+virShmObjectListPtr
Re: [libvirt] [PATCH V2 1/3] libxl: fix ref counting of libxlMigrationDstArgs
On 27.08.2015 23:38, Jim Fehlig wrote:
Michal Privoznik wrote:
On 07.08.2015 19:53, Jim Fehlig wrote:
This patch fixes some flawed logic around ref counting the
libxlMigrationDstArgs object.
First, when adding sockets to the event loop with
virNetSocketAddIOCallback(), the generic virObjectFreeCallback()
was registered as a free function, with libxlMigrationDstArgs as
its parameter. A reference was also taken on
libxlMigrationDstArgs for each successful call to
virNetSocketAddIOCallback(). The rational behind this logic was
that the libxlMigrationDstArgs object had to out-live the socket
objects. But virNetSocketAddIOCallback() already takes a
reference on socket objects, ensuring their life until removed
from the event loop and unref'ed in virNetSocketEventFree(). We
only need to ensure libxlMigrationDstArgs lives until
libxlDoMigrateReceive() finishes, which can be done by simply
unref'ing libxlMigrationDstArgs at the end of
libxlDoMigrateReceive().
The second flaw was unref'ing the sockets in the failure path of
libxlMigrateReceive() and at the end of libxlDoMigrateReceive().
As mentioned above, the sockets are already unref'ed by
virNetSocketEventFree() when removed from the event loop.
Attempting to unref the socket a second time resulted in a
libvirtd crash since the socket was previously unref'ed and
disposed.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
V2: Initialize args in libxlDomainMigrationPrepare
src/libxl/libxl_migration.c | 20 ++--
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index aa9547b..f9673c8 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -112,11 +112,11 @@ libxlDoMigrateReceive(void *opaque)
virNetSocketUpdateIOCallback(socks[i], 0);
This is pre-existing, but since the socket callback is removed right
after, it does not make much sense to update its events to listen for.
Right. Should be removed since I'm touching this code.
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
-virObjectUnref(socks[i]);
This will leak the socks[i] object, wouldn't it?
Yes, you are right. I verified the virNetSocket was not disposed with
the missing unref.
I mean, in
libxlDomainMigrationPrepare() on line 392 virNetSocketNewListenTCP() is
called. This initialize the array with object pointers. Then
virNetSocketAddIOCallback() + virNetSocketRemoveIOCallback() pair keep
the ref counter consistent. This makes me think you should not remove
this line.
socks[i] = NULL;
}
args-nsocks = 0;
VIR_FORCE_CLOSE(recvfd);
+virObjectUnref(args);
}
@@ -164,11 +164,11 @@ libxlMigrateReceive(virNetSocketPtr sock,
virNetSocketUpdateIOCallback(socks[i], 0);
virNetSocketRemoveIOCallback(socks[i]);
virNetSocketClose(socks[i]);
-virObjectUnref(socks[i]);
socks[i] = NULL;
}
args-nsocks = 0;
VIR_FORCE_CLOSE(recvfd);
+virObjectUnref(args);
}
static int
@@ -318,7 +318,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
virNetSocketPtr *socks = NULL;
size_t nsocks = 0;
int nsocks_listen = 0;
-libxlMigrationDstArgs *args;
+libxlMigrationDstArgs *args = NULL;
size_t i;
int ret = -1;
@@ -420,22 +420,12 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
VIR_EVENT_HANDLE_READABLE,
libxlMigrateReceive,
args,
- virObjectFreeCallback) 0)
+ NULL) 0)
continue;
-/*
- * Successfully added sock to event loop. Take a ref on args to
- * ensure it is not freed until sock is removed from the event
loop.
- * Ref is dropped in virObjectFreeCallback after being removed
- * from the event loop.
- */
-virObjectRef(args);
nsocks_listen++;
}
-/* Done with args in this function, drop reference */
-virObjectUnref(args);
-
if (!nsocks_listen)
goto error;
@@ -448,6 +438,8 @@ libxlDomainMigrationPrepare(virConnectPtr dconn,
virObjectUnref(socks[i]);
}
VIR_FREE(socks);
+virObjectUnref(args);
+
/* Remove virDomainObj from domain list */
if (vm) {
virDomainObjListRemove(driver-domains, vm);
Otherwise looking good.
How does it look with the following squashed in?
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 8db3aea..9609e06 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -112,9 +112,9 @@ libxlDoMigrateReceive(void *opaque)
/* Remove all listen socks from event
