- Devel - libvirt List Archives

Re: [libvirt] Constantly changing USB product ID
by Doug Goldstein 28 Mar '12

28 Mar '12

On Tue, Mar 27, 2012 at 10:48 AM, Jaap Winius <jwinius(a)umrk.nl> wrote: > Hi folks, > > Recently I learned how to configure KVM with USB pass-though functionality. > In my case I configured my guest domain with this block of code: > > <hostdev mode='subsystem' type='usb' managed='yes'> > <source> > <vendor id='0x0c93'/> > <product id='0x1772'/> > <address bus='1' device='4'/> > </source> > </hostdev> > > At first this worked fine, but then later the guest domain refused to start > because the USB device was absent. When I checked, I found that its product > ID had mysteriously changed to 1771. Later it was back at 1772. Now it > appears that the USB device I am dealing with has a product ID that changes > back and forth between 1771 and 1772 at random. > > Apparently, the Windows program running on the guest domain is designed to > deal with this nonsense, but the question is, Can KVM be configured to deal > with it? Something like <product id='0x177*'/> would be useful, but that > doesn't work. > > Any ideas would be much appreciated. > > Thanks, > > Jaap > FWIW, if someone needs a device to test a potential patch with just take any rooted Android phone. e.g. an HTC at the HBOOT menu presents itself as a different product ID than when the phone is normally booted. You can also do this with an iPhone, just hold down the power and the home button to boot it into a "recovery" mode where it'll have a different product ID than normal. -- Doug Goldstein

3 2

[libvirt] [PATCH] qemu: eliminate nested switch, simplify code
by Laine Stump 28 Mar '12

28 Mar '12

qemuBuildHostNetStr had a switch-within-a-switch where both were looking at the same variable. This was apparently to take advantage of code common to three different cases (while also taking care of some code that was different). However, there were only 2 lines common to all, one of those can be eliminated by merging it into the virAsprintfs that are in each case. On top of that, all the extra empty cases cause Coverity complaints (because they are unreachable), but absence of the empty cases causes a compile error due to "enumeration value not handled in switch". The solution is to just make each toplevel case independent, folding in the common code to each. --- src/qemu/qemu_command.c | 55 ++++++++++++++++++----------------------------- 1 files changed, 21 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3d2bb6b..6b246d6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2721,8 +2721,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_DIRECT: - virBufferAddLit(&buf, "tap"); - virBufferAsprintf(&buf, "%cfd=%s", type_sep, tapfd); + virBufferAsprintf(&buf, "tap%cfd=%s", type_sep, tapfd); type_sep = ','; is_tap = true; break; @@ -2742,40 +2741,28 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, break; case VIR_DOMAIN_NET_TYPE_CLIENT: + virBufferAsprintf(&buf, "socket%cconnect=%s:%d", + type_sep, + net->data.socket.address, + net->data.socket.port); + type_sep = ','; + break; + case VIR_DOMAIN_NET_TYPE_SERVER: + virBufferAsprintf(&buf, "socket%clisten=%s:%d", + type_sep, + net->data.socket.address, + net->data.socket.port); + type_sep = ','; + break; + case VIR_DOMAIN_NET_TYPE_MCAST: - virBufferAddLit(&buf, "socket"); - switch (netType) { - case VIR_DOMAIN_NET_TYPE_CLIENT: - virBufferAsprintf(&buf, "%cconnect=%s:%d", - type_sep, - net->data.socket.address, - net->data.socket.port); - break; - case VIR_DOMAIN_NET_TYPE_SERVER: - virBufferAsprintf(&buf, "%clisten=%s:%d", - type_sep, - net->data.socket.address, - net->data.socket.port); - break; - case VIR_DOMAIN_NET_TYPE_MCAST: - virBufferAsprintf(&buf, "%cmcast=%s:%d", - type_sep, - net->data.socket.address, - net->data.socket.port); - break; - case VIR_DOMAIN_NET_TYPE_USER: - case VIR_DOMAIN_NET_TYPE_ETHERNET: - case VIR_DOMAIN_NET_TYPE_NETWORK: - case VIR_DOMAIN_NET_TYPE_BRIDGE: - case VIR_DOMAIN_NET_TYPE_INTERNAL: - case VIR_DOMAIN_NET_TYPE_DIRECT: - case VIR_DOMAIN_NET_TYPE_HOSTDEV: - case VIR_DOMAIN_NET_TYPE_LAST: - break; - } - type_sep = ','; - break; + virBufferAsprintf(&buf, "socket%cmcast=%s:%d", + type_sep, + net->data.socket.address, + net->data.socket.port); + type_sep = ','; + break; case VIR_DOMAIN_NET_TYPE_USER: default: -- 1.7.7.6

