[libvirt] [PATCH] Valid pool format type for logical volume pool should be auto and lvm2
The logical volume pool supports formats auto and lvm2 Signed-off-by: Shanzhi Yu s...@redhat.com --- src/conf/storage_conf.c | 2 +- src/conf/storage_conf.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8b6fd79..3d61273 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -77,7 +77,7 @@ VIR_ENUM_IMPL(virStoragePoolFormatDisk, VIR_ENUM_IMPL(virStoragePoolFormatLogical, VIR_STORAGE_POOL_LOGICAL_LAST, - unknown, lvm2) + auto, lvm2) VIR_ENUM_IMPL(virStorageVolFormatDisk, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 04d99eb..2b08ac5 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -462,7 +462,7 @@ typedef enum { VIR_ENUM_DECL(virStoragePoolFormatDisk) typedef enum { -VIR_STORAGE_POOL_LOGICAL_UNKNOWN = 0, +VIR_STORAGE_POOL_LOGICAL_AUTO = 0, VIR_STORAGE_POOL_LOGICAL_LVM2 = 1, VIR_STORAGE_POOL_LOGICAL_LAST, } virStoragePoolFormatLogical; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator
On Tue, 2014-06-17 at 12:40 -0600, Jim Fehlig wrote: Ian Campbell wrote: On Mon, 2014-06-16 at 17:11 -0600, Jim Fehlig wrote: This function exists in Xen 4.2 as well, in libxl.h. Any ideas on how to handle this? I'm not aware of an existing macro to check for func 'foo' defined in header 'bar'. Is writing a custom macro along these lines a good solution? A bad solution I tried was hacking the test to check libxl version via libxl_get_version_info(), but that API does not work if not running Xen. Given that it exists in everything from 4.2 onwards why do you need to check for it? Hrm, right. I had the half-brained idea to use this to solve the failures I saw when testing this series against Xen 4.2 https://www.redhat.com/archives/libvir-list/2014-June/msg00170.html I think the solution to that specific problem is to use Xen 4.2 config as the baseline. But it got me thinking about the general problem you mentioned near the end of this mail https://www.redhat.com/archives/libvir-list/2014-June/msg00032.html With virJSONStringCompare in 1/1, Daniel provides a way to handle existence of new fields showing up in the json. But what if I want to write a test where the expected data is not supported on earlier versions? E.g. how would I add a test to check conversion of 'tpm ...' to 'vtpms: [ ]' and expect that to work when running 'make check' against a 4.2 libxl where vtpms were not yet supported? I suppose each such test would have to probe for the feature it checks and skip if not found. Right. You'd probably want to gate such a test case on the corresponding LIBXL_HAVE_XXX #define from libxl.h. Ian. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt] xen: handle root= in xen-xm configuration files.
On Tue, 2014-06-17 at 16:24 +0100, Ian Campbell wrote: +if (xenXMConfigGetString(conf, extra, extra, NULL) 0) This was subtly broken. The default needs to be . -8-- From 539412a6deac8b928c82945d692ef20a49535d65 Mon Sep 17 00:00:00 2001 From: Ian Campbell ian.campb...@citrix.com Date: Tue, 17 Jun 2014 15:48:48 +0100 Subject: [PATCH] xen: handle root= in xen-xm configuration files. In addition to extra= xm supported a root= option which was supposed to be incorporated into the final command line. Handle that for virsh domxml-from-native xen-xm. Tested with the libxl backend. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- v2: extra should default to . --- .gnulib|2 +- --- WTF. I stripped this out of the patch shown below... src/xenxs/xen_xm.c | 14 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index b2db97d..745041b 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -339,6 +339,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, def-os.nBootDevs++; } } else { +const char *extra, *root; + if (xenXMConfigCopyStringOpt(conf, bootloader, def-os.bootloader) 0) goto cleanup; if (xenXMConfigCopyStringOpt(conf, bootargs, def-os.bootloaderArgs) 0) @@ -348,8 +350,18 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, goto cleanup; if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd) 0) goto cleanup; -if (xenXMConfigCopyStringOpt(conf, extra, def-os.cmdline) 0) +if (xenXMConfigGetString(conf, extra, extra, ) 0) +goto cleanup; +if (xenXMConfigGetString(conf, root, root, NULL) 0) goto cleanup; + +if (root) { +if (virAsprintf(def-os.cmdline, root=%s %s, root, extra) 0) +goto cleanup; +} else { +if (VIR_STRDUP(def-os.cmdline, extra) 0) +goto cleanup; +} } if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon, -- 1.7.10.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] libxl: Add a test suite for libxl option generator
On Tue, Jun 17, 2014 at 12:40:30PM -0600, Jim Fehlig wrote: Ian Campbell wrote: On Mon, 2014-06-16 at 17:11 -0600, Jim Fehlig wrote: This function exists in Xen 4.2 as well, in libxl.h. Any ideas on how to handle this? I'm not aware of an existing macro to check for func 'foo' defined in header 'bar'. Is writing a custom macro along these lines a good solution? A bad solution I tried was hacking the test to check libxl version via libxl_get_version_info(), but that API does not work if not running Xen. Given that it exists in everything from 4.2 onwards why do you need to check for it? Hrm, right. I had the half-brained idea to use this to solve the failures I saw when testing this series against Xen 4.2 https://www.redhat.com/archives/libvir-list/2014-June/msg00170.html I think the solution to that specific problem is to use Xen 4.2 config as the baseline. But it got me thinking about the general problem you mentioned near the end of this mail https://www.redhat.com/archives/libvir-list/2014-June/msg00032.html With virJSONStringCompare in 1/1, Daniel provides a way to handle existence of new fields showing up in the json. But what if I want to write a test where the expected data is not supported on earlier versions? E.g. how would I add a test to check conversion of 'tpm ...' to 'vtpms: [ ]' and expect that to work when running 'make check' against a 4.2 libxl where vtpms were not yet supported? I suppose each such test would have to probe for the feature it checks and skip if not found. My last approach was to allow the JSON comparator to skip certain types of missing data but I don't think that's sufficiently flexible anymore. I'm thinking instead, that we could maintain a set of context paths which designate things that are new in each version, which would indicate things that should be skipped with old versions. So we with 4.3, we'd list /c_info/driver_domain /c_info/pvh and any other newly added fields. Then when running the test on 4.2 if the expected XML contained either of those paths, we'd skip them when comparing against the actual JSON. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] Only set SELinux seclabel if supported by the host.
Hi, On Tue, Jun 17, 2014 at 04:01:53PM +0200, Cédric Bosdonnat wrote: This code depends on new API in libvirt-gconfig to extract the secmodels handled by the host. --- Diff to v3: * Added yet another missing g_object_unref. * Fixed the logic for supportsSelinux libvirt-sandbox/libvirt-sandbox-builder.c | 49 +++ 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c index 48b3acc..d6b5735 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder.c +++ b/libvirt-sandbox/libvirt-sandbox-builder.c @@ -322,12 +322,10 @@ static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *build return TRUE; } - -static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder G_GNUC_UNUSED, -GVirSandboxConfig *config G_GNUC_UNUSED, -const gchar *statedir G_GNUC_UNUSED, -GVirConfigDomain *domain, -GError **error G_GNUC_UNUSED) +static gboolean gvir_sandbox_builder_construct_security_selinux (GVirSandboxBuilder *builder, + GVirSandboxConfig *config, + GVirConfigDomain *domain, + GError **error) { GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new(); const char *label = gvir_sandbox_config_get_security_label(config); @@ -360,6 +358,45 @@ static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil return TRUE; } +static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder, +GVirSandboxConfig *config, +const gchar *statedir G_GNUC_UNUSED, +GVirConfigDomain *domain, +GError **error) +{ +GVirConnection *connection = gvir_sandbox_builder_get_connection(builder); +GVirConfigCapabilities *configCapabilities; +GVirConfigCapabilitiesHost *hostCapabilities; +GList *secmodels, *iter; +gboolean supportsSelinux = FALSE; + +/* What security models are available on the host? */ +if (!(configCapabilities = gvir_connection_get_capabilities(connection, error))) { Missing g_object_unref(connection); here too. +return FALSE; +} + +hostCapabilities = gvir_config_capabilities_get_host(configCapabilities); + +secmodels = gvir_config_capabilities_host_get_secmodels(hostCapabilities); +for (iter = secmodels; iter != NULL; iter = iter-next) { +if (g_str_equal(gvir_config_capabilities_host_secmodel_get_model( +GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter-data)), selinux)) +supportsSelinux = TRUE; +g_object_unref(iter-data); +} + +g_list_free(secmodels); +g_object_unref(hostCapabilities); +g_object_unref(configCapabilities); +g_object_unref(connection); + +if (supportsSelinux) +return gvir_sandbox_builder_construct_security_selinux(builder, config, + domain, error); + +return TRUE; Wondering whether this we should return FALSE when we did nothing because we only support SELinux. Patch is fine otherwise, I can squash these changes in before pushing if you don't want to send yet another iteration ;) Christophe pgpIK4BjaJOW5.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virsh: man: Fix examples and docs for virsh version
The version command now takes the --daemon parameter for a while and the output example was pretty outdated. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1110673 --- tools/virsh.pod | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 35cf878..b522798 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -206,23 +206,28 @@ option instead. For example: quit this interactive terminal -=item Bversion +=item Bversion [I--daemon] Will print out the major version info about what this built from. +If I--daemon is specified then the version of the libvirt daemon +is included in the output. =over 4 BExample -Bvirsh version - -Compiled against library: libvir 0.0.6 - -Using library: libvir 0.0.6 - -Using API: Xen 3.0.0 - -Running hypervisor: Xen 3.0.0 + $ virsh version + Compiled against library: libvirt 1.2.3 + Using library: libvirt 1.2.3 + Using API: QEMU 1.2.3 + Running hypervisor: QEMU 2.0.50 + + $ virsh version --daemon + Compiled against library: libvirt 1.2.3 + Using library: libvirt 1.2.3 + Using API: QEMU 1.2.3 + Running hypervisor: QEMU 2.0.50 + Running against daemon: 1.2.6 =back -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] virsh: man: Tweak some old docs problems
Peter Krempa (2): virsh: man: Fix examples and docs for virsh version virsh: man: Correctly spell QEMU tools/virsh.pod | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] virsh: man: Correctly spell QEMU
s/QEemu/QEMU/g --- tools/virsh.pod | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index b522798..ceec1a0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -19,7 +19,7 @@ under the GNU Lesser General Public License. Virtualization of the Linux Operating System means the ability to run multiple instances of Operating Systems concurrently on a single hardware system where the basic resources are driven by a Linux instance. The library aims at -providing a long term stable C API. It currently supports Xen, QEmu, +providing a long term stable C API. It currently supports Xen, QEMU, KVM, LXC, OpenVZ, VirtualBox and VMware ESX. The basic structure of most virsh usage is: @@ -260,11 +260,11 @@ this is used to connect to the local Xen hypervisor =item qemu:///system -connect locally as root to the daemon supervising QEmu and KVM domains +connect locally as root to the daemon supervising QEMU and KVM domains =item qemu:///session -connect locally as a normal user to his own set of QEmu and KVM domains +connect locally as a normal user to his own set of QEMU and KVM domains =item lxc:/// -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH libvirt] xen: handle root= in xen-xm configuration files.
On Tue, 2014-06-17 at 15:36 -0600, Jim Fehlig wrote: Eric Blake wrote: On 06/17/2014 09:24 AM, Ian Campbell wrote: In addition to extra= xm supported a root= option which was supposed to be incorporated into the final command line. Handle that for virsh domxml-from-native xen-xm. Tested with the libxl backend. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- .gnulib|2 +- src/xenxs/xen_xm.c | 14 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.gnulib b/.gnulib index d55899f..e8e0eb6 16 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit d55899fd2c5794ac85ecb14d5e2f646a89e4b4dd +Subproject commit e8e0eb6bfb728685ec8d5afd924e41b18e9d928d Was the submodule bump intended? No, sorry, I've no idea how that happened (/me curses git submodules yet again). Once I understand that, then this patch (minus the .gnulib bump) seems okay. NB I just sent out a v2 -- extra should default to not NULL for this to work as intended. BTW, if cmdline contains root=, I noticed that domxml-to-native will put it in extra= instead of creating a root= entry. E.g. cmdlineroot=/dev/bla foo=bar/cmdline converts to extra=root=/dev/bla foo=bar, which is still valid config so perhaps not such a big deal. I think this is fine. Personally I consider the root= stuff to be a weird wart, in that it effectively exposes details of the Linux command line syntax in the xm/xl cfg file. It's far better IMHO to ignore it and write root=foo in the actual command line. Ian. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/12] Expose IOMMU and VFIO capabilities from virCaps
On 30.05.2014 11:11, Daniel P. Berrange wrote: On Thu, May 29, 2014 at 10:32:45AM +0200, Michal Privoznik wrote: This piece of information may be useful for management application to decide if a domain with a device passthrough can be started on given host or not. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Notes: I'm not very happy with the element names, but they're the best I could come up with so far. If you have any better suggestion I am all ears. docs/formatcaps.html.in | 8 +++- docs/schemas/capability.rng | 12 src/conf/capabilities.c | 4 tests/capabilityschemadata/caps-qemu-kvm.xml | 2 ++ tests/capabilityschemadata/caps-test.xml | 2 ++ tests/capabilityschemadata/caps-test2.xml| 2 ++ tests/capabilityschemadata/caps-test3.xml| 2 ++ tests/xencapsdata/xen-i686-pae-hvm.xml | 2 ++ tests/xencapsdata/xen-i686-pae.xml | 2 ++ tests/xencapsdata/xen-i686.xml | 2 ++ tests/xencapsdata/xen-ia64-be-hvm.xml| 2 ++ tests/xencapsdata/xen-ia64-be.xml| 2 ++ tests/xencapsdata/xen-ia64-hvm.xml | 2 ++ tests/xencapsdata/xen-ia64.xml | 2 ++ tests/xencapsdata/xen-ppc64.xml | 2 ++ tests/xencapsdata/xen-x86_64-hvm.xml | 2 ++ tests/xencapsdata/xen-x86_64.xml | 2 ++ 17 files changed, 51 insertions(+), 1 deletion(-) diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in index d060a5b..eb8c905 100644 --- a/docs/formatcaps.html.in +++ b/docs/formatcaps.html.in @@ -35,6 +35,8 @@ BIOS you will see/p lt;suspend_disk/gt; lt;suspend_hybrid/gt; lt;power_management/gt; +lt;kvmgt;truelt;/kvmgt; +lt;vfiogt;truelt;/vfiogt; lt;/hostgt;/span lt;!-- xen-3.0-x86_64 --gt; @@ -78,7 +80,11 @@ BIOS you will see/p Suspend-to-Disk (S4) and Hybrid-Suspend (a combination of S3 and S4). In case the host does not support any such feature, then an empty lt;power_management/gt; - tag will be shown. /p + tag will be shown. Then, two elements + codelt;kvm/gt;/code and codelt;vfio/gt;/code + expose the fact, whether the host supports legacy device + passthrough with IOMMU cooperation or newer Virtual function + I/O./p pThe second block (in blue) indicates the paravirtualization support of the Xen support, you will see the os_type of xen to indicate a paravirtual kernel, then architecture diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index d2d9776..3b378eb 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -48,6 +48,18 @@ zeroOrMore ref name='secmodel'/ /zeroOrMore + element name='kvm' + choice + valuefalse/value + valuetrue/value + /choice + /element + element name='vfio' + choice + valuefalse/value + valuetrue/value + /choice + /element /element /define diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 9561ba3..a91f37b 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -901,6 +901,10 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(buf, /secmodel\n); } +/* KVM and VFIO features */ +virBufferAsprintf(buf, kvm%s/kvm\n, caps-host.legacyKVMPassthrough ? true : false); +virBufferAsprintf(buf, vfio%s/vfio\n, caps-host.VFIOPassthrough ? true : false); Ah, so this is missing from the previous patch. In fact the splt between this previous patch is a bit confusing. I'd expect the previous patch to only change the src/conf/capabilities* files and generic test cases. While this patch only change the driver code and driver specific test cases. So this XML is really trying to provide a list of the valid enumeration options for the driver element of hostdev devices. This is quite a common request for many domain XML elements, so I feel we ought to do something that is not so ad-hoc here. When we get into this world of providing info about supported options for devices, we really need to be able to express this on a fine granularity that the capabilities XML allows for. For example, different emulator binaries will support different options, as will many different architectures, or even different machine types of virtualization types. So it feels like we need a new API for this, that accepts info about the machine we're trying to launch. eg char * virConnectGetEmulatorCapabilties(virConnectPtr conn, const char *emulatorbin, const char *machine, const char *virttype); What's this virttype argument? NB I didn't
Re: [libvirt] [PATCH 0/2] virsh: man: Tweak some old docs problems
On 06/18/2014 11:28 AM, Peter Krempa wrote: Peter Krempa (2): virsh: man: Fix examples and docs for virsh version virsh: man: Correctly spell QEMU tools/virsh.pod | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) ACK signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/12] Expose IOMMU and VFIO capabilities from virCaps
On Wed, Jun 18, 2014 at 11:51:51AM +0200, Michal Privoznik wrote: On 30.05.2014 11:11, Daniel P. Berrange wrote: diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 9561ba3..a91f37b 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -901,6 +901,10 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAddLit(buf, /secmodel\n); } +/* KVM and VFIO features */ +virBufferAsprintf(buf, kvm%s/kvm\n, caps-host.legacyKVMPassthrough ? true : false); +virBufferAsprintf(buf, vfio%s/vfio\n, caps-host.VFIOPassthrough ? true : false); Ah, so this is missing from the previous patch. In fact the splt between this previous patch is a bit confusing. I'd expect the previous patch to only change the src/conf/capabilities* files and generic test cases. While this patch only change the driver code and driver specific test cases. So this XML is really trying to provide a list of the valid enumeration options for the driver element of hostdev devices. This is quite a common request for many domain XML elements, so I feel we ought to do something that is not so ad-hoc here. When we get into this world of providing info about supported options for devices, we really need to be able to express this on a fine granularity that the capabilities XML allows for. For example, different emulator binaries will support different options, as will many different architectures, or even different machine types of virtualization types. So it feels like we need a new API for this, that accepts info about the machine we're trying to launch. eg char * virConnectGetEmulatorCapabilties(virConnectPtr conn, const char *emulatorbin, const char *machine, const char *virttype); What's this virttype argument? Matches the same attribute in domain XML - domain type='kvm|qemu|xen|uml|...' NB I didn't include 'architecture' since that's implicit in the emulatorbin chosen. The 'char *' return value would be an XML schema As for the XML schema, I haven't given it huge thought, but perhaps something that loosely mirrors the XML schema is desirable. the XML schema? You mean the capabilities XML schema? I mean the new emulator XML schema should roughly follow the structure of the domain XML schema. so if domain XML has domaindevicesdisk.../disk/devices/domain then, we'd want to keep this kind of nesting when reporting info about what the disk element supports for its various enums. So for the hostdev driver type enum I could imagine starting off with: emulatorCapabilities path/usr/bin/qemyu-system-x86_64/path domainkvm/domain archx86_64/arch machinepc-1.0/machine devices hostdev driver I find this misleading. I mean, why kvm and vfio is under devices/hostdev/driver? IIUC, you were attempting to expose flags showing whether the host kernel supports vfio or kvm PCI passthrough. Apps would then consume that to decide what the hostdev mode='subsystem' type='pci' for values of the driver name='kvm|vfio|...' attribute. Of course that's only half the problem because they also need to know if the QEMU binary supports the 'vfio' scheme too. I'm saying that instead of having capabilities which represent what the host kernel supports, we should directly expose information on whether you can use the attribute in the driver element of the hostdev. This captures both kernel and QEMU supportability. enum name=driver valuekvm/value valuevfio/value /enum /driver /hostdev /devices /emulatorCapabilities I would expect that the 'maxCpus' hack we stuffed into the existing capabilities XML would be added to this too eg vcpus max=120/ at the top level So let me see if I understand correctly. To represent emulatorCapabilities in memory we need another structure, say: typedef struct _virEmulatorCapabilities virEmulatorCapabilities; typedef virEmulatorCapabilities *virEmulatorCapabilitiesPtr; struct _virEmulatorCapabilities { some values here }; which will live in src/conf/capabilities.*. Can you shed a light on the structure internals and where it should be created? IIUC, it'll be created in the virConnectGetEmulatorCapabilties(), then formated and immediately disposed. Ignore the existing capabilities XML and its code. This should be a new XML schema that is completely separate giving information specifically related to features of the emulator in question. We'd not use the existing virCapabilities structure. Instead I'd expect that we directly populate this new schema with info from the qemuCapsPtr object. This mechanism will let us address this more general problem that both libguestfs and virt-manager have repeatedly asked for - a way to
Re: [libvirt] [PATCH 0/2] virsh: man: Tweak some old docs problems
On 06/18/14 12:06, Ján Tomko wrote: On 06/18/2014 11:28 AM, Peter Krempa wrote: Peter Krempa (2): virsh: man: Fix examples and docs for virsh version virsh: man: Correctly spell QEMU tools/virsh.pod | 29 + 1 file changed, 17 insertions(+), 12 deletions(-) ACK Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] Only set SELinux seclabel if supported by the host.
On Wed, 2014-06-18 at 11:11 +0200, Christophe Fergeau wrote: Hi, On Tue, Jun 17, 2014 at 04:01:53PM +0200, Cédric Bosdonnat wrote: This code depends on new API in libvirt-gconfig to extract the secmodels handled by the host. --- Diff to v3: * Added yet another missing g_object_unref. * Fixed the logic for supportsSelinux libvirt-sandbox/libvirt-sandbox-builder.c | 49 +++ 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/libvirt-sandbox/libvirt-sandbox-builder.c b/libvirt-sandbox/libvirt-sandbox-builder.c index 48b3acc..d6b5735 100644 --- a/libvirt-sandbox/libvirt-sandbox-builder.c +++ b/libvirt-sandbox/libvirt-sandbox-builder.c @@ -322,12 +322,10 @@ static gboolean gvir_sandbox_builder_construct_devices(GVirSandboxBuilder *build return TRUE; } - -static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder G_GNUC_UNUSED, -GVirSandboxConfig *config G_GNUC_UNUSED, -const gchar *statedir G_GNUC_UNUSED, -GVirConfigDomain *domain, -GError **error G_GNUC_UNUSED) +static gboolean gvir_sandbox_builder_construct_security_selinux (GVirSandboxBuilder *builder, + GVirSandboxConfig *config, + GVirConfigDomain *domain, + GError **error) { GVirConfigDomainSeclabel *sec = gvir_config_domain_seclabel_new(); const char *label = gvir_sandbox_config_get_security_label(config); @@ -360,6 +358,45 @@ static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *buil return TRUE; } +static gboolean gvir_sandbox_builder_construct_security(GVirSandboxBuilder *builder, +GVirSandboxConfig *config, +const gchar *statedir G_GNUC_UNUSED, +GVirConfigDomain *domain, +GError **error) +{ +GVirConnection *connection = gvir_sandbox_builder_get_connection(builder); +GVirConfigCapabilities *configCapabilities; +GVirConfigCapabilitiesHost *hostCapabilities; +GList *secmodels, *iter; +gboolean supportsSelinux = FALSE; + +/* What security models are available on the host? */ +if (!(configCapabilities = gvir_connection_get_capabilities(connection, error))) { Missing g_object_unref(connection); here too. Oops, I forgot that one case indeed. +return FALSE; +} + +hostCapabilities = gvir_config_capabilities_get_host(configCapabilities); + +secmodels = gvir_config_capabilities_host_get_secmodels(hostCapabilities); +for (iter = secmodels; iter != NULL; iter = iter-next) { +if (g_str_equal(gvir_config_capabilities_host_secmodel_get_model( +GVIR_CONFIG_CAPABILITIES_HOST_SECMODEL(iter-data)), selinux)) +supportsSelinux = TRUE; +g_object_unref(iter-data); +} + +g_list_free(secmodels); +g_object_unref(hostCapabilities); +g_object_unref(configCapabilities); +g_object_unref(connection); + +if (supportsSelinux) +return gvir_sandbox_builder_construct_security_selinux(builder, config, + domain, error); + +return TRUE; Wondering whether this we should return FALSE when we did nothing because we only support SELinux. No idea what the original intent was... but we shouldn't fail if we just not using any security label: that may be a valid use case. Patch is fine otherwise, I can squash these changes in before pushing if you don't want to send yet another iteration ;) Well, that would save a few mails to the mailing list ;) -- Cedric -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] NL_RECV RETURNED WITH ERROR: NO BUFFER SPACE AVAILABLE ERROR FOR IXGBE LOAD
On 06/17/2014 06:16 AM, vaughan wrote: Hi experts, Release: OL7 Kernel: 3.10.0-121.el7.x86_64 Noticed below error on OL7 server, while loading Intel 10gigabit nic driver module , ixgbe in syslog journal: nl_recv returned with error: No buffer space available --- Complete syslog content for ixgbe module load : un 16 20:46:10 ca-ostest432 kernel: ixgbe: Intel(R) 10 Gigabit PCI Express Network Driver - version 3.15.1-k Jun 16 20:46:10 ca-ostest432 kernel: ixgbe: Copyright (c) 1999-2013 Intel Corporation. Jun 16 20:46:10 ca-ostest432 kvm: 1 guest now active Jun 16 20:46:10 ca-ostest432 kvm: 0 guests now active Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: Multiqueue Enabled: Rx Queue count = 16, Tx Queue count = 16 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: (PCI Express:5.0GT/s:Width x8) 00:1b:21:c8:24:74 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: MAC: 2, PHY: 9, SFP+: 3, PBA No: E70856-007 Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: PCI Express bandwidth of 32GT/s available Jun 16 20:46:10 ca-ostest432 kernel: ixgbe :13:00.0: (Speed:5.0GT/s, Width: x8, Encoding Loss:20%) Jun 16 20:46:10 ca-ostest432 journal: nl_recv returned with error: No buffer space available A very similar problem (probably the same, but you're showing the kernel error message rather than the error logged by libvirt) was reported and addressed in libnl3 quite awhile back. libnl3 originally set the default buffer size to 4096, which wasn't enough for SRIOV cards with lots of VFs. So they increased it to 4 * 4096, which should be plenty for anybody. That libnl3 patch is present in RHEL7.0 (currently at 3.2.21.6). Can you verify the version of libnl3 you are running, and that it contains this code if (page_size == 0) page_size = getpagesize() * 4; in the function lib/nl.c:nl_recv() (previously it was just page_size = getpagesize();). If you don't have that patch in your libnl3 package, please backport the upstream commit that makes that change. If you do have that patch in your libnl3, perhaps you have gotten a different ixgbe driver from somewhere (we did test against ixgbe with the maximum number of VFs, so there would have to be something different in your driver). It would be good to figure out the source of the problem before applying any fix anywhere - much better to understand the cause, and right now I don't think we do; what is creating the need for such a large buffer in your case, but not for others who use the same driver with the same number of VFs?). Up to now our position has been that this problem should be fixed in libnl, so we have preferred to not patch libvirt for it, but instead get libnl fixed. If we do decide to patch libvirt, I think it would be better to turn on message peeking for nl_recv (nl_socket_enable_msg_peek()), as that would solve the problem totally and permanently (the upstream maintainer of libnl is reluctant to turn that on by default due to potential performance problems in other users of libnl) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] fix issues in vbox found by make check on RHEL6
Pavel Hrdina (2): tests: fix vbox snapshot xmls vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc' src/vbox/vbox_snapshot_conf.c | 2 +- tests/vboxsnapshotxmldata/2disks-1snap.vbox| 292 +++ tests/vboxsnapshotxmldata/2disks-2snap.vbox| 584 +++--- .../vboxsnapshotxmldata/2disks-3snap-brother.vbox | 292 +++ tests/vboxsnapshotxmldata/2disks-3snap.vbox| 876 ++--- 5 files changed, 1023 insertions(+), 1023 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc'
The function 'xmlSaveFormatFileEnc' has a last option to set if you want to format dumped xml with whitespaces or not. Older libxml2, the one used in RHEL6, take this option as it is but newer libxml2 check this option if it's true or not. This small difference somehow makes things messy on RHEL6 and generated xml had extra new line and extra whitespaces. We should pass 1 instead if -1 because the -1 confuses the libxml2. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/vbox/vbox_snapshot_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index 3f7ad78..7af1231 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -1211,7 +1211,7 @@ virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachinePtr machine, } } -if (xmlSaveFormatFileEnc(filePath, xml, ISO-8859-1, -1) 0) { +if (xmlSaveFormatFileEnc(filePath, xml, ISO-8859-1, 1) 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(Unable to save the xml)); goto cleanup; -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
On Mon, 2014-06-02 at 14:44 +0100, George Dunlap wrote: == Open questions == Those are things I think I know; there are a couple of pertinent factual questions which I'm not sure of: * Is it possible to specify PVUSB controllers and attach USB devices to them before the guest is up and running (i.e., before the frontend is connected)? It looks like xend had a syntax for specifying virtual controllers and attaching virtual devices, so it seems like it should be possible. Unless the PVUSB drivers are radically different to other PV devices this ought to be possible and should just work. (Essentially you just preload the backend with all the settings and htey get handled when the f.e. turns up) * Is it possible to connect a USB 1.1 device to a PVUSB controller which has been specified 2.0, or would there have to be a separate virtual controller for each? I'm a bit surprised that PVUSB exposes the concept of a version to the FE at all. I suppose there is some USBish reason why the f.e. would care. But I don't know the answer to your question. * Is it possible for the toolstack to tell if dom0 (or whatever the specified backend domain) has PVUSB support before starting the guest? After creating the nodes with state == XenbusStateInitialising (1) the toolstack waits for the backend to move to XenbusStateInitWait (2) before continuing, with a timeout. So you will detect this in a controlled way. You can't tell before try the setup though since the driver might be autoloaded. (Assuming pvusbback is the same as everything else) * Is it possible for the toolstack to tell if domU has a working and connected PVUSB front-end? It can observe the state variable being 4 I suppose. Why do you need to know? * Do we want to be able to create virtual hubs for qemu-backed controllers at some point in the future? Is there any groundwork we want to lay for that? qemu-backed emulated or PV controllers? I don't think emulated would make sense for a PV guest and if qemu wanted to be a PV backend it would have to implement the usual xenstore watches etc. I suppose a backend type a la libxl_device_disk's = phy|tap|qdisk might be needed for this, but I think you can probably add that in a compatible way in the future if necessary. == Design questions == Then based on that, we have several design questions. * How much of the controller thing should be exposed via libxl? Via xl? * This series only allows you to specify the protocol, either PV or DEVICE_MODEL. Would it be better, for instance, to get rid of that, and instead allow the user to specify the creation of USB controllers, allow them to have a type of HCI (or emulated) or PV, and allow the user to specify which controller to attach a specific device to? * How much smarts should we have in the libxl / xl about creating emulated/virtual controllers and of what kinds? * How much / what kind of smarts should be in libxl / xl about choosing a controller to plug a device into? Dunno * 4. Your proposed design looked ok to me. * What about config file syntax? Should we try to reuse / extend the current config syntax, or create a new one? Should the new one allow the specification of controllers? Should it perhaps *require* the specification of controllers? We should at least strive for any existing xm config files which use USB to continue working, but that needn't imply that the preferred xl syntax looks that way. Of course if the xm syntax is horribly terribly broken then we might make a concious choice not to carry it forward, but the default should be compatibility. For reference, here are some example config snippets from the current xl / xm config files: -- snip -- # HVM USB usb=1 usbdevice=['tablet','host:4.3'] # HVM USB, not compatible with the above usbversion=3 # xend's PVUSB config file; this creates one virtual controller, then # plugs hostdev 1.8 into port 1 vusb=['usbver=2, numports=4, port_1=1-8'] Oh my. That last one is quite exciting. -- snip -- Given that, here is a potential config file format: -- snip -- # Create two controllers, one pv, one emulated usbcontroller=['type=pv,name=pv0,usbversion=2,numports=4', 'type=emul,name=hci0,usbversion=2'] # Create a controller with the defaults; PV for PV guests, emul for HVM guests usbcontroller=[''] # Same as above, but defaulting to version 2 usbversion=2 # I think we should be able to automatically detect which format to # use; so I think we should re-use usbdevice. I could be convinced otherwise. usbdevice=['type=tablet','type=hostdev,hostaddr=4.3,bus=pv0'] Does this require that the usbcontroller have been specified? I think it would be good if xl would by default create some number of appropriate controllers without my having to specify them explicitly, iow just saying usbdevice=[...] should be enough. I'm not saying that you shouldn't support more specific syntax for people who want
Re: [libvirt] [PATCH 1/2] tests: fix vbox snapshot xmls
On 06/18/14 14:29, Pavel Hrdina wrote: On RHEL6 the vboxsnapshotxmltest fails because of wrong xml that is generated by libvirt. However the core issue is in the xml data itself with the wrong indentation. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- tests/vboxsnapshotxmldata/2disks-1snap.vbox| 292 +++ tests/vboxsnapshotxmldata/2disks-2snap.vbox| 584 +++--- .../vboxsnapshotxmldata/2disks-3snap-brother.vbox | 292 +++ tests/vboxsnapshotxmldata/2disks-3snap.vbox| 876 ++--- 4 files changed, 1022 insertions(+), 1022 deletions(-) ACK, git show -b verifies that it's a whitespace only change Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc'
On 06/18/14 14:29, Pavel Hrdina wrote: The function 'xmlSaveFormatFileEnc' has a last option to set if you want to format dumped xml with whitespaces or not. Older libxml2, the one used in RHEL6, take this option as it is but newer libxml2 check this option if it's true or not. This small difference somehow makes things messy on RHEL6 and generated xml had extra new line and extra whitespaces. We should pass 1 instead if -1 because the -1 confuses the libxml2. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/vbox/vbox_snapshot_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
On 06/18/2014 01:57 PM, Ian Campbell wrote: * Is it possible for the toolstack to tell if domU has a working and connected PVUSB front-end? It can observe the state variable being 4 I suppose. Why do you need to know? It might be nice to be able to create both a pv and an emulated controller in the config file, and then have xl usb-attach [foo] to automatically plug it into the PV controller if the PV frontend seems to be up, and into the emulated controller if it doesn't seem to be up. But that doesn't change the elements of the interface, just what the default would be if the controller field is empty. * Do we want to be able to create virtual hubs for qemu-backed controllers at some point in the future? Is there any groundwork we want to lay for that? qemu-backed emulated or PV controllers? I don't think emulated would make sense for a PV guest and if qemu wanted to be a PV backend it would have to implement the usual xenstore watches etc. I mean, emulate an actual USB hub -- you know, it plugs into your USB controller and you can plug other USB devices into it. I'm inclined not to bother with it at this point. -- snip -- Given that, here is a potential config file format: -- snip -- # Create two controllers, one pv, one emulated usbcontroller=['type=pv,name=pv0,usbversion=2,numports=4', 'type=emul,name=hci0,usbversion=2'] # Create a controller with the defaults; PV for PV guests, emul for HVM guests usbcontroller=[''] # Same as above, but defaulting to version 2 usbversion=2 # I think we should be able to automatically detect which format to # use; so I think we should re-use usbdevice. I could be convinced otherwise. usbdevice=['type=tablet','type=hostdev,hostaddr=4.3,bus=pv0'] Does this require that the usbcontroller have been specified? I think it would be good if xl would by default create some number of appropriate controllers without my having to specify them explicitly, iow just saying usbdevice=[...] should be enough. I'm not saying that you shouldn't support more specific syntax for people who want more control, just that it shouldn't be required to do so. (I'm just talking xl here, at the libxl layer I think it would be fine to require them to be explicit). That makes sense. * Rather than having to specify a controller, automatically hot-plug controllers as-needed. At the xl level I think this would be good. OK, sounds good. Thanks for the input. -George -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc'
On 18.6.2014 15:11, Peter Krempa wrote: On 06/18/14 14:29, Pavel Hrdina wrote: The function 'xmlSaveFormatFileEnc' has a last option to set if you want to format dumped xml with whitespaces or not. Older libxml2, the one used in RHEL6, take this option as it is but newer libxml2 check this option if it's true or not. This small difference somehow makes things messy on RHEL6 and generated xml had extra new line and extra whitespaces. We should pass 1 instead if -1 because the -1 confuses the libxml2. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- src/vbox/vbox_snapshot_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK, Peter Thanks, pushed Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v7 RFC 0/2] libxl USB prototype and design discussion
On 06/18/2014 01:57 PM, Ian Campbell wrote: * Is it possible to connect a USB 1.1 device to a PVUSB controller which has been specified 2.0, or would there have to be a separate virtual controller for each? I'm a bit surprised that PVUSB exposes the concept of a version to the FE at all. I suppose there is some USBish reason why the f.e. would care. But I don't know the answer to your question. Simon, Can you put Test different USB version devices with different PVUSB bus version numbers on your to-do list? (i.e., find USB 1.1, 2.0, and 3.0 devices, and try each of them with usbversion set to 1, 2, and 3). Thanks! -George -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: network interface tags vs. portgroups
Ping - any opinions on this? On 06/10/2014 01:01 PM, Laine Stump wrote: A couple releases ago (commit 7d5bf484, first appeared in 1.2.2) I modified the domain interface status xml to show what resources are actually in use for an interface, superseding the interface config in the cases where they conflict with each other. In particular, if there is an interface of type='network' that references a portgroup of the network in the source element, the interface status will not contain a source element showing the network and portgroup names, but instead the source resulting from applying the config is shown. For example, given the following domain interface and network definitions: interface type='network' source network='mynet' portgroup='xyzzy'/ ... /interface network namemynet/name forward mode='bridge'/\ bridge name='br0'/ portgroup name='xyzzy' bandwidth inbound average='1000' peak='5000' floor='200' burst='1024'/ outbound average='128' peak='256' burst='256'/ /bandwidth /portgroup /network the status that was previously displayed when the domain was running would be identical to the config above (except that it would also contain the tap device name and alias). But now the status will be this: interface type='bridge' source bridge='br0'/ bandwidth inbound average='1000' peak='5000' floor='200' burst='1024'/ outbound average='128' peak='256' burst='256'/ /bandwidth ... /interface The advantage here is that a hook script for the domain will be able to see the bandwidth (and vlan and physical device, if any) that are actually being used by the domain's interface. Because the config and status both use the same elements/attributes, we can only show one or the other; the thinking was that normally the status will be what is desired, and anyone who really wants to look at the config should use the VIR_DOMAIN_XML_INACTIVE flag when calling virDomainGetXMLDesc(). As you would expect, a few months later (after the release of 1.2.4) someone on IRC checked in with a problem caused by this change - they had been using the portgroup name in the source element of the interface to determine what action to take during migration; they didn't even have any libvirt config stored in the portgroup, but were just using its name as a tag. Since the portgroup name is only a part of the source element when the interface is type='network', they now don't have a tag in the xml to use for their decision (and since they aren't explicitly calling virDomainGetXMLDesc() themselves, they can't simply get the INACTIVE xml to solve their problem). This use of a portgroup name struck me as potentially useful (although it is a slight hijacking of the original intent of portgroups), so I would like to restore that functionality. I came up with a couple different ways to solve the problem, and am looking for opinions before I spend any time on either. Solution 1: My initial thought was to just restore the portgroup name in the status XML; that could be done by moving the portgroup name out of the network-specific part of the object and into common data for all interface types (this way it could show up in the source element no matter what is the network type). However, once we've done that it becomes enticing to allow specification of a portgroup even in cases where the interface type != network; in those cases though, the portgroup would be *only* a tag to be used by external entities; this would lead to lax checking for existence of the specified portgroup, and may end up with people misspelling a portgroup name, then mistakenly believing that (e.g.) they had a bandwidth limit applied to a domain interface when none was in fact in effect. (alternately, we could allow it only if the interface *config* was for type='network', but that seems somehow logically broken, and you can bet that eventually someone would ask for us to allow it for all types) Solution 2: An alternate idea I had was to add a new tag name='x'/ element to interfaces, networks, and portgroups. An interface could have multiple tags, and would assume the tags of its network when active. A tag would be purely for use by external entities - it would mean nothing to libvirt. For example, given this extreme example: interface type='network' source network='mynet' portgroup='xyzzy'/ tag name='wumpus'/ ... /interface network namemynet/name tag name='twisty'/ forward mode='bridge'/\ bridge name='br0'/ portgroup name='xyzzy' tag name='xyzzytag'/ bandwidth inbound average='1000' peak='5000' floor='200' burst='1024'/ outbound average='128' peak='256' burst='256'/ /bandwidth /portgroup /network network when the
[libvirt] [php PATCH] Use long variable type for zend_parse_parameters.
This patch fixes a bug where zend_parse_parameters would segfault on certain PHP version (spotted on PHP 5.3.2 64bit) where type specifier is long l and variable reference is int or unsigned int. Changing the variable type from int or unsigned int to long fixes the problem for me and is a known issue [1]. This did not happen in newer PHP version (5.3.10). [1] https://bugs.php.net/bug.php?id=59289 --- src/libvirt-php.c | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 07ae137..224943d 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -3283,8 +3283,8 @@ PHP_FUNCTION(libvirt_domain_get_metadata) { php_libvirt_domain *domain = NULL; zval *zdomain; - int type = 0; - unsigned int flags = 0; + long type = 0; + long flags = 0; char *uri = NULL; int uri_len; char *ret = NULL; @@ -3328,8 +3328,8 @@ PHP_FUNCTION(libvirt_domain_set_metadata) char *metadata = NULL; char *key = NULL; char *uri = NULL; - int type = 0; - unsigned int flags = 0; + long type = 0; + long flags = 0; int rc; GET_DOMAIN_FROM_ARGS (rlsssl, zdomain, type, metadata, metadata_len, key, key_len, uri, uri_len, flags); @@ -3566,7 +3566,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) { php_libvirt_domain *domain=NULL; zval *zdomain; - unsigned int screen = 0; + long screen = 0; int fd = -1; char file[] = /tmp/libvirt-php-tmp-XX; virStreamPtr st = NULL; @@ -3668,7 +3668,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot) int port = -1; char *hostname = NULL; int hostname_len; - int scancode = 10; + long scancode = 10; char *path; char name[1024] = { 0 }; int use_builtin = 0; @@ -3908,9 +3908,9 @@ PHP_FUNCTION(libvirt_domain_send_pointer_event) char *xml = NULL; char *hostname = NULL; int hostname_len; - int pos_x = 0; - int pos_y = 0; - int clicked = 0; + long pos_x = 0; + long pos_y = 0; + long clicked = 0; int release = 1; int ret; @@ -4352,12 +4352,12 @@ PHP_FUNCTION(libvirt_domain_new) // char *emulator; char *iso_image = NULL; int iso_image_len; - int vcpus = -1; - int memMB = -1; + long vcpus = -1; + long memMB = -1; zval *disks, *networks; tVMDisk *vmDisks = NULL; tVMNetwork *vmNetworks = NULL; - int maxmemMB = -1; + long maxmemMB = -1; HashTable *arr_hash; int numDisks, numNets, i; zval **data; // removed **zvalue @@ -4642,7 +4642,7 @@ PHP_FUNCTION(libvirt_domain_get_interface_devices) */ PHP_FUNCTION(libvirt_domain_change_vcpus) { - int numCpus, flags = 0; + long numCpus, flags = 0; php_libvirt_domain *domain=NULL; zval *zdomain; @@ -6266,7 +6266,7 @@ PHP_FUNCTION(libvirt_domain_snapshot_delete) { zval *zsnapshot; php_libvirt_snapshot *snapshot; - int flags = 0; + long flags = 0; int retval; GET_SNAPSHOT_FROM_ARGS(r|l,zsnapshot, flags); @@ -6766,7 +6766,7 @@ PHP_FUNCTION(libvirt_storagevolume_delete) { php_libvirt_volume *volume=NULL; zval *zvolume; - int flags = 0; + long flags = 0; int retval = 0; GET_VOLUME_FROM_ARGS(r|l,zvolume,flags); @@ -7057,7 +7057,7 @@ PHP_FUNCTION(libvirt_storagepool_refresh) { php_libvirt_storagepool *pool = NULL; zval *zpool; - unsigned long flags = 0; + long flags = 0; int retval; GET_STORAGEPOOL_FROM_ARGS (r|l, zpool, flags); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt] xen: handle root= in xen-xm configuration files.
Ian Campbell wrote: On Tue, 2014-06-17 at 16:24 +0100, Ian Campbell wrote: +if (xenXMConfigGetString(conf, extra, extra, NULL) 0) This was subtly broken. The default needs to be . -8-- From 539412a6deac8b928c82945d692ef20a49535d65 Mon Sep 17 00:00:00 2001 From: Ian Campbell ian.campb...@citrix.com Date: Tue, 17 Jun 2014 15:48:48 +0100 Subject: [PATCH] xen: handle root= in xen-xm configuration files. In addition to extra= xm supported a root= option which was supposed to be incorporated into the final command line. Handle that for virsh domxml-from-native xen-xm. Tested with the libxl backend. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- v2: extra should default to . --- .gnulib|2 +- --- WTF. I stripped this out of the patch shown below... src/xenxs/xen_xm.c | 14 +- 2 files changed, 14 insertions(+), 2 deletions(-) ACK and pushed; thanks! Regards, Jim diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c index b2db97d..745041b 100644 --- a/src/xenxs/xen_xm.c +++ b/src/xenxs/xen_xm.c @@ -339,6 +339,8 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, def-os.nBootDevs++; } } else { +const char *extra, *root; + if (xenXMConfigCopyStringOpt(conf, bootloader, def-os.bootloader) 0) goto cleanup; if (xenXMConfigCopyStringOpt(conf, bootargs, def-os.bootloaderArgs) 0) @@ -348,8 +350,18 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, goto cleanup; if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd) 0) goto cleanup; -if (xenXMConfigCopyStringOpt(conf, extra, def-os.cmdline) 0) +if (xenXMConfigGetString(conf, extra, extra, ) 0) +goto cleanup; +if (xenXMConfigGetString(conf, root, root, NULL) 0) goto cleanup; + +if (root) { +if (virAsprintf(def-os.cmdline, root=%s %s, root, extra) 0) +goto cleanup; +} else { +if (VIR_STRDUP(def-os.cmdline, extra) 0) +goto cleanup; +} } if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH python] Correct virDomainMigrateToURI3 definition
Ping? (Adding Daniel Veillard, listed as owner for libvirt-python.git, as this was previously just sent to the list.) On 6/5/2014 11:43 AM, Jason Andryuk wrote: dconnuri is a string, so update the definition to match. Without this, the generated python would fail when passed a string. --- libvirt-override-api.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml index d5b25b5..935e04d 100644 --- a/libvirt-override-api.xml +++ b/libvirt-override-api.xml @@ -562,7 +562,7 @@ infoMigrate the domain object from its current host to the destination host given by URI./info arg name='domain' type='virDomainPtr' info='a domain object'/ - arg name='dconnuri' type='virConnectPtr' info='URI for target libvirtd if @flags includes VIR_MIGRATE_PEER2PEER'/ + arg name='dconnuri' type='char *' info='URI for target libvirtd if @flags includes VIR_MIGRATE_PEER2PEER'/ arg name='params' type='char *' info='dictionary with migration parameters'/ arg name='flags' type='unsigned int' info='an ORapos;ed set of virDomainMigrateFlags'/ return type='int' info='0 in case of success, -1 in case of failure.'/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Valid pool format type for logical volume pool should be auto and lvm2
On 06/18/2014 03:25 AM, Shanzhi Yu wrote: The logical volume pool supports formats auto and lvm2 Signed-off-by: Shanzhi Yu s...@redhat.com --- src/conf/storage_conf.c | 2 +- src/conf/storage_conf.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8b6fd79..3d61273 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -77,7 +77,7 @@ VIR_ENUM_IMPL(virStoragePoolFormatDisk, VIR_ENUM_IMPL(virStoragePoolFormatLogical, VIR_STORAGE_POOL_LOGICAL_LAST, - unknown, lvm2) + auto, lvm2) VIR_ENUM_IMPL(virStorageVolFormatDisk, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 04d99eb..2b08ac5 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -462,7 +462,7 @@ typedef enum { VIR_ENUM_DECL(virStoragePoolFormatDisk) typedef enum { -VIR_STORAGE_POOL_LOGICAL_UNKNOWN = 0, +VIR_STORAGE_POOL_LOGICAL_AUTO = 0, VIR_STORAGE_POOL_LOGICAL_LVM2 = 1, VIR_STORAGE_POOL_LOGICAL_LAST, } virStoragePoolFormatLogical; NACK See http://libvirt.org/storage.html for details on how 'auto' is used. Although not specifically called out for Logical Volume pools in the documentation - it follows how other pools (filesystem and network filesystem) use auto. Also remember how libvirt works - 'calloc' memory resulting in a 'default' of 0 (zero). Without the presence of a format type, it'll be up to the pool code to determine what's there and what to default to. There's only one format type for Logical Volume pools - so it's probably fairly easy to guess what's in the pool... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [php PATCH] Use long variable type for zend_parse_parameters.
Hi Dawid, I've applied it to the libvirt-php git repository. See: http://libvirt.org/git/?p=libvirt-php.git;a=commit;h=d3b3afa7d37541984d1e80e4ab46cd3e582ea60d Thanks, Michal 2014-06-18 19:11 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com: 2014-06-18 19:11 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com: This patch fixes a bug where zend_parse_parameters would segfault on certain PHP version (spotted on PHP 5.3.2 64bit) where type specifier is long l and variable reference is int or unsigned int. Changing the variable type from int or unsigned int to long fixes the problem for me and is a known issue [1]. This did not happen in newer PHP version (5.3.10). [1] https://bugs.php.net/bug.php?id=59289 --- src/libvirt-php.c | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 07ae137..224943d 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -3283,8 +3283,8 @@ PHP_FUNCTION(libvirt_domain_get_metadata) { php_libvirt_domain *domain = NULL; zval *zdomain; - int type = 0; - unsigned int flags = 0; + long type = 0; + long flags = 0; char *uri = NULL; int uri_len; char *ret = NULL; @@ -3328,8 +3328,8 @@ PHP_FUNCTION(libvirt_domain_set_metadata) char *metadata = NULL; char *key = NULL; char *uri = NULL; - int type = 0; - unsigned int flags = 0; + long type = 0; + long flags = 0; int rc; GET_DOMAIN_FROM_ARGS (rlsssl, zdomain, type, metadata, metadata_len, key, key_len, uri, uri_len, flags); @@ -3566,7 +3566,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot_api) { php_libvirt_domain *domain=NULL; zval *zdomain; - unsigned int screen = 0; + long screen = 0; int fd = -1; char file[] = /tmp/libvirt-php-tmp-XX; virStreamPtr st = NULL; @@ -3668,7 +3668,7 @@ PHP_FUNCTION(libvirt_domain_get_screenshot) int port = -1; char *hostname = NULL; int hostname_len; - int scancode = 10; + long scancode = 10; char *path; char name[1024] = { 0 }; int use_builtin = 0; @@ -3908,9 +3908,9 @@ PHP_FUNCTION(libvirt_domain_send_pointer_event) char *xml = NULL; char *hostname = NULL; int hostname_len; - int pos_x = 0; - int pos_y = 0; - int clicked = 0; + long pos_x = 0; + long pos_y = 0; + long clicked = 0; int release = 1; int ret; @@ -4352,12 +4352,12 @@ PHP_FUNCTION(libvirt_domain_new) // char *emulator; char *iso_image = NULL; int iso_image_len; - int vcpus = -1; - int memMB = -1; + long vcpus = -1; + long memMB = -1; zval *disks, *networks; tVMDisk *vmDisks = NULL; tVMNetwork *vmNetworks = NULL; - int maxmemMB = -1; + long maxmemMB = -1; HashTable *arr_hash; int numDisks, numNets, i; zval **data; // removed **zvalue @@ -4642,7 +4642,7 @@ PHP_FUNCTION(libvirt_domain_get_interface_devices) */ PHP_FUNCTION(libvirt_domain_change_vcpus) { - int numCpus, flags = 0; + long numCpus, flags = 0; php_libvirt_domain *domain=NULL; zval *zdomain; @@ -6266,7 +6266,7 @@ PHP_FUNCTION(libvirt_domain_snapshot_delete) { zval *zsnapshot; php_libvirt_snapshot *snapshot; - int flags = 0; + long flags = 0; int retval; GET_SNAPSHOT_FROM_ARGS(r|l,zsnapshot, flags); @@ -6766,7 +6766,7 @@ PHP_FUNCTION(libvirt_storagevolume_delete) { php_libvirt_volume *volume=NULL; zval *zvolume; - int flags = 0; + long flags = 0; int retval = 0; GET_VOLUME_FROM_ARGS(r|l,zvolume,flags); @@ -7057,7 +7057,7 @@ PHP_FUNCTION(libvirt_storagepool_refresh) { php_libvirt_storagepool *pool = NULL; zval *zpool; - unsigned long flags = 0; + long flags = 0; int retval; GET_STORAGEPOOL_FROM_ARGS (r|l, zpool, flags); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [php PATCH] Fix compiler warnings after int to long conversion
The previous patch [1] caused compiler warnings after variable types were changed from int to long and this patch fixes this. [1] https://www.redhat.com/archives/libvir-list/2014-June/msg00835.html --- src/libvirt-php.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 224943d..6d6fa81 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -3928,7 +3928,7 @@ PHP_FUNCTION(libvirt_domain_send_pointer_event) RETURN_FALSE; } - DPRINTF(%s: x = %d, y = %d, clicked = %d, release = %d, hostname = %s...\n, PHPFUNC, pos_x, pos_y, clicked, release, hostname); + DPRINTF(%s: x = %d, y = %d, clicked = %d, release = %d, hostname = %s...\n, PHPFUNC, (int) pos_x, (int) pos_y, (int) clicked, release, hostname); ret = vnc_send_pointer_event(hostname, tmp, pos_x, pos_y, clicked, release); if (ret == 0) { DPRINTF(%s: Pointer event result is %d\n, PHPFUNC, ret); @@ -4428,7 +4428,7 @@ PHP_FUNCTION(libvirt_domain_new) numNets = i; snprintf(tmpname, sizeof(tmpname), %s-install, name); - DPRINTF(%s: Name is '%s', memMB is %d, maxmemMB is %d\n, PHPFUNC, tmpname, memMB, maxmemMB); + DPRINTF(%s: Name is '%s', memMB is %d, maxmemMB is %d\n, PHPFUNC, tmpname, (int) memMB, (int) maxmemMB); tmp = installation_get_xml(1, conn-conn, tmpname, memMB, maxmemMB, NULL /* arch */, NULL, vcpus, iso_image, vmDisks, numDisks, vmNetworks, numNets, @@ -6272,7 +6272,7 @@ PHP_FUNCTION(libvirt_domain_snapshot_delete) GET_SNAPSHOT_FROM_ARGS(r|l,zsnapshot, flags); retval = virDomainSnapshotDelete(snapshot-snapshot, flags); - DPRINTF(%s: virDomainSnapshotDelete(%p, %d) returned %d\n, PHPFUNC, snapshot-snapshot, flags, retval); + DPRINTF(%s: virDomainSnapshotDelete(%p, %d) returned %d\n, PHPFUNC, snapshot-snapshot, (int) flags, retval); if (retval == -1) RETURN_FALSE; RETURN_TRUE; } @@ -6772,7 +6772,7 @@ PHP_FUNCTION(libvirt_storagevolume_delete) GET_VOLUME_FROM_ARGS(r|l,zvolume,flags); retval = virStorageVolDelete(volume-volume, flags); - DPRINTF(%s: virStorageVolDelete(%p, %d) returned %d\n, PHPFUNC, volume-volume, flags, retval); + DPRINTF(%s: virStorageVolDelete(%p, %d) returned %d\n, PHPFUNC, volume-volume, (int) flags, retval); if (retval != 0) { set_error_if_unset(Cannot delete storage volume TSRMLS_CC); RETURN_FALSE; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [python PATCH] build: provide wrapper makefile
After years of finger training, I'm so used to 'make check' just working, that I lose quite a bit of time re-learning that in this project, it is spelled 'python setup.py build check'. A shim makefile bridges the gap. * Makefile: New file. Signed-off-by: Eric Blake ebl...@redhat.com --- I'd like to add this to the repo, but even if it gets rejected, I'll still keep it in my local tree :) Makefile | 18 ++ 1 file changed, 18 insertions(+) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 000..6c8da0a --- /dev/null +++ b/Makefile @@ -0,0 +1,18 @@ +# Shim wrapper around setup.py to allow for familiar build targets + +PYTHON ?= python + +all: + $(PYTHON) setup.py build + +install: all + $(PYTHON) setup.py install + +clean: + $(PYTHON) setup.py clean + +check: all + $(PYTHON) setup.py test + +rpm: + $(PYTHON) setup.py rpm -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [python PATCH] build: provide wrapper makefile
On Thu, Jun 19, 2014 at 12:51 AM, Eric Blake ebl...@redhat.com wrote: After years of finger training, I'm so used to 'make check' just working, that I lose quite a bit of time re-learning that in this project, it is spelled 'python setup.py build check'. A shim makefile bridges the gap. * Makefile: New file. I see no harm in adding this. Definitely makes a developer's life easier. :) Signed-off-by: Eric Blake ebl...@redhat.com --- I'd like to add this to the repo, but even if it gets rejected, I'll still keep it in my local tree :) Makefile | 18 ++ 1 file changed, 18 insertions(+) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 000..6c8da0a --- /dev/null +++ b/Makefile @@ -0,0 +1,18 @@ +# Shim wrapper around setup.py to allow for familiar build targets + +PYTHON ?= python + +all: + $(PYTHON) setup.py build + +install: all + $(PYTHON) setup.py install + +clean: + $(PYTHON) setup.py clean + +check: all + $(PYTHON) setup.py test + +rpm: + $(PYTHON) setup.py rpm -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [php PATCH] Fix compiler warnings after int to long conversion
Hi Dawid, thanks for the information. Well, please rebase to the latest commit of my master branch and resend, thanks a lot! Michal 2014-06-18 22:05 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com: Ugh, now I know what happened - my local master branch has commits I have not sent pull requests for yet so it wasn't in 100% in sync with remote. Do you want me to resend the patch to the ML? On Wed, 2014-06-18 at 16:02 -0400, Dawid Zamirski wrote: Hi Michal, I'm pretty sure I did git pull right before sending the patch. Here's what I did exactly: On master branch: git pull git checkout -b parse-param-fix origin/master created original patch commit git format-patch -1 git send-email --no-chain-reply-to --annotate 0001-Use-long-variable-type-for-zend_parse_parameters.patch then I've noticed the warnings (still on parse-param-fix branch) create patch commit git fetch --all git pull --rebase git send-email --no-chain-reply-to --annotate origin/master I guess that before starting the waring fix patch I should have create a new local branch: git checkout master git pull git checkout -b warning-fix origin/master and then work from there. Regards, Dawid On Wed, 2014-06-18 at 21:28 +0200, Michal Novotny wrote: Hi Dawid, thanks for the patch, I'll apply it when I have time to do so. However, the patch is not critical as it's in the DPRINTF debug macro (for production environment you should disable the DEBUG macro). Also, please make sure you are you the latest git tree (by running git pull before writing the patch) as I'm having issues applying some of the patches cleanly. Thanks, Michal 2014-06-18 21:09 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com: The previous patch [1] caused compiler warnings after variable types were changed from int to long and this patch fixes this. [1] https://www.redhat.com/archives/libvir-list/2014-June/msg00835.html --- src/libvirt-php.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 224943d..6d6fa81 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -3928,7 +3928,7 @@ PHP_FUNCTION(libvirt_domain_send_pointer_event) RETURN_FALSE; } - DPRINTF(%s: x = %d, y = %d, clicked = %d, release = % d, hostname = %s...\n, PHPFUNC, pos_x, pos_y, clicked, release, hostname); + DPRINTF(%s: x = %d, y = %d, clicked = %d, release = % d, hostname = %s...\n, PHPFUNC, (int) pos_x, (int) pos_y, (int) clicked, release, hostname); ret = vnc_send_pointer_event(hostname, tmp, pos_x, pos_y, clicked, release); if (ret == 0) { DPRINTF(%s: Pointer event result is %d\n, PHPFUNC, ret); @@ -4428,7 +4428,7 @@ PHP_FUNCTION(libvirt_domain_new) numNets = i; snprintf(tmpname, sizeof(tmpname), %s-install, name); - DPRINTF(%s: Name is '%s', memMB is %d, maxmemMB is %d \n, PHPFUNC, tmpname, memMB, maxmemMB); + DPRINTF(%s: Name is '%s', memMB is %d, maxmemMB is %d \n, PHPFUNC, tmpname, (int) memMB, (int) maxmemMB); tmp = installation_get_xml(1, conn-conn, tmpname, memMB, maxmemMB, NULL /* arch */, NULL, vcpus, iso_image, vmDisks, numDisks, vmNetworks, numNets, @@ -6272,7 +6272,7 @@ PHP_FUNCTION(libvirt_domain_snapshot_delete) GET_SNAPSHOT_FROM_ARGS(r|l,zsnapshot, flags); retval = virDomainSnapshotDelete(snapshot-snapshot, flags); - DPRINTF(%s: virDomainSnapshotDelete(%p, %d) returned %d\n, PHPFUNC, snapshot-snapshot, flags, retval); + DPRINTF(%s: virDomainSnapshotDelete(%p, %d) returned %d\n, PHPFUNC, snapshot-snapshot, (int) flags, retval); if (retval == -1) RETURN_FALSE; RETURN_TRUE; } @@ -6772,7 +6772,7 @@ PHP_FUNCTION(libvirt_storagevolume_delete) GET_VOLUME_FROM_ARGS(r|l,zvolume,flags); retval = virStorageVolDelete(volume-volume, flags); - DPRINTF(%s: virStorageVolDelete(%p, %d) returned %d \n, PHPFUNC, volume-volume, flags, retval); + DPRINTF(%s: virStorageVolDelete(%p, %d) returned %d \n, PHPFUNC, volume-volume, (int) flags, retval); if (retval != 0) { set_error_if_unset(Cannot delete
Re: [libvirt] [PATCH V2 0/2] libxl: support interface type=network
Jim Fehlig wrote: Patch 1 is a V2 of http://www.redhat.com/archives/libvir-list/2014-June/msg00382.html Patch 2 is a result of Laine's comments to V1. It ensures script is only allowed with interface types ethernet and bridge. Jim Fehlig (2): libxl: support interface type=network libxl: limit support for specifying an interface script src/libxl/libxl_conf.c | 63 ++ 1 file changed, 58 insertions(+), 5 deletions(-) Any comments on this version? Thanks! Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Initial implementation of new job control api
This is my initial definition of a new internal job control api. I am working on this as a part of the google summer of code. These patches contain the core job control api and deal only with managing individual jobs. I am currently working on writing code using this api to manage jobs in domains, in such a way that I will be able to replace the current job control code in qemu and libxl. Ultimately I will use this to implement job control in the storage driver which is my ultimate goal for the summer of code. --- src/Makefile.am | 1 + src/util/virjobcontrol.c | 574 +++ src/util/virjobcontrol.h | 342 3 files changed, 917 insertions(+) create mode 100644 src/util/virjobcontrol.c create mode 100644 src/util/virjobcontrol.h diff --git a/src/Makefile.am b/src/Makefile.am index 2b9ac61..77de0e7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -118,6 +118,7 @@ UTIL_SOURCES = \ util/virinitctl.c util/virinitctl.h \ util/viriptables.c util/viriptables.h \ util/viriscsi.c util/viriscsi.h \ + util/virjobcontrol.h util/virjobcontrol.c \ util/virjson.c util/virjson.h \ util/virkeycode.c util/virkeycode.h \ util/virkeyfile.c util/virkeyfile.h \ diff --git a/src/util/virjobcontrol.c b/src/util/virjobcontrol.c new file mode 100644 index 000..04a5246 --- /dev/null +++ b/src/util/virjobcontrol.c @@ -0,0 +1,574 @@ +/* + * virjobcontrol.c Core implementation of job control + * + * Copyright (C) 2014 Tucker DiNapoli + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * http://www.gnu.org/licenses/. + * + * Author: Tucker DiNapoli + */ + +#include config.h + +#include virjobcontrol.h +#include viralloc.h +#include virtime.h +#include virlog.h +VIR_LOG_INIT(virjobcontrol); + +VIR_ENUM_IMPL(virJob, 4, + none, + query, + modify, + destroy, +); +/* + No files other then this and virjobcontrol.c should need to + have access to the core implmentation of jobs. The code in these + files is intended to serve as a base for job control independent of + drivers. +*/ + +#define LOCK_JOB(job) \ +virMutexLock(job-lock) +#define UNLOCK_JOB(job) \ +virMutexUnlock(job-lock) +#define LOCK_JOB_INFO(job) \ +virMutexLock(job-info-lock) +#define UNLOCK_JOB_INFO(job)\ +virMutexUnlock(job-info-lock) +#define GET_CURRENT_TIME(time) \ +if (virTimeMillisNow(time) 0) { \ +return -1; \ +} + + +#define CHECK_FLAG_ATOMIC(job, flag) (virAtomicIntGet(job-flags) VIR_JOB_FLAG_##flag) +#define CHECK_FLAG(job, flag) (job-flags VIR_JOB_FLAG_##flag) +#define SET_FLAG_ATOMIC(job, flag) (virAtomicIntOr(job-flags, VIR_JOB_FLAG_##flag)) +#define SET_FLAG(job, flag) (job-flags |= VIR_JOB_FLAG_##flag) +#define UNSET_FLAG_ATOMIC(job, flag) (virAtomicIntAnd(job-flags, (~VIR_JOB_FLAG_##flag))) +#define UNSET_FLAG(job, flag) (job-flags = (~VIR_JOB_FLAG_##flag)) +#define CLEAR_FLAGS_ATOMIC(job) (virAtomicIntSet(job-flags, VIR_JOB_FLAG_NONE)) +#define CLEAR_FLAGS(job) (job-flags = VIR_JOB_FLAG_NONE) + +typedef struct _jobHashEntry { +virJobID id; +virJobObjPtr job; +struct _jobHashEntry *next; +} jobHashEntry; + +typedef struct _jobHash { +jobHashEntry **table; +size_t size; +size_t num_entries; +virRWLock lock; +} jobHash; + +/* Using this incures a cost on every call to a job control function + that uses the job control hash table, but it means that no one using + job control needs to call an initialization function to use it. + + The other option would be to have a function: + virJobControlInit(void) + { + return virOnce(job_once, jobControlInit); + } + and require anyone using job control to call it. + */ +static struct _jobHash *job_hash; /* Hash table that contians all current jobs */ +static int init_err = 0; +static virOnceControl job_once = VIR_ONCE_CONTROL_INITIALIZER; +static void +jobControlInit(void) +{ +if (VIR_ALLOC_QUIET(job_hash)
Re: [libvirt] [php PATCH] Fix compiler warnings after int to long conversion
On Wed, 2014-06-18 at 22:50 +0200, Michal Novotny wrote: Hi Dawid, thanks for the information. Well, please rebase to the latest commit of my master branch and resend, thanks a lot! Michal Ok, now I'm puzzled. I've rebased the patch from clean master and the diff ended up identical to the original one. To double check, I've also successfully re-applied original mbox file on a fresh git clone, like this: git clone git://libvirt.org/libvirt-php git am ~/\[php_PATCH\]_Fix_compiler_warnings_after_int_to_long_conversion.mbox Applying: Fix compiler warnings after int to long conversion It seems that my original patch was correctly generated against remote's master due to: git send-email --no-chain-reply-to --annotate origin/master which means it was made against remote master branch, not my local copy that was dirty. At this point, I'm not sure what else I can do on my end. Sorry for the noise with such a simple patch... Regards, Dawid 2014-06-18 22:05 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com: Ugh, now I know what happened - my local master branch has commits I have not sent pull requests for yet so it wasn't in 100% in sync with remote. Do you want me to resend the patch to the ML? On Wed, 2014-06-18 at 16:02 -0400, Dawid Zamirski wrote: Hi Michal, I'm pretty sure I did git pull right before sending the patch. Here's what I did exactly: On master branch: git pull git checkout -b parse-param-fix origin/master created original patch commit git format-patch -1 git send-email --no-chain-reply-to --annotate 0001-Use-long-variable-type-for-zend_parse_parameters.patch then I've noticed the warnings (still on parse-param-fix branch) create patch commit git fetch --all git pull --rebase git send-email --no-chain-reply-to --annotate origin/master I guess that before starting the waring fix patch I should have create a new local branch: git checkout master git pull git checkout -b warning-fix origin/master and then work from there. Regards, Dawid On Wed, 2014-06-18 at 21:28 +0200, Michal Novotny wrote: Hi Dawid, thanks for the patch, I'll apply it when I have time to do so. However, the patch is not critical as it's in the DPRINTF debug macro (for production environment you should disable the DEBUG macro). Also, please make sure you are you the latest git tree (by running git pull before writing the patch) as I'm having issues applying some of the patches cleanly. Thanks, Michal 2014-06-18 21:09 GMT+02:00 Dawid Zamirski dzamir...@dattobackup.com: The previous patch [1] caused compiler warnings after variable types were changed from int to long and this patch fixes this. [1] https://www.redhat.com/archives/libvir-list/2014-June/msg00835.html --- src/libvirt-php.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libvirt-php.c b/src/libvirt-php.c index 224943d..6d6fa81 100644 --- a/src/libvirt-php.c +++ b/src/libvirt-php.c @@ -3928,7 +3928,7 @@ PHP_FUNCTION(libvirt_domain_send_pointer_event) RETURN_FALSE; } - DPRINTF(%s: x = %d, y = %d, clicked = %d, release = % d, hostname = %s...\n, PHPFUNC, pos_x, pos_y, clicked, release, hostname); + DPRINTF(%s: x = %d, y = %d, clicked = %d, release = % d, hostname = %s...\n, PHPFUNC, (int) pos_x, (int) pos_y, (int) clicked, release, hostname); ret = vnc_send_pointer_event(hostname, tmp, pos_x, pos_y, clicked, release); if (ret == 0) { DPRINTF(%s: Pointer event result is %d\n, PHPFUNC, ret); @@ -4428,7 +4428,7 @@ PHP_FUNCTION(libvirt_domain_new) numNets = i; snprintf(tmpname, sizeof(tmpname), %
[libvirt] [PATCH v3] blockjob: use stable disk string in job event
When the block job event was first added, it was for block pull, where the active layer of the disk remains the same name. It was also in a day where we only cared about local files, and so we always had a canonical absolute file name. But two things have changed since then: we now have network disks, where determining a single absolute string does not really make sense; and we have two-phase jobs (copy and active commit) where the name of the active layer changes between the first event (ready, on the old name) and second (complete, on the pivoted name). Adam Litke reported that having an unstable string between events makes life harder for clients. Furthermore, all of our API that operate on a particular disk of a domain accept multiple strings: not only the absolute name of the active layer, but also the destination device name (such as 'vda'). As this latter name is stable, even for network sources, it serves as a better string to supply in block job events. But backwards-compatibility demands that we should not change the name handed to users unless they explicitly request it. Therefore, this patch adds a new event, BLOCK_JOB_2 (alas, I couldn't think of any nicer name - but at least Migrate2 and Migrate3 are precedent for a number suffix). We must double up on emitting both old-style and new-style events according to what clients have registered for (see also how IOError and IOErrorReason emits double events, but there the difference was a larger struct rather than changed meaning of one of the struct members). Unfortunately, adding a new event isn't something that can easily be broken into pieces, so the commit is rather large. * include/libvirt/libvirt.h.in (virDomainEventID): Add a new id for VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2. (virConnectDomainEventBlockJobCallback): Document new semantics. * src/conf/domain_event.c (_virDomainEventBlockJob): Rename field, to ensure we catch all clients. (virDomainEventBlockJobNew): Add parameter. (virDomainEventBlockJobDispose) (virDomainEventBlockJobNewFromObj) (virDomainEventBlockJobNewFromDom) (virDomainEventDispatchDefaultFunc): Adjust clients. (virDomainEventBlockJob2NewFromObj) (virDomainEventBlockJob2NewFromDom): New functions. * src/conf/domain_event.h: Add new prototypes. * src/libvirt_private.syms (domain_event.h): Export new functions. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Generate two different events. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Likewise. * src/remote/remote_protocol.x (remote_domain_event_block_job_2_msg): New struct. (REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2): New RPC. * src/remote/remote_driver.c (remoteDomainBuildEventBlockJob2): New handler. (remoteEvents): Register new event. * daemon/remote.c (remoteRelayDomainEventBlockJob2): New handler. (domainEventCallbacks): Register new event. * tools/virsh-domain.c (vshEventCallbacks): Likewise. (vshEventBlockJobPrint): Adjust client. * src/remote_protocol-structs: Regenerate. Signed-off-by: Eric Blake ebl...@redhat.com --- v3: don't do s/path/disk/ on code that is not shared; s/disk/dst/ on new code that is not shared; touch up libvirt.c docs, actually test with python bindings daemon/remote.c | 39 +++ include/libvirt/libvirt.h.in | 19 --- src/conf/domain_event.c | 44 +++- src/conf/domain_event.h | 11 +++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_driver.c | 8 +++- src/qemu/qemu_process.c | 7 +++ src/remote/remote_driver.c | 31 +++ src/remote/remote_protocol.x | 16 +++- src/remote_protocol-structs | 8 tools/virsh-domain.c | 7 +-- 11 files changed, 176 insertions(+), 16 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 34c96c9..7199764 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -931,6 +931,44 @@ remoteRelayDomainEventDeviceRemoved(virConnectPtr conn, } +static int +remoteRelayDomainEventBlockJob2(virConnectPtr conn, +virDomainPtr dom, +const char *dst, +int type, +int status, +void *opaque) +{ +daemonClientEventCallbackPtr callback = opaque; +remote_domain_event_block_job_2_msg data; + +if (callback-callbackID 0 || +!remoteRelayDomainEventCheckACL(callback-client, conn, dom)) +return -1; + +VIR_DEBUG(Relaying domain block job 2 event %s %d %s %i, %i, callback %d, + dom-name, dom-id, dst, type, status, callback-callbackID); + +/* build return data */ +memset(data, 0, sizeof(data)); +data.callbackID = callback-callbackID; +if (VIR_STRDUP(data.dst, dst) 0) +goto error; +data.type = type; +data.status = status; +make_nonnull_domain(data.dom, dom); + +
[libvirt] [python PATCH 1/2] blockjob: support new BLOCK_JOB_2 event
Libvirt 1.2.6 is introducing a new block job event that passes disk information by target device rather than host file name. At the python level, we are just a passthrough, so we can reuse all the existing code and just wire up the new enum value. * libvirt-override-virConnect.py (_dispatchDomainEventBlockPullCallback): Rename... (_dispatchDomainEventBlockJobCallback): ...to this, and make generic to both events. * libvirt-override.c (libvirt_virConnectDomainEventBlockJobCallback): Match naming. (libvirt_virConnectDomainEventRegisterAny): Allow new registration. Signed-off-by: Eric Blake ebl...@redhat.com --- libvirt-override-virConnect.py | 6 +++--- libvirt-override.c | 10 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libvirt-override-virConnect.py b/libvirt-override-virConnect.py index c228eb2..31d71a3 100644 --- a/libvirt-override-virConnect.py +++ b/libvirt-override-virConnect.py @@ -113,14 +113,14 @@ authScheme, subject, opaque) return 0 -def _dispatchDomainEventBlockPullCallback(self, dom, path, type, status, cbData): -Dispatches events to python user domain blockJob event callbacks +def _dispatchDomainEventBlockJobCallback(self, dom, disk, type, status, cbData): +Dispatches events to python user domain blockJob/blockJob2 event callbacks try: cb = cbData[cb] opaque = cbData[opaque] -cb(self, virDomain(self, _obj=dom), path, type, status, opaque) +cb(self, virDomain(self, _obj=dom), disk, type, status, opaque) return 0 except AttributeError: pass diff --git a/libvirt-override.c b/libvirt-override.c index 8fd856b..eb1d5e2 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -6085,7 +6085,7 @@ libvirt_virConnectDomainEventGraphicsCallback(virConnectPtr conn ATTRIBUTE_UNUSE static int libvirt_virConnectDomainEventBlockJobCallback(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, - const char *path, + const char *disk, int type, int status, void *opaque) @@ -6114,9 +6114,9 @@ libvirt_virConnectDomainEventBlockJobCallback(virConnectPtr conn ATTRIBUTE_UNUSE /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)_dispatchDomainEventBlockPullCallback, + (char*)_dispatchDomainEventBlockJobCallback, (char*)OsiiO, -pyobj_dom, path, type, status, pyobj_cbData); +pyobj_dom, disk, type, status, pyobj_cbData); Py_DECREF(pyobj_cbData); Py_DECREF(pyobj_dom); @@ -6506,6 +6506,7 @@ libvirt_virConnectDomainEventDeviceRemovedCallback(virConnectPtr conn ATTRIBUTE_ } #endif /* LIBVIR_CHECK_VERSION(1, 1, 1) */ + static PyObject * libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject *self, PyObject *args) @@ -6561,6 +6562,9 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject *self, cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventGenericCallback); break; case VIR_DOMAIN_EVENT_ID_BLOCK_JOB: +#if LIBVIR_CHECK_VERSION(1, 2, 6) +case VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2: +#endif /* LIBVIR_CHECK_VERSION(1, 2, 6) */ cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventBlockJobCallback); break; case VIR_DOMAIN_EVENT_ID_DISK_CHANGE: -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [python PATCH 2/2] event-test: add missing events
Update the example to be able to trace all events. * examples/event-test.py (main): Match full list of domain events. (myDomainEventIOErrorReasonCallback) (myDomainEventControlErrorCallback) (myDomainEventBlockJobCallback, myDomainEventBlockJob2Callback) (blockJobTypeToString, blockJobStatusToString): New functions. Signed-off-by: Eric Blake ebl...@redhat.com --- examples/event-test.py | 26 ++ 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/examples/event-test.py b/examples/event-test.py index aa3c5a7..cd85de3 100644 --- a/examples/event-test.py +++ b/examples/event-test.py @@ -456,6 +456,14 @@ def domDetailToString(event, detail): ) return domEventStrings[event][detail] +def blockJobTypeToString(type): +blockJobTypes = ( unknown, Pull, Copy, Commit, ActiveCommit, ) +return blockJobTypes[type] + +def blockJobStatusToString(status): +blockJobStatus = ( Completed, Failed, Canceled, Ready, ) +return blockJobStatus[status] + def myDomainEventCallback1 (conn, dom, event, detail, opaque): print(myDomainEventCallback1 EVENT: Domain %s(%s) %s %s % (dom.name(), dom.ID(), domEventToString(event), @@ -477,10 +485,14 @@ def myDomainEventWatchdogCallback(conn, dom, action, opaque): def myDomainEventIOErrorCallback(conn, dom, srcpath, devalias, action, opaque): print(myDomainEventIOErrorCallback: Domain %s(%s) %s %s %d % (dom.name(), dom.ID(), srcpath, devalias, action)) - +def myDomainEventIOErrorReasonCallback(conn, dom, srcpath, devalias, action, reason, opaque): +print(myDomainEventIOErrorReasonCallback: Domain %s(%s) %s %s %d %s % (dom.name(), dom.ID(), srcpath, devalias, action, reason)) def myDomainEventGraphicsCallback(conn, dom, phase, localAddr, remoteAddr, authScheme, subject, opaque): print(myDomainEventGraphicsCallback: Domain %s(%s) %d %s % (dom.name(), dom.ID(), phase, authScheme)) - +def myDomainEventControlErrorCallback(conn, dom, opaque): +print(myDomainEventControlErrorCallback: Domain %s(%s) % (dom.name(), dom.ID())) +def myDomainEventBlockJobCallback(conn, dom, disk, type, status, opaque): +print(myDomainEventBlockJobCallback: Domain %s(%s) %s on disk %s %s % (dom.name(), dom.ID(), blockJobTypeToString(type), disk, blockJobStatusToString(status))) def myDomainEventDiskChangeCallback(conn, dom, oldSrcPath, newSrcPath, devAlias, reason, opaque): print(myDomainEventDiskChangeCallback: Domain %s(%s) disk change oldSrcPath: %s newSrcPath: %s devAlias: %s reason: %s % ( dom.name(), dom.ID(), oldSrcPath, newSrcPath, devAlias, reason)) @@ -501,6 +513,8 @@ def myDomainEventPMSuspendDiskCallback(conn, dom, reason, opaque): def myDomainEventDeviceRemovedCallback(conn, dom, dev, opaque): print(myDomainEventDeviceRemovedCallback: Domain %s(%s) device removed: %s % ( dom.name(), dom.ID(), dev)) +def myDomainEventBlockJob2Callback(conn, dom, disk, type, status, opaque): +print(myDomainEventBlockJob2Callback: Domain %s(%s) %s on disk %s %s % (dom.name(), dom.ID(), blockJobTypeToString(type), disk, blockJobStatusToString(status))) ## # Network events @@ -591,14 +605,17 @@ def main(): vc.registerCloseCallback(myConnectionCloseCallback, None) -#Add 2 callbacks to prove this works with more than just one +#Add 2 lifecycle callbacks to prove this works with more than just one vc.domainEventRegister(myDomainEventCallback1,None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_LIFECYCLE, myDomainEventCallback2, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_REBOOT, myDomainEventRebootCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_RTC_CHANGE, myDomainEventRTCChangeCallback, None) -vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR, myDomainEventIOErrorCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_WATCHDOG, myDomainEventWatchdogCallback, None) +vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR, myDomainEventIOErrorCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_GRAPHICS, myDomainEventGraphicsCallback, None) +vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON, myDomainEventIOErrorReasonCallback, None) +vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_CONTROL_ERROR, myDomainEventControlErrorCallback, None) +vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_BLOCK_JOB, myDomainEventBlockJobCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_DISK_CHANGE, myDomainEventDiskChangeCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_TRAY_CHANGE, myDomainEventTrayChangeCallback, None)
Re: [libvirt] [PATCH libvirt] xen: handle root= in xen-xm configuration files.
On 06/18/2014 11:46 AM, Jim Fehlig wrote: In addition to extra= xm supported a root= option which was supposed to be incorporated into the final command line. Handle that for virsh domxml-from-native xen-xm. Tested with the libxl backend. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- v2: extra should default to . --- .gnulib|2 +- --- WTF. I stripped this out of the patch shown below... src/xenxs/xen_xm.c | 14 +- 2 files changed, 14 insertions(+), 2 deletions(-) ACK and pushed; thanks! I'm now seeing test failures on Fedora 20, non-xen setup: TEST: xmconfigtest 1) Xen XM-2-XML Parse paravirt-old-pvfb ... OK 2) Xen XM-2-XML Format paravirt-old-pvfb ... Offset 324 Expect [] Actual [ cmdline/cmdline ] ... FAILED 3) Xen XM-2-XML Parse paravirt-old-pvfb-vncdisplay ... OK 4) Xen XM-2-XML Format paravirt-old-pvfb-vncdisplay ... Offset 324 Expect [] Actual [ cmdline/cmdline ] ... FAILED 5) Xen XM-2-XML Parse paravirt-new-pvfb ... OK and several others. Looks like something in the code is outputting an empty cmdline element when it is not needed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/5] blockjob: turn on qemu capability bit for active commit
Use the probing functionality added in the last patch to turn on a capability bit when active commit is present, and gate active commit on that capability. While at it, leave a few more bread-crumbs about what blockjob sync/async means, and enforce that sync cancel is only ever encountered with blockpull (at this point, RHEL 6.2 is likely to be the only platform to have sync cancel). Modifying qemucapabilitiestest is painful; the .replies files would be so much easier if they had comments correlating which command generated the given reply. Maybe I'll fix that up later... * src/qemu/qemu_capabilities.h (QEMU_CAPS_ACTIVE_COMMIT): New capability. (QEMU_CAPS_BLOCKJOB_SYNC): Enhance comment. * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Likewise. (qemuDomainBlockCopy): Likewise. (qemuDomainBlockCommit): Use the new bit * src/qemu/qemu_capabilities.c (virQEMUCaps): Name the new bit. (virQEMUCapsProbeQMPCommands): Set it. * tests/qemucapabilitiesdata/caps_1.3.1-1.replies: Update. * tests/qemucapabilitiesdata/caps_1.4.2-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.5.3-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.6.0-1.replies: Likewise. * tests/qemucapabilitiesdata/caps_1.6.50-1.replies: Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 3 +- src/qemu/qemu_driver.c | 16 +++--- tests/qemucapabilitiesdata/caps_1.3.1-1.replies | 62 +--- tests/qemucapabilitiesdata/caps_1.4.2-1.replies | 62 +--- tests/qemucapabilitiesdata/caps_1.5.3-1.replies | 62 +--- tests/qemucapabilitiesdata/caps_1.6.0-1.replies | 62 +--- tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 +--- 8 files changed, 193 insertions(+), 143 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 08c3d04..b96fec1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, usb-kbd, /* 165 */ host-pci-multidomain, msg-timestamp, + active-commit, ); @@ -2149,6 +2150,12 @@ virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, VIR_FORCE_CLOSE(fd); } +/* Probe for active commit of qemu 2.1 (for now, we are choosing + * to ignore the fact that qemu 2.0 can also do active commit) */ +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCK_COMMIT) +qemuMonitorBlockCommit(mon, bogus, NULL, NULL, 0) == -2) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_ACTIVE_COMMIT); + return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d755caa..617986d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -127,7 +127,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_SCSI_BLOCK = 88, /* -device scsi-block */ QEMU_CAPS_TRANSACTION= 89, /* transaction monitor command */ QEMU_CAPS_BLOCKJOB_SYNC = 90, /* RHEL 6.2 block_job_cancel */ -QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1 block-job-cancel */ +QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1/RHEL 6.3 block-job-cancel */ QEMU_CAPS_SCSI_CD= 92, /* -device scsi-cd */ QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */ QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */ @@ -206,6 +206,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */ QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain 0 in host pci address */ QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */ +QEMU_CAPS_ACTIVE_COMMIT = 168, /* block-commit works without 'top' */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ca58d6b..004b63d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15130,8 +15130,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm, * to prevent newly scheduled block jobs from confusing us. */ if (mode == BLOCK_JOB_ABORT) { if (!async) { -/* Older qemu that lacked async reporting also lacked - * active commit, so we can hardcode the event to pull. +/* Older qemu (RHEL 6.2) that lacked async reporting also + * lacked copy and commit, so we can hardcode type_pull. * We have to generate two variants of the event. */ int type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL; int status = VIR_DOMAIN_BLOCK_JOB_CANCELED; @@ -15291,6 +15291,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm, goto endjob; } +/* Ensure that no one backports copy to RHEL 6.2, where cancel + * behaved differently */ if (!(virQEMUCapsGet(priv-qemuCaps,
[libvirt] [PATCH v3 1/5] blockjob: allow omitted arguments to QMP block-commit
We are about to turn on support for active block commit. Although qemu 2.0 was the first version to mostly support it, that version mis-handles 0-length files, and doesn't have anything available for easy probing. But qemu 2.1 fixed bugs, and made life simpler by letting the 'top' argument be optional. Unless someone begs for active commit with qemu 2.0, for now we are just going to enable it only by probing for qemu 2.1 behavior (anyone backporting active commit can also backport the optional argument behavior). Although all our actual uses of block-commit supply arguments for both base and top, we can omit both arguments and use a bogus device string to trigger an interesting behavior in qemu. All QMP commands first do argument validation, failing with GenericError if a mandatory argument is missing. Once that passes, the code in the specific command gets to do further checking, and the qemu developers made sure that if device is the only supplied argument, then the block-commit code will look up the device first, with a failure of DeviceNotFound, before attempting any further argument validation (most other validations fail with GenericError). Thus, the category of error class can reliably be used to decipher whether the top argument was optional, which in turn implies a working active commit. Since we expect our bogus device string to trigger an error either way, the code is written to return a distinct return value without spamming the logs. * src/qemu/qemu_monitor.h (qemuMonitorBlockCommit): Allow NULL for top and base, for probing purposes. * src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Likewise. * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit): Likewise. * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit): Likewise, with special return values based on probe. * tests/qemumonitorjsontest.c (mymain): Enable... (testQemuMonitorJSONqemuMonitorJSONBlockCommit2): ...a new test. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_monitor.c | 2 +- src/qemu/qemu_monitor.h | 3 +-- src/qemu/qemu_monitor_json.c | 20 -- src/qemu/qemu_monitor_json.h | 3 +-- tests/qemumonitorjsontest.c | 49 5 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 80d7b9d..013a6ad 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3240,7 +3240,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, top, base, bandwidth); + mon, device, NULLSTR(top), NULLSTR(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 31b3204..1c7857e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -663,8 +663,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *top, const char *base, unsigned long bandwidth) -ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) -ATTRIBUTE_NONNULL(4); +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index bedd959..0e4262d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3467,14 +3467,30 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device, cmd = qemuMonitorJSONMakeCommand(block-commit, s:device, device, U:speed, speed, - s:top, top, - s:base, base, + S:top, top, + S:base, base, NULL); if (!cmd) return -1; if ((ret = qemuMonitorJSONCommand(mon, cmd, reply)) 0) goto cleanup; +if (!top !base) { +/* Normally we always specify top and base; but omitting them + * allows for probing whether qemu is new enough to support + * live commit. */ +if (qemuMonitorJSONHasError(reply, DeviceNotFound)) { +VIR_DEBUG(block-commit supports active commit); +ret = -2; +} else { +/* This is a false negative for qemu 2.0; but probably not + * worth the additional complexity to worry about it */ +VIR_DEBUG(block-commit requires 'top' parameter, + assuming it lacks active commit); +ret = -3; +} +goto cleanup; +} ret =
[libvirt] [PATCH v3 4/5] blockcommit: track job type in xml
A future patch is going to wire up qemu active block commit jobs; but as they have similar events and are canceled/pivoted in the same way as block copy jobs, it is easiest to track all bookkeeping for the commit job by reusing the mirror element. This patch adds domain XML to track which job was responsible for creating a mirroring situation, and adds a job='copy' attribute to all existing uses of mirror. Along the way, it also massages the qemu monitor backend to read the new field in order to generate the correct type of libvirt job (even though it requires a future patch to actually cause a qemu event that can be reported as an active commit). * docs/schemas/domaincommon.rng: Enhance schema. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (_virDomainDiskDef): Add a field. * src/conf/domain_conf.c (virDomainBlockJobType): String conversion. (virDomainDiskDefParseXML): Parse job type. (virDomainDiskDefFormat): Output job type. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish active from regular commit. * src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type. (qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type on completion. * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml: Update tests. * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise. * tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New file. * tests/qemuxml2xmltest.c (mymain): Drive new test. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/formatdomain.html.in | 20 ++-- docs/schemas/domaincommon.rng | 13 src/conf/domain_conf.c | 33 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 3 ++ src/qemu/qemu_process.c| 18 +++ .../qemuxml2argv-disk-active-commit.xml| 37 ++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +-- tests/qemuxml2xmltest.c| 1 + 10 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 79b85d5..f8a5238 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1878,16 +1878,18 @@ /dd dtcodemirror/code/dt dd -This element is present if the hypervisor has started a block -copy operation (via the codevirDomainBlockRebase/code -API), where the mirror location in the codesource/code -sub-element will eventually have the same contents as the -source, and with the file format in the +This element is present if the hypervisor has started a +long-running block job operation, where the mirror location in +the codesource/code sub-element will eventually have the +same contents as the source, and with the file format in the sub-element codeformat/code (which might differ from the -format of the source). The details of the codesource/code -sub-element are determined by the codetype/code attribute -of the mirror, similar to what is done for the -overall codedisk/code device element. If +format of the source). The codejob/code attribute +mentions which API started the operation (copy for +the codevirDomainBlockRebase/code API, or active-commit +for the codevirDomainBlockCommit/code API). The details +of the codesource/code sub-element are determined by +the codetype/code attribute of the mirror, similar to what +is done for the overall codedisk/code device element. If attribute codeready/code is present, then it is known the disk is ready to pivot; otherwise, the disk is probably still copying. For now, this element only valid in output; it is diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 33d0308..fe943f9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4186,6 +4186,13 @@ /attribute /optional optional +attribute name='job' + choice +valuecopy/value + /choice +/attribute + /optional + optional interleave ref name='diskSourceFile'/ optional @@ -4195,6 +4202,12 @@ /optional /group group !-- preferred format -- + attribute name='job' +choice + valuecopy/value + valueactive-commit/value +/choice + /attribute interleave ref name=diskSource/ optional diff --git
[libvirt] [PATCH v3 0/5] Next round of active commit support
Browseable at: http://repo.or.cz/w/libvirt/ericb.git/shortlog/refs/heads/gluster or git checkout git://repo.or.cz/libvirt/ericb.git gluster Resolves some of the review comments, and in particular FINALLY turns on capability probing. However, I can't push this to libvirt until Jeff's patch actually makes it into qemu.git: https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg04060.html and still lacking cleanup of the persistent XML after a pivot Eric Blake (5): blockjob: allow omitted arguments to QMP block-commit blockjob: turn on qemu capability bit for active commit virsh: expose new active commit controls blockcommit: track job type in xml blockcommit: turn on active commit docs/formatdomain.html.in | 20 +++ docs/schemas/domaincommon.rng | 13 + src/conf/domain_conf.c | 33 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 7 +++ src/qemu/qemu_capabilities.h | 3 +- src/qemu/qemu_driver.c | 59 +++- src/qemu/qemu_monitor.c| 2 +- src/qemu/qemu_monitor.h| 3 +- src/qemu/qemu_monitor_json.c | 20 ++- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_process.c| 18 --- tests/qemucapabilitiesdata/caps_1.3.1-1.replies| 62 -- tests/qemucapabilitiesdata/caps_1.4.2-1.replies| 62 -- tests/qemucapabilitiesdata/caps_1.5.3-1.replies| 62 -- tests/qemucapabilitiesdata/caps_1.6.0-1.replies| 62 -- tests/qemucapabilitiesdata/caps_1.6.50-1.replies | 62 -- tests/qemumonitorjsontest.c| 49 + .../qemuxml2argv-disk-active-commit.xml| 37 + .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +- tests/qemuxml2xmltest.c| 1 + tools/virsh-domain.c | 56 --- tools/virsh.pod| 32 +++ 24 files changed, 484 insertions(+), 191 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/5] virsh: expose new active commit controls
Add knobs to virsh to manage a 2-phase active commit of the top layer, similar to knobs already present on blockcopy. While this code will fail until later patches actually implement the new knobs in the qemu driver, doing it now proves that the API is usable and also makes it easier for testing the qemu changes as they are made. * tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot, and --finish options, modeled after blockcopy. (blockJobImpl): Support --active flag. * tools/virsh.pod (blockcommit): Document new flags. (blockjob): Mention 2-phase commit interaction. Signed-off-by: Eric Blake ebl...@redhat.com --- tools/virsh-domain.c | 56 ++-- tools/virsh.pod | 32 +- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d136862..0ae1538 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1492,6 +1492,10 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW; if (vshCommandOptBool(cmd, delete)) flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE; +if (vshCommandOptBool(cmd, active) || +vshCommandOptBool(cmd, pivot) || +vshCommandOptBool(cmd, keep-overlay)) +flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE; ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags); break; case VSH_CMD_BLOCK_JOB_COPY: @@ -1592,13 +1596,18 @@ static const vshCmdOptDef opts_block_commit[] = { .type = VSH_OT_DATA, .help = N_(path of top file to commit from (default top of chain)) }, +{.name = active, + .type = VSH_OT_BOOL, + .help = N_(trigger two-stage active commit of top file) +}, {.name = delete, .type = VSH_OT_BOOL, .help = N_(delete files that were successfully committed) }, {.name = wait, .type = VSH_OT_BOOL, - .help = N_(wait for job to complete) + .help = N_(wait for job to complete +(with --active, wait for job to sync)) }, {.name = verbose, .type = VSH_OT_BOOL, @@ -1606,7 +1615,15 @@ static const vshCmdOptDef opts_block_commit[] = { }, {.name = timeout, .type = VSH_OT_INT, - .help = N_(with --wait, abort if copy exceeds timeout (in seconds)) + .help = N_(implies --wait, abort if copy exceeds timeout (in seconds)) +}, +{.name = pivot, + .type = VSH_OT_BOOL, + .help = N_(implies --active --wait, pivot when commit is synced) +}, +{.name = keep-overlay, + .type = VSH_OT_BOOL, + .help = N_(implies --active --wait, quit when commit is synced) }, {.name = async, .type = VSH_OT_BOOL, @@ -1620,8 +1637,11 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; bool ret = false; -bool blocking = vshCommandOptBool(cmd, wait); bool verbose = vshCommandOptBool(cmd, verbose); +bool pivot = vshCommandOptBool(cmd, pivot); +bool finish = vshCommandOptBool(cmd, keep-overlay); +bool active = vshCommandOptBool(cmd, active) || pivot || finish; +bool blocking = vshCommandOptBool(cmd, wait); int timeout = 0; struct sigaction sig_action; struct sigaction old_sig_action; @@ -1632,7 +1652,12 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) bool quit = false; int abort_flags = 0; +blocking |= vshCommandOptBool(cmd, timeout) || pivot || finish; if (blocking) { +if (pivot finish) { +vshError(ctl, %s, _(cannot mix --pivot and --keep-overlay)); +return false; +} if (vshCommandOptTimeoutToMs(ctl, cmd, timeout) 0) return false; if (vshCommandOptStringReq(ctl, cmd, path, path) 0) @@ -1650,8 +1675,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) sigaction(SIGINT, sig_action, old_sig_action); GETTIMEOFDAY(start); -} else if (verbose || vshCommandOptBool(cmd, timeout) || - vshCommandOptBool(cmd, async)) { +} else if (verbose || vshCommandOptBool(cmd, async)) { vshError(ctl, %s, _(missing --wait option)); return false; } @@ -1683,6 +1707,8 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) if (verbose) vshPrintJobProgress(_(Block Commit), info.end - info.cur, info.end); +if (active info.cur == info.end) +break; GETTIMEOFDAY(curr); if (intCaught || (timeout @@ -1709,7 +1735,25 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) /* printf [100 %] */ vshPrintJobProgress(_(Block Commit), 0, 1); } -vshPrint(ctl, \n%s, quit ? _(Commit aborted) : _(Commit complete)); +if (!quit pivot) { +abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT; +if (virDomainBlockJobAbort(dom, path, abort_flags) 0) { +
[libvirt] [PATCH v3 5/5] blockcommit: turn on active commit
With this in place, I can (finally!) now do: virsh blockcommit $dom vda --shallow --verbose --pivot and watch qemu shorten the backing chain by one, followed by libvirt automatically updating the dumpxml output, effectively undoing the work of virsh snapshot-commit --no-metadata --disk-only. Commit is S much faster than blockpull, when I'm still fairly close in time to when the temporary qcow2 wrapper file was created via a snapshot operation! Still not done: the persistent XML is not updated; which means stopping and restarting a persistent guest will use the wrong file and likely fail to boot. * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live commit to regular files. Signed-off-by: Eric Blake ebl...@redhat.com --- src/qemu/qemu_driver.c | 40 +++- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d19ca18..089668d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15495,9 +15495,12 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned int baseIndex = 0; const char *top_parent = NULL; bool clean_access = false; +virStorageSourcePtr mirror = NULL; + -/* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */ -virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1); +/* XXX Add support for COMMIT_DELETE */ +virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | + VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, -1); if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; @@ -15545,9 +15548,6 @@ qemuDomainBlockCommit(virDomainPtr dom, top_parent))) goto endjob; -/* FIXME: qemu 2.0 supports active commit, but as a two-stage - * process; qemu 2.1 is further improving active commit. We need - * to start supporting it in libvirt. */ if (topSource == disk-src) { if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_ACTIVE_COMMIT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, @@ -15561,6 +15561,14 @@ qemuDomainBlockCommit(virDomainPtr dom, disk-dst); goto endjob; } +if (disk-mirror) { +virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, + _(disk '%s' already in active block job), + disk-dst); +goto endjob; +} +if (VIR_ALLOC(mirror) 0) +goto endjob; } else if (flags VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) { virReportError(VIR_ERR_INVALID_ARG, _(active commit requested but '%s' is not active), @@ -15607,6 +15615,21 @@ qemuDomainBlockCommit(virDomainPtr dom, VIR_DISK_CHAIN_READ_WRITE) 0)) goto endjob; +/* For an active commit, clone enough of the base to act as the mirror */ +if (mirror) { +/* XXX Support network commits */ +if (baseSource-type != VIR_STORAGE_TYPE_FILE +baseSource-type != VIR_STORAGE_TYPE_BLOCK) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(active commit to non-file not supported yet)); +goto endjob; +} +mirror-type = baseSource-type; +if (VIR_STRDUP(mirror-path, baseSource-path) 0) +goto endjob; +mirror-format = baseSource-format; +} + /* Start the commit operation. Pass the user's original spelling, * if any, through to qemu, since qemu may behave differently * depending on whether the input was specified as relative or @@ -15619,6 +15642,12 @@ qemuDomainBlockCommit(virDomainPtr dom, bandwidth); qemuDomainObjExitMonitor(driver, vm); +if (ret == 0 mirror) { +disk-mirror = mirror; +mirror = NULL; +disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; +} + endjob: if (ret 0 clean_access) { /* Revert access to read-only, if possible. */ @@ -15629,6 +15658,7 @@ qemuDomainBlockCommit(virDomainPtr dom, top_parent, VIR_DISK_CHAIN_READ_ONLY); } +virStorageSourceFree(mirror); if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list