[libvirt] [PATCH 2/2 (v3)] util: fallback to ioctl(SIOCBRDELBR) if netlink RTM_DELLINK fails
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);
+}
+
+
+#elif defined(HAVE_STRUCT_IFREQ) defined(SIOCBRDELBR)
+int
+virNetDevBridgeDelete(const char *brname)
+{
+return virNetDevBridgeDeleteWithIoctl(brname);
+}
+
+
#elif defined(HAVE_STRUCT_IFREQ) defined(SIOCIFDESTROY)
-int virNetDevBridgeDelete(const char *brname)
+int
+virNetDevBridgeDelete(const char *brname)
{
int s;
struct ifreq ifr;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 9c4da1f..954736a 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -220,7 +220,7 @@ virNetDevMacVLanCreate(const char *ifname,
*/
int virNetDevMacVLanDelete(const char *ifname)
{
-return virNetlinkDelLink(ifname);
+return virNetlinkDelLink(ifname, NULL);
}
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0052ef9..0276522 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -281,6 +281,10 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
* virNetlinkDelLink:
*
* @ifname: Name of the link
+ * @fallback: pointer to an alternate function that will
+ *be called to perform the delete if RTM_DELLINK fails
+ *with EOPNOTSUPP (any other error will simply be treated
+ *as an error).
*
* delete a network link (aka interface aka device) with the given
* name. This works for many different types of network devices,
@@ -289,7 +293,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
* Returns 0 on success, -1 on fatal error.
*/
int
-virNetlinkDelLink(const char *ifname)
+virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback)
{
int rc = -1;
struct nlmsghdr *resp = NULL;
@@ -325,6 +329,10 @@ virNetlinkDelLink(const char *ifname)
if (resp-nlmsg_len NLMSG_LENGTH(sizeof(*err)))
goto malformed_resp;
+if (-err-error == EOPNOTSUPP fallback) {
+rc = fallback(ifname);
+goto cleanup;
+}
if (err-error) {
virReportSystemError(-err-error,
_(error
[libvirt] [PATCH] hostdev: skip ACS check when using VFIO for device assignment
The ACS checks are meaningless when using the more modern VFIO driver
for device assignment since VFIO has its own more complete and exact
checks, but I didn't realize that when I added support for VFIO. This
patch eliminates the ACS check when preparing PCI devices for
assignment if VFIO is being used.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1256486
---
src/util/virhostdev.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 529753c..0bc5120 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1,6 +1,6 @@
/* virhostdev.c: hostdev management
*
- * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
* Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
*
@@ -230,7 +230,6 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr
*hostdevs, int nhostdevs)
virObjectUnref(list);
return NULL;
}
-
if (virPCIDeviceListAdd(list, dev) 0) {
virPCIDeviceFree(dev);
virObjectUnref(list);
@@ -579,7 +578,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr
hostdev_mgr,
struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, dom_name,
usesVfio};
-if (!virPCIDeviceIsAssignable(dev, strict_acs_check)) {
+if (!usesVfio !virPCIDeviceIsAssignable(dev, strict_acs_check)) {
virReportError(VIR_ERR_OPERATION_INVALID,
_(PCI device %s is not assignable),
virPCIDeviceGetName(dev));
--
2.1.0
--
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 25.08.2015 18:42, Dmitry Guryanov wrote:
On 08/25/2015 12:04 PM, 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;
}
Could you, please, send this change in a separate patch, because it's a sort
of bugfix to common libvirt code and not related to our migration?
ok
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);
.
+
+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 = connuri-port;
+ret = 0;
+
This is generally not correct, since you are passing the port of libvirt's
connection to vz connection, it works if you don't specify port in URI, port
is 0 in this case, and libprlsdk accepts it.
I'm not agree. This function is called with hypervisor specific uri where port
is hypervisor and not libvirtd port.
+ cleanup:
+
+virURIFree(connuri);
+if (ret 0) {
+virURIFree(vzuri);
+vzuri = NULL;
+}
+
+return vzuri;
+}
+
+#define VZ_MIGRATION_FLAGS (0)
+
+#define VZ_MIGRATION_PARAMETERS (NULL)
+
+static int
+vzDomainMigratePerform3(virDomainPtr domain,
+const char *xmlin ATTRIBUTE_UNUSED,
+const char *cookiein ATTRIBUTE_UNUSED,
+int cookieinlen ATTRIBUTE_UNUSED,
+char **cookieout ATTRIBUTE_UNUSED,
+int *cookieoutlen ATTRIBUTE_UNUSED,
+const char *dconnuri ATTRIBUTE_UNUSED,
+const char *uri,
+unsigned long flags,
+const char *dname ATTRIBUTE_UNUSED,
+unsigned long bandwidth ATTRIBUTE_UNUSED)
+{
+int ret = -1;
+virDomainObjPtr dom = NULL;
+virConnectPtr dconn = NULL;
+virURIPtr vzuri = NULL;
+unsigned char session_uuid[VIR_UUID_BUFLEN];
+vzConnPtr privconn = domain-conn-privateData;
+char *cookie = NULL;
+int cookielen = 0;
+
+virCheckFlags(VZ_MIGRATION_FLAGS, -1);
+
+if (!(vzuri = vzMakeVzUri(uri)))
+goto cleanup;
+
+if (!(dom = vzDomObjFromDomain(domain)))
+goto cleanup;
+
+dconn = virConnectOpen(uri);
+if (dconn == NULL) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _(Failed to connect to remote libvirt URI %s: %s),
+ uri, virGetLastErrorMessage());
+goto cleanup;
+}
+
+/* NULL and zero elements are unused */
+/* domxml is passed as or otherwise we will fail at rpc call */
+if (virDomainMigratePrepare3(dconn, NULL, 0, cookie, cookielen, NULL,
NULL, 0, NULL, 0, ) 0)
Could you split this line?
ok
+goto cleanup;
+
+if (vzParseCookie(cookie, session_uuid) 0)
Re: [libvirt] [PATCH v3 4/5] vz: support misc migration options
On 25.08.2015 18:54, Dmitry Guryanov wrote: On 08/25/2015 12:04 PM, nshirokovs...@virtuozzo.com wrote: From: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com Migration API has a lot of options. This patch intention is to provide support for those options that can be trivially supported and give estimation for other options support in this commit message. I. Supported. 1. VIR_MIGRATE_COMPRESSED. Means 'use compression when migration domain memory'. It is supported but quite uncommon way: vz migration demands that this option should be set. This is due to vz is hardcoded to moving VMs memory using compression. So anyone who wants to migrate vz domain should set this option thus declaring it knows it uses compression. Why bother? May be just support this option and ignore if it is not set or don't support at all as we can't change behaviour in this aspect. Well I believe that this option is, first, inherent to hypervisor implementation as we have a task of moving domain memory to different place and, second, we have a tradeoff here between cpu and network resources and some managment should choose the stratery via this option. If we choose ignoring or unsupporting implementation than this option has a too vague meaning. Let's go into more detail. First if we ignore situation where option is not set than we put user into fallacy that vz hypervisor don't use compression and thus have lower cpu usage. Second approach is to not support the option. The main reason not to follow this way is that 'not supported and not set' is indistinguishable from 'supported and not set' and thus again fool the user. 2. VIR_MIGRATE_LIVE. Means 'reduce domain downtime by suspending it as lately as possible' which technically means 'migrate as much domain memory as possible before suspending'. Supported in same manner as VIR_MIGRATE_COMPRESSED as both vz VMs and CTs are always migrated via live scheme. One may be fooled by vz sdk flags of migration api: PVMT_HOT_MIGRATION(aka live) and PVMT_WARM_MIGRATION(aka normal). Current implementation ignore these flags and always use live migration. 3. VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE. This two comes together. Vz domain are alwasy persistent so we have to support demand option VIR_MIGRATE_PERSIST_DEST is set and VIR_MIGRATE_UNDEFINE_SOURCE is not (and this is done just by unsupporting it). 4. VIR_MIGRATE_PAUSED. Means 'don't resume domain on destination'. This is trivially supported as we have a corresponding option in vz migration. 5. VIR_MIGRATE_OFFLINE. Means 'migrate only XML definition of a domain'. It is a forcing option that is it is ignored if domain is running and must be set to migrate stopped domain. Vz implemenation follows this unformal definition with one exception: non-shared disks will be migrated too. This desicion is on par with VIR_MIGRATE_NON_SHARED_DISK condideration(see last part of this notes). All that said the minimal command to migrate vz domain looks next: migrate --direct $DOMAIN $STUB --migrateuri $DESTINATION --live --persistent --compressed. Not good. Say if you want to just migrate a domain without further details you will get error messages until you add these options to command line. I think there is a lack of notion 'default' behaviour in all these aspects. If we have it we could just issue: migrate $DOMAIN $DESTINATION For vz this would give default compression for example, for qemu - default no-compression. Then we could have flags --compressed and -no-compressed and for vz the latter would give unsupported error. II. Unsupported. 1. VIR_MIGRATE_UNSAFE. Vz disks are always have 'cache=none' set (this is not reflected in current version of vz driver and will be fixed soon). So we need not to support this option. 2. VIR_MIGRATE_CHANGE_PROTECTION. Unsupported as we have no appopriate support from vz sdk. Although we have locks they are advisory and cant help us. 3. VIR_MIGRATE_TUNNELLED. Means 'use libvirtd to libvirtd connection to pass hypervisor migration traffic'. Unsupported as not among vz hypervisor usecases. 4. p2p migration which is exposed via *toURI* interface with VIR_MIGRATE_PEER2PEER flag set. It doesn't make sense for vz migration as it is a variant of managed migration which is qemu specific. 5. VIR_MIGRATE_ABORT_ON_ERROR, VIR_MIGRATE_AUTO_CONVERGE, VIR_MIGRATE_RDMA_PIN_ALL, VIR_MIGRATE_NON_SHARED_INC, VIR_MIGRATE_PARAM_DEST_XML, VIR_MIGRATE_PARAM_BANDWIDTH, VIR_MIGRATE_PARAM_GRAPHICS_URI, VIR_MIGRATE_PARAM_LISTEN_ADDRESS, VIR_MIGRATE_PARAM_MIGRATE_DISKS. Without further discussion. They are just not usecases of vz hypevisor. III. Undecided and thus unsupported. 6. VIR_MIGRATE_NON_SHARED_DISK. The meaning of this option is not clear to me. Look, if qemu domain has a non-shared disk than it will refuse to migrate. But after you specify this option it will refuse too. You need to create image file
Re: [libvirt] [PATCH 0/2] Fix bridge creation/deletion on 2.6.x kernels
On 08/26/2015 05:01 AM, Martin Kletzander wrote: On Tue, Aug 25, 2015 at 11:47:53PM -0400, Laine Stump wrote: These two patches fix this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1252780 which was also separate reported in libvirt-users a couple days ago. Most people running CentOS6 or RHEL6 also run the downstream version of libvirt-0.10.2 that has many patches backported from later upstream releases, but not the patch described in the commit logs of these proposed patches. And the problem being fixed here only occurs on a 2.6.x kernel, while most people running upstream libvirt are running something much more modern. That's why not many people have seen the bug that these patches fix. Still, we claim to fully support upstream on RHEL6/CentOS6, so we'd better do it right! Two notes: 1) I tried timing 20 iterations of net-start...net-delete with these patches vs. a libvirt build that simply calls the ioctls in the beginnng (i.e. it doesn't try netlink then fall back to ioctl if that fails), and the time to completion was within 0.25%, so the extra overhead of trying one then the other isn't significant even on the older machines that will always fall back to ioctl. 2) There are 2 different versions of Patch 2/2. I did the first one as an exercise to see how ugly it would be to *not* duplicate the entire virNetlinkDelLink() function inside the new virNetDevBridgeDelete(). It was pretty ugly, so I made a cleaner version that duplicates the code instead. I would be okay pushing either version, but I think I slightly prefer the one that duplicates code and remains uncomplicated. To be honest, I dislike both approaches. Sure, the second one is not that messy, but looking at those patches I have another idea. At first I thought that since there are not that many callers of virNetlinkDelLink(), what if we did not report the error inside that function at all and just return the errno and callers would report errors themselves. But that's not good either, virNetlinkDelLink() has very nice error reporting. Right - there is too much inormation that would be lost by leaving error reporting up to the caller, and that leads to bug reports of the type This is broken. rather than This has a problem with XYZ. So what if we took the second version, but the code that's duplicated (e.g. preparing netlink messages) that can fail by itself could be moved into its own functions The s on the end of that is the crucial part - it would take two functions, one for the part before checking the error code, and one for the part after. How about this: give virNetlinkDelLink() a new argument fallbackDeleteFunc which would either be NULL, or a pointer to virNetDevBridgeDeleteWithIoctl() (in this case)? That avoids needing to split virNetlinkDelLink into multiple parts as well as the duplication of code, and I think it's much less obtuse than the boolean thing of V1. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Fix bridge creation/deletion on 2.6.x kernels
On 26.08.2015 17:53, Laine Stump wrote: snip/ How about this: give virNetlinkDelLink() a new argument fallbackDeleteFunc which would either be NULL, or a pointer to virNetDevBridgeDeleteWithIoctl() (in this case)? That avoids needing to split virNetlinkDelLink into multiple parts as well as the duplication of code, and I think it's much less obtuse than the boolean thing of V1. D'oh! I mean, if someone calls virNetlink*() he can assume that the function will talk to kernel via netlink. This would break the assumption. But I because I am unable to come up with a better idea, I think it's our best option. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Fix bridge creation/deletion on 2.6.x kernels
On Wed, Aug 26, 2015 at 07:22:10PM +0200, Michal Privoznik wrote: On 26.08.2015 17:53, Laine Stump wrote: snip/ How about this: give virNetlinkDelLink() a new argument fallbackDeleteFunc which would either be NULL, or a pointer to virNetDevBridgeDeleteWithIoctl() (in this case)? That avoids needing to split virNetlinkDelLink into multiple parts as well as the duplication of code, and I think it's much less obtuse than the boolean thing of V1. D'oh! I mean, if someone calls virNetlink*() he can assume that the function will talk to kernel via netlink. This would break the assumption. But I because I am unable to come up with a better idea, I think it's our best option. As hacky as the previous one, but at least clean and understandable, in my opinion. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] target-i386: Enable check mode by default
Current default behavior of QEMU is to silently disable features that
are not supported by the host when a CPU model is requested in the
command-line. This means that in addition to risking breaking guest ABI
by default, we are silent about it.
I would like to enable enforce by default, but this can easily break
existing production systems because of the way libvirt makes assumptions
about CPU models today (this will change in the future, once QEMU
provide a proper interface for checking if a CPU model is runnable).
But there's no reason we should be silent about it. So, change
target-i386 to enable check mode by default so at least we have some
warning printed to stderr (and hopefully logged somewhere) when QEMU
disables a feature that is not supported by the host system.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cfb8aa7..32fee00 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3121,7 +3121,7 @@ static Property x86_cpu_properties[] = {
DEFINE_PROP_BOOL(hv-relaxed, X86CPU, hyperv_relaxed_timing, false),
DEFINE_PROP_BOOL(hv-vapic, X86CPU, hyperv_vapic, false),
DEFINE_PROP_BOOL(hv-time, X86CPU, hyperv_time, false),
-DEFINE_PROP_BOOL(check, X86CPU, check_cpuid, false),
+DEFINE_PROP_BOOL(check, X86CPU, check_cpuid, true),
DEFINE_PROP_BOOL(enforce, X86CPU, enforce_cpuid, false),
DEFINE_PROP_BOOL(kvm, X86CPU, expose_kvm, true),
DEFINE_PROP_UINT32(level, X86CPU, env.cpuid_level, 0),
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/2] qemu: fix the audit log is not correct for memory device
On 08/13/2015 10:15 AM, Luyao Huang wrote: First review: http://www.redhat.com/archives/libvir-list/2015-July/msg00982.html Change in v2: - split it to two patches - fix some small mistake in hot-unplug part I change the code in qemuDomainAttachMemory like what we do in other attach deivce functions. And I have removed the jump in qemuDomainRemoveMemoryDevice, since i just see only one place need jump to audit, so i refactor the code (thanks John's advise). Luyao Huang (2): qemu: fix the audit log is not correct after hot-plug memory success qemu: fix the audit log is not correct after hot-unplug memory device src/qemu/qemu_hotplug.c | 35 +-- 1 file changed, 17 insertions(+), 18 deletions(-) These look much cleaner code wise - had to clean up the commit messages, but otherwise fine. I have pushed the two patches. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Remove backingStore when changing cdrom media source
On 07/28/2015 09:56 AM, Peter Krempa wrote:
Since the code is changing the source image path by modifying the
existing XML snippet the backingStore element does not get dropped.
The element is ignored though but it might start being used in the
future.
Before this patch, you'd get:
$ virsh change-media --eject --print-xml 10 hdc
disk type=file device=cdrom
driver name=qemu type=qcow2/
backingStore type=file index=1
format type=qcow2/
source file=/var/lib/libvirt/images/vm.1436949097/
backingStore/
/backingStore
target dev=hdc bus=ide/
...
/disk
After:
$ virsh change-media --eject --print-xml 10 hdc
disk type=file device=cdrom
driver name=qemu type=qcow2/
target dev=hdc bus=ide/
...
/disk
---
tools/virsh-domain.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
Code will need to be updated with recent virsh changes... I'm not quite
sure I understand what's being done or why it's necessary from the
commit explanation.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a61656f..d4d6987 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11415,6 +11415,7 @@ vshUpdateDiskXML(xmlNodePtr disk_node,
vshUpdateDiskXMLType type)
{
xmlNodePtr source = NULL;
+xmlNodePtr child;
char *device_type = NULL;
char *ret = NULL;
@@ -11430,10 +11431,16 @@ vshUpdateDiskXML(xmlNodePtr disk_node,
}
/* find the current source subelement */
-for (source = disk_node-children; source; source = source-next) {
-if (source-type == XML_ELEMENT_NODE
-xmlStrEqual(source-name, BAD_CAST source))
-break;
+for (child = disk_node-children; child; child = child-next) {
+if (child-type == XML_ELEMENT_NODE
+xmlStrEqual(child-name, BAD_CAST source))
+source = child;
+
+if (child-type == XML_ELEMENT_NODE
+xmlStrEqual(child-name, BAD_CAST backingStore)) {
+xmlUnlinkNode(child);
+xmlFreeNode(child);
+}
here you've freed child and from how I read the existing code when
'source' goes through the same process, there's a 'source = NULL;'
following, so it seems there should be a child = NULL; added.
Without that, would the next iteration of the loop reference something
that was free'd? That is child is local to this function, it's not
passed by reference, thus a subsequent ; child; child = child-next
may be by chance pointing to something valid, but could be pointing to
something invalid too.
I think with the merge with latest changes and the child = NULL this is
ACK-able.
John
}
if (type == VSH_UPDATE_DISK_XML_EJECT) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [python PATCH] examples: small fix for nodestats.py example
On 07/30/2015 10:19 PM, Luyao Huang wrote: Add nodestats.py in MANIFEST.in and add a small description for nodestats.py in README Signed-off-by: Luyao Huang lhu...@redhat.com --- MANIFEST.in | 1 + examples/README | 3 +++ 2 files changed, 4 insertions(+) ACK and pushed. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] lxc: Inherit namespace feature
On Thu, Aug 20, 2015 at 07:16:17PM +0530, ik.nitk wrote: 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. ACK and pushed to GIT master. Thanks for taking the time to work on this feature ! 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] Libvirt Application Development Guide Using Python is now complete
On Tue, Aug 25, 2015 at 08:47:37AM -0500, David Ashley wrote: All - I have finished work on the Libvirt Application Development Guide Using Python book for the Fedora Docs Team. It is currently under review before being published. I have confirmed that I can offer the guide back the the libvirt team for their own use. The guide currently resides in a git repository and can be obtained with git clone git://git.fedorahosted.org/git/docs/libvirt_application_development_guide_using_python.git Please let me know if you publish the guide as I would like to keep track of all the publishers so I can notify them about future updates. How do you want to manage this doc going forward. I'd personally have a preference for hosting it on the primary libvirt git server, rather than on Fedora hosted, so it is in the same place as all the other libvirt related repositories. We'd of course give you direct ability to push updates to the GIT repo in that case. Is there any workflow reason why you would need it to continue to be hosted on Fedora ? We'd also likely set up a job to automatically build it and publish HTML + PDFs on libvirt.org 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] [PATCHv3 0/2] Added waiting for DAD to finish for bridge address.
Thank you! Waiting in hope you have not forgotten. :) On 08/17/2015 10:11 PM, Laine Stump wrote: On 08/17/2015 10:48 AM, Maxim Perevedentsev wrote: Hello guys! Just a humble reminder of pending request :-) Any suggestions about patches maybe? Sorry for the delay. Pretty much everybody (including me) is at KVM Forum this week. I'll try to make some comments on the patches as soon as I can. -- Your sincerely, Maxim Perevedentsev -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Don't save/set MAC address for macvtap+passthrough+802.1Qbh
On Wed, Aug 26, 2015 at 12:29:20AM -0400, Laine Stump wrote:
Before libvirt sets the MAC address of the physdev (the physical
ethernet device) linked to a macvtap passthrough device, it always
saves the previous MAC address to restore when the guest is finished
(following a leave nothing behind policy). It has even done this for
macvtap devices that have an 802.1Qbh port profile attached to
them. It turns out that this is unnecessary, because the port profile
Associate/Disassociate operations do that for us.
Beyond that, with a recent change to the way we retrieve the MAC
address (commit cb3fe38c), all attempts to start a macvtap passthrough
device with an 802.1Qbh port profile attached to a Cisco VMFEX card
(which uses the enic driver in the kernel) to fail.
This patch puts extra qualifiers around both the save/set and the
restore of the physdev address, so that it isn't done if there is an
802.1Qbh port profile associated with it.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1257004
---
Stefan - do you know if this save/restore of MAC address is also
unnecessary/erroneous for 802.1Qbg? If so, I'll just disable it any
time a virtportprofile is present, since those are the only two port
profile types that are valid with macvtap anyway.
src/util/virnetdevmacvlan.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 213b8eb..9c4da1f 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -777,11 +777,17 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
*tgifname,
* address must be reset when the VM is shut down.
* This is especially important when using SRIOV capable cards that
* emulate their switch in firmware.
+ *
+ * Note that this saving/setting of the MAC address must *NOT* be
+ * done if the interface is setup with an 802.1Qbh. In that case,
+ * the 802.1Qbh port associate operation will take care of
+ * saving/setting the MAC address.
*/
-if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) {
-if (virNetDevReplaceNetConfig(linkdev, -1, macaddress, -1, stateDir)
0)
-return -1;
-}
+if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU
+!(virtPortProfile
+ virtPortProfile-virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH)
+virNetDevReplaceNetConfig(linkdev, -1, macaddress, -1, stateDir) 0)
+return -1;
if (tgifname) {
if ((ret = virNetDevExists(tgifname)) 0)
@@ -913,7 +919,9 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char
*ifname,
int ret = 0;
int vf = -1;
-if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU)
+if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU
+!(virtPortProfile
+ virtPortProfile-virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH))
ignore_value(virNetDevRestoreNetConfig(linkdev, vf, stateDir));
I was thinking this could be dealt with in the Replace/Restore
functions, but that would require more work and, more importantly, it
is already dealt with on other callers of these functions, so...
ACK
if (ifname) {
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Fix bridge creation/deletion on 2.6.x kernels
On Tue, Aug 25, 2015 at 11:47:53PM -0400, Laine Stump wrote: These two patches fix this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1252780 which was also separate reported in libvirt-users a couple days ago. Most people running CentOS6 or RHEL6 also run the downstream version of libvirt-0.10.2 that has many patches backported from later upstream releases, but not the patch described in the commit logs of these proposed patches. And the problem being fixed here only occurs on a 2.6.x kernel, while most people running upstream libvirt are running something much more modern. That's why not many people have seen the bug that these patches fix. Still, we claim to fully support upstream on RHEL6/CentOS6, so we'd better do it right! Two notes: 1) I tried timing 20 iterations of net-start...net-delete with these patches vs. a libvirt build that simply calls the ioctls in the beginnng (i.e. it doesn't try netlink then fall back to ioctl if that fails), and the time to completion was within 0.25%, so the extra overhead of trying one then the other isn't significant even on the older machines that will always fall back to ioctl. 2) There are 2 different versions of Patch 2/2. I did the first one as an exercise to see how ugly it would be to *not* duplicate the entire virNetlinkDelLink() function inside the new virNetDevBridgeDelete(). It was pretty ugly, so I made a cleaner version that duplicates the code instead. I would be okay pushing either version, but I think I slightly prefer the one that duplicates code and remains uncomplicated. To be honest, I dislike both approaches. Sure, the second one is not that messy, but looking at those patches I have another idea. At first I thought that since there are not that many callers of virNetlinkDelLink(), what if we did not report the error inside that function at all and just return the errno and callers would report errors themselves. But that's not good either, virNetlinkDelLink() has very nice error reporting. So what if we took the second version, but the code that's duplicated (e.g. preparing netlink messages) that can fail by itself could be moved into its own functions that would be called in both virNetlinkDelLink() and virNetDevBridgeDelete() so that the duplication would be minimal and more prone to differentiation in the future. Laine Stump (2): util: fallback to ioctl(SIOCBRADDBR) if netlink RTM_NEWLINK fails util: fallback to ioctl(SIOCBRDELBR) if netlink RTM_DELLINK fails src/util/virnetdevbridge.c | 112 src/util/virnetdevmacvlan.c | 2 +- src/util/virnetlink.c | 17 +-- src/util/virnetlink.h | 2 +- 4 files changed, 98 insertions(+), 35 deletions(-) -- 2.1.0 -- 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 v3 2/5] vz: add migration backbone code
On 25.08.2015 20:28, Dmitry Guryanov wrote:
On 08/25/2015 07:18 PM, Daniel P. Berrange wrote:
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.
I'm trying to understand why you need $STUB as dummy data.
IIRC, when doing direct migration, you should be providing a valid
URI for the first parameter, and not need the second uri.
virsh uses virDomainMigrateToURI3 function, and for some reason it differs
destination URI for direct and peer-to-peer migrations. For p2p it uses
pconnuri argument and for direct -VIR_MIGRATE_PARAM_URI
http://libvirt.org/html/libvirt-libvirt-domain.html#VIR_MIGRATE_PARAM_URI
from typed params list.
AFAIU in virDomainMigrateToURI3 function, dconnuri argument is treated as URI
to destination libvirt daemon.
As direct migration doesn't need this kind of connection this argument is
ignored.
However as suggested vz migration implementation uses connection to destination
libvirtd daemon
to get destination hypervisor session uuid I should probably use dconnuri for
for that purporse
not hypervisor URI. This is related what Dmitry mentioned in previous letters
where there
was a confusion between hypervisor and remote access to daemon ports.
This would break a invariant mentioned in the documentation that direct
migration
does not use dconnuri. Probably it is not too bad too get away from this
restriction.
+
+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)
+{
+vzConnPtr privconn = conn-privateData;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(*cookieout = vzFormatCookie(privconn-session_uuid)))
+goto cleanup;
+*cookieoutlen = strlen(*cookieout) + 1;
+ret = 0;
+
+ 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;
+}
+}
+
+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 = connuri-port;
+ret = 0;
+
+ cleanup:
+
+virURIFree(connuri);
+if (ret 0) {
+virURIFree(vzuri);
+vzuri = NULL;
+}
+
+return vzuri;
+}
+
+#define VZ_MIGRATION_FLAGS (0)
+
+#define VZ_MIGRATION_PARAMETERS (NULL)
+
+static int
+vzDomainMigratePerform3(virDomainPtr domain,
+const char *xmlin ATTRIBUTE_UNUSED,
+const char *cookiein ATTRIBUTE_UNUSED,
+int cookieinlen ATTRIBUTE_UNUSED,
+char **cookieout ATTRIBUTE_UNUSED,
+int *cookieoutlen ATTRIBUTE_UNUSED,
+const char *dconnuri ATTRIBUTE_UNUSED,
+const char *uri,
I think you should be using 'dconnuri' rather than 'uri', so
then you would not need the $STUB dummy parameter.
Regards,
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix link to page for Virtuozzo driver
--- docs/index.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/index.html.in b/docs/index.html.in index 7b0c149..c4025de 100644 --- a/docs/index.html.in +++ b/docs/index.html.in @@ -72,7 +72,7 @@ The a href=http://libvirt.org/drvphyp.html;IBM PowerVM/a hypervisor /li li -The a href=http://libvirt.org/drvirtuozzo.html;Virtuozzo/a hypervisor +The a href=http://libvirt.org/drvvirtuozzo.html;Virtuozzo/a hypervisor /li li The a href=http://libvirt.org/drvbhyve.html;Bhyve/a hypervisor -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Start daemon only after filesystems are mounted
On Tue, Aug 25, 2015 at 05:12:20PM +0200, Martin Kletzander wrote: When images are on mounted filesystems, there is no guarantee libvirtd will start before they are mounted. In systemd world, this is done with 'After=systemd.mount' in the service file. Signed-off-by: Martin Kletzander mklet...@redhat.com --- daemon/libvirtd.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 1759ac8a0946..c461f9462ae5 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -5,6 +5,7 @@ After=network.target After=dbus.service After=iscsid.service After=apparmor.service +After=systemd.mount What is this systemd.mount unit you're referring to ? AFAIK there is no such unit - systemd.mount is a man page describing the format of *.mount units. Do you instead mean to use local-fs.target which blocks until all local filesystems are mounted ? 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] Start daemon only after filesystems are mounted
On Wed, Aug 26, 2015 at 11:41:54AM +0100, Daniel P. Berrange wrote: On Tue, Aug 25, 2015 at 05:12:20PM +0200, Martin Kletzander wrote: When images are on mounted filesystems, there is no guarantee libvirtd will start before they are mounted. In systemd world, this is done with 'After=systemd.mount' in the service file. Signed-off-by: Martin Kletzander mklet...@redhat.com --- daemon/libvirtd.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 1759ac8a0946..c461f9462ae5 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -5,6 +5,7 @@ After=network.target After=dbus.service After=iscsid.service After=apparmor.service +After=systemd.mount What is this systemd.mount unit you're referring to ? AFAIK there is no such unit - systemd.mount is a man page describing the format of *.mount units. Do you instead mean to use local-fs.target which blocks until all local filesystems are mounted ? I want this to apply to remote filesystems as well. I thought systemd.mount covers all activated .mount units, but it wouldn't cover neither local-fs nor remote-fs targets. So looking at systemd.special(7) man-page (oh, why is this more complicated then it must be), I think I must use both After=local-fs.target and After=remote-fs.target as well. Feel free to correct me as I admit I know nothing, to be polite, about systemd, so I'm reading wrong man pages all the time. 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 :| signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Start daemon only after filesystems are mounted
On Wed, Aug 26, 2015 at 02:13:54PM +0200, Martin Kletzander wrote: On Wed, Aug 26, 2015 at 11:41:54AM +0100, Daniel P. Berrange wrote: On Tue, Aug 25, 2015 at 05:12:20PM +0200, Martin Kletzander wrote: When images are on mounted filesystems, there is no guarantee libvirtd will start before they are mounted. In systemd world, this is done with 'After=systemd.mount' in the service file. Signed-off-by: Martin Kletzander mklet...@redhat.com --- daemon/libvirtd.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in index 1759ac8a0946..c461f9462ae5 100644 --- a/daemon/libvirtd.service.in +++ b/daemon/libvirtd.service.in @@ -5,6 +5,7 @@ After=network.target After=dbus.service After=iscsid.service After=apparmor.service +After=systemd.mount What is this systemd.mount unit you're referring to ? AFAIK there is no such unit - systemd.mount is a man page describing the format of *.mount units. Do you instead mean to use local-fs.target which blocks until all local filesystems are mounted ? I want this to apply to remote filesystems as well. I thought systemd.mount covers all activated .mount units, but it wouldn't cover neither local-fs nor remote-fs targets. So looking at systemd.special(7) man-page (oh, why is this more complicated then it must be), I think I must use both After=local-fs.target and After=remote-fs.target as well. Yes, after both remote-fs.target local-fs.target sounds reasonable. 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] virt-aa-helper: add NVRAM store file for read/write
On Fri, 2015-08-21 at 11:01 +0200, Guido Günther wrote:
Hi,
On Thu, Aug 20, 2015 at 10:58:59AM -0700, Peter Kieser wrote:
Some UEFI firmwares may want to use a non-volatile memory to store some
variables.
If AppArmor is enabled, and NVRAM store file is set currently virt-aa-helper
does
not add the NVRAM store file to the template. Add this file for read/write
when
this functionality is defined in domain XML.
I'm not an export on apparmor things but it makes sense to me.
ACK
ACK from me too. Just pushed it.
--
Cedric
Cheers,
-- Guido
Signed-off-by: Peter Kieser pe...@kieser.ca
---
src/security/virt-aa-helper.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 4ce1e7a..2f93172 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1047,6 +1047,10 @@ get_files(vahControl * ctl)
if (vah_add_file(buf, ctl-def-os.loader-path, r) != 0)
goto cleanup;
+if (ctl-def-os.loader ctl-def-os.loader-nvram)
+if (vah_add_file(buf, ctl-def-os.loader-nvram, rw) != 0)
+goto cleanup;
+
for (i = 0; i ctl-def-ngraphics; i++) {
if (ctl-def-graphics[i]-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC
ctl-def-graphics[i]-data.vnc.socket
--
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hostdev: skip ACS check when using VFIO for device assignment
On 08/26/2015 04:13 AM, Martin Kletzander wrote:
On Wed, Aug 26, 2015 at 02:07:32AM -0400, Laine Stump wrote:
The ACS checks are meaningless when using the more modern VFIO driver
for device assignment since VFIO has its own more complete and exact
checks, but I didn't realize that when I added support for VFIO. This
patch eliminates the ACS check when preparing PCI devices for
assignment if VFIO is being used.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1256486
I don't remember how it's possible as neatly as I managed to do
somehow, but I believe git has good support for 'Key: Value' on one
line so that you can then easily see, for example, patches that have
Key == Resolves etc., so I'm using Resolves: link-to-bz, but as I
said, I forgot how to use that formatting neatly (it was almost like
one parameter only). So this is just an FYI, no need to change it.
Interesting, I didn't know about that.
---
src/util/virhostdev.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 529753c..0bc5120 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1,6 +1,6 @@
/* virhostdev.c: hostdev management
*
- * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
* Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
*
@@ -230,7 +230,6 @@
virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int
nhostdevs)
virObjectUnref(list);
return NULL;
}
-
if (virPCIDeviceListAdd(list, dev) 0) {
virPCIDeviceFree(dev);
virObjectUnref(list);
@@ -579,7 +578,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr
hostdev_mgr,
struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr,
dom_name,
usesVfio};
-if (!virPCIDeviceIsAssignable(dev, strict_acs_check)) {
+if (!usesVfio !virPCIDeviceIsAssignable(dev,
strict_acs_check)) {
I suppose we could unset the strict check in case of VFIO as that
would work too, but was not meant for this job.
Just in case someone were to be incredibly intelligent and try to use
legacy assignment and vfio in the same system, we can't just disable the
global strict_acs check. Besides, even if the strict ACS check is
disabled, we still go poking around in sysfs (calling
virPCIDeviceIsBehindSwitchLackingACS()), which we really *don't* want to
do, as that is causing problems for people attempting to do vfio device
assignment from a session-mode libvirt (which should otherwise work as
long as the device is pre-bound to vfio-pci and all the device node
permissions are set). Who wants to do vfio device assignment from
session-mode libvirt? gnome-boxes of course! They want to do GPU
passthrough.
ACK as-is.
Thanks!
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] hostdev: skip ACS check when using VFIO for device assignment
On Wed, Aug 26, 2015 at 02:07:32AM -0400, Laine Stump wrote:
The ACS checks are meaningless when using the more modern VFIO driver
for device assignment since VFIO has its own more complete and exact
checks, but I didn't realize that when I added support for VFIO. This
patch eliminates the ACS check when preparing PCI devices for
assignment if VFIO is being used.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1256486
I don't remember how it's possible as neatly as I managed to do
somehow, but I believe git has good support for 'Key: Value' on one
line so that you can then easily see, for example, patches that have
Key == Resolves etc., so I'm using Resolves: link-to-bz, but as I
said, I forgot how to use that formatting neatly (it was almost like
one parameter only). So this is just an FYI, no need to change it.
---
src/util/virhostdev.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 529753c..0bc5120 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -1,6 +1,6 @@
/* virhostdev.c: hostdev management
*
- * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
* Copyright (C) 2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
*
@@ -230,7 +230,6 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr
*hostdevs, int nhostdevs)
virObjectUnref(list);
return NULL;
}
-
if (virPCIDeviceListAdd(list, dev) 0) {
virPCIDeviceFree(dev);
virObjectUnref(list);
@@ -579,7 +578,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr
hostdev_mgr,
struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, dom_name,
usesVfio};
-if (!virPCIDeviceIsAssignable(dev, strict_acs_check)) {
+if (!usesVfio !virPCIDeviceIsAssignable(dev, strict_acs_check)) {
I suppose we could unset the strict check in case of VFIO as that
would work too, but was not meant for this job.
ACK as-is.
virReportError(VIR_ERR_OPERATION_INVALID,
_(PCI device %s is not assignable),
virPCIDeviceGetName(dev));
--
2.1.0
--
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] qemu: Label correct per-VM path when starting
On Tue, Aug 25, 2015 at 11:53:15AM +0200, Martin Kletzander wrote: Commit f1f68ca33433825ce0deed2d96f1990200bc6618 overused mdir_name() event though it was not needed in the latest version, hence labelling directory one level up in the tree and not the one it should. If anyone with SElinux managed to try run a domain with guest agent set up, it's highly possible that they will need to run 'restorecon -F /var/lib/libvirt/qemu/channel/target' to fix what was done. Signed-off-by: Martin Kletzander mklet...@redhat.com --- src/qemu/qemu_process.c | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) ACK Pavel -- 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 Wed, Aug 26, 2015 at 10:19:43AM +0300, Nikolay Shirokovskiy wrote: On 25.08.2015 20:28, Dmitry Guryanov wrote: On 08/25/2015 07:18 PM, Daniel P. Berrange wrote: 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. I'm trying to understand why you need $STUB as dummy data. IIRC, when doing direct migration, you should be providing a valid URI for the first parameter, and not need the second uri. virsh uses virDomainMigrateToURI3 function, and for some reason it differs destination URI for direct and peer-to-peer migrations. For p2p it uses pconnuri argument and for direct -VIR_MIGRATE_PARAM_URI http://libvirt.org/html/libvirt-libvirt-domain.html#VIR_MIGRATE_PARAM_URI from typed params list. AFAIU in virDomainMigrateToURI3 function, dconnuri argument is treated as URI to destination libvirt daemon. As direct migration doesn't need this kind of connection this argument is ignored. However as suggested vz migration implementation uses connection to destination libvirtd daemon to get destination hypervisor session uuid I should probably use dconnuri for for that purporse not hypervisor URI. This is related what Dmitry mentioned in previous letters where there was a confusion between hypervisor and remote access to daemon ports. This would break a invariant mentioned in the documentation that direct migration does not use dconnuri. Probably it is not too bad too get away from this restriction. Before you change the code yet again, I'll take a closer look at the migration code and try to figure out a clear correct answer. This bit of libvirt is so confusing it is easy to get wrong 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] Fix link to page for Virtuozzo driver
On 08/26/2015 07:34 AM, Sergey Bronnikov wrote: --- docs/index.html.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Ugh... missed that ;-) ACK and pushed. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] lxc: ensure setns() syscall is defined
Older versions of glibc don't provide the setns() syscall
function wrapper, so we must define it ourselves to prevent
build failure on old distros.
Signed-off-by: Daniel P. Berrange berra...@redhat.com
Pushed as a RHEL-6 build-break fix
---
src/lxc/lxc_container.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 8011ed0..feb8fad 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -95,6 +95,40 @@ VIR_LOG_INIT(lxc.lxc_container);
# define CLONE_NEWNET 0x4000 /* New network namespace */
#endif
+/*
+ * Workaround older glibc. While kernel may support the setns
+ * syscall, the glibc wrapper might not exist. If that's the
+ * case, use our own.
+ */
+#ifndef __NR_setns
+# if defined(__x86_64__)
+# define __NR_setns 308
+# elif defined(__i386__)
+# define __NR_setns 346
+# elif defined(__arm__)
+# define __NR_setns 375
+# elif defined(__aarch64__)
+# define __NR_setns 375
+# elif defined(__powerpc__)
+# define __NR_setns 350
+# elif defined(__s390__)
+# define __NR_setns 339
+# endif
+#endif
+
+#ifndef HAVE_SETNS
+# if defined(__NR_setns)
+# include sys/syscall.h
+
+static inline int setns(int fd, int nstype)
+{
+return syscall(__NR_setns, fd, nstype);
+}
+# else /* !__NR_setns */
+# error Please determine the syscall number for setns on your architecture
+# endif
+#endif
+
/* messages between parent and container */
typedef char lxc_message_t;
#define LXC_CONTINUE_MSG 'c'
--
2.4.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] libvirt_lxc: Claim success for --help
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; } } -- 2.4.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] lxc_container: Turn lxcAttachNS into calling virProcessSetNamespaces
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;
}
--
2.4.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/4] Couple of LXC fixes
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. 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(-) -- 2.4.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] util: Allow virProcessSetNamespaces() to have sparse FD list
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
--
2.4.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] Revert lxc: ensure setns() syscall is defined
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(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index eb7cad6..b621d1d 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -95,40 +95,6 @@ VIR_LOG_INIT(lxc.lxc_container);
# define CLONE_NEWNET 0x4000 /* New network namespace */
#endif
-/*
- * Workaround older glibc. While kernel may support the setns
- * syscall, the glibc wrapper might not exist. If that's the
- * case, use our own.
- */
-#ifndef __NR_setns
-# if defined(__x86_64__)
-# define __NR_setns 308
-# elif defined(__i386__)
-# define __NR_setns 346
-# elif defined(__arm__)
-# define __NR_setns 375
-# elif defined(__aarch64__)
-# define __NR_setns 375
-# elif defined(__powerpc__)
-# define __NR_setns 350
-# elif defined(__s390__)
-# define __NR_setns 339
-# endif
-#endif
-
-#ifndef HAVE_SETNS
-# if defined(__NR_setns)
-# include sys/syscall.h
-
-static inline int setns(int fd, int nstype)
-{
-return syscall(__NR_setns, fd, nstype);
-}
-# else /* !__NR_setns */
-# error Please determine the syscall number for setns on your architecture
-# endif
-#endif
-
/* messages between parent and container */
typedef char lxc_message_t;
#define LXC_CONTINUE_MSG 'c'
--
2.4.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [python PATCH] examples: small fix for nodestats.py example
On 08/27/2015 06:06 AM, John Ferlan wrote: On 07/30/2015 10:19 PM, Luyao Huang wrote: Add nodestats.py in MANIFEST.in and add a small description for nodestats.py in README Signed-off-by: Luyao Huang lhu...@redhat.com --- MANIFEST.in | 1 + examples/README | 3 +++ 2 files changed, 4 insertions(+) ACK and pushed. Thanks a lot for your review. John Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 0/2] qemu: fix the audit log is not correct for memory device
On 08/27/2015 05:54 AM, John Ferlan wrote: On 08/13/2015 10:15 AM, Luyao Huang wrote: First review: http://www.redhat.com/archives/libvir-list/2015-July/msg00982.html Change in v2: - split it to two patches - fix some small mistake in hot-unplug part I change the code in qemuDomainAttachMemory like what we do in other attach deivce functions. And I have removed the jump in qemuDomainRemoveMemoryDevice, since i just see only one place need jump to audit, so i refactor the code (thanks John's advise). Luyao Huang (2): qemu: fix the audit log is not correct after hot-plug memory success qemu: fix the audit log is not correct after hot-unplug memory device src/qemu/qemu_hotplug.c | 35 +-- 1 file changed, 17 insertions(+), 18 deletions(-) These look much cleaner code wise - had to clean up the commit messages, but otherwise fine. I have pushed the two patches. Thanks a lot for your review and help. John Luyao -- 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 26.08.2015 21:05, 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(-) ACK. But don't forget that the counterpart is broken too. Once you destroy network, on RHEL-6, you can't start it up, because of the same problem. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virt-aa-helper: Improve valid_path
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:
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)
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))
+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
[libvirt] Entering freeze for libvirt-1.2.19
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/ 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
Re: [libvirt] [PATCH 2/2 (v3)] util: fallback to ioctl(SIOCBRDELBR) if netlink RTM_DELLINK fails
On 08/26/2015 09:24 PM, Michal Privoznik wrote: On 26.08.2015 21:05, 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(-) ACK. But don't forget that the counterpart is broken too. Once you destroy network, on RHEL-6, you can't start it up, because of the same problem. Yeah, that was patch 1/2 (which hasn't been reviewed yet (hint hint :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
