[libvirt] RFC: extending domcapabilities XML
by Cole Robinson
Hi all,
There's a couple qemu bits I want to expose in domcapabilities XML, but there
isn't an existing pattern to follow so I'd like to get some feedback first.
Hopefully it can serve as a template for future domcapabilities extensions
1) USB controller models
Not strictly important for my needs, but this is an incremental step. Do we do
some kind of nested XML like
<controller supported='yes'>
<type name='usb'/>
<enum name='model'>
<value>nec-xhci</value>
...
</enum>
</type>
</controller>
Or come up with a unique device-level enum name to track this, like we do for
hostdev subsysType:
<controller supported='yes'>
<enum name='usbModel'>
<value>nec-xhci</value>
...
</enum>
</controller>
2) new usb ports= attribute
This is for the patches I just sent. There's two questions here: how does XML
look like for a non-enum? Maybe:
<controller supported='yes'>
<parameter name='ports' supported='yes'/>
</controller>
(or maybe <option .../>)
However, ports is only supported for type=usb model=nec-xhci. Do we represent
that? If so what does the XML look like? Do we go crazy nested like:
<controller supported='yes'>
<parameter name='type' value='usb'>
<parameter name='model' value='nec-xhci'/>
<parameter name='ports' supported='yes'/>
</parameter>
</parameter>
</controller>
But that's kind of intense. It gives the illusion that we can write a parser
to automatically handle additions for introspecting what libvirt supports, but
in practice I don't know if that's ever going to work. Instead I think it's
better to invent new top level strings to represent these types of cases,
rather than try to encode an infinite level of dependency in the XML hierarchy.
<controller supported='yes'>
<capability name='usbXHCIPorts' supported='yes'/>
</controller>
The question then is if the <capability> element is the proper name. Could be
<feature> too but that's quite overloaded in libvirt XML.
3) spice gl
whether spice gl is supported or not is a factor of libvirt version, qemu
version, and spice version that qemu was compiled against, which is a lot of
pieces to track. So exposing it over domcapabilities will be nice. Something like:
<graphics supported='yes'/>
<enum name='spiceGL'>
<value>off</value>
<value>on</value>
</enum>
</graphics>
Similarly we want to know that virtio-vga has virgl support via <acceleration
accel3d='yes'/>
<video supported='yes'/>
<enum name='virtioVGAAccel3D'>
<value>yes</value>
<value>no</value>
</enum>
</video>
Comments welcome.
Thanks,
Cole
8 years, 7 months
[libvirt] [PATCH] rpc: Don't leak fd via CreateXMLWithFiles
by Cole Robinson
From: Ben Gray <ben.r.gray(a)gmail.com>
FD passing APIs like CreateXMLWithFiles or OpenGraphicsFD will leak
file descriptors. The user passes in an fd, which is dup()'d in
virNetClientProgramCall. The new fd is what is transfered to the
server virNetClientIOWriteMessage.
Once all the fds have been written though, the parent msg->fds list
is immediately free'd, so the individual fds are never closed.
This closes each FD as its send to the server, so all fds have been
closed by the time msg->fds is free'd.
https://bugzilla.redhat.com/show_bug.cgi?id=1159766
Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
---
Ben's patch, my comments
src/rpc/virnetclient.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 781e74c..d8ed15b 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1177,20 +1177,21 @@ virNetClientIOWriteMessage(virNetClientPtr client,
if (thecall->msg->bufferOffset == thecall->msg->bufferLength) {
size_t i;
for (i = thecall->msg->donefds; i < thecall->msg->nfds; i++) {
int rv;
if ((rv = virNetSocketSendFD(client->sock, thecall->msg->fds[i])) < 0)
return -1;
if (rv == 0) /* Blocking */
return 0;
thecall->msg->donefds++;
+ VIR_FORCE_CLOSE(thecall->msg->fds[i]);
}
thecall->msg->donefds = 0;
thecall->msg->bufferOffset = thecall->msg->bufferLength = 0;
VIR_FREE(thecall->msg->fds);
VIR_FREE(thecall->msg->buffer);
if (thecall->expectReply)
thecall->mode = VIR_NET_CLIENT_MODE_WAIT_RX;
else
thecall->mode = VIR_NET_CLIENT_MODE_COMPLETE;
}
--
2.7.3
8 years, 7 months
[libvirt] [PATCH 0/3] conf: reject '/' in object names
by Cole Robinson
An embedded '/' in object names doesn't really work for any
of our stateful drivers, so let's explicitly reject it.
https://bugzilla.redhat.com/show_bug.cgi?id=639923
https://bugzilla.redhat.com/show_bug.cgi?id=787604
Cole Robinson (3):
conf: domain: reject name containing '/'
conf: network: reject name containing '/'
conf: storage: pool: reject name containing '/'
src/conf/domain_conf.c | 8 ++++++++
src/conf/domain_conf.h | 1 +
src/conf/network_conf.c | 6 ++++++
src/conf/storage_conf.c | 6 ++++++
src/openvz/openvz_driver.c | 5 +++--
src/phyp/phyp_driver.c | 1 +
src/vbox/vbox_common.c | 1 +
src/vmx/vmx.c | 3 ++-
src/xenapi/xenapi_driver.c | 1 +
tests/genericxml2xmlindata/generic-name-slash-fail.xml | 17 +++++++++++++++++
tests/genericxml2xmltest.c | 3 +++
11 files changed, 49 insertions(+), 3 deletions(-)
create mode 100644 tests/genericxml2xmlindata/generic-name-slash-fail.xml
--
2.7.4
8 years, 7 months
[libvirt] [PATCH] conf: format runtime DAC seclabel, unless MIGRATABLE
by Cole Robinson
We historically format runtime seclabel selinux/apparmor values,
however we skip formatting runtime DAC values. This was added in
commit 990e46c4542349f838e001d30638872576c389e9
Author: Marcelo Cerri <mhcerri(a)linux.vnet.ibm.com>
Date: Fri Aug 31 13:40:41 2012 +0200
conf: Avoid formatting auto-generated DAC labels
to maintain migration compatibility with libvirt < 0.10.0.
However the formatting was skipped unconditionally. Instead only
skip formatting in the VIR_DOMAIN_DEF_FORMAT_MIGRATABLE case.
https://bugzilla.redhat.com/show_bug.cgi?id=1215833
---
src/conf/domain_conf.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index db567f5..0557912 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18741,7 +18741,8 @@ virDomainEventActionDefFormat(virBufferPtr buf,
static void
virSecurityLabelDefFormat(virBufferPtr buf,
- virSecurityLabelDefPtr def)
+ virSecurityLabelDefPtr def,
+ unsigned int flags)
{
const char *sectype = virDomainSeclabelTypeToString(def->type);
@@ -18751,11 +18752,13 @@ virSecurityLabelDefFormat(virBufferPtr buf,
if (def->type == VIR_DOMAIN_SECLABEL_DEFAULT)
return;
- /* To avoid backward compatibility issues, suppress DAC and 'none' labels
- * that are automatically generated.
+ /* libvirt versions prior to 0.10.0 support just a single seclabel element
+ * in the XML, and that would typically be filled with type=selinux.
+ * Don't format it in the MIGRATABLE case, for backwards compatibility
*/
if ((STREQ_NULLABLE(def->model, "dac") ||
- STREQ_NULLABLE(def->model, "none")) && def->implicit)
+ STREQ_NULLABLE(def->model, "none")) && def->implicit &&
+ (flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE))
return;
virBufferAsprintf(buf, "<seclabel type='%s'",
@@ -22886,7 +22889,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAddLit(buf, "</devices>\n");
for (n = 0; n < def->nseclabels; n++)
- virSecurityLabelDefFormat(buf, def->seclabels[n]);
+ virSecurityLabelDefFormat(buf, def->seclabels[n], flags);
if (def->namespaceData && def->ns.format) {
if ((def->ns.format)(buf, def->namespaceData) < 0)
--
2.7.3
8 years, 7 months
[libvirt] [PATCHv2] conf: log error when incorrect PCI root controller is added to domain
by Laine Stump
libvirt may automatically add a pci-root or pcie-root controller to a
domain, depending on the arch/machinetype, and it hopefully always
makes the right decision about which to add (since in all cases these
controllers are an implicit part of the virtual machine).
But it's always possible that someone will create a config that
explicitly supplies the wrong type of PCI controller for the selected
machinetype. In the past that would lead to an error later when
libvirt was trying to assign addresses to other devices, for example:
XML error: PCI bus is not compatible with the device at
0000:00:02.0. Device requires a PCI Express slot, which is not
provided by bus 0000:00
(that's the error message that appears if you replace the pcie-root
controller in a Q35 domain with a pci-root controller).
This patch adds a check at the same place that the implicit
controllers are added (to ensure that the same logic is used to check
which type of pci root is correct). If a pci controller with index='0'
is already present, we verify that it is of the model that we would
have otherwise added automatically; if not, an error is logged:
The PCI controller with index='0' must be " model='pcie-root' for
this machine type, " but model='pci-root' was found instead.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1004602
---
Change from V1 - add test cases per Cole's suggestion (one for 440fx,
one for Q35)
src/qemu/qemu_domain.c | 47 ++++++++++++++++------
.../qemuxml2argv-440fx-wrong-root.xml | 28 +++++++++++++
.../qemuxml2argv-q35-wrong-root.xml | 27 +++++++++++++
tests/qemuxml2argvtest.c | 9 +++++
4 files changed, 99 insertions(+), 12 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 51d4830..86b7d13 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1437,6 +1437,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
{
bool addDefaultUSB = true;
int usbModel = -1; /* "default for machinetype" */
+ int pciRoot; /* index within def->controllers */
bool addImplicitSATA = false;
bool addPCIRoot = false;
bool addPCIeRoot = false;
@@ -1538,14 +1539,26 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
def, VIR_DOMAIN_CONTROLLER_TYPE_SATA, 0, -1) < 0)
goto cleanup;
+ pciRoot = virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0);
+
/* NB: any machine that sets addPCIRoot to true must also return
* true from the function qemuDomainSupportsPCI().
*/
- if (addPCIRoot &&
- virDomainDefMaybeAddController(
- def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
- VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
- goto cleanup;
+ if (addPCIRoot) {
+ if (pciRoot >= 0) {
+ if (def->controllers[pciRoot]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("The PCI controller with index='0' must be "
+ "model='pci-root' for this machine type, "
+ "but model='%s' was found instead"),
+ virDomainControllerModelPCITypeToString(def->controllers[pciRoot]->model));
+ goto cleanup;
+ }
+ } else if (!virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)) {
+ goto cleanup;
+ }
+ }
/* When a machine has a pcie-root, make sure that there is always
* a dmi-to-pci-bridge controller added as bus 1, and a pci-bridge
@@ -1555,15 +1568,25 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
* true from the function qemuDomainSupportsPCI().
*/
if (addPCIeRoot) {
+ if (pciRoot >= 0) {
+ if (def->controllers[pciRoot]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("The PCI controller with index='0' must be "
+ "model='pcie-root' for this machine type, "
+ "but model='%s' was found instead"),
+ virDomainControllerModelPCITypeToString(def->controllers[pciRoot]->model));
+ goto cleanup;
+ }
+ } else if (!virDomainDefAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) {
+ goto cleanup;
+ }
if (virDomainDefMaybeAddController(
- def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
- VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0 ||
- virDomainDefMaybeAddController(
- def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1,
- VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 ||
+ def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1,
+ VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 ||
virDomainDefMaybeAddController(
- def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2,
- VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0) {
+ def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0) {
goto cleanup;
}
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml
new file mode 100644
index 0000000..93fef08
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-440fx-wrong-root.xml
@@ -0,0 +1,28 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <title>A description of the test machine.</title>
+ <description>
+ A test of qemu's minimal configuration.
+ This test also tests the description and title elements.
+ </description>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-kvm</emulator>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pcie-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml
new file mode 100644
index 0000000..836de52
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35-wrong-root.xml
@@ -0,0 +1,27 @@
+<domain type='qemu'>
+ <name>q35-test</name>
+ <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
+ <memory unit='KiB'>2097152</memory>
+ <currentMemory unit='KiB'>2097152</currentMemory>
+ <vcpu placement='static' cpuset='0-1'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-kvm</emulator>
+ <controller type='pci' index='0' model='pci-root'/>
+ <controller type='pci' index='1' model='dmi-to-pci-bridge'>
+ <model name='i82801b11-bridge'/>
+ </controller>
+ <controller type='pci' index='2' model='pci-bridge'>
+ <model name='pci-bridge'/>
+ <target chassisNr='56'/>
+ </controller>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8842b2f..b43f03c 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1536,6 +1536,15 @@ mymain(void)
QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+ DO_TEST_PARSE_ERROR("q35-wrong-root",
+ QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_ICH9_USB_EHCI1,
+ QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+ QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+ DO_TEST_PARSE_ERROR("440fx-wrong-root", NONE);
+
DO_TEST_FAILURE("pcie-root-port-too-many",
QEMU_CAPS_DEVICE_PCI_BRIDGE,
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
--
2.5.5
8 years, 7 months
[libvirt] [PATCH v2] configure: Remove build time checks for (ip|ip6|eb)tables
by Cole Robinson
And the 'ip' tool. There isn't much benefit to checking this at
configure time when we have infrastructure nowadays for looking up
binaries in the PATH
---
v2:
Keep the virFileIsExecutable check
configure.ac | 12 ------
src/util/virfirewall.c | 18 +++++----
src/util/virnetdev.c | 6 +--
tests/virfirewalltest.c | 98 ++++++++++++++++++++++++-------------------------
4 files changed, 62 insertions(+), 72 deletions(-)
diff --git a/configure.ac b/configure.ac
index de5f430..35ae16e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -694,18 +694,6 @@ if test x"$with_rhel5_api" = x"yes"; then
AC_DEFINE([WITH_RHEL5_API], [1], [whether building for the RHEL-5 API])
fi
-AC_PATH_PROG([IP_PATH], [ip], /sbin/ip, [/usr/sbin:$PATH])
-AC_DEFINE_UNQUOTED([IP_PATH], "$IP_PATH", [path to ip binary])
-
-AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH])
-AC_DEFINE_UNQUOTED([IPTABLES_PATH], "$IPTABLES_PATH", [path to iptables binary])
-
-AC_PATH_PROG([IP6TABLES_PATH], [ip6tables], /sbin/ip6tables, [/usr/sbin:$PATH])
-AC_DEFINE_UNQUOTED([IP6TABLES_PATH], "$IP6TABLES_PATH", [path to ip6tables binary])
-
-AC_PATH_PROG([EBTABLES_PATH], [ebtables], /sbin/ebtables, [/usr/sbin:$PATH])
-AC_DEFINE_UNQUOTED([EBTABLES_PATH], "$EBTABLES_PATH", [path to ebtables binary])
-
dnl
dnl Checks for the OpenVZ driver
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index f26fd86..0c8e3bf 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -47,9 +47,9 @@ typedef virFirewallGroup *virFirewallGroupPtr;
VIR_ENUM_DECL(virFirewallLayerCommand)
VIR_ENUM_IMPL(virFirewallLayerCommand, VIR_FIREWALL_LAYER_LAST,
- EBTABLES_PATH,
- IPTABLES_PATH,
- IP6TABLES_PATH);
+ "ebtables",
+ "iptables",
+ "ip6tables");
VIR_ENUM_DECL(virFirewallLayerFirewallD)
VIR_ENUM_IMPL(virFirewallLayerFirewallD, VIR_FIREWALL_LAYER_LAST,
@@ -134,13 +134,13 @@ static void
virFirewallCheckUpdateLocking(void)
{
const char *iptablesArgs[] = {
- IPTABLES_PATH, "-w", "-L", "-n", NULL,
+ "iptables", "-w", "-L", "-n", NULL,
};
const char *ip6tablesArgs[] = {
- IP6TABLES_PATH, "-w", "-L", "-n", NULL,
+ "ip6tables", "-w", "-L", "-n", NULL,
};
const char *ebtablesArgs[] = {
- EBTABLES_PATH, "--concurrent", "-L", NULL,
+ "ebtables", "--concurrent", "-L", NULL,
};
if (lockOverride)
return;
@@ -182,17 +182,19 @@ virFirewallValidateBackend(virFirewallBackend backend)
if (backend == VIR_FIREWALL_BACKEND_DIRECT) {
const char *commands[] = {
- IPTABLES_PATH, IP6TABLES_PATH, EBTABLES_PATH
+ "iptables", "ip6tables", "ebtables"
};
size_t i;
for (i = 0; i < ARRAY_CARDINALITY(commands); i++) {
- if (!virFileIsExecutable(commands[i])) {
+ char *path = virFindFileInPath(commands[i]);
+ if (!path || !virFileIsExecutable(path)) {
virReportSystemError(errno,
_("direct firewall backend requested, but %s is not available"),
commands[i]);
return -1;
}
+ VIR_FREE(path);
}
VIR_DEBUG("found iptables/ip6tables/ebtables, using direct backend");
}
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index bb17b84..75e45fd 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1469,7 +1469,7 @@ int virNetDevSetIPAddress(const char *ifname,
virCommandAddArgList(cmd, "broadcast", bcaststr, NULL);
virCommandAddArg(cmd, "alias");
# else
- cmd = virCommandNew(IP_PATH);
+ cmd = virCommandNew("ip");
virCommandAddArgList(cmd, "addr", "add", NULL);
virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
if (peerstr)
@@ -1506,7 +1506,7 @@ virNetDevAddRoute(const char *ifname,
goto cleanup;
if (!(gatewaystr = virSocketAddrFormat(gateway)))
goto cleanup;
- cmd = virCommandNew(IP_PATH);
+ cmd = virCommandNew("ip");
virCommandAddArgList(cmd, "route", "add", NULL);
virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
virCommandAddArgList(cmd, "via", gatewaystr, "dev", ifname,
@@ -1544,7 +1544,7 @@ int virNetDevClearIPAddress(const char *ifname,
virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
virCommandAddArg(cmd, "-alias");
# else
- cmd = virCommandNew(IP_PATH);
+ cmd = virCommandNew("ip");
virCommandAddArgList(cmd, "addr", "del", NULL);
virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
virCommandAddArgList(cmd, "dev", ifname, NULL);
diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
index f1f29c6..976e883 100644
--- a/tests/virfirewalltest.c
+++ b/tests/virfirewalltest.c
@@ -128,11 +128,11 @@ VIR_MOCK_WRAP_RET_ARGS(dbus_connection_send_with_reply_and_block,
if (fwBuf) {
if (STREQ(type, "ipv4"))
- virBufferAddLit(fwBuf, IPTABLES_PATH);
+ virBufferAddLit(fwBuf, "iptables");
else if (STREQ(type, "ipv4"))
- virBufferAddLit(fwBuf, IP6TABLES_PATH);
+ virBufferAddLit(fwBuf, "ip6tables");
else
- virBufferAddLit(fwBuf, EBTABLES_PATH);
+ virBufferAddLit(fwBuf, "ebtables");
}
for (i = 0; i < nargs; i++) {
if (fwBuf) {
@@ -204,8 +204,8 @@ testFirewallSingleGroup(const void *opaque)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
const struct testFirewallData *data = opaque;
fwDisabled = data->fwDisabled;
@@ -263,8 +263,8 @@ testFirewallRemoveRule(const void *opaque)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
const struct testFirewallData *data = opaque;
virFirewallRulePtr fwrule;
@@ -329,10 +329,10 @@ testFirewallManyGroups(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
- IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A OUTPUT --jump DROP\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
+ "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A OUTPUT --jump DROP\n";
const struct testFirewallData *data = opaque;
fwDisabled = data->fwDisabled;
@@ -423,10 +423,10 @@ testFirewallIgnoreFailGroup(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
- IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A OUTPUT --jump DROP\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+ "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A OUTPUT --jump DROP\n";
const struct testFirewallData *data = opaque;
fwDisabled = data->fwDisabled;
@@ -498,10 +498,10 @@ testFirewallIgnoreFailRule(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
- IPTABLES_PATH " -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A OUTPUT --jump DROP\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+ "iptables -A OUTPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A OUTPUT --jump DROP\n";
const struct testFirewallData *data = opaque;
fwDisabled = data->fwDisabled;
@@ -572,8 +572,8 @@ testFirewallNoRollback(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n";
const struct testFirewallData *data = opaque;
fwDisabled = data->fwDisabled;
@@ -642,11 +642,11 @@ testFirewallSingleRollback(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
- IPTABLES_PATH " -D INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
- IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+ "iptables -D INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
+ "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
const struct testFirewallData *data = opaque;
fwDisabled = data->fwDisabled;
@@ -732,10 +732,10 @@ testFirewallManyRollback(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
- IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
- IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+ "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
+ "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
const struct testFirewallData *data = opaque;
fwDisabled = data->fwDisabled;
@@ -825,14 +825,14 @@ testFirewallChainedRollback(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
- IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
- IPTABLES_PATH " -D INPUT --source-host 192.168.122.127 --jump REJECT\n"
- IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"
- IPTABLES_PATH " -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
- IPTABLES_PATH " -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
+ "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n"
+ "iptables -A INPUT --source-host 192.168.122.255 --jump REJECT\n"
+ "iptables -D INPUT --source-host 192.168.122.127 --jump REJECT\n"
+ "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n"
+ "iptables -D INPUT --source-host 192.168.122.255 --jump REJECT\n"
+ "iptables -D INPUT --source-host '!192.168.122.1' --jump REJECT\n";
const struct testFirewallData *data = opaque;
fwDisabled = data->fwDisabled;
@@ -976,11 +976,11 @@ testFirewallQueryHook(const char *const*args,
int *status,
void *opaque ATTRIBUTE_UNUSED)
{
- if (STREQ(args[0], IPTABLES_PATH) &&
+ if (STREQ(args[0], "iptables") &&
STREQ(args[1], "-L")) {
if (VIR_STRDUP(*output, TEST_FILTER_TABLE_LIST) < 0)
*status = 127;
- } else if (STREQ(args[0], IPTABLES_PATH) &&
+ } else if (STREQ(args[0], "iptables") &&
STREQ(args[1], "-t") &&
STREQ(args[2], "nat") &&
STREQ(args[3], "-L")) {
@@ -1026,15 +1026,15 @@ testFirewallQuery(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
const char *actual = NULL;
const char *expected =
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
- IPTABLES_PATH " -L\n"
- IPTABLES_PATH " -t nat -L\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.130 --jump REJECT\n"
- IPTABLES_PATH " -A INPUT --source-host '!192.168.122.129' --jump REJECT\n"
- IPTABLES_PATH " -A INPUT --source-host '!192.168.122.129' --jump REJECT\n"
- IPTABLES_PATH " -A INPUT --source-host 192.168.122.128 --jump REJECT\n"
- IPTABLES_PATH " -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
+ "iptables -A INPUT --source-host 192.168.122.1 --jump ACCEPT\n"
+ "iptables -A INPUT --source-host 192.168.122.127 --jump REJECT\n"
+ "iptables -L\n"
+ "iptables -t nat -L\n"
+ "iptables -A INPUT --source-host 192.168.122.130 --jump REJECT\n"
+ "iptables -A INPUT --source-host '!192.168.122.129' --jump REJECT\n"
+ "iptables -A INPUT --source-host '!192.168.122.129' --jump REJECT\n"
+ "iptables -A INPUT --source-host 192.168.122.128 --jump REJECT\n"
+ "iptables -A INPUT --source-host '!192.168.122.1' --jump REJECT\n";
const struct testFirewallData *data = opaque;
expectedLineNum = 0;
--
2.7.3
8 years, 7 months
[libvirt] [PATCH] qemu: conf: Run through the parser even without config file
by Peter Krempa
The parser sets a few defaults if the are not present in the config. We
should run that even when the file is missing.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1325075
---
src/qemu/qemu_conf.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 77ef4fe..0bd9f06 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -477,12 +477,11 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
*/
if (access(filename, R_OK) == -1) {
VIR_INFO("Could not read qemu config file %s", filename);
- return 0;
+ } else {
+ if (!(conf = virConfReadFile(filename, 0)))
+ goto cleanup;
}
- if (!(conf = virConfReadFile(filename, 0)))
- goto cleanup;
-
#define CHECK_TYPE(name, typ) \
if (p && p->type != (typ)) { \
virReportError(VIR_ERR_INTERNAL_ERROR, \
--
2.8.1
8 years, 7 months
[libvirt] [PATCH 0/2] Fix migrations with automatic vnc sockets
by Martin Kletzander
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1326270
Martin Kletzander (2):
qemu: Unref cfg in qemuDomainDefPostParse
qemu: Regenerate VNC socket paths
src/qemu/qemu_domain.c | 22 +++++++++++-
.../qemuxml2argv-graphics-vnc-autosocket.args | 22 ++++++++++++
.../qemuxml2argv-graphics-vnc-autosocket.xml | 34 +++++++++++++++++++
.../qemuxml2xmlout-graphics-vnc-autosocket.xml | 39 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 7 ++++
5 files changed, 123 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml
--
2.8.1
8 years, 7 months
[libvirt] interface "peer address" patches are broken
by Laine Stump
I haven't had the time to fix it yet, but wanted to point out that the
patches that are supposed to support setting the peer address of
tap/macvlan devices do not work. The problem is that the peer attribute
isn't being included when the XML for a a domain interface is being
formatted.
I also sent the following message last week as a followup to the patch
series; possibly it wasn't noticed for that reason:.
I'll make a patch to fix the formatting when I can so that I can test my
theory about address vs. peer on lxc vs qemu. But if it's fixed by
someone else in the meantime, all the better :-)
On 04/20/2016 12:36 PM, Laine Stump wrote:
> Something I've found myself worrying about lately while driving in the
> car or nodding off to sleep - are the "address" and "peer" attributes
> effectively used in the same way for all network connection types and
> both hypervisors? I think the answer may be "no", and if so we need to
> fix that before they go out in a release.
>
> In particular, when an lxc domain's interface has:
>
> <ip address='192.168.128.1'/>
>
> That is the IP address seen by the guest, not the host. So I would
> assume that if an LXC domain had:
>
>
> <ip address='192.168.128.1' peer='192.168.128.2'/>
>
> that 192.168.128.1 would still be the IP address see by the guest, and
> 192.168.128.2 would be the IP address on the host side; and it should
> be the same for qemu.
>
> From what I can see of the code, though, on a qemu domain, the IP
> address is set for the tap device's own IP, meaning that it would show
> up on the *host* side, while the peer address would be what the host
> expects to be at the other end of the tap device (i.e. the guest
> side), so the two attributes are used for the *opposite* end of the
> PTP link in lxc vs. qemu.
>
> I think that, instead, the "address" attribute should *always* be the
> IP address that is seen/used by the guest, and the "peer" attribute
> should be the IP address that is seen/used by the host. (perhaps
> "peer" could be replaced with some other name, like "host" or
> "hostAddress" to avoid confusion? (don't like either of those
> alternatives, but I don't really like peer either)).
>
> Aside from that, I can see that these patches have been pushed in the
> code that I'm running, and I've been trying to add "peer='blah'" to
> interface IP addresses on my test machine, but it's just removed from
> the config. Have you tested what got pushed? Has something gone wrong?
>
> Since there hasn't been a release with these patches included yet,
> there is still time to fix it at least to be consistent (assuming that
> my suspicions are correct; I've been unable to test it myself for the
> reason above).
8 years, 7 months
