[libvirt] [PATCH] qemu: don't munge user input during block commit
by Eric Blake
While investigating https://bugzilla.redhat.com/show_bug.cgi?id=1061827
I noticed that we pass user input unscathed for block-pull, but
always pass a canonical absolute name through for block-commit.
[Note that we probably _ought_ to validate that the user's request
for block-pull actually matches the backing chain, the way we already
do for block-commit - but that's a separate issue. Further note that
the ability to pass user input through unscathed allows backdoors
such as specifying a backing image that is a network URI such as
a gluster disk, instead of forcing things to the local file system;
which is an area still under active investigation on whether libvirt
needs to behave differently for network disks.]
Since qemu may write the name that the user passed in as the backing
file, a user may have a reason to want a relative file name passed
through to qemu, and always munging things to absolute prevents that.
Put another way, if you have the backing chain:
[A] <- [B(back=./A)] <- [C(back=./B)]
and commit B into A (virsh blockcommit $dom vda --base A --top B),
the metadata of C will have to be re-written. But should it be
rewritten as [C(back=./A)] or as [C(back=/path/to/A)]? Still up in
the air is whether qemu's decision should be based on whether B
and/or C had relative paths, or on whether the --base and/or
--top arguments to the command were relative paths; but if we always
pass a canonical name, we've prevented the spelling of the command
arguments from being part of the hueristics that qemu uses.
I also audited the code, and verified that we never call
qemuMonitorBlockCommit() with a NULL base, either before or after
the change to qemu_driver.c.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Preserve user's
spelling, since absolute vs. relative matters to qemu.
* src/qemu/qemu_monitor.h (qemuMonitorBlockCommit): Base is never
null.
* src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I was _hoping_ that this would solve the mentioned bugzilla. But
even with this applied, qemu still ended up writing an absolute
backing file name into the active file in the backing chain, so that
bug has been reassigned back to qemu - it probably has to do with
the fact that libvirt always spawns qemu with -drive pointing to
an absolute name, which is unrelated to what this patch fixes.
Therefore, I'm a little bit hesitant to apply this patch, but
wanted to post it for review anyway.
src/qemu/qemu_driver.c | 11 ++++++++---
src/qemu/qemu_monitor.c | 4 ++--
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 2 +-
src/qemu/qemu_monitor_json.h | 5 +++--
5 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9aad2dc..31adf78 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15383,10 +15383,15 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
VIR_DISK_CHAIN_READ_WRITE) < 0))
goto endjob;
- /* Start the commit operation. */
+ /* Start the commit operation. Pass the user's original spelling,
+ * if any, through to qemu, since qemu behaves differently
+ * depending on whether the input was specified as relative or
+ * absolute (that is, our absolute top_canon may do the wrong
+ * thing if the user specified a name). */
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorBlockCommit(priv->mon, device, top_canon, base_canon,
- bandwidth);
+ ret = qemuMonitorBlockCommit(priv->mon, device,
+ top ? top : top_canon,
+ base ? base : base_canon, bandwidth);
qemuDomainObjExitMonitor(driver, vm);
endjob:
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index a2769db..e4707b7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1,7 +1,7 @@
/*
* qemu_monitor.c: interaction with QEMU monitor console
*
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -3188,7 +3188,7 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
unsigned long long speed;
VIR_DEBUG("mon=%p, device=%s, top=%s, base=%s, bandwidth=%ld",
- mon, device, NULLSTR(top), NULLSTR(base), bandwidth);
+ mon, device, top, base, bandwidth);
/* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
* limited to LLONG_MAX also for unsigned values */
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index eabf000..7bb465b 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -647,7 +647,8 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
const char *top,
const char *base,
unsigned long bandwidth)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4);
int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
const char *cmd,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index ee3ae15..40d6d1a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3307,7 +3307,7 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
"s:device", device,
"U:speed", speed,
"s:top", top,
- base ? "s:base" : NULL, base,
+ "s:base", base,
NULL);
if (!cmd)
return -1;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index a93c51e..ef71588 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -1,7 +1,7 @@
/*
* qemu_monitor_json.h: interaction with QEMU monitor console
*
- * Copyright (C) 2006-2009, 2011-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2009, 2011-2014 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -258,7 +258,8 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
const char *top,
const char *base,
unsigned long long bandwidth)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4);
int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
const char *cmd_str,
--
1.8.5.3
11 years, 1 month
[libvirt] [PATCH] iptables: don't log command probe failures
by Eric Blake
Commit b9dd878f caused a regression in iptables interaction by
logging non-zero status at a higher level than VIR_INFO. Revert
that portion of the commit, as well as adding a comment explaining
why we check the status ourselves.
Reported by Nehal J Wani.
* src/util/viriptables.c (virIpTablesOnceInit): Undo log regression.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/util/viriptables.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 9e03cc4..45f7789 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -60,6 +60,7 @@ static int
virIpTablesOnceInit(void)
{
virCommandPtr cmd;
+ int status;
#if HAVE_FIREWALLD
firewall_cmd_path = virFindFileInPath("firewall-cmd");
@@ -70,7 +71,8 @@ virIpTablesOnceInit(void)
cmd = virCommandNew(firewall_cmd_path);
virCommandAddArgList(cmd, "--state", NULL);
- if (virCommandRun(cmd, NULL) < 0) {
+ /* don't log non-zero status */
+ if (virCommandRun(cmd, &status) < 0 || status != 0) {
VIR_INFO("firewall-cmd found but disabled for iptables");
VIR_FREE(firewall_cmd_path);
firewall_cmd_path = NULL;
@@ -87,7 +89,8 @@ virIpTablesOnceInit(void)
cmd = virCommandNew(IPTABLES_PATH);
virCommandAddArgList(cmd, "-w", "-L", "-n", NULL);
- if (virCommandRun(cmd, NULL) < 0) {
+ /* don't log non-zero status */
+ if (virCommandRun(cmd, &status) < 0 || status != 0) {
VIR_INFO("xtables locking not supported by your iptables");
} else {
VIR_INFO("using xtables locking for iptables");
--
1.8.5.3
11 years, 1 month
[libvirt] [PATCH] build: work around FreeBSD stdlib.h bug
by Eric Blake
POSIX requires that <stdlib.h> expose WIFEXITED and friends,
but FreeBSD and others fail to comply. We can work around it
manually by including <sys/wait.h>, or we can work around it
automatically by using gnulib's system-posix module.
* bootstrap.conf (gnulib_modules): Add system-posix.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I'll wait for a review on this one - we don't use system(),
and it feels a little bit odd to list the system() module
merely for its side effect of a fixed <stdlib.h>. The alternative
is to continue using <sys/wait.h> everywhere that we use
WIFEXITED and friends.
bootstrap.conf | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index d24a714..bc6ce80 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -1,6 +1,6 @@
# Bootstrap configuration.
-# Copyright (C) 2010-2013 Red Hat, Inc.
+# Copyright (C) 2010-2014 Red Hat, Inc.
# This library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
@@ -112,6 +112,7 @@ strerror_r-posix
strptime
strsep
strtok_r
+system-posix
sys_stat
sys_wait
termios
--
1.8.5.3
11 years, 1 month
[libvirt] [PATCH v3] nwfilter: Deactivate iptables MAC address check where needed
by Stefan Berger
From: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
Recent Linux iptables (3.11.7) refuses to create iptables MAC address
check rules using -m mac --mac-source <addr> where previous versions
still allowed it. So we now need to deactivate the filtering rules for
when the incoming traffic is filtered before it is sent into the VM.
Those are typically the chains that start with FO-* or start with FP-*
when they are being built.
Adapt the documentation to reflect the fact that srcmacaddr, when
used in iptables rules, should be regarded as deprecated due to the
above mentioned problems.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/formatnwfilter.html.in | 54 +++++++++++--------------------
src/nwfilter/nwfilter_ebiptables_driver.c | 29 ++++++++++-------
2 files changed, 36 insertions(+), 47 deletions(-)
diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in
index 4b95fce..5c06bf2 100644
--- a/docs/formatnwfilter.html.in
+++ b/docs/formatnwfilter.html.in
@@ -1209,7 +1209,9 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
+ <td>MAC address of sender; this option is deprecated and
+ MAC address filtering using for example the mac protocol
+ above should be used</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1320,22 +1322,9 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
- </tr>
- <tr>
- <td>srcmacmask</td>
- <td>MAC_MASK</td>
- <td>Mask applied to MAC address of sender</td>
- </tr>
- <tr>
- <td>dstmacaddr</td>
- <td>MAC_ADDR</td>
- <td>MAC address of destination</td>
- </tr>
- <tr>
- <td>dstmacmask</td>
- <td>MAC_MASK</td>
- <td>Mask applied to MAC address of destination</td>
+ <td>MAC address of sender; this option is deprecated and
+ MAC address filtering using for example the mac protocol
+ above should be used</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1429,22 +1418,9 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
- </tr>
- <tr>
- <td>srcmacmask</td>
- <td>MAC_MASK</td>
- <td>Mask applied to MAC address of sender</td>
- </tr>
- <tr>
- <td>dstmacaddr</td>
- <td>MAC_ADDR</td>
- <td>MAC address of destination</td>
- </tr>
- <tr>
- <td>dstmacmask</td>
- <td>MAC_MASK</td>
- <td>Mask applied to MAC address of destination</td>
+ <td>MAC address of sender; this option is deprecated and
+ MAC address filtering using for example the mac protocol
+ above should be used</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1529,7 +1505,9 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
+ <td>MAC address of sender; this option is deprecated and
+ MAC address filtering using for example the mac protocol
+ above should be used</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1640,7 +1618,9 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
+ <td>MAC address of sender; this option is deprecated and
+ MAC address filtering using for example the mac protocol
+ above should be used</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1735,7 +1715,9 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
+ <td>MAC address of sender; this option is deprecated and
+ MAC address filtering using for example the mac protocol
+ above should be used</td>
</tr>
<tr>
<td>srcipaddr</td>
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 352c08f..bea9535 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -972,7 +972,7 @@ static int
iptablesHandleSrcMacAddr(virBufferPtr buf,
virNWFilterVarCombIterPtr vars,
nwItemDescPtr srcMacAddr,
- bool directionIn,
+ const char *chain, bool directionIn,
bool *srcmacskipped)
{
char macaddr[VIR_MAC_STRING_BUFLEN];
@@ -984,6 +984,14 @@ iptablesHandleSrcMacAddr(virBufferPtr buf,
return 0;
}
+ /* recent Linux iptables does not allow this filtering rule to be
+ * applied to the FP-/FO- chains
+ */
+ if (chain[1] == CHAINPREFIX_HOST_OUT_TEMP) {
+ *srcmacskipped = true;
+ return 0;
+ }
+
if (printDataType(vars,
macaddr, sizeof(macaddr),
srcMacAddr) < 0)
@@ -1366,7 +1374,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.tcpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1421,7 +1429,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.udpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1454,7 +1462,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.udpliteHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1482,7 +1490,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.espHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1506,11 +1514,10 @@ _iptablesCreateRuleInstance(bool directionIn,
virBufferAddLit(&buf, " -p ah");
bufUsed = virBufferUse(&buf);
-
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.ahHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1538,7 +1545,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.sctpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1574,7 +1581,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.icmpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1636,7 +1643,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.igmpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1664,7 +1671,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.allHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
--
1.8.1.4
11 years, 1 month
[libvirt] Get disk size for disk type="network"
by Qiang Fu
Hi,
In the output of "virsh dumpxml" command the disk section is as follows:
<disk type='network' device='disk'>
<driver name='qemu' type='raw' cache='writeback'/>
<auth username='svl-cc1-cinder-user'>
<secret type='ceph' uuid='e80afa94-a64c-486c-9e34-d55e85f26406'/>
</auth>
<source protocol='rbd'
name='svl-cc1-cinder-volume-1/volume-0a4cdeab-863d-4394-845f-008b993bc4e9'>
<host name='10.34.114.211' port='6789'/>
<host name='10.34.114.212' port='6789'/>
<host name='10.34.114.213' port='6789'/>
</source>
<target dev='vda' bus='virtio'/>
<serial>0a4cdeab-863d-4394-845f-008b993bc4e9</serial>
<alias name='virtio-disk0'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04'
function='0x0'/>
</disk>
In this case how can I use libvirt to get the disk/volume size information?
Thanks,
Bruce
11 years, 1 month
[libvirt] [PATCH] libxl: support sexpr in native to XML conversion
by Jim Fehlig
Supporting sexpr in connectDomainXMLFromNative in the libxl driver
adds flexibility for users importing legacy Xen configuration into
libvirt. E.g. this patch allows importing previous xend-managed
domains from /var/lib/xend/domains/<dom-uuid>/config.sxp into the
libvirt libxl driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a79efcb..65d80a2 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -46,6 +46,7 @@
#include "libxl_driver.h"
#include "libxl_conf.h"
#include "xen_xm.h"
+#include "xen_sxpr.h"
#include "virtypedparam.h"
#include "viruri.h"
#include "virstring.h"
@@ -62,6 +63,7 @@
#define LIBXL_DOM_REQ_HALT 4
#define LIBXL_CONFIG_FORMAT_XM "xen-xm"
+#define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr"
/* Number of Xen scheduler parameters */
#define XEN_SCHED_CREDIT_NPARAM 2
@@ -2867,8 +2869,9 @@ libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
}
static char *
-libxlConnectDomainXMLFromNative(virConnectPtr conn, const char * nativeFormat,
- const char * nativeConfig,
+libxlConnectDomainXMLFromNative(virConnectPtr conn,
+ const char *nativeFormat,
+ const char *nativeConfig,
unsigned int flags)
{
libxlDriverPrivatePtr driver = conn->privateData;
@@ -2882,22 +2885,33 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, const char * nativeFormat,
if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0)
goto cleanup;
- if (STRNEQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
+ if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) {
+ if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
+ goto cleanup;
+
+ if (!(def = xenParseXM(conf,
+ cfg->verInfo->xen_version_major,
+ cfg->caps))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("parsing xm config failed"));
+ goto cleanup;
+ }
+ } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_SEXPR)) {
+ /* only support latest xend config format */
+ if (!(def = xenParseSxprString(nativeConfig,
+ XEND_CONFIG_VERSION_3_1_0,
+ NULL,
+ -1))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("parsing sxpr config failed"));
+ goto cleanup;
+ }
+ } else {
virReportError(VIR_ERR_INVALID_ARG,
_("unsupported config type %s"), nativeFormat);
goto cleanup;
}
- if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0)))
- goto cleanup;
-
- if (!(def = xenParseXM(conf,
- cfg->verInfo->xen_version_major,
- cfg->caps))) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("parsing xm config failed"));
- goto cleanup;
- }
-
xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE);
cleanup:
--
1.8.1.4
11 years, 1 month
[libvirt] [PATCH] storage: Fix bugs in VIR_APPEND_ELEMENT series
by John Ferlan
>From commit id 'd53bbfd1'
Found one core and one possible memory leak. Core seen during local
virt-test/tp_libvirt run for the vol_create_from test. The memory leak
was seen by inspection during a review of all VIR_APPEND_ELEMENT changes
In storage_backend_disk/virStorageBackendDiskMakeDataVol(), the 'vol'
needs to be kept around since it's used later, so use the _COPY macro.
This caused a segv in libvirtd:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe87c3700 (LWP 6919)]
virStorageBackendDiskMakeDataVol (vol=0x0, groups=0x7fffc8000d70, pool=0x7fffc8002460) at storage/storage_backend_disk.c:66
66 if (vol->target.path == NULL) {
In storage_backend_rbd/virStorageBackendRBDRefreshPool() there's a failure
path where the 'vol' needs to go through virStorageVolDefFree() since it
wouldn't be appended.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_disk.c | 3 ++-
src/storage/storage_backend_rbd.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index b261773..2b6011b 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -57,7 +57,8 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
*/
tmp = strrchr(groups[0], '/');
if (VIR_STRDUP(vol->name, tmp ? tmp + 1 : groups[0]) < 0 ||
- VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) {
+ VIR_APPEND_ELEMENT_COPY(pool->volumes.objs,
+ pool->volumes.count, vol) < 0) {
virStorageVolDefFree(vol);
return -1;
}
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index bc52474..d78dbe4 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -401,6 +401,7 @@ static int virStorageBackendRBDRefreshPool(virConnectPtr conn,
}
if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) < 0) {
+ virStorageVolDefFree(vol);
virStoragePoolObjClearVols(pool);
goto cleanup;
}
--
1.8.5.3
11 years, 1 month
[libvirt] [PATCH v2] nwfilter: Deactivate iptables MAC address check where needed
by Stefan Berger
From: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
Recent Linux iptables (3.11.7) refuses to create iptables MAC address
check rules using -m mac --mac-source <addr> where previous versions
still allowed it. So we now need to deactivate the filtering rules for
when the incoming traffic is filtered before it is sent into the VM.
Those are typically the chains that start with FO-* or start with FP-*
when they are being built.
Adapt the documentation to reflect the fact that srcmacaddr, when
used in iptables rules, should be regarded as deprecated due to the
above mentioned problems.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/formatnwfilter.html.in | 42 +++++--------------------------
src/nwfilter/nwfilter_ebiptables_driver.c | 29 +++++++++++++--------
2 files changed, 24 insertions(+), 47 deletions(-)
diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in
index 4b95fce..ee23d8e 100644
--- a/docs/formatnwfilter.html.in
+++ b/docs/formatnwfilter.html.in
@@ -1209,7 +1209,7 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
+ <td>MAC address of sender; this option is deprecated</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1320,22 +1320,7 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
- </tr>
- <tr>
- <td>srcmacmask</td>
- <td>MAC_MASK</td>
- <td>Mask applied to MAC address of sender</td>
- </tr>
- <tr>
- <td>dstmacaddr</td>
- <td>MAC_ADDR</td>
- <td>MAC address of destination</td>
- </tr>
- <tr>
- <td>dstmacmask</td>
- <td>MAC_MASK</td>
- <td>Mask applied to MAC address of destination</td>
+ <td>MAC address of sender; this option is deprecated</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1429,22 +1414,7 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
- </tr>
- <tr>
- <td>srcmacmask</td>
- <td>MAC_MASK</td>
- <td>Mask applied to MAC address of sender</td>
- </tr>
- <tr>
- <td>dstmacaddr</td>
- <td>MAC_ADDR</td>
- <td>MAC address of destination</td>
- </tr>
- <tr>
- <td>dstmacmask</td>
- <td>MAC_MASK</td>
- <td>Mask applied to MAC address of destination</td>
+ <td>MAC address of sender; this option is deprecated</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1529,7 +1499,7 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
+ <td>MAC address of sender; this option is deprecated</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1640,7 +1610,7 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
+ <td>MAC address of sender; this option is deprecated</td>
</tr>
<tr>
<td>srcipaddr</td>
@@ -1735,7 +1705,7 @@
<tr>
<td>srcmacaddr</td>
<td>MAC_ADDR</td>
- <td>MAC address of sender</td>
+ <td>MAC address of sender; this option is deprecated</td>
</tr>
<tr>
<td>srcipaddr</td>
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
index 352c08f..ae20bf0 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -972,7 +972,7 @@ static int
iptablesHandleSrcMacAddr(virBufferPtr buf,
virNWFilterVarCombIterPtr vars,
nwItemDescPtr srcMacAddr,
- bool directionIn,
+ const char *chain, bool directionIn,
bool *srcmacskipped)
{
char macaddr[VIR_MAC_STRING_BUFLEN];
@@ -984,6 +984,14 @@ iptablesHandleSrcMacAddr(virBufferPtr buf,
return 0;
}
+ /* recent Linux iptables does not allow this filteirng rule to be
+ * applied to all FO-* chains
+ */
+ if (chain[1] == CHAINPREFIX_HOST_OUT_TEMP) {
+ *srcmacskipped = true;
+ return 0;
+ }
+
if (printDataType(vars,
macaddr, sizeof(macaddr),
srcMacAddr) < 0)
@@ -1366,7 +1374,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.tcpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1421,7 +1429,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.udpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1454,7 +1462,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.udpliteHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1482,7 +1490,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.espHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1506,11 +1514,10 @@ _iptablesCreateRuleInstance(bool directionIn,
virBufferAddLit(&buf, " -p ah");
bufUsed = virBufferUse(&buf);
-
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.ahHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1538,7 +1545,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.sctpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1574,7 +1581,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.icmpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1636,7 +1643,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.igmpHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
@@ -1664,7 +1671,7 @@ _iptablesCreateRuleInstance(bool directionIn,
if (iptablesHandleSrcMacAddr(&buf,
vars,
&rule->p.allHdrFilter.dataSrcMACAddr,
- directionIn,
+ chain, directionIn,
&srcMacSkipped) < 0)
goto err_exit;
--
1.8.1.4
11 years, 1 month
[libvirt] Google Summer of Code 2014 - Rewriting Virtual-Box Driver
by prashanth sunder
Hello,
I am applying for the project idea "Rewriting Virtual-Box Driver" suggested
by Michal Privoznik(mprivozn(a)redhat.com) in the "Qemu" ideas page (
http://wiki.qemu.org/Google_Summer_of_Code_2014).
In the short span of time that I had after my discussion with Mr.
Privoznik, the current Virtual Box driver code is not structured very
well(lot of #ifdef's in the current code) and very hard to read.
Since libvirt has to support all versions of Virtual Box, the current code
decides on what code to compile based on the value of the macro
VBOX_API_VERSION
that is checked during ./configure.
The only thing after discussion that we were able to come up with is
putting
version specific code under particular files and only compile the those
necessary
files during "make"
For instance,
if a function X() supports version A, B, C we will have function X() in
vbox_A.c just for version A, function X() in vbox_B.c just for version B
and so on.
Any other suggestions than the above mentioned one which
can be used to fix the problem??
Regards,
Prashanth Sunder
11 years, 1 month
[libvirt] [PATCH] Fix sec label setup when attaching to QEMU processes
by Daniel P. Berrange
When attaching to a QEMU process, the def->seclabels array is
going to be empty. The qemuProcessAttach method must thus
populate it with data for the security drivers.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/qemu/qemu_process.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1f00840..7d41c93 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4479,6 +4479,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
virDomainPausedReason reason;
virSecurityLabelPtr seclabel = NULL;
virSecurityLabelDefPtr seclabeldef = NULL;
+ bool seclabelgen = false;
virSecurityManagerPtr* sec_managers = NULL;
const char *model;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
@@ -4529,10 +4530,16 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
goto error;
for (i = 0; sec_managers[i]; i++) {
+ seclabelgen = false;
+ VIR_ERROR("Iter %zu", i);
model = virSecurityManagerGetModel(sec_managers[i]);
seclabeldef = virDomainDefGetSecurityLabelDef(vm->def, model);
- if (seclabeldef == NULL)
- goto error;
+ VIR_ERROR("model %s def %p", model, seclabeldef);
+ if (seclabeldef == NULL) {
+ if (!(seclabeldef = virDomainDefGenSecurityLabelDef(model)))
+ goto error;
+ seclabelgen = true;
+ }
seclabeldef->type = VIR_DOMAIN_SECLABEL_STATIC;
if (VIR_ALLOC(seclabel) < 0)
goto error;
@@ -4546,6 +4553,12 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED,
if (VIR_STRDUP(seclabeldef->label, seclabel->label) < 0)
goto error;
VIR_FREE(seclabel);
+
+ if (seclabelgen) {
+ if (VIR_APPEND_ELEMENT(vm->def->seclabels, vm->def->nseclabels, seclabeldef) < 0)
+ goto error;
+ seclabelgen = false;
+ }
}
VIR_DEBUG("Creating domain log file");
@@ -4692,6 +4705,8 @@ error:
VIR_FORCE_CLOSE(logfile);
VIR_FREE(seclabel);
VIR_FREE(sec_managers);
+ if (seclabelgen)
+ virSecurityLabelDefFree(seclabeldef);
virDomainChrSourceDefFree(monConfig);
virObjectUnref(cfg);
virObjectUnref(caps);
--
1.8.5.3
11 years, 1 month