2 1

Re: [libvirt] [PATCH] qemu: live change of the bridge used by a guest network device
by Laine Stump 28 Mar '12

28 Mar '12

Is this patch too much "new code" to push now? https://www.redhat.com/archives/libvir-list/2012-March/msg01228.html The reasons I ask are two-fold - 1) it sat for a long time before anybody reviewed it. 2) I just noticed that there is a bug in the existing code that fails to flag a change in bridge name as an error (although it does nothing with it), and this patch would remedy that, so in one way it could be considered a bugfix.

2 1

[libvirt] [libvirt-glib] Do not encode entities in XML attributes
by Christophe Fergeau 28 Mar '12

28 Mar '12

libxml2 will properly escape < > and " in XML attribute content. If we use xmlEncodeEntitiesReentrant for attributes, this causes issues with UTF8 filenames (gvir_config_domain_disk_set_source for example): the filename UTF8 characters will be substituted with entities (é -> é), but when it uses this filename, libvirt will use it as is and will fail to find the file. I've tested that with this change gnome-boxes can open ISOs in directories with 'é' in their names, and in directories with '&foo;xx<' in their names. --- libvirt-gconfig/libvirt-gconfig-object.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 9a142c9..ee3584a 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -711,7 +711,6 @@ gvir_config_object_set_attribute(GVirConfigObject *object, ...) while (TRUE) { const char *name; const char *value; - xmlChar *encoded_value; name = va_arg(args, const char *); if (name == NULL) { @@ -723,9 +722,7 @@ gvir_config_object_set_attribute(GVirConfigObject *object, ...) g_warn_if_reached(); break; } - encoded_value = xmlEncodeEntitiesReentrant(doc, (xmlChar*)value); - xmlNewProp(object->priv->node, (xmlChar *)name, encoded_value); - xmlFree(encoded_value); + xmlNewProp(object->priv->node, (xmlChar *)name, (xmlChar *)value); } va_end(args); } @@ -780,17 +777,11 @@ gvir_config_object_set_attribute_with_type(GVirConfigObject *object, ...) str = g_strdup_printf("%d", val); break; } - case G_TYPE_STRING: { - xmlDocPtr doc; - xmlChar *enc_str; - + case G_TYPE_STRING: str = va_arg(args, char *); - g_object_get(G_OBJECT(object->priv->doc), "doc", &doc, NULL); - enc_str = xmlEncodeEntitiesReentrant(doc, (xmlChar*)str); - str = g_strdup((char *)enc_str); - xmlFree(enc_str); + xmlNewProp(object->priv->node, (xmlChar *)name, (xmlChar *)str); + str = NULL; break; - } case G_TYPE_BOOLEAN: { gboolean val; val = va_arg(args, gboolean); -- 1.7.9.3

2 1

[libvirt] [PATCH] qemu: Make migration fail when port profile association fails on the dst host
by Christian Benvenuti (benve) 28 Mar '12

28 Mar '12

In the current V3 migration protocol, Libvirt does not check the result of the function qemuMigrationVPAssociatePortProfiles This means that it is possible for a migration to complete successfully even when the VM loses network connectivity on the destination host. With this change libvirt aborts the migration (during the "finish" step) when the above function fails, that is to say when at least one of the port profile associations fails. Signed-off by: Christian Benvenuti <benve(a)cisco.com>

2 2

[libvirt] [PATCHv2] virsh: plug memory leaks on failure path
by Alex Jia 28 Mar '12

28 Mar '12

