March 2013 - Devel - libvirt List Archives

[libvirt] [PATCH] nwfilter: probe for inverted ctdir
by Stefan Berger 22 Mar '13

22 Mar '13

Linux netfilter at some point inverted the meaning of the '--ctdir reply' and newer netfilter implementations now expect '--ctdir original' instread and vice-versa. We probe for this netfilter change via a UDP message over loopback and 3 filtering rules applied to INPUT. If the sent byte arrives, the newer netfilter implementation has been detected. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 123 ++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -27,6 +27,10 @@ #include <string.h> #include <sys/stat.h> #include <fcntl.h> +#include <arpa/inet.h> +#include <sys/select.h> +#include <sys/time.h> +#include <unistd.h> #include "internal.h" @@ -85,6 +89,12 @@ static char *iptables_cmd_path; static char *ip6tables_cmd_path; static char *grep_cmd_path; +/* + * --ctdir original vs. reply's meaning was inverted in the netfilter + * at some point. We probe for it. + */ +static bool iptables_ctdir_corrected = false; + #define PRINT_ROOT_CHAIN(buf, prefix, ifname) \ snprintf(buf, sizeof(buf), "libvirt-%c-%s", prefix, ifname) #define PRINT_CHAIN(buf, prefix, ifname, suffix) \ @@ -1262,6 +1272,9 @@ iptablesEnforceDirection(int directionIn virNWFilterRuleDefPtr rule, virBufferPtr buf) { + if (iptables_ctdir_corrected) + directionIn = !directionIn; + if (rule->tt != VIR_NWFILTER_RULE_DIRECTION_INOUT) virBufferAsprintf(buf, " -m conntrack --ctdir %s", (directionIn) ? "Original" @@ -4304,6 +4317,113 @@ ebiptablesDriverTestCLITools(void) return ret; } +static void +ebiptablesDriverProbeCtdir(void) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + static const char cmdline[] = + "$IPT -%c INPUT %c -i lo -p udp --dport %hu " + "-m state --state ESTABLISHED -j ACCEPT " CMD_SEPARATOR + "$IPT -%c INPUT %c -i lo -p udp --dport %hu " + "-m conntrack --ctdir original -j ACCEPT " CMD_SEPARATOR + "$IPT -%c INPUT %c -i lo -p udp --dport %hu -j DROP"; + /* + * Above '--ctdir original' gets this test to receive a message on + * 'fixed' netfilter. + */ + unsigned short port; + int ssockfd = -1, csockfd = -1; + struct sockaddr_in serveraddr = { + .sin_family = AF_INET, + }; + fd_set readfds; + struct timeval timeout = { + .tv_sec = 0, + .tv_usec = 1000 * 200, + }; + int n; + + if (inet_aton("127.0.0.1", &serveraddr.sin_addr) == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "inet_aton failed"); + goto cleanup; + } + + if ((ssockfd = socket(AF_INET, SOCK_DGRAM, 0)) < 0 || + (csockfd = socket(AF_INET, SOCK_DGRAM, 0)) < 0) { + VIR_ERROR(_("Could not open UDP socket")); + goto cleanup; + } + + for (port = 0xffff; port > 1024; port--) { + serveraddr.sin_port = htons(port); + if (bind(ssockfd, (struct sockaddr *)&serveraddr, + sizeof(serveraddr)) == 0) + break; + } + if (port == 1024) { + VIR_ERROR(_("Could not bind to any UDP socket")); + goto cleanup; + } + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + virBufferAsprintf(&buf, cmdline, + 'I', '1', port, + 'I', '2', port, + 'I', '3', port); + + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + + if (ebiptablesExecCLI(&buf, NULL, NULL) < 0) { + VIR_ERROR(_("Could not apply iptables rules")); + goto cleanup_iptables; + } + + if (sendto(csockfd, cmdline, 1, 0, (struct sockaddr *)&serveraddr, + sizeof(serveraddr)) < 0) { + VIR_ERROR(_("Could not send to UDP socket")); + goto cleanup_iptables; + } + + FD_ZERO(&readfds); + FD_SET(ssockfd, &readfds); + + while (true) { + n = select(ssockfd + 1, &readfds, NULL, NULL, &timeout); + if (n < 0) { + if (errno == EINTR) + continue; + VIR_ERROR(_("Select failed")); + goto cleanup_iptables; + } + if (n == 0) { + VIR_INFO("Ctdir probing received no data -- 'old' netfilter"); + goto cleanup_iptables; + } + VIR_INFO("Ctdir probing received data -- 'fixed' netfilter"); + iptables_ctdir_corrected = true; + break; + } + +cleanup_iptables: + virBufferFreeAndReset(&buf); + + NWFILTER_SET_IPTABLES_SHELLVAR(&buf); + virBufferAsprintf(&buf, cmdline, + 'D', ' ', port, + 'D', ' ', port, + 'D', ' ', port); + ebiptablesExecCLI(&buf, NULL, NULL); + +cleanup: + virBufferFreeAndReset(&buf); + VIR_FORCE_CLOSE(ssockfd); + VIR_FORCE_CLOSE(csockfd); +} + static int ebiptablesDriverInit(bool privileged) { @@ -4341,6 +4461,9 @@ ebiptablesDriverInit(bool privileged) return -ENOTSUP; } + if (iptables_cmd_path) + ebiptablesDriverProbeCtdir(); + ebiptables_driver.flags = TECHDRV_FLAG_INITIALIZED; return 0;