Leaks are introduced in commit 1cf0e3d and fe383bb. Fixing memory leaks, in addition, the patch also fixs a potential missing return value issue in 'if (from)' statement, without the fixing, although the programming met a error, the subsequent codes will be executed continually. * tools/virsh.c (cmdSnapshotList): fix memory leaks and missing return value. * How to reproduce? % virsh snapshot-list <domain> --parent --roots % virsh snapshot-list <domain> --parent --tree % virsh snapshot-list <domain> --roots --tree actual result: error: --parent and --roots are mutually exclusive error: Failed to disconnect from the hypervisor, 1 leaked reference(s) error: --parent and --tree are mutually exclusive error: Failed to disconnect from the hypervisor, 1 leaked reference(s) error: --roots and --tree are mutually exclusive error: Failed to disconnect from the hypervisor, 1 leaked reference(s) ...... % virsh snapshot-create-as <domain> --name "hello" % virsh snapshot-create-as <domain> --name "libvirt" % virsh snapshot-list <domain> --roots --from "hello" actual result: error: --roots and --from are mutually exclusive Name Creation Time State ------------------------------------------------------------ libvirt 2012-03-28 13:46:51 +0800 running Signed-off-by: Alex Jia <ajia(a)redhat.com> --- tools/virsh.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 5009b6b..8ee25c3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -16357,23 +16357,24 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "roots")) { vshError(ctl, "%s", _("--parent and --roots are mutually exclusive")); - return false; + goto cleanup; } if (tree) { vshError(ctl, "%s", _("--parent and --tree are mutually exclusive")); - return false; + goto cleanup; } parent_filter = 1; } else if (vshCommandOptBool(cmd, "roots")) { if (tree) { vshError(ctl, "%s", _("--roots and --tree are mutually exclusive")); - return false; + goto cleanup; } if (from) { vshError(ctl, "%s", _("--roots and --from are mutually exclusive")); + goto cleanup; } flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS; } @@ -16381,7 +16382,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (tree) { vshError(ctl, "%s", _("--leaves and --tree are mutually exclusive")); - return false; + goto cleanup; } flags |= VIR_DOMAIN_SNAPSHOT_LIST_LEAVES; } -- 1.7.1

2 2

[libvirt] [PATCH 1/2] python: Add new helper functions for python to C integral conversion
by Guannan Ren 28 Mar '12

28 Mar '12

int libvirt_intUnwrap(PyObject *obj, int *val); int libvirt_uintUnwrap(PyObject *obj, unsigned int *val); int libvirt_longUnwrap(PyObject *obj, long *val); int libvirt_ulongUnwrap(PyObject *obj, unsigned long *val); int libvirt_longlongUnwrap(PyObject *obj, long long *val); int libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val); int libvirt_doubleUnwrap(PyObject *obj, double *val); int libvirt_boolUnwrap(PyObject *obj, bool *val); --- python/typewrappers.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++++ python/typewrappers.h | 9 +++ 2 files changed, 151 insertions(+), 0 deletions(-) diff --git a/python/typewrappers.c b/python/typewrappers.c index 3f200b3..6ad01df 100644 --- a/python/typewrappers.c +++ b/python/typewrappers.c @@ -119,6 +119,148 @@ libvirt_constcharPtrWrap(const char *str) return (ret); } +int +libvirt_intUnwrap(PyObject *obj, int *val) +{ + long long_val; + + /* If obj is type of PyInt_Type, PyInt_AsLong converts it + * to C long type directly. If it is of PyLong_Type, PyInt_AsLong + * will call PyLong_AsLong() to deal with it automatically. + */ + long_val = PyInt_AsLong(obj); + if ((long_val == -1) && PyErr_Occurred()) + return -1; + + if ((int)long_val == long_val) { + *val = long_val; + } else { + PyErr_SetString(PyExc_OverflowError, + "Python int too large to convert to C int"); + return -1; + } + return 0; +} + +int +libvirt_uintUnwrap(PyObject *obj, unsigned int *val) +{ + long long_val; + + long_val = PyInt_AsLong(obj); + if ((long_val == -1) && PyErr_Occurred()) + return -1; + + if ((unsigned int)long_val == long_val) { + *val = long_val; + } else { + PyErr_SetString(PyExc_OverflowError, + "Python int too large to convert to C unsigned int"); + return -1; + } + return 0; +} + +int +libvirt_longUnwrap(PyObject *obj, long *val) +{ + long long_val; + + long_val = PyInt_AsLong(obj); + if ((long_val == -1) && PyErr_Occurred()) + return -1; + + *val = long_val; + return 0; +} + +int +libvirt_ulongUnwrap(PyObject *obj, unsigned long *val) +{ + long long_val; + + long_val = PyInt_AsLong(obj); + if ((long_val == -1) && PyErr_Occurred()) + return -1; + + *val = long_val; + return 0; +} + +int +libvirt_longlongUnwrap(PyObject *obj, long long *val) +{ + long long llong_val; + + /* If obj is of PyInt_Type, PyLong_AsLongLong + * will call PyInt_AsLong() to handle it automatically. + */ + llong_val = PyLong_AsLongLong(obj); + if ((llong_val == -1) && PyErr_Occurred()) + return -1; + + *val = llong_val; + return 0; +} + +int +libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val) +{ + unsigned long long ullong_val = -1; + + /* The PyLong_AsUnsignedLongLong doesn't check the type of + * obj, only accept argument of PyLong_Type, so we check it instead. + */ + if (PyInt_Check(obj)) { + ullong_val = PyInt_AsLong(obj); + } else if (PyLong_Check(obj)) { + ullong_val = PyLong_AsUnsignedLongLong(obj); + } else { + PyErr_SetString(PyExc_TypeError, "an integer is required"); + } + + if ((ullong_val == -1) && PyErr_Occurred()) + return -1; + + *val = ullong_val; + return 0; +} + +int +libvirt_doubleUnwrap(PyObject *obj, double *val) +{ + double double_val; + + double_val = PyFloat_AsDouble(obj); + if ((double_val == -1) && PyErr_Occurred()) + return -1; + + *val = double_val; + return 0; +} + +int +libvirt_boolUnwrap(PyObject *obj, bool *val) +{ + int ret = -1; + + /* We only accept PyInt_Type, PyLong_Type and PyBool_Type + * as the boolean representation. + */ + if (PyInt_Check(obj) || + PyLong_Check(obj) || PyBool_Check(obj)) { + ret = PyObject_IsTrue(obj); + if (ret < 0) + return ret; + + *val = ret > 0; + return 0; + } else { + PyErr_SetString(PyExc_TypeError, "an integer or bool is required"); + } + return ret; +} + PyObject * libvirt_virDomainPtrWrap(virDomainPtr node) { diff --git a/python/typewrappers.h b/python/typewrappers.h index 1e34dfa..af68bce 100644 --- a/python/typewrappers.h +++ b/python/typewrappers.h @@ -7,6 +7,7 @@ */ #include <Python.h> +#include <stdbool.h> #include "libvirt/libvirt.h" #include "libvirt/virterror.h" @@ -163,6 +164,14 @@ PyObject * libvirt_ulonglongWrap(unsigned long long val); PyObject * libvirt_charPtrWrap(char *str); PyObject * libvirt_charPtrSizeWrap(char *str, Py_ssize_t size); PyObject * libvirt_constcharPtrWrap(const char *str); +int libvirt_intUnwrap(PyObject *obj, int *val); +int libvirt_uintUnwrap(PyObject *obj, unsigned int *val); +int libvirt_longUnwrap(PyObject *obj, long *val); +int libvirt_ulongUnwrap(PyObject *obj, unsigned long *val); +int libvirt_longlongUnwrap(PyObject *obj, long long *val); +int libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val); +int libvirt_doubleUnwrap(PyObject *obj, double *val); +int libvirt_boolUnwrap(PyObject *obj, bool *val); PyObject * libvirt_virConnectPtrWrap(virConnectPtr node); PyObject * libvirt_virDomainPtrWrap(virDomainPtr node); PyObject * libvirt_virNetworkPtrWrap(virNetworkPtr node); -- 1.7.7.5

2 11

Re: [libvirt] Constantly changing USB product ID
by Avi Kivity 28 Mar '12

28 Mar '12

On 03/28/2012 02:41 PM, Avi Kivity wrote: > On 03/27/2012 05:48 PM, Jaap Winius wrote: > > Hi folks, > > > > Recently I learned how to configure KVM with USB pass-though > > functionality. In my case I configured my guest domain with this block > > of code: > > > > <hostdev mode='subsystem' type='usb' managed='yes'> > > <source> > > <vendor id='0x0c93'/> > > <product id='0x1772'/> > > <address bus='1' device='4'/> > > </source> > > </hostdev> > > > > At first this worked fine, but then later the guest domain refused to > > start because the USB device was absent. When I checked, I found that > > its product ID had mysteriously changed to 1771. Later it was back at > > 1772. Now it appears that the USB device I am dealing with has a > > product ID that changes back and forth between 1771 and 1772 at random. > > > > Apparently, the Windows program running on the guest domain is > > designed to deal with this nonsense, but the question is, Can KVM be > > configured to deal with it? Something like <product id='0x177*'/> > > would be useful, but that doesn't work. > > > > Any ideas would be much appreciated. > > > > This is really strange. What kind of device is this? > > I've filed an RFE [1] for virt-manager for assigning USB host devices > opportunistically, that is if they're plugged they're assigned, and if > not the guest starts without them. If it were implemented, you could > assign both 0x1771 and 0x1772 and whichever ID the device is today would > get assigned. > > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=804432 > btw, the correct place for this discussion is likely the libvirt mailing list, or maybe the virt-manager list if it exists. -- error compiling committee.c: too many arguments to function

1 0

[libvirt] Modern CPU models cannot be used with libvirt
by Jiri Denemark 28 Mar '12

28 Mar '12

Hi, Recently I realized that all modern CPU models defined in /etc/qemu/target-x86_64.conf are useless when qemu is used through libvirt. That's because we start qemu with -nodefconfig which results in qemu ignoring that file with CPU model definitions. We have a very good reason for using -nodefconfig because we need to control the ABI presented to a guest OS and we don't want any configuration file that can contain lots of things including device definitions to be read by qemu. However, we would really like the new CPU models to be understood by qemu even if used through libvirt. What would be the best way to solve this? I suspect this could have been already discussed in the past but obviously a workable solution was either not found or just not implemented. Jirka

15 103

[libvirt] RFC: storage migration via pre-copy and streaming mirrors
by Eric Blake 28 Mar '12

28 Mar '12

This is the counter-proposal to my earlier RFC for storage migration via snapshot mirrors[1], resulting from a NACK on the code review for that earlier proposal[2]. In particular, this proposal fleshes out some of Paolo's design overview on the qemu wiki[3]. [1] https://www.redhat.com/archives/libvir-list/2012-March/msg00578.html [2] https://www.redhat.com/archives/libvir-list/2012-March/msg01033.html [3] http://wiki.qemu.org/Features/SnapshotsMultipleDevices My plan is to have everything in this RFC coded up in the next couple of days (hopefully no later than Thursday); this has missed the feature freeze for 0.9.11, so it should not be applied upstream until after the weekend release, as one of the first patches for 0.9.12. Backport-wise, the new flags can be backported as far back as the 0.9.10 .so API, but the new virDomainBlockCopy() API cannot be exported when doing a backport without breaking .so versions (although it's implemenation can be used internally). Additions ========= The following new error code will be added: VIR_ERR_BLOCK_COPY_IN_PROGRESS The following new API will be added: int virDomainBlockCopy(virDomainPtr dom, const char *disk, const char *base, const char *dest, const char *format, unsigned long bandwidth, unsigned int flags); The following new named values will be added: enum virDomainBlockJobType (used in virDomainBlockJobInfo): VIR_DOMAIN_BLOCK_JOB_TYPE_COPY = 2 The following new flags will be added: for virDomainBlockRebase: VIR_DOMAIN_BLOCK_REBASE_SHALLOW = 1 << 0 VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT = 1 << 1 VIR_DOMAIN_BLOCK_REBASE_COPY = 1 << 2 for virDomainBlockCopy: VIR_DOMAIN_BLOCK_COPY_SHALLOW = 1 << 0 VIR_DOMAIN_BLOCK_COPY_REUSE_EXT = 1 << 1 for virDomainBlockJobAbort: VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT Add some XML: Under //domain/drivers/disk, next to <source file='...'/>, add <mirror file='...'/> Semantics ========= virDomainBlockCopy sets up a BLOCK_JOB_TYPE_COPY job. 'disk' names the disk to be copied (can be 'vda' or '/path/to/source', as with other block commands) and must not be NULL. 'base' names the path to the backing file in the chain of the source that will be the new backing file of the destination; if this parameter is NULL, then the destination file defaults to a complete block pull, but the COPY_SHALLOW flag instead requests a pull of just the top file in the source backing chain. 'dest' names the copy being created and must not be NULL; normally, this file is created by the hypervisor/libvirt, but the COPY_REUSE_EXT flag lets an application pass in a pre-created file (allowing metadata to include a relative instead of absolute backing file name). 'format' gives the format of the copy, or NULL to either probe the format of a COPY_REUSE_EXT dest or to reuse the same format as the source. flags cannot contain COPY_SHALLOW unless 'base' is NULL. Once a block copy job is started, calls to virDomainGetBlockJobInfo() for the same 'disk' will report an info with VIR_DOMAIN_BLOCK_JOB_TYPE_COPY as the type. This job never completes on its own, but must be stopped by the user (this enables mirroring to continue until the user informs libvirt that any backing files, perhaps located at different locations as specified by relative path names using REUSE_EXT, have been externally copied into place). There are two phases to a TYPE_COPY job. In the first phase, cur < end when querying progress, calls to virDomainBlockJobAbort(dom, disk, 0) will cancel the operation and revert to the source, and calls to virDomainBlockJobAbort(dom, disk, VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) will fail with VIR_ERR_BLOCK_COPY_IN_PROGRESS. In the second phase, cur == end when querying progress, calls to virDomainBlockJobAbort(dom, disk, 0) will break the mirroring and revert to the source, while calls to virDomainBlockJobAbort(dom, disk, VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) will break the mirroring and pivot to the destination. Use of VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT on a non-copy block job will fail with VIR_ERR_INVALID_ARG. virDomainBlockRebase(dom, disk, dest, bandwidth, VIR_DOMAIN_BLOCK_REBASE_COPY | (flags & 3)) is shorthand for virDomainBlockCopy(dom, disk, NULL, dest, NULL, bandwidth, (flags & 3)) - that is, use of the REBASE_COPY flag treats the BlockRebase 'base' argument as the BlockCopy 'dest' argument, creates the destination with the same file format as the source (or probes the backing file format if REBASE_REUSE_EXT is used), and passes the COPY_SHALLOW and COPY_REUSE_EXT flags through (note that the similarly named flags were conveniently chosen to be the same values). Attempts to use REBASE_SHALLOW or REBASE_REUSE_EXT without also using REBASE_COPY will fail with VIR_ERR_INVALID_ARG. While a copy operation is in place, virDomainGetXMLDesc (dumpxml) will show the <mirror> element for that <disk>. Initial Implementation ====================== When virDomainBlockCopy is called (perhaps via the virDomainBlockRebase alias), it first sets up a mirror using the 'drive-mirror' monitor command and the destination file name. The mirror is opened with the 'existing' mode if _REUSE_EXT is present; otherwise it is opened with the 'absolute-paths' mode if _SHALLOW is present or 'no-backing-file' mode if no flags are present. Next, the function calls the 'block_stream' monitor command to start the streaming. The streaming command uses 'base' as its starting point, except that when _SHALLOW was specified, libvirt will use the backing file of the source disk, rather than NULL (this can be obtained by the 'query-block' monitor command; although someday libvirt should start tracking this information in <domain> XML rather than relying on qemu). At this point, control returns to the user, and the stream proceeds in the background; virDomainBlockJobSetSpeed can tune the speed of the block streaming. At least in the initial implementation, as long as the block job is active, libvirt will prevent 'virDomainMigrate', 'virDomainSave', 'virDomainSnapshotCreateXML' in general, and 'virDomainDetachDevice' of the disk being mirrored, all with the new VIR_ERR_BLOCK_COPY_IN_PROGRESS error. This is because I don't have an easy way to resume mirroring when restarting a new qemu process; preventing these actions until the user first cancels the ongoing mirroring will result in fewer corner cases that libvirt has to worry about. This also implies that the initial implementation will fail for persistent domains, and only be useful for transient domains. Attempts to define a domain with a <mirror> element are rejected, leaving <mirror> as output-only XML useful in restoring state when restarting libvirtd. It may be possible to add persistent support in the future, once we determine how to make qemu resume a mirrored block device; at that point, it would be possible to specify <mirror> in domain xml during domain creation or during device hotplug. When the block stream finishes, qemu will send an event to libvirt (libvirt will also have to manually check for completion on a libvirtd restart, based on whether cur == end in the block job info). I'm not yet sure whether to expose this event to the user so that they do not have to poll the block job info, or whether to consume it internally. At any rate, before this event occurs, the BLOCK_JOB_ABORT_PIVOT flag is rejected, and virDomainBlockJobAbort without flags uses the 'block_job_cancel' monitor command to stop the streaming early, then the 'drive-reopen' monitor command to break the mirroring back to the source; it is feasible that there is a race where a 'block_job_cancel' can be called after the pull is complete but before the completion event has been processed, so the code must proceed on to the 'drive-reopen' even if the job cancel fails. After the event occurs, virDomainBlockJobAbort only needs to use the 'drive-reopen' monitor command, with either the source or the destination file depending on the BLOCK_JOB_ABORT_PIVOT flag. Until 'drive-reopen' is made atomic in qemu (by adding code to support it inside 'transaction'), the user risks a block job abort rendering the disk unusable, because the source was closed before the destination was opened; hopefully this situation is rare, in part because libvirt will do stat() checks and SELinux labeling on destination files before starting qemu monitor commands, as a sanity check that qemu will be able to use the specified files. If qemu ever adds atomic 'drive-reopen' support, we can add a new flag BLOCK_JOB_ABORT_ATOMIC that fails on older qemu, and ensures the use of 'transaction' on the newer qemu that supports an atomic reopen. If a block mirror is aborted (whether by the user calling virDomainBlockJobAbort with no flag, or by the qemu process ending due to things like a guest-initiated shutdown), then the mirror can be safely discarded, and restarting the domain will be unmirrored where the virDomainBlockRebase can be called again from scratch. Examples ======== For some examples, starting with base <- snap1 <- snap2 <- snap3 as the backing chain for disk 'vda', virDomainBlockCopy(dom, "vda", NULL, "/path/to/copy", NULL, 0) would set up a job that results in /path/to/copy being the same file format as snap3, but containing the entire chain virDomainBlockCopy(dom, "vda", "/path/to/snap1", "/path/to/copy", "qed", 0) would set up a job that results in base <- snap1 <- copy as the mirrored backing chain, and ensuring that copy is formatted as qed regardless of the format of snap3 virDomainBlockCopy(dom, "vda", NULL, "/path/to/copy", NULL, VIR_DOMAIN_BLOCK_COPY_SHALLOW) is shorthand for virDomainBlockCopy(dom, "vda", "/path/to/snap2", "/path/to/copy", NULL, 0) and results in base <- snap1 <- snap2 <- copy creates copy using the same format as snap3 virDomainBlockCopy(dom, "vda", "/path/to/snap2", "/path/to/copy", NULL, VIR_DOMAIN_BLOCK_COPY_REUSE_EXT) requires /path/to/copy to already exist, probes it for existing format (which might be different from snap3), and proceeds to mirror everything so that snap2 is the base of copy (and the user is at fault if the pre-existing file doesn't call out a backing file that happens to be identical in content to snap2) oVirt will probably use the sequence: - use qemu-img to create an empty qcow2 file with relative backing name to the destination storage - call virDomainBlockRebase(dom, disk, "/path/to/copy", VIR_DOMAIN_BLOCK_REBASE_COPY | VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) - copy the base files from source to destination storage (this can be done in parallel, either before or after the virDomainBlockRebase call) - wait for the block pull to finish (either by waiting for an event if I propagate the event, or by polling virDomainBlockJobInfo, or even by polling virDomainBlockJobAbort(VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) and checking for VIR_ERR_BLOCK_COPY_IN_PROGRESS - once both the base files are in place and the block pull half of the copy job is complete (and without regard to whether the block stream or the external base file copying completed first), call virDomainBlockJobAbort(,VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) to reopen to the new storage domain chain Comparison to first RFC ======================= This proposal exposes only one disk at a time, while the earlier virDomainSnapshotCreateXML <mirror> approach could atomically set up mirroring on multiple disks. However, the nature of block jobs being a background process means that parallel jobs can be run on independent disks, so the user can do the overall block migration with the time cost of the slowest disk, rather than having to do things serially with the time cost of all disks added together. This proposal avoids having to create an intermediate snapshot, so the pull is more efficient and the source chain does not get longer, no matter how many times the process is aborted and restarted. This proposal can expose the no-backing-file mode, while the snapshot approach did not. To survive across libvirtd restarts, the snapshot approach was using <domainsnapshot> to store the mirroring status in a user-visible location; this approach has to modify the internal live xml (alongside other internal data, such as the qemu pid). Or perhaps I can add a <mirror> subelement to <disk> of a domain and make it user-visible after all, and treat that as an output-only parameter for now. Both approaches face the dilemma of how to start a new qemu process with mirroring intact, and my solution in both patch series will be to prevent any action that would force libvirt to save domain state until after the user has first canceled all current mirroring jobs. This limitation is not permanent - if future qemu provides better ways to restart mirroring, and as libvirt is taught to store the full backing chain in <domain> xml instead of probing it on the fly, we can relax this restriction in the future. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

2 2