2 3

[libvirt] [PATCH] Fix parsing of bond interface XML
by Jim Fehlig 21 Mar '13

21 Mar '13

Noticed that parsing bond interface XML containing the miimon element fails <interface type="bond" name="bond0"> ... <bond mode="active-backup"> <miimon freq="100" carrier="netif"/> ... </bond> </interface> This configuration does not contain the optional updelay and downdelay attributes, but parsing will fail due to returning the result of virXPathULong (a -1 when the attribute doesn't exist) from virInterfaceDefParseBond after examining the updelay attribute. I considered just adding a ret = 0; near the bottom of virInterfaceDefParseBond, but see there is no cleanup in the error label. Instead, just return failure where failure occurs and return success if the end of the function is reached. --- src/conf/interface_conf.c | 56 +++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 9301ec0..3d45d5c 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -572,61 +572,58 @@ error: static int virInterfaceDefParseBond(virInterfaceDefPtr def, xmlXPathContextPtr ctxt) { - int ret = -1; + int res; unsigned long tmp; def->data.bond.mode = virInterfaceDefParseBondMode(ctxt); if (def->data.bond.mode < 0) - goto error; + return -1; - ret = virInterfaceDefParseBondItfs(def, ctxt); - if (ret != 0) - goto error; + if (virInterfaceDefParseBondItfs(def, ctxt) != 0) + return -1; if (virXPathNode("./miimon[1]", ctxt) != NULL) { def->data.bond.monit = VIR_INTERFACE_BOND_MONIT_MII; - ret = virXPathULong("string(./miimon/@freq)", ctxt, &tmp); - if ((ret == -2) || (ret == -1)) { + res = virXPathULong("string(./miimon/@freq)", ctxt, &tmp); + if ((res == -2) || (res == -1)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bond interface miimon freq missing or invalid")); - goto error; + return -1; } def->data.bond.frequency = (int) tmp; - ret = virXPathULong("string(./miimon/@downdelay)", ctxt, &tmp); - if (ret == -2) { + res = virXPathULong("string(./miimon/@downdelay)", ctxt, &tmp); + if (res == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bond interface miimon downdelay invalid")); - goto error; - } else if (ret == 0) { + return -1; + } else if (res == 0) { def->data.bond.downdelay = (int) tmp; } - ret = virXPathULong("string(./miimon/@updelay)", ctxt, &tmp); - if (ret == -2) { + res = virXPathULong("string(./miimon/@updelay)", ctxt, &tmp); + if (res == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bond interface miimon updelay invalid")); - goto error; - } else if (ret == 0) { + return -1; + } else if (res == 0) { def->data.bond.updelay = (int) tmp; } def->data.bond.carrier = virInterfaceDefParseBondMiiCarrier(ctxt); - if (def->data.bond.carrier < 0) { - ret = -1; - goto error; - } + if (def->data.bond.carrier < 0) + return -1; } else if (virXPathNode("./arpmon[1]", ctxt) != NULL) { def->data.bond.monit = VIR_INTERFACE_BOND_MONIT_ARP; - ret = virXPathULong("string(./arpmon/@interval)", ctxt, &tmp); - if ((ret == -2) || (ret == -1)) { + res = virXPathULong("string(./arpmon/@interval)", ctxt, &tmp); + if ((res == -2) || (res == -1)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bond interface arpmon interval missing or invalid")); - goto error; + return -1; } def->data.bond.interval = (int) tmp; @@ -635,18 +632,15 @@ virInterfaceDefParseBond(virInterfaceDefPtr def, if (def->data.bond.target == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bond interface arpmon target missing")); - ret = -1; - goto error; + return -1; } def->data.bond.validate = virInterfaceDefParseBondArpValid(ctxt); - if (def->data.bond.validate < 0) { - ret = -1; - goto error; - } + if (def->data.bond.validate < 0) + return -1; } -error: - return ret; + + return 0; } static int -- 1.8.0.1

2 2

[libvirt] [PATCH] Correct invalid RNG schemas.
by Martin Kletzander 21 Mar '13

21 Mar '13

The 'trang' utility, which is able to transform '.rng' files into '.rnc' files, reported some errors in our schemas that weren't caught by the tools we use in the build. I haven't added a test for this, but the validity can be checked by the following command: trang -I rng -O rnc domain.rng domain.rnc There were unescaped minuses in regular expressions and we were constraining int (which is by default in the range of [-2^31;2^31-1] to maximum of 2^32. But what we wanted was exactly an unsignedInt. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Thanks to that, the '.rnc' files can be used by nxml-mode which makes editing libvirt xml files a *lot* easier. docs/schemas/domaincommon.rng | 2 +- docs/schemas/nwfilter.rng | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c4e7b7a..3240e1c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3929,7 +3929,7 @@ </define> <define name='aliasName'> <data type="string"> - <param name="pattern">[a-zA-Z0-9_-]+</param> + <param name="pattern">[a-zA-Z0-9_\-]+</param> </data> </define> <define name='alias'> diff --git a/docs/schemas/nwfilter.rng b/docs/schemas/nwfilter.rng index cfd9ba5..f1aa699 100644 --- a/docs/schemas/nwfilter.rng +++ b/docs/schemas/nwfilter.rng @@ -308,25 +308,25 @@ <choice> <value>root</value> <data type="string"> - <param name="pattern">mac[a-zA-Z0-9_\.:-]{0,9}</param> + <param name="pattern">mac[a-zA-Z0-9_\.:\-]{0,9}</param> </data> <data type="string"> - <param name="pattern">stp[a-zA-Z0-9_\.:-]{0,9}</param> + <param name="pattern">stp[a-zA-Z0-9_\.:\-]{0,9}</param> </data> <data type="string"> - <param name="pattern">vlan[a-zA-Z0-9_\.:-]{0,8}</param> + <param name="pattern">vlan[a-zA-Z0-9_\.:\-]{0,8}</param> </data> <data type="string"> - <param name="pattern">arp[a-zA-Z0-9_\.:-]{0,9}</param> + <param name="pattern">arp[a-zA-Z0-9_\.:\-]{0,9}</param> </data> <data type="string"> - <param name="pattern">rarp[a-zA-Z0-9_\.:-]{0,8}</param> + <param name="pattern">rarp[a-zA-Z0-9_\.:\-]{0,8}</param> </data> <data type="string"> - <param name="pattern">ipv4[a-zA-Z0-9_\.:-]{0,8}</param> + <param name="pattern">ipv4[a-zA-Z0-9_\.:\-]{0,8}</param> </data> <data type="string"> - <param name="pattern">ipv6[a-zA-Z0-9_\.:-]{0,8}</param> + <param name="pattern">ipv6[a-zA-Z0-9_\.:\-]{0,8}</param> </data> </choice> </attribute> @@ -950,10 +950,7 @@ <param name="pattern">0x[0-9a-fA-F]{1,8}</param> </data> - <data type="int"> - <param name="minInclusive">0</param> - <param name="maxInclusive">4294967295</param> - </data> + <data type="unsignedInt"/> </choice> </define> -- 1.8.1.5

2 2

[libvirt] [PATCH RFC] virsh: Fix semantics of --config for "update-device" command
by Peter Krempa 21 Mar '13

21 Mar '13

The man page states that with --config the next boot is affected. This can be understood as if _only_ the next bood was affected. This isn't true if the machine is running. This patch adds the full --live, --config, --current infrastructure and tweaks stuff to correctly support the obsolete --persistent flag. --- Notes: - This patch will be greatly simplified with macros from: http://www.redhat.com/archives/libvir-list/2013-March/msg00268.html - There are multiple places like this in virsh that will need update too. (detach-device for example) - https://bugzilla.redhat.com/show_bug.cgi?id=921398 tools/virsh-domain.c | 53 +++++++++++++++++++++++++++++++++++++++++----------- tools/virsh.pod | 22 +++++++++++++++------- 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e9da11f..33545f6 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9304,13 +9304,21 @@ static const vshCmdOptDef opts_update_device[] = { .help = N_("XML file") }, {.name = "persistent", - .type = VSH_OT_ALIAS, - .help = "config" + .type = VSH_OT_BOOL, + .help = N_("make live change persistent") }, {.name = "config", .type = VSH_OT_BOOL, .help = N_("affect next boot") }, + {.name = "live", + .type = VSH_OT_BOOL, + .help = N_("affect running domain") + }, + {.name = "current", + .type = VSH_OT_BOOL, + .help = N_("affect current domain") + }, {.name = "force", .type = VSH_OT_BOOL, .help = N_("force device update") @@ -9325,7 +9333,34 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; char *buffer = NULL; bool ret = false; - unsigned int flags; + unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool persistent = vshCommandOptBool(cmd, "persistent"); + + if (persistent) { + if (current || config || live) { + vshError(ctl, "%s", _("--persistent is incompatible with " + "--current, --live or --config")); + return false; + } + + flags = VIR_DOMAIN_AFFECT_CONFIG; + } else { + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified " + "exclusively")); + return false; + } + } else { + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + } + } if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -9333,19 +9368,15 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) goto cleanup; + if (persistent && + virDomainIsActive(dom) == 1) + flags |= VIR_DOMAIN_AFFECT_LIVE; + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { vshReportError(ctl); goto cleanup; } - if (vshCommandOptBool(cmd, "config")) { - flags = VIR_DOMAIN_AFFECT_CONFIG; - if (virDomainIsActive(dom) == 1) - flags |= VIR_DOMAIN_AFFECT_LIVE; - } else { - flags = VIR_DOMAIN_AFFECT_LIVE; - } - if (vshCommandOptBool(cmd, "force")) flags |= VIR_DOMAIN_DEVICE_MODIFY_FORCE; diff --git a/tools/virsh.pod b/tools/virsh.pod index 7fb89e4..8bb0490 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1887,18 +1887,26 @@ If I<--config> is specified, alter persistent configuration, effect observed on next boot, for compatibility purposes, I<--persistent> is alias of I<--config>. -=item B<update-device> I<domain> I<file> [I<--config>] [I<--force>] +=item B<update-device> I<domain> I<file> [I<--force>] +[[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] Update the characteristics of a device associated with I<domain>, -based on the device definition in an XML I<file>. If the I<--config> -option is used, the changes will take affect the next time libvirt -starts the domain. For compatibility purposes, I<--persistent> is -alias of I<--config>. The I<--force> option can be used to force -device update, e.g., to eject a CD-ROM even if it is locked/mounted in -the domain. See the documentation at +based on the device definition in an XML I<file>. The I<--force> option +can be used to force device update, e.g., to eject a CD-ROM even if it is +locked/mounted in the domain. See the documentation at L<http://libvirt.org/formatdomain.html#elementsDevices> to learn about libvirt XML format for a device. +If I<--live> is specified, affect a running domain. +If I<--config> is specified, affect the next startup of a persistent domain. +If I<--current> is specified, affect the current domain state. +Both I<--live> and I<--config> flags may be given, but I<--current> is +exclusive. Not specifying any flag is the same as specifying I<--current>. + +For compatibility purposes, I<--persistent> is alias of I<--config> and +I<--live> if the domain is running. This flag isn't compatible with the +other domain status flags. + =item B<change-media> I<domain> I<path> [I<--eject>] [I<--insert>] [I<--update>] [I<source>] [I<--force>] [[I<--live>] [I<--config>] | [I<--current>]] -- 1.8.1.5

3 2

[libvirt] [PATCH v9 0/3] DEVICE_DELETED event
by Michael S. Tsirkin 21 Mar '13

21 Mar '13

libvirt has a long-standing bug: when removing the device, it can request removal but does not know when the removal completes. Add an event so we can fix this in a robust way. First patch only adds the event with ID, second patch adds a path field. Split this way for ease of backport (stable downstreams without QOM would want to only take the first patch). Event without fields is still useful as management can use it to poll device list to figure out which device was removed. Signed-off-by: Michael S. Tsirkin <mst(a)redhat.com> If there are no more comments I'll stick this on my pci branch. Changes from v8: - reorder qom destruction so no need to change unparent Changes from v7: - none, v7 was malformed series sent by mistake Changes from v6: - make empty event use data: {}, Markus prefers this Changes from v5: - Emit an empty event on unnamed devices in patch 1/3, as suggested by Markus Changes from v4: - Add extra triggers and extra fields as requested by Markus Changes from v3: - Document that we only emit events for devices with and ID, as suggested by Markus Changes from v2: - move event toward the end of device_unparent, so that parents are reported after their children, as suggested by Paolo Changes from v1: - move to device_unparent - address comments by Andreas and Eric -- Anthony Liguori Michael S. Tsirkin (3): qdev: DEVICE_DELETED event qom: call class destructor before unparent qmp: add path to device_deleted event QMP/qmp-events.txt | 18 ++++++++++++++++++ hw/qdev.c | 14 ++++++++++++++ include/monitor/monitor.h | 1 + monitor.c | 1 + qapi-schema.json | 4 +++- qom/object.c | 6 +++--- 6 files changed, 40 insertions(+), 4 deletions(-) -- MST

4 9

[libvirt] IPv6 migration
by Ján Tomko 21 Mar '13

21 Mar '13

Hello. We can only tell QEMU on the destination to listen either on IPv6 or on IPv4. If we're supplied with a numeric v6 address, that's the only thing we need to know to set the listen address to [::]. For hostnames, we can either assume this based on how it resolves by default on the destination (we keep trying all the resolved addresses on the source, but this might break a few cases), which John found disgusting, so that leaves user input: How about a VIR_DOMAIN_MIGRATE_IPV6 flag, depending on which we set the listen address on the destination and creating a new function virNetSocketNewConnectTCPHints, where we would add IPv4/IPv6 hint based on the presence/absence of this flag? Would it be better to auto-add this flag for numeric addresses, or just check for them both in the Prepare phase on the dest (and set listen_addr accordingly) and before connecting from the source, so we don't use the IPv4 hint (in case we wanted to use the partial fix for just numeric addresses for older libvirt). Jan Bug: https://bugzilla.redhat.com/show_bug.cgi?id=846013 v3: https://www.redhat.com/archives/libvir-list/2013-February/msg01379.html unfortunately I posted v3 before reading John's feedback to v1: https://www.redhat.com/archives/libvir-list/2013-February/msg01059.html

3 4

[libvirt] libvirt-tck test failure with identity patches
by Guido Günther 21 Mar '13

21 Mar '13

Hi, the qemu session libvirt-tck test currently fails with: [19:58:21] scripts/domain/050-transient-lifecycle.t ............ ok 59684 ms [19:59:20] scripts/domain/051-transient-autostart.t ............ ok 2001 ms [19:59:22] scripts/domain/060-persistent-lifecycle.t ........... ok 2943 ms [19:59:25] scripts/domain/061-persistent-autostart.t ........... ok 2050 ms [19:59:28] scripts/domain/065-persistent-redefine.t ............ ok 2050 ms [19:59:30] scripts/domain/070-transient-to-persistent.t ........ ok 2025 ms [19:59:32] scripts/domain/080-unique-id-define.t ............... ok 3353 ms [19:59:35] scripts/domain/081-unique-id-create.t ............... Dubious, test returned 22 (wstat 5632, 0x1600) All 12 subtests passed [19:59:41] scripts/domain/082-unique-id-caching.t .............. Bailout called. Further testing stopped: failed to setup test harness: libvirt error code: 1, message: internal error Cannot initialize thread local for current identity Runnig 081-unique-id-create.t and 082-unique-id-caching.t separately works so there's some corruption going on. Any idea what could be triggering this? It stared with one of: ebf78be4c277cffae57d99daa199a9b3c1cf9804 Set the current client identity during API call dispatch d5e83ad9b7c74e434349ede076dc573a3cc50384 Add ability to get a virIdentity from a virNetServerClientPtr 8c5d28c1ad5d42b8f3599d52a3dfed32f88c4edc Add API to get the system identity 8726e91b3a165fa1094155218f3a3b65dbc932c5 Add APIs for associating a virIdentityPtr with the current thread 3aabe27247711324df2bfa623e9a5e8d2442e3a5 Define internal APIs for managing identities 51997e50fa9a54c4bfce3cb2dd43b53418135d18 Add APIs to get at more client security data Any idea? I'll try to dig deeper once I find some more time. Cheers, -- Guido

2 2

[libvirt] [PATCH] qemu: Support setting the 'removable' flag for USB disks
by anonym 21 Mar '13

21 Mar '13

This adds an attribute named 'removable' to the 'target' element of disks, which controls the removable flag. For instance, on a Linux guest it controls the value of /sys/block/$dev/removable. This option is only valid for USB disks (i.e. bus='usb'), and its default value is 'off', which is the same behaviour as before. To achieve this, 'removable=on' is appended to the '-device usb-storage' parameter sent to qemu when adding a USB disk via '-disk'. For versions of qemu only supporting '-usbdevice disk:' for adding USB disks this feature always remains 'off' since there's no support for passing such an option. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=922495 --- docs/formatdomain.html.in | 8 +++-- docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 35 ++++++++++++++++++-- src/conf/domain_conf.h | 9 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 6 ++++ .../qemuxml2argv-disk-usb-device-removable.args | 8 +++++ .../qemuxml2argv-disk-usb-device-removable.xml | 27 +++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 9 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8a3c3b7..384da4f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1498,9 +1498,13 @@ removable disks (i.e. CDROM or Floppy disk), the value can be either "open" or "closed", defaults to "closed". NB, the value of <code>tray</code> could be updated while the domain is running. - <span class="since">Since 0.0.3; <code>bus</code> attribute since 0.4.3; + The optional attribute <code>removable</code> sets the + removable flag for USB disks, and its value can be either "on" + or "off", defaulting to "off". <span class="since">Since + 0.0.3; <code>bus</code> attribute since 0.4.3; <code>tray</code> attribute since 0.9.11; "usb" attribute value since - after 0.4.4; "sata" attribute value since 0.9.7</span> + after 0.4.4; "sata" attribute value since 0.9.7; "removable" attribute + value since X.Y.Z</span> </dd> <dt><code>iotune</code></dt> <dd>The optional <code>iotune</code> element provides the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 9792065..eab6aa3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1164,6 +1164,14 @@ </choice> </attribute> </optional> + <optional> + <attribute name="removable"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> </element> </define> <define name="geometry"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3278e9c..551bac3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -709,6 +709,10 @@ VIR_ENUM_IMPL(virDomainDiskTray, VIR_DOMAIN_DISK_TRAY_LAST, "closed", "open"); +VIR_ENUM_IMPL(virDomainDiskRemovable, VIR_DOMAIN_DISK_REMOVABLE_LAST, + "on", + "off"); + VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST, "default", @@ -3996,6 +4000,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, char *authUUID = NULL; char *usageType = NULL; char *tray = NULL; + char *removable = NULL; char *logical_block_size = NULL; char *physical_block_size = NULL; char *wwn = NULL; @@ -4149,6 +4154,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, target = virXMLPropString(cur, "dev"); bus = virXMLPropString(cur, "bus"); tray = virXMLPropString(cur, "tray"); + removable = virXMLPropString(cur, "removable"); /* HACK: Work around for compat with Xen * driver in previous libvirt releases */ @@ -4556,6 +4562,24 @@ virDomainDiskDefParseXML(virCapsPtr caps, def->tray_status = VIR_DOMAIN_DISK_TRAY_CLOSED; } + if (removable) { + if ((def->removable = virDomainDiskRemovableTypeFromString(removable)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk removable status '%s'"), removable); + goto error; + } + + if (def->bus != VIR_DOMAIN_DISK_BUS_USB) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("removable is only valid for usb disks")); + goto error; + } + } else { + if (def->bus == VIR_DOMAIN_DISK_BUS_USB) { + def->removable = VIR_DOMAIN_DISK_REMOVABLE_OFF; + } + } + if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && def->bus != VIR_DOMAIN_DISK_BUS_FDC) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -4754,6 +4778,7 @@ cleanup: VIR_FREE(target); VIR_FREE(source); VIR_FREE(tray); + VIR_FREE(removable); VIR_FREE(trans); while (nhosts > 0) { virDomainDiskHostDefFree(&hosts[nhosts - 1]); @@ -12915,10 +12940,14 @@ virDomainDiskDefFormat(virBufferPtr buf, if ((def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY || def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) && def->tray_status != VIR_DOMAIN_DISK_TRAY_CLOSED) - virBufferAsprintf(buf, " tray='%s'/>\n", + virBufferAsprintf(buf, " tray='%s'", virDomainDiskTrayTypeToString(def->tray_status)); - else - virBufferAddLit(buf, "/>\n"); + if (def->bus == VIR_DOMAIN_DISK_BUS_USB && + def->removable != VIR_DOMAIN_DISK_REMOVABLE_OFF) { + virBufferAsprintf(buf, " removable='%s'", + virDomainDiskRemovableTypeToString(def->removable)); + } + virBufferAddLit(buf, "/>\n"); /*disk I/O throttling*/ if (def->blkdeviotune.total_bytes_sec || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 96f11ba..0f4f0d7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -518,6 +518,13 @@ enum virDomainDiskTray { VIR_DOMAIN_DISK_TRAY_LAST }; +enum virDomainDiskRemovable { + VIR_DOMAIN_DISK_REMOVABLE_ON, + VIR_DOMAIN_DISK_REMOVABLE_OFF, + + VIR_DOMAIN_DISK_REMOVABLE_LAST +}; + enum virDomainDiskGeometryTrans { VIR_DOMAIN_DISK_TRANS_DEFAULT = 0, VIR_DOMAIN_DISK_TRANS_NONE, @@ -612,6 +619,7 @@ struct _virDomainDiskDef { char *src; char *dst; int tray_status; + int removable; int protocol; size_t nhosts; virDomainDiskHostDefPtr hosts; @@ -2346,6 +2354,7 @@ VIR_ENUM_DECL(virDomainDiskIo) VIR_ENUM_DECL(virDomainDiskSecretType) VIR_ENUM_DECL(virDomainDiskSGIO) VIR_ENUM_DECL(virDomainDiskTray) +VIR_ENUM_DECL(virDomainDiskRemovable) VIR_ENUM_DECL(virDomainIoEventFd) VIR_ENUM_DECL(virDomainVirtioEventIdx) VIR_ENUM_DECL(virDomainDiskCopyOnRead) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5cad990..0d1a9d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -156,6 +156,7 @@ virDomainDiskIoTypeToString; virDomainDiskPathByName; virDomainDiskProtocolTransportTypeFromString; virDomainDiskProtocolTransportTypeToString; +virDomainDiskRemovableTypeToString; virDomainDiskRemove; virDomainDiskRemoveByName; virDomainDiskTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4891b65..c04cecf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3219,6 +3219,11 @@ qemuBuildDriveDevStr(virDomainDefPtr def, if (disk->product) virBufferAsprintf(&opt, ",product=%s", disk->product); + if (disk->bus == VIR_DOMAIN_DISK_BUS_USB && + disk->removable != VIR_DOMAIN_DISK_REMOVABLE_OFF) { + virBufferAsprintf(&opt, ",removable=%s", + virDomainDiskRemovableTypeToString(disk->removable)); + } if (virBufferError(&opt)) { virReportOOMError(); goto error; @@ -9391,6 +9396,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, disk->type = VIR_DOMAIN_DISK_TYPE_FILE; disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; disk->bus = VIR_DOMAIN_DISK_BUS_USB; + disk->removable = VIR_DOMAIN_DISK_REMOVABLE_OFF; if (!(disk->dst = strdup("sda")) || VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) goto no_memory; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args new file mode 100644 index 0000000..36be080 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/dev/HostVG/QEMUGuest1,if=none,id=drive-ide0-0-0 -device ide-drive,\ +bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=/tmp/usbdisk.img,\ +if=none,id=drive-usb-disk0 -device usb-storage,drive=drive-usb-disk0,\ +id=usb-disk0,removable=on -device virtio-balloon-pci,id=balloon0,bus=pci.0,\ +addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml new file mode 100644 index 0000000..6ee3e9b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-usb-device-removable.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='file' device='disk'> + <source file='/tmp/usbdisk.img'/> + <target dev='sda' bus='usb' removable='on'/> + </disk> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e76d844..2eca7ac 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -516,6 +516,8 @@ mymain(void) DO_TEST("disk-usb", NONE); DO_TEST("disk-usb-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("disk-usb-device-removable", + QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-scsi-device", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); -- 1.7.10.4

3 3

[libvirt] [PATCHv2] python: Fix emulatorpin API bindings
by Peter Krempa 21 Mar '13

21 Mar '13

The addition of emulator pinning APIs didn't think of doing the right job with python APIs for them. The default generator produced unusable code for this. This patch switches to proper code as in the case of domain Vcpu pining. This change can be classified as a python API-breaker but in the state the code was before I doubt anyone was able to use it successfully. --- python/generator.py | 2 + python/libvirt-override-api.xml | 18 +++++- python/libvirt-override.c | 118 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 2 deletions(-) diff --git a/python/generator.py b/python/generator.py index 6a25c2d..0aeb675 100755 --- a/python/generator.py +++ b/python/generator.py @@ -418,6 +418,8 @@ skip_impl = ( 'virDomainPinVcpu', 'virDomainPinVcpuFlags', 'virDomainGetVcpuPinInfo', + 'virDomainGetEmulatorPinInfo', + 'virDomainPinEmulator', 'virSecretGetValue', 'virSecretSetValue', 'virSecretGetUUID', diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 5976fb2..c720610 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -237,10 +237,24 @@ <function name='virDomainGetVcpuPinInfo' file='python'> <info>Query the CPU affinity setting of all virtual CPUs of domain</info> <return type='unsigned char *' info='the array of cpumap'/> - <arg name='domain' type='virDomainPtr' info='pointer to domain object, or NULL for Domain0'/> + <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> + <arg name='flags' type='int' info='an OR'ed set of virDomainModificationImpact'/> + </function> + <function name='virDomainGetEmulatorPinInfo' file='python'> + <info>Query the CPU affinity setting of the emulator process of domain</info> + <return type='unsigned char *' info='the array of cpumap'/> + <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> <arg name='flags' type='int' info='an OR'ed set of virDomainModificationImpact'/> </function> - <function name='virDomainSetSchedulerParameters' file='python'> + <function name='virDomainPinEmulator' file='python'> + <info>Dynamically change the real CPUs which can be allocated to the emulator process of a domain. + This function requires privileged access to the hypervisor.</info> + <return type='int' info='0 in case of success, -1 in case of failure.'/> + <arg name='domain' type='virDomainPtr' info='pointer to domain object, or NULL for Domain0'/> + <arg name='cpumap' type='unsigned char *' info='pointer to a bit map of real CPUs (in 8-bit bytes) (IN) Each bit set to 1 means that corresponding CPU is usable. Bytes are stored in little-endian order: CPU0-7, 8-15... In each byte, lowest CPU number is least significant bit.'/> + <arg name='flags' type='int' info='flags to specify'/> + </function> + <function name='virDomainSetSchedulerParameters' file='python'> <info>Change the scheduler parameters</info> <return type='int' info='-1 in case of error, 0 in case of success.'/> <arg name='domain' type='virDomainPtr' info='pointer to domain object'/> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 9637598..f6573e1 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -1664,6 +1664,122 @@ cleanup: return VIR_PY_NONE; } + +static PyObject * +libvirt_virDomainPinEmulator(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain, *pycpumap; + unsigned char *cpumap = NULL; + int cpumaplen, i, tuple_size, cpunum; + int i_retval; + unsigned int flags; + + if (!PyArg_ParseTuple(args, (char *)"OOi:virDomainPinVcpu", + &pyobj_domain, &pycpumap, &flags)) + return NULL; + + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0) + return VIR_PY_INT_FAIL; + + cpumaplen = VIR_CPU_MAPLEN(cpunum); + + if (!PyTuple_Check(pycpumap)) { + PyErr_SetString(PyExc_TypeError, "Unexpected type, tuple is required"); + return NULL; + } + + if ((tuple_size = PyTuple_Size(pycpumap)) == -1) + return NULL; + + if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) + return PyErr_NoMemory(); + + for (i = 0; i < tuple_size; i++) { + PyObject *flag = PyTuple_GetItem(pycpumap, i); + bool b; + + if (!flag || libvirt_boolUnwrap(flag, &b) < 0) { + VIR_FREE(cpumap); + return VIR_PY_INT_FAIL; + } + + if (b) + VIR_USE_CPU(cpumap, i); + else + VIR_UNUSE_CPU(cpumap, i); + } + + for (; i < cpunum; i++) + VIR_UNUSE_CPU(cpumap, i); + + LIBVIRT_BEGIN_ALLOW_THREADS; + i_retval = virDomainPinEmulator(domain, cpumap, cpumaplen, flags); + LIBVIRT_END_ALLOW_THREADS; + + VIR_FREE(cpumap); + + if (i_retval < 0) + return VIR_PY_INT_FAIL; + + return VIR_PY_INT_SUCCESS; +} + + +static PyObject * +libvirt_virDomainGetEmulatorPinInfo(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr domain; + PyObject *pyobj_domain; + PyObject *pycpumap; + unsigned char *cpumap; + size_t cpumaplen; + size_t pcpu; + unsigned int flags; + int ret; + int cpunum; + + if (!PyArg_ParseTuple(args, (char *)"Oi:virDomainEmulatorPinInfo", + &pyobj_domain, &flags)) + return NULL; + + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + + if ((cpunum = getPyNodeCPUCount(virDomainGetConnect(domain))) < 0) + return VIR_PY_NONE; + + cpumaplen = VIR_CPU_MAPLEN(cpunum); + + if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) + return PyErr_NoMemory(); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virDomainGetEmulatorPinInfo(domain, cpumap, cpumaplen, flags); + LIBVIRT_END_ALLOW_THREADS; + if (ret < 0) { + VIR_FREE(cpumap); + return VIR_PY_NONE; + } + + if (!(pycpumap = PyTuple_New(cpunum))) { + VIR_FREE(cpumap); + return NULL; + } + + for (pcpu = 0; pcpu < cpunum; pcpu++) + PyTuple_SET_ITEM(pycpumap, pcpu, + PyBool_FromLong(VIR_CPU_USABLE(cpumap, cpumaplen, + 0, pcpu))); + + VIR_FREE(cpumap); + return pycpumap; +} + + /************************************************************************ * * * Global error handler at the Python level * @@ -6705,6 +6821,8 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainPinVcpu", libvirt_virDomainPinVcpu, METH_VARARGS, NULL}, {(char *) "virDomainPinVcpuFlags", libvirt_virDomainPinVcpuFlags, METH_VARARGS, NULL}, {(char *) "virDomainGetVcpuPinInfo", libvirt_virDomainGetVcpuPinInfo, METH_VARARGS, NULL}, + {(char *) "virDomainGetEmulatorPinInfo", libvirt_virDomainGetEmulatorPinInfo, METH_VARARGS, NULL}, + {(char *) "virDomainPinEmulator", libvirt_virDomainPinEmulator, METH_VARARGS, NULL}, {(char *) "virConnectListStoragePools", libvirt_virConnectListStoragePools, METH_VARARGS, NULL}, {(char *) "virConnectListDefinedStoragePools", libvirt_virConnectListDefinedStoragePools, METH_VARARGS, NULL}, {(char *) "virConnectListAllStoragePools", libvirt_virConnectListAllStoragePools, METH_VARARGS, NULL}, -- 1.8.1.5

2 2

[libvirt] [PATCH] qemu: Un-mark volume as mirrored/copied if blockjob copy fails
by Peter Krempa 21 Mar '13

21 Mar '13

When the blockjob fails for some reason an event is emitted but the disk wasn't unmarked as being part of a active block copy operation. --- src/qemu/qemu_process.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2465938..ada864b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -970,6 +970,9 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY && status == VIR_DOMAIN_BLOCK_JOB_READY) disk->mirroring = true; + if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY && + status == VIR_DOMAIN_BLOCK_JOB_FAILED) + VIR_FREE(disk->mirror); } virObjectUnlock(vm); -- 1.8.1.5

2 2