April 2019 - Devel - libvirt List Archives

[libvirt] [PATCH 0/7] qemu: use domCaps for validation
by Cole Robinson 06 Aug '19

06 Aug '19

I'm trying to remove some hurdles and pitfalls WRT extending domaincapabilities data. One issue is that it's too easy to add invalid data to it, or let the data become out of date. For example the first two patches of this series add <rng model=X> domcaps reporting. The logic to fill in the domcaps data from qemuCaps is nearly identical to the logic we use to validate rng->model in qemuDomainRNGDefValidate. If just those patches are added, and later a new qemu rng model was introduced, a future patch could easily miss updated domaincapabilities output. This series aims to set up a pattern to prevent these types of issues from sneaking in. A function virDomainCapsDeviceDefValidate is added which will use domcaps data to perform validation against a devicedef. The existing qemu <rng> model validation is moved there. This ensures that any future <rng> model additions, if they want to work in the qemu driver, effectively need to extend domaincapabilities as well. It's also theoretically useful for other drivers too. One issue is that at DomainDefValidate time we don't have domCaps handy, or any cache layer for domCaps assembling. Patch #4 adds a domCapsCache hashtable to the virQEMUCaps class for caching domCaps builds based on the full tuple of emulator+machine+arch+virttype. If qemuCaps need to be regenerated, the domCaps cache is wiped out for us so we don't need to worry about the data being stale, it's tied to the lifetime of a qemuCaps instance. Cole Robinson (7): conf: domcaps: Report device <rng> qemu: capabilities: fill in domcaps <rng> qemu: conf: add virQEMUDriverGetDomainCapabilities qemu: conf: Cache domCaps in qemuCaps conf: domcaps: Add virDomainCapsDeviceDefValidate qemu: domain: Call virDomainCapsDeviceDefValidate qemu: Move rng model validation to domcaps docs/formatdomaincaps.html.in | 35 ++++++++ docs/schemas/domaincaps.rng | 10 +++ src/conf/domain_capabilities.c | 83 ++++++++++++++++++ src/conf/domain_capabilities.h | 14 ++++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 41 +++++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_conf.c | 84 +++++++++++++++++++ src/qemu/qemu_conf.h | 7 ++ src/qemu/qemu_domain.c | 38 +++------ src/qemu/qemu_driver.c | 18 +--- .../qemu_1.7.0.x86_64.xml | 9 ++ .../qemu_2.12.0-virt.aarch64.xml | 11 +++ .../qemu_2.12.0.ppc64.xml | 11 +++ .../qemu_2.12.0.s390x.xml | 11 +++ .../qemu_2.12.0.x86_64.xml | 11 +++ .../qemu_2.6.0-virt.aarch64.xml | 11 +++ .../qemu_2.6.0.aarch64.xml | 11 +++ .../domaincapsschemadata/qemu_2.6.0.ppc64.xml | 11 +++ .../qemu_2.6.0.x86_64.xml | 11 +++ .../domaincapsschemadata/qemu_2.7.0.s390x.xml | 11 +++ .../qemu_2.8.0-tcg.x86_64.xml | 11 +++ .../domaincapsschemadata/qemu_2.8.0.s390x.xml | 11 +++ .../qemu_2.8.0.x86_64.xml | 11 +++ .../qemu_2.9.0-q35.x86_64.xml | 11 +++ .../qemu_2.9.0-tcg.x86_64.xml | 11 +++ .../qemu_2.9.0.x86_64.xml | 11 +++ .../domaincapsschemadata/qemu_3.0.0.s390x.xml | 11 +++ .../qemu_4.0.0.x86_64.xml | 11 +++ 29 files changed, 488 insertions(+), 40 deletions(-) -- 2.21.0

3 16

[libvirt] [python PATCH v7] Add virDomainCheckpoint APIs
by Eric Blake 29 Jul '19

29 Jul '19

Copies heavily from existing virDomainSnapshot handling, regarding what special cases the generator has to be taught and what overrides need to be written. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Python counterparts to my incremental backup patches. An earlier version was already reviewed by Dan; the main diff here is the addition of virDomainSnapshotCreateXML2 handling. HACKING | 2 + MANIFEST.in | 1 + generator.py | 37 ++++++++-- libvirt-override-api.xml | 12 ++++ libvirt-override-virDomain.py | 13 ++++ libvirt-override-virDomainCheckpoint.py | 19 +++++ libvirt-override.c | 96 +++++++++++++++++++++++++ sanitytest.py | 11 +-- typewrappers.c | 13 ++++ typewrappers.h | 10 +++ 10 files changed, 206 insertions(+), 8 deletions(-) create mode 100644 libvirt-override-virDomainCheckpoint.py diff --git a/HACKING b/HACKING index 6eeb9e6..39e7cd3 100644 --- a/HACKING +++ b/HACKING @@ -28,6 +28,8 @@ hand written source files the virConnect class - libvirt-override-virDomain.py - high level overrides in the virDomain class + - libvirt-override-virDomainCheckpoint.py - high level overrides in + the virDomainCheckpoint class - libvirt-override-virDomainSnapshot.py - high level overrides in the virDomainSnapshot class - libvirt-override-virStoragePool.py - high level overrides in diff --git a/MANIFEST.in b/MANIFEST.in index b6788f4..5d2f559 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -23,6 +23,7 @@ include libvirt-override.c include libvirt-override.py include libvirt-override-virConnect.py include libvirt-override-virDomain.py +include libvirt-override-virDomainCheckpoint.py include libvirt-override-virDomainSnapshot.py include libvirt-override-virStoragePool.py include libvirt-override-virStream.py diff --git a/generator.py b/generator.py index ffa3ce5..a16f9b1 100755 --- a/generator.py +++ b/generator.py @@ -35,6 +35,7 @@ libvirt_headers = [ "libvirt", "libvirt-common", "libvirt-domain", + "libvirt-domain-checkpoint", "libvirt-domain-snapshot", "libvirt-event", "libvirt-host", @@ -364,6 +365,10 @@ py_types = { 'virStream *': ('O', "virStream", "virStreamPtr", "virStreamPtr"), 'const virStream *': ('O', "virStream", "virStreamPtr", "virStreamPtr"), + 'virDomainCheckpointPtr': ('O', "virDomainCheckpoint", "virDomainCheckpointPtr", "virDomainCheckpointPtr"), + 'virDomainCheckpoint *': ('O', "virDomainCheckpoint", "virDomainCheckpointPtr", "virDomainCheckpointPtr"), + 'const virDomainCheckpoint *': ('O', "virDomainCheckpoint", "virDomainCheckpointPtr", "virDomainCheckpointPtr"), + 'virDomainSnapshotPtr': ('O', "virDomainSnapshot", "virDomainSnapshotPtr", "virDomainSnapshotPtr"), 'virDomainSnapshot *': ('O', "virDomainSnapshot", "virDomainSnapshotPtr", "virDomainSnapshotPtr"), 'const virDomainSnapshot *': ('O', "virDomainSnapshot", "virDomainSnapshotPtr", "virDomainSnapshotPtr"), @@ -536,6 +541,8 @@ skip_function = ( 'virSaveLastError', # We have our own python error wrapper 'virFreeError', # Only needed if we use virSaveLastError 'virConnectListAllDomains', # overridden in virConnect.py + 'virDomainListAllCheckpoints', # overridden in virDomain.py + 'virDomainCheckpointListAllChildren', # overridden in virDomainCheckpoint.py 'virDomainListAllSnapshots', # overridden in virDomain.py 'virDomainSnapshotListAllChildren', # overridden in virDomainSnapshot.py 'virConnectListAllStoragePools', # overridden in virConnect.py @@ -582,6 +589,7 @@ skip_function = ( "virStoragePoolRef", "virStorageVolRef", "virStreamRef", + "virDomainCheckpointRef", "virDomainSnapshotRef", # This functions shouldn't be called via the bindings (and even the docs @@ -594,6 +602,8 @@ skip_function = ( "virNWFilterGetConnect", "virStoragePoolGetConnect", "virStorageVolGetConnect", + "virDomainCheckpointGetConnect", + "virDomainCheckpointGetDomain", "virDomainSnapshotGetConnect", "virDomainSnapshotGetDomain", @@ -1023,6 +1033,8 @@ classes_type = { "virStream *": ("._o", "virStream(self, _obj=%s)", "virStream"), "virConnectPtr": ("._o", "virConnect(_obj=%s)", "virConnect"), "virConnect *": ("._o", "virConnect(_obj=%s)", "virConnect"), + "virDomainCheckpointPtr": ("._o", "virDomainCheckpoint(self,_obj=%s)", "virDomainCheckpoint"), + "virDomainCheckpoint *": ("._o", "virDomainCheckpoint(self, _obj=%s)", "virDomainCheckpoint"), "virDomainSnapshotPtr": ("._o", "virDomainSnapshot(self,_obj=%s)", "virDomainSnapshot"), "virDomainSnapshot *": ("._o", "virDomainSnapshot(self, _obj=%s)", "virDomainSnapshot"), } @@ -1031,7 +1043,7 @@ primary_classes = ["virDomain", "virNetwork", "virInterface", "virStoragePool", "virStorageVol", "virConnect", "virNodeDevice", "virSecret", "virNWFilter", "virNWFilterBinding", - "virStream", "virDomainSnapshot"] + "virStream", "virDomainCheckpoint", "virDomainSnapshot"] classes_destructors = { "virDomain": "virDomainFree", @@ -1043,6 +1055,7 @@ classes_destructors = { "virSecret": "virSecretFree", "virNWFilter": "virNWFilterFree", "virNWFilterBinding": "virNWFilterBindingFree", + "virDomainCheckpoint": "virDomainCheckpointFree", "virDomainSnapshot": "virDomainSnapshotFree", # We hand-craft __del__ for this one #"virStream": "virStreamFree", @@ -1053,6 +1066,7 @@ class_skip_connect_impl = { } class_domain_impl = { + "virDomainCheckpoint": True, "virDomainSnapshot": True, } @@ -1171,6 +1185,18 @@ def nameFixup(name, classe, type, file): elif name[0:12] == "virDomainGet": func = name[12:] func = func[0:1].lower() + func[1:] + elif name[0:31] == "virDomainCheckpointLookupByName": + func = name[9:] + func = func[0:1].lower() + func[1:] + elif name[0:28] == "virDomainCheckpointCreateXML": + func = name[9:] + func = func[0:1].lower() + func[1:] + elif name[0:26] == "virDomainCheckpointCurrent": + func = name[9:] + func = func[0:1].lower() + func[1:] + elif name[0:19] == "virDomainCheckpoint": + func = name[19:] + func = func[0:1].lower() + func[1:] elif name[0:29] == "virDomainSnapshotLookupByName": func = name[9:] func = func[0:1].lower() + func[1:] @@ -1183,6 +1209,9 @@ def nameFixup(name, classe, type, file): elif name[0:20] == "virDomainSnapshotNum": func = name[9:] func = func[0:1].lower() + func[1:] + elif name[0:27] == "virDomainSnapshotCreateXML2": + func = name[9:] + func = func[0:1].lower() + func[1:] elif name[0:26] == "virDomainSnapshotCreateXML": func = name[9:] func = func[0:1].lower() + func[1:] @@ -1501,7 +1530,7 @@ def buildWrappers(module): "virStorageVol", "virNodeDevice", "virSecret","virStream", "virNWFilter", "virNWFilterBinding" ]: classes.write(" def __init__(self, conn, _obj=None):\n") - elif classname in [ 'virDomainSnapshot' ]: + elif classname in [ "virDomainCheckpoint", "virDomainSnapshot" ]: classes.write(" def __init__(self, dom, _obj=None):\n") else: classes.write(" def __init__(self, _obj=None):\n") @@ -1513,7 +1542,7 @@ def buildWrappers(module): classes.write(" self._conn = conn\n" + \ " if not isinstance(conn, virConnect):\n" + \ " self._conn = conn._conn\n") - elif classname in [ "virDomainSnapshot" ]: + elif classname in [ "virDomainCheckpoint", "virDomainSnapshot" ]: classes.write(" self._dom = dom\n") classes.write(" self._conn = dom.connect()\n") classes.write(" if type(_obj).__name__ not in [\"PyCapsule\", \"PyCObject\"]:\n") @@ -1641,7 +1670,7 @@ def buildWrappers(module): classes.write( " if ret is None:raise libvirtError('%s() failed', vol=self)\n" % (name)) - elif classname == "virDomainSnapshot": + elif classname in [ "virDomainCheckpoint", "virDomainSnapshot"]: classes.write( " if ret is None:raise libvirtError('%s() failed', dom=self._dom)\n" % (name)) diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml index 7f578e0..1edf0c5 100644 --- a/libvirt-override-api.xml +++ b/libvirt-override-api.xml @@ -576,6 +576,18 @@ <arg name='flags' type='unsigned int' info='flags'/> <return type='int' info="0 on success, -1 on error"/> </function> + <function name='virDomainListAllCheckpoints' file='python'> + <info>returns the list of checkpoints for the given domain</info> + <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> + <arg name='flags' type='unsigned int' info='flags'/> + <return type='char *' info='the list of checkpoints or None in case of error'/> + </function> + <function name='virDomainCheckpointListAllChildren' file='python'> + <info>collect the list of child checkpoint names for the given checkpoint</info> + <arg name='checkpoint' type='virDomainCheckpointPtr' info='pointer to the checkpoint'/> + <arg name='flags' type='unsigned int' info='flags'/> + <return type='char *' info='the list of checkpoints or None in case of error'/> + </function> <function name='virDomainGetBlockJobInfo' file='python'> <info>Get progress information for a block job</info> <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> diff --git a/libvirt-override-virDomain.py b/libvirt-override-virDomain.py index 7c417b8..7ce34f1 100644 --- a/libvirt-override-virDomain.py +++ b/libvirt-override-virDomain.py @@ -11,6 +11,19 @@ return retlist + def listAllCheckpoints(self, flags=0): + """List all checkpoints and returns a list of checkpoint objects""" + ret = libvirtmod.virDomainListAllCheckpoints(self._o, flags) + if ret is None: + raise libvirtError("virDomainListAllCheckpoints() failed", conn=self) + + retlist = list() + for chkptr in ret: + retlist.append(virDomainCheckpoint(self, _obj=chkptr)) + + return retlist + + def createWithFiles(self, files, flags=0): """Launch a defined domain. If the call succeeds the domain moves from the defined to the running domains pools. diff --git a/libvirt-override-virDomainCheckpoint.py b/libvirt-override-virDomainCheckpoint.py new file mode 100644 index 0000000..371b0fd --- /dev/null +++ b/libvirt-override-virDomainCheckpoint.py @@ -0,0 +1,19 @@ + def getConnect(self): + """Get the connection that owns the domain that a checkpoint was created for""" + return self.connect() + + def getDomain(self): + """Get the domain that a checkpoint was created for""" + return self.domain() + + def listAllChildren(self, flags=0): + """List all child checkpoints and returns a list of checkpoint objects""" + ret = libvirtmod.virDomainCheckpointListAllChildren(self._o, flags) + if ret is None: + raise libvirtError("virDomainCheckpointListAllChildren() failed", conn=self) + + retlist = list() + for chkptr in ret: + retlist.append(virDomainCheckpoint(self, _obj=chkptr)) + + return retlist diff --git a/libvirt-override.c b/libvirt-override.c index c5e2908..777f533 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -2325,6 +2325,98 @@ libvirt_virConnectListDefinedDomains(PyObject *self ATTRIBUTE_UNUSED, goto cleanup; } +#if LIBVIR_CHECK_VERSION(5, 1, 0) +static PyObject * +libvirt_virDomainListAllCheckpoints(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval = NULL; + virDomainCheckpointPtr *chks = NULL; + int c_retval; + ssize_t i; + virDomainPtr dom; + PyObject *pyobj_dom; + unsigned int flags; + + if (!PyArg_ParseTuple(args, (char *)"OI:virDomainListAllCheckpoints", + &pyobj_dom, &flags)) + return NULL; + dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainListAllCheckpoints(dom, &chks, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval < 0) + return VIR_PY_NONE; + + if (!(py_retval = PyList_New(c_retval))) + goto cleanup; + + for (i = 0; i < c_retval; i++) { + VIR_PY_LIST_SET_GOTO(py_retval, i, + libvirt_virDomainCheckpointPtrWrap(chks[i]), error); + chks[i] = NULL; + } + + cleanup: + for (i = 0; i < c_retval; i++) + if (chks[i]) + virDomainCheckpointFree(chks[i]); + VIR_FREE(chks); + return py_retval; + + error: + Py_CLEAR(py_retval); + goto cleanup; +} + +static PyObject * +libvirt_virDomainCheckpointListAllChildren(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval = NULL; + virDomainCheckpointPtr *chks = NULL; + int c_retval; + ssize_t i; + virDomainCheckpointPtr parent; + PyObject *pyobj_parent; + unsigned int flags; + + if (!PyArg_ParseTuple(args, (char *)"OI:virDomainCheckpointListAllChildren", + &pyobj_parent, &flags)) + return NULL; + parent = (virDomainCheckpointPtr) PyvirDomainCheckpoint_Get(pyobj_parent); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainCheckpointListAllChildren(parent, &chks, flags); + LIBVIRT_END_ALLOW_THREADS; + + if (c_retval < 0) + return VIR_PY_NONE; + + if (!(py_retval = PyList_New(c_retval))) + goto cleanup; + + for (i = 0; i < c_retval; i++) { + VIR_PY_LIST_SET_GOTO(py_retval, i, + libvirt_virDomainCheckpointPtrWrap(chks[i]), error); + chks[i] = NULL; + } + + cleanup: + for (i = 0; i < c_retval; i++) + if (chks[i]) + virDomainCheckpointFree(chks[i]); + VIR_FREE(chks); + return py_retval; + + error: + Py_CLEAR(py_retval); + goto cleanup; +} +#endif /* LIBVIR_CHECK_VERSION(5, 1, 0) */ + static PyObject * libvirt_virDomainSnapshotListNames(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) @@ -10100,6 +10192,10 @@ static PyMethodDef libvirtMethods[] = { #if LIBVIR_CHECK_VERSION(1, 0, 3) {(char *) "virDomainGetJobStats", libvirt_virDomainGetJobStats, METH_VARARGS, NULL}, #endif /* LIBVIR_CHECK_VERSION(1, 0, 3) */ +#if LIBVIR_CHECK_VERSION(5, 1, 0) + {(char *) "virDomainListAllCheckpoints", libvirt_virDomainListAllCheckpoints, METH_VARARGS, NULL}, + {(char *) "virDomainCheckpointListAllChildren", libvirt_virDomainCheckpointListAllChildren, METH_VARARGS, NULL}, +#endif /* LIBVIR_CHECK_VERSION(5, 1, 0) */ {(char *) "virDomainSnapshotListNames", libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL}, #if LIBVIR_CHECK_VERSION(0, 9, 13) {(char *) "virDomainListAllSnapshots", libvirt_virDomainListAllSnapshots, METH_VARARGS, NULL}, diff --git a/sanitytest.py b/sanitytest.py index 68dde6b..b044231 100644 --- a/sanitytest.py +++ b/sanitytest.py @@ -249,13 +249,13 @@ for name in sorted(basicklassmap): # Remove 'Get' prefix from most APIs, except those in virConnect # and virDomainSnapshot namespaces which stupidly used a different # convention which we now can't fix without breaking API - if func[0:3] == "Get" and klass not in ["virConnect", "virDomainSnapshot", "libvirt"]: + if func[0:3] == "Get" and klass not in ["virConnect", "virDomainCheckpoint", "virDomainSnapshot", "libvirt"]: if func not in ["GetCPUStats", "GetTime"]: func = func[3:] # The object creation and lookup APIs all have to get re-mapped # into the parent class - if func in ["CreateXML", "CreateLinux", "CreateXMLWithFiles", + if func in ["CreateXML", "CreateXML2", "CreateLinux", "CreateXMLWithFiles", "DefineXML", "CreateXMLFrom", "LookupByUUID", "LookupByUUIDString", "LookupByVolume" "LookupByName", "LookupByID", "LookupByName", "LookupByKey", "LookupByPath", @@ -266,7 +266,7 @@ for name in sorted(basicklassmap): if klass != "virDomain": func = klass[3:] + func - if klass == "virDomainSnapshot": + if klass in [ "virDomainCheckpoint", "virDomainSnapshot"]: klass = "virDomain" func = func[6:] elif klass == "virStorageVol" and func in ["StorageVolCreateXMLFrom", "StorageVolCreateXML"]: @@ -297,10 +297,13 @@ for name in sorted(basicklassmap): if func[0:6] == "Change": klass = "virConnect" - # Need to special case the snapshot APIs + # Need to special case the checkpoint and snapshot APIs if klass == "virDomainSnapshot" and func in ["Current", "ListNames", "Num"]: klass = "virDomain" func = "snapshot" + func + elif klass == "virDomainCheckpoint" and func == "Current": + klass = "virDomain" + func = "checkpoint" + func # Names should start with lowercase letter... func = func[0:1].lower() + func[1:] diff --git a/typewrappers.c b/typewrappers.c index 9ba14b4..cd7a70b 100644 --- a/typewrappers.c +++ b/typewrappers.c @@ -568,6 +568,19 @@ libvirt_virStreamPtrWrap(virStreamPtr node) return ret; } +PyObject * +libvirt_virDomainCheckpointPtrWrap(virDomainCheckpointPtr node) +{ + PyObject *ret; + + if (node == NULL) { + return VIR_PY_NONE; + } + + ret = libvirt_buildPyObject(node, "virDomainCheckpointPtr", NULL); + return ret; +} + PyObject * libvirt_virDomainSnapshotPtrWrap(virDomainSnapshotPtr node) { diff --git a/typewrappers.h b/typewrappers.h index 4423774..198397b 100644 --- a/typewrappers.h +++ b/typewrappers.h @@ -128,6 +128,15 @@ typedef struct { } PyvirStream_Object; +#define PyvirDomainCheckpoint_Get(v) (((v) == Py_None) ? NULL : \ + (((PyvirDomainCheckpoint_Object *)(v))->obj)) + +typedef struct { + PyObject_HEAD + virDomainCheckpointPtr obj; +} PyvirDomainCheckpoint_Object; + + #define PyvirDomainSnapshot_Get(v) (((v) == Py_None) ? NULL : \ (((PyvirDomainSnapshot_Object *)(v))->obj)) @@ -204,6 +213,7 @@ PyObject * libvirt_virSecretPtrWrap(virSecretPtr node); PyObject * libvirt_virNWFilterPtrWrap(virNWFilterPtr node); PyObject * libvirt_virNWFilterBindingPtrWrap(virNWFilterBindingPtr node); PyObject * libvirt_virStreamPtrWrap(virStreamPtr node); +PyObject * libvirt_virDomainCheckpointPtrWrap(virDomainCheckpointPtr node); PyObject * libvirt_virDomainSnapshotPtrWrap(virDomainSnapshotPtr node); -- 2.20.1

2 2

[libvirt] [PATCH v4 00/25] Fix and enable owner remembering
by Michal Privoznik 09 Jul '19

09 Jul '19

This is meant for next release to have the most time possible for testing. Some of the patches were ACKed in v3 already but since they don't make sense on their own I haven't pushed them. v4 of: https://www.redhat.com/archives/libvir-list/2019-March/msg01948.html As usual, you can find (not only these) patches on my github: https://github.com/zippy2/libvirt branch xattr_fixes_v4 diff to v3: - Some new patches (qemusecuritytest and qemusecuritymock) - Some other fixes raised by Cole in review of v3 (like double error reporting and others) - Remembering is done only for paths that cannot be shared between domains. This renders refcounting needless because the refcounter can't ever be greater than one. Nevertheless, I'm keeping it in because in the long run I might come up with a solution to the problem of shared resources and having refcounters might help. Michal Prívozník (25): qemusecuritymock: Mock virProcessRunInFork qemusecuritymock: Fix bit arithmetic qemusecuritymock: Actually set error on failure qemusecuritymock: Introduce and use freePaths() qemusecuritytest: Drop unused variable qemusecuritytest: Use AUTOFREE/AUTOUNREF qemusecuritytest: Fix capabilities loading tools: Slightly rework libvirt_recover_xattrs.sh virSecuritySELinuxRestoreAllLabel: Print @migrated in the debug message too virfile: Make virFileGetXAttr report errors virFileSetXAttr: Report error on failure virFileRemoveXAttr: Report error on failure security: Don't skip label restore on file systems lacking XATTRs security: Document @restore member of transaction list security_dac: Allow caller to suppress owner remembering security_selinux: Allow caller to suppress owner remembering qemusecuritymock: Allow some paths to be not restored security: Don't remember owner for shared resources security: Introduce virSecurityManagerMoveImageMetadata security_util: Introduce virSecurityMoveRememberedLabel security_dac: Implement virSecurityManagerMoveImageMetadata security_selinux: Implement virSecurityManagerMoveImageMetadata qemu_security: Implement qemuSecurityMoveImageMetadata qemu: Move image security metadata on snapshot activity Revert "qemu: Temporary disable owner remembering" docs/news.xml | 13 ++ src/libvirt_private.syms | 2 + src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 5 + src/qemu/qemu_blockjob.c | 6 + src/qemu/qemu_conf.c | 4 + src/qemu/qemu_driver.c | 17 +- src/qemu/qemu_security.c | 19 +++ src/qemu/qemu_security.h | 5 + src/qemu/test_libvirtd_qemu.aug.in | 1 + src/security/security_dac.c | 171 +++++++++++++++---- src/security/security_driver.h | 5 + src/security/security_manager.c | 39 +++++ src/security/security_manager.h | 4 + src/security/security_nop.c | 10 ++ src/security/security_selinux.c | 263 ++++++++++++++++++++--------- src/security/security_stack.c | 20 +++ src/security/security_util.c | 73 +++++++- src/security/security_util.h | 5 + src/util/virfile.c | 78 +++++++-- src/util/virfile.h | 5 + src/util/virprocess.h | 3 +- tests/qemusecuritymock.c | 76 +++++++-- tests/qemusecuritytest.c | 146 ++++++++++------ tests/qemusecuritytest.h | 4 +- tools/libvirt_recover_xattrs.sh | 50 +++--- 26 files changed, 802 insertions(+), 223 deletions(-) -- 2.21.0

4 64

[libvirt] [PATCH v8 00/21] Incremental backups
by Eric Blake 07 Jul '19

07 Jul '19

The API is fairly stable, but there are still probably code cleanups worth adding to the qemu driver implementation. I've also seen a desire on the list to support all APIs in the test driver, so I'll try and whip that together for my next round of patches. If we are confident that the API is worth having in 5.3 (especially since the earlier part of this series already has positive reviews), then it's probably worth committing that part of the series now even while I still address review comments on the later patches. I've pushed a tag backup-v8 to both my libvirt.git and libvirt-python.git repos to match: https://repo.or.cz/libvirt/ericb.git/shortlog/refs/tags/backup-v8 https://repo.or.cz/libvirt-python/ericb.git/shortlog/refs/tags/backup-v8 Here's hoping we're happy enough with the API, including the fact that rudimentary operation of pull mode backups work with qemu 4.0-rc1, for this to make it into the 5.2 release. Notable diffs from v7: - address several review comments (although I didn't get to every comment later in the series) - actually support job id of 0 (it was documented but not working in v7) - add support for Unix socket pull mode backups - improve checkpoint XML handling (more tests, separate postparse actions out from initial parse) - rebase to master 001/21:[0044] [FC] 'backup: Document new XML for checkpoints' 002/21:[----] [--] 'backup: Document new XML for backups' 003/21:[0006] [FC] 'backup: Introduce virDomainCheckpoint APIs' 004/21:[0002] [FC] 'backup: Introduce virDomainBackup APIs' 005/21:[----] [--] 'backup: Document nuances between different state capture APIs' 006/21:[0304] [FC] 'backup: Parse and output checkpoint XML' 007/21:[0007] [FC] 'backup: Allow for lists of checkpoint objects' 008/21:[0004] [FC] 'backup: Add new domain:checkpoint access control' 009/21:[0043] [FC] 'backup: Implement backup APIs for remote driver' 010/21:[down] 'conf: Add parameter to virDomainDiskSourceFormat' 011/21:[0019] [FC] 'backup: Parse and output backup XML' 012/21:[0006] [FC] 'backup: Implement virsh support for checkpoints' 013/21:[0002] [FC] 'backup: Implement virsh support for backup' 014/21:[down] 'backup: Prepare for Unix sockets in QMP nbd-server-start' 015/21:[0008] [FC] 'backup: Add new qemu monitor interactions' 016/21:[0022] [FC] 'backup: qemu: Implement metadata tracking for checkpoint APIs' 017/21:[0004] [FC] 'backup: Wire up qemu checkpoint commands over QMP' 018/21:[0019] [FC] 'backup: qemu: Implement framework for backup job APIs' 019/21:[0042] [FC] 'backup: Wire up qemu full pull backup commands over QMP' 020/21:[----] [-C] 'backup: qemu: Wire up qemu full push backup commands over QMP' 021/21:[0003] [FC] 'backup: implement qemu incremental pull backup' Eric Blake (21): backup: Document new XML for checkpoints backup: Document new XML for backups backup: Introduce virDomainCheckpoint APIs backup: Introduce virDomainBackup APIs backup: Document nuances between different state capture APIs backup: Parse and output checkpoint XML backup: Allow for lists of checkpoint objects backup: Add new domain:checkpoint access control backup: Implement backup APIs for remote driver conf: Add parameter to virDomainDiskSourceFormat backup: Parse and output backup XML backup: Implement virsh support for checkpoints backup: Implement virsh support for backup backup: Prepare for Unix sockets in QMP nbd-server-start backup: Add new qemu monitor interactions backup: qemu: Implement metadata tracking for checkpoint APIs backup: Wire up qemu checkpoint commands over QMP backup: qemu: Implement framework for backup job APIs backup: Wire up qemu full pull backup commands over QMP backup: qemu: Wire up qemu full push backup commands over QMP backup: implement qemu incremental pull backup include/libvirt/libvirt-domain-checkpoint.h | 161 ++ include/libvirt/libvirt-domain-snapshot.h | 8 +- include/libvirt/libvirt-domain.h | 47 +- include/libvirt/libvirt.h | 5 +- src/access/viraccessperm.h | 6 + src/conf/backup_conf.h | 97 + src/conf/checkpoint_conf.h | 103 ++ src/conf/domain_conf.h | 3 + src/conf/virconftypes.h | 12 + src/conf/virdomaincheckpointobjlist.h | 74 + src/conf/virdomainmomentobjlist.h | 7 +- src/conf/virdomainobjlist.h | 7 +- src/driver-hypervisor.h | 79 + src/qemu/qemu_block.h | 3 + src/qemu/qemu_blockjob.h | 1 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_conf.h | 2 + src/qemu/qemu_domain.h | 19 + src/qemu/qemu_monitor.h | 27 +- src/qemu/qemu_monitor_json.h | 22 +- tools/virsh-checkpoint.h | 29 + tools/virsh-completer.h | 4 + tools/virsh-util.h | 3 + tools/virsh.h | 1 + docs/Makefile.am | 3 + docs/apibuild.py | 2 + docs/docs.html.in | 10 +- docs/domainstatecapture.html.in | 315 ++++ docs/format.html.in | 2 + docs/formatbackup.html.in | 184 ++ docs/formatcheckpoint.html.in | 204 +++ docs/formatsnapshot.html.in | 2 + docs/index.html.in | 4 +- docs/schemas/domainbackup.rng | 219 +++ docs/schemas/domaincheckpoint.rng | 87 + examples/object-events/event-test.c | 3 + libvirt.spec.in | 3 + mingw-libvirt.spec.in | 6 + po/POTFILES | 3 + src/Makefile.am | 2 + src/access/viraccessperm.c | 3 +- src/conf/Makefile.inc.am | 6 + src/conf/backup_conf.c | 538 ++++++ src/conf/checkpoint_conf.c | 636 +++++++ src/conf/domain_conf.c | 22 +- src/conf/snapshot_conf.c | 3 +- src/conf/virdomaincheckpointobjlist.c | 223 +++ src/conf/virdomainmomentobjlist.c | 40 +- src/conf/virdomainobjlist.c | 11 + src/conf/virdomainsnapshotobjlist.c | 2 +- src/libvirt-domain-checkpoint.c | 750 ++++++++ src/libvirt-domain-snapshot.c | 89 + src/libvirt-domain.c | 237 ++- src/libvirt_private.syms | 32 + src/libvirt_public.syms | 24 + src/qemu/qemu_block.c | 12 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_conf.c | 5 + src/qemu/qemu_domain.c | 221 ++- src/qemu/qemu_driver.c | 1577 +++++++++++++++++ src/qemu/qemu_migration.c | 9 +- src/qemu/qemu_monitor.c | 72 +- src/qemu/qemu_monitor_json.c | 222 ++- src/qemu/qemu_process.c | 9 + src/remote/remote_daemon_dispatch.c | 20 + src/remote/remote_driver.c | 32 +- src/remote/remote_protocol.x | 258 ++- src/remote_protocol-structs | 139 ++ src/rpc/gendispatch.pl | 32 +- tests/Makefile.am | 13 +- tests/domainbackupxml2xmlin/backup-pull.xml | 9 + tests/domainbackupxml2xmlin/backup-push.xml | 9 + tests/domainbackupxml2xmlin/empty.xml | 1 + tests/domainbackupxml2xmlout/backup-pull.xml | 9 + tests/domainbackupxml2xmlout/backup-push.xml | 9 + tests/domainbackupxml2xmlout/empty.xml | 7 + tests/domaincheckpointxml2xmlin/empty.xml | 1 + tests/domaincheckpointxml2xmlin/sample.xml | 7 + tests/domaincheckpointxml2xmlin/size.xml | 4 + tests/domaincheckpointxml2xmlout/empty.xml | 7 + .../internal-active-invalid.xml | 53 + .../internal-inactive-invalid.xml | 53 + tests/domaincheckpointxml2xmlout/redefine.xml | 63 + tests/domaincheckpointxml2xmlout/sample.xml | 12 + tests/domaincheckpointxml2xmlout/size.xml | 11 + tests/domaincheckpointxml2xmltest.c | 223 +++ tests/qemublocktest.c | 3 +- .../caps_4.0.0.riscv32.xml | 2 + .../caps_4.0.0.riscv64.xml | 2 + .../caps_4.0.0.x86_64.xml | 2 + tests/qemumonitorjsontest.c | 32 +- tests/virschematest.c | 4 + tests/virstoragetest.c | 2 +- tools/Makefile.am | 1 + tools/virsh-checkpoint.c | 1370 ++++++++++++++ tools/virsh-completer.c | 51 + tools/virsh-domain-monitor.c | 23 + tools/virsh-domain.c | 268 ++- tools/virsh-snapshot.c | 37 +- tools/virsh-util.c | 11 + tools/virsh.c | 2 + tools/virsh.pod | 302 +++- 102 files changed, 9502 insertions(+), 100 deletions(-) create mode 100644 include/libvirt/libvirt-domain-checkpoint.h create mode 100644 src/conf/backup_conf.h create mode 100644 src/conf/checkpoint_conf.h create mode 100644 src/conf/virdomaincheckpointobjlist.h create mode 100644 tools/virsh-checkpoint.h create mode 100644 docs/domainstatecapture.html.in create mode 100644 docs/formatbackup.html.in create mode 100644 docs/formatcheckpoint.html.in create mode 100644 docs/schemas/domainbackup.rng create mode 100644 docs/schemas/domaincheckpoint.rng create mode 100644 src/conf/backup_conf.c create mode 100644 src/conf/checkpoint_conf.c create mode 100644 src/conf/virdomaincheckpointobjlist.c create mode 100644 src/libvirt-domain-checkpoint.c create mode 100644 tests/domainbackupxml2xmlin/backup-pull.xml create mode 100644 tests/domainbackupxml2xmlin/backup-push.xml create mode 100644 tests/domainbackupxml2xmlin/empty.xml create mode 100644 tests/domainbackupxml2xmlout/backup-pull.xml create mode 100644 tests/domainbackupxml2xmlout/backup-push.xml create mode 100644 tests/domainbackupxml2xmlout/empty.xml create mode 100644 tests/domaincheckpointxml2xmlin/empty.xml create mode 100644 tests/domaincheckpointxml2xmlin/sample.xml create mode 100644 tests/domaincheckpointxml2xmlin/size.xml create mode 100644 tests/domaincheckpointxml2xmlout/empty.xml create mode 100644 tests/domaincheckpointxml2xmlout/internal-active-invalid.xml create mode 100644 tests/domaincheckpointxml2xmlout/internal-inactive-invalid.xml create mode 100644 tests/domaincheckpointxml2xmlout/redefine.xml create mode 100644 tests/domaincheckpointxml2xmlout/sample.xml create mode 100644 tests/domaincheckpointxml2xmlout/size.xml create mode 100644 tests/domaincheckpointxml2xmltest.c create mode 100644 tools/virsh-checkpoint.c -- 2.20.1

2 46

[libvirt] [PATCHv3 0/5] Implement debugcon chardev
by Ján Tomko 19 Jun '19

19 Jun '19

v2: https://www.redhat.com/archives/libvir-list/2019-February/msg00293.html v3: * dropped the pointless isa-prefix * use DO_TEST_CAPS_LATEST * trimmed the XML * compiles with gcc * only make irq optional * autofill iobase Ján Tomko (3): qemu: introduce qemuDomainChrSerialTargetModel qemu: make irq optional when formatting the ISA address qemu: autoadd iobase to debugcon chardev Nikolay Shirokovskiy (2): conf: add debugcon chardev guest interface qemu: implement debugcon chardev docs/formatdomain.html.in | 3 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 5 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 25 +++++++++++++--- src/qemu/qemu_domain.c | 12 ++++++++ .../isa-serial-debugcon.x86_64-latest.args | 30 +++++++++++++++++++ .../qemuxml2argvdata/isa-serial-debugcon.xml | 21 +++++++++++++ tests/qemuxml2argvtest.c | 1 + .../isa-serial-debugcon.xml | 30 +++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 11 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/isa-serial-debugcon.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/isa-serial-debugcon.xml create mode 100644 tests/qemuxml2xmloutdata/isa-serial-debugcon.xml -- 2.19.2

2 14

Re: [libvirt] [PATCH] storage: escape ipv6 for ceph mon hosts to librados
by Yi Li 18 Jun '19

18 Jun '19

> >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses, > >so they are often referenced by IP rather than hostname for > >convenience, or to avoid relying on DNS. Using IPv4 addresses as the > >host name works already, but IPv6 addresses require rbd-specific > > If you include the escaping in the XML, does it currently work? > <host name='[2205::192:168:205:141]' port='6789'/> Yes, It works. > > > >escaping because the colon is used as an option separator in the > >string passed to librados. > > > >Escape these colons, and enclose the IPv6 address in square brackets > >so it is distinguished from the port, which is currently mandatory. > > > >Signed-off-by: Yi Li <yili(a)winhong.com> > >--- > > docs/schemas/storagepool.rng | 5 ++++- > > src/storage/storage_backend_rbd.c | 13 ++++++++++--- > > tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml | 13 +++++++++++++ > > tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 ++++++++++++++++ > > tests/storagepoolxml2xmltest.c | 1 + > > 5 files changed, 44 insertions(+), 4 deletions(-) > > create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml > > create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml > > > >diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c > >index f8c968e..3056563 100644 > >--- a/src/storage/storage_backend_rbd.c > >+++ b/src/storage/storage_backend_rbd.c > >@@ -268,9 +268,16 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, > > source->hosts[i].name); > > } else if (source->hosts[i].name != NULL && > > source->hosts[i].port) { > >- virBufferAsprintf(&mon_host, "%s:%d,", > >- source->hosts[i].name, > >- source->hosts[i].port); > >+ /* assume host containing : is ipv6 */ > >+ if (strchr(source->hosts[i].name, ':')) { > > if (virSocketAddrNumericFamily(listenAddress) == AF_INET6) > > By using this helper function, we won't try to escape an address that is > already escaped. > > Also, instead of copying the whole virBuffer call twice, it would be > nicer to assign the format to a temporary variable like we do in qemuMigrationDstPrepare > > Jano Good point. I'd sending a v2. > > > >+ virBufferAsprintf(&mon_host, "[%s]:%d,", > >+ source->hosts[i].name, > >+ source->hosts[i].port); > >+ } else { > >+ virBufferAsprintf(&mon_host, "%s:%d,", > >+ source->hosts[i].name, > >+ source->hosts[i].port); > >+ } > > } else { > > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > _("received malformed monitor, check the XML definition")); On Sun, Apr 28, 2019 at 1:10 PM winhong-yili <yili(a)winhong.com> wrote: > > >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses, > >so they are often referenced by IP rather than hostname for > >convenience, or to avoid relying on DNS. Using IPv4 addresses as the > >host name works already, but IPv6 addresses require rbd-specific > > If you include the escaping in the XML, does it currently work? > <host name='[2205::192:168:205:141]' port='6789'/> > > >escaping because the colon is used as an option separator in the > >string passed to librados. > > > >Escape these colons, and enclose the IPv6 address in square brackets > >so it is distinguished from the port, which is currently mandatory. > > > >Signed-off-by: Yi Li <yili(a)winhong.com> > >--- > > docs/schemas/storagepool.rng | 5 ++++- > > src/storage/storage_backend_rbd.c | 13 ++++++++++--- > > tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml | 13 +++++++++++++ > > tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 ++++++++++++++++ > > tests/storagepoolxml2xmltest.c | 1 + > > 5 files changed, 44 insertions(+), 4 deletions(-) > > create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml > > create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml > > > >diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c > >index f8c968e..3056563 100644 > >--- a/src/storage/storage_backend_rbd.c > >+++ b/src/storage/storage_backend_rbd.c > >@@ -268,9 +268,16 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, > > source->hosts[i].name); > > } else if (source->hosts[i].name != NULL && > > source->hosts[i].port) { > >- virBufferAsprintf(&mon_host, "%s:%d,", > >- source->hosts[i].name, > >- source->hosts[i].port); > >+ /* assume host containing : is ipv6 */ > >+ if (strchr(source->hosts[i].name, ':')) { > > if (virSocketAddrNumericFamily(listenAddress) == AF_INET6) > > By using this helper function, we won't try to escape an address that is > already escaped. > > Also, instead of copying the whole virBuffer call twice, it would be > nicer to assign the format to a temporary variable like we do in qemuMigrationDstPrepare > > Jano > > >+ virBufferAsprintf(&mon_host, "[%s]:%d,", > >+ source->hosts[i].name, > >+ source->hosts[i].port); > >+ } else { > >+ virBufferAsprintf(&mon_host, "%s:%d,", > >+ source->hosts[i].name, > >+ source->hosts[i].port); > >+ } > > } else { > > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > _("received malformed monitor, check the XML definition"));

2 2

[libvirt] [PATCH v3 00/15] implement cgroups v2 support
by Pavel Hrdina 18 Jun '19

18 Jun '19

In cgroups v2 there is no devices controller, eBPF should be used instead. Changes in v3: - removed workaround for kernel bug [1] - added documentation how to get the eBPF program Changes in v2: - fixed build on bsd and older kernels without cgroup BPF - cgroup bpf devices code moved to separate file Documentation for eBPF: <http://man7.org/linux/man-pages/man2/bpf.2.html> <https://www.kernel.org/doc/Documentation/networking/filter.txt> <https://docs.cilium.io/en/v1.3/bpf/> [1] <https://bugzilla.redhat.com/show_bug.cgi?id=1656432> Pavel Hrdina (15): util: introduce virbpf helpers vircgroup: introduce virCgroupV2DevicesAvailable vircgroup: introduce virCgroupV2DevicesAttachProg vircgroup: introduce virCgroupV2DevicesDetectProg vircgroup: introduce virCgroupV2DevicesCreateProg vircgroup: introduce virCgroupV2DevicesPrepareProg vircgroup: introduce virCgroupV2DevicesRemoveProg vircgroup: introduce virCgroupV2DeviceGetPerms vircgroup: introduce virCgroupV2DevicesGetKey vircgroup: introduce virCgroupV2AllowDevice vircgroup: introduce virCgroupV2DenyDevice vircgroup: introduce virCgroupV2AllowAllDevices vircgroup: introduce virCgroupV2DenyAllDevices vircgroup: workaround devices in hybrid mode vircgroupmock: mock virCgroupV2DevicesAvailable configure.ac | 6 + include/libvirt/virterror.h | 2 + src/Makefile.am | 2 + src/libvirt_private.syms | 26 ++ src/util/Makefile.inc.am | 4 + src/util/virbpf.c | 438 +++++++++++++++++++ src/util/virbpf.h | 259 ++++++++++++ src/util/vircgroup.c | 3 +- src/util/vircgroupbackend.h | 3 +- src/util/vircgrouppriv.h | 10 + src/util/vircgroupv1.c | 9 +- src/util/vircgroupv2.c | 117 +++++- src/util/vircgroupv2devices.c | 670 ++++++++++++++++++++++++++++++ src/util/vircgroupv2devices.h | 57 +++ src/util/virerror.c | 2 + tests/vircgroupdata/hybrid.parsed | 2 +- tests/vircgroupmock.c | 7 + tests/vircgrouptest.c | 4 +- 18 files changed, 1613 insertions(+), 8 deletions(-) create mode 100644 src/util/virbpf.c create mode 100644 src/util/virbpf.h create mode 100644 src/util/vircgroupv2devices.c create mode 100644 src/util/vircgroupv2devices.h -- 2.20.1

2 33

[libvirt] QMP; unsigned 64-bit ints; JSON standards compliance
by Daniel P. Berrangé 05 Jun '19

05 Jun '19

The QEMU QMP service is based on JSON which is nice because that is a widely supported "standard" data format..... ....except QEMU's implementation (and indeed most impls) are not strictly standards compliant. Specifically the problem is around representing 64-bit integers, whether signed or unsigned. The JSON standard declares that largest integer is 2^53-1 and the likewise the smallest is -(2^53-1): http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_sa… A crazy limit inherited from its javascript origins IIUC. QEMU, and indeed many applications, want to handle 64-bit integers. The C JSON library impls have traditionally mapped integers to the data type 'long long int' which gives a min/max of -(2^63) / 2^63-1. QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1. Libvirt has historically used the YAJL library which uses 'long long int' and thus can't officially go beyond 2^63-1 values. Fortunately it lets libvirt get at the raw json string, so libvirt can re-parse the value to get an 'unsigned long long'. We recently tried to switch to Jansson because YAJL has a dead upstream for many years and countless unanswered bugs & patches. Unfortunately we forgot about this need for 2^64-1 max, and Jansson also uses 'long long int' and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It also provides no backdoor for libvirt todo its own integer parsing. Thus we had to abort our switch to jansson as it broke parsing QEMU's JSON: https://bugzilla.redhat.com/show_bug.cgi?id=1614569 Other JSON libraries we've investigated have similar problems. I imagine the same may well be true of non-C based JOSN impls, though I've not investigated in any detail. Essentially libvirt is stuck with either using the dead YAJL library forever, or writing its own JSON parser (most likely copying QEMU's JSON code into libvirt's git). This feels like a very unappealing situation to be in as not being able to use a JSON library of our choice is loosing one of the key benefits of using a standard data format. Thus I'd like to see a solution to this to allow QMP to be reliably consumed by any JSON library that exists. I can think of some options: 1. Encode unsigned 64-bit integers as signed 64-bit integers. This follows the example that most C libraries map JSON ints to 'long long int'. This is still relying on undefined behaviour as apps don't need to support > 2^53-1. Apps would need to cast back to 'unsigned long long' for those QMP fields they know are supposed to be unsigned. 2. Encode all 64-bit integers as a pair of 32-bit integers. This is fully compliant with the JSON spec as each half is fully within the declared limits. App has to split or assemble the 2 pieces from/to a signed/unsigned 64-bit int as needed. 3. Encode all 64-bit integers as strings The application has todo all parsing/formatting client side. None of these changes are backwards compatible, so I doubt we could make the change transparently in QMP. Instead we would have to have a QMP greeting message capability where the client can request enablement of the enhanced integer handling. Any of the three options above would likely work for libvirt, but I would have a slight preference for either 2 or 3, so that we become 100% standards compliant. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

5 22

[libvirt] [PATCH v2 for 5.3.0] storage_backend_rbd: Ignore rbd_diff_iterate error() if needed
by Michal Privoznik 21 May '19

21 May '19

When cloning an RBD volume we try to find a snapshot which is not different to the image we're trying to clone. This boils down to calling rbd_diff_iterate() or rbd_diff_iterate2() on systems with newer ceph. These two are passed a callback - virStorageBackendRBDIterateCb() which sets @diff to 1 and returns -1. The idea is to stop iterating as soon as possible (i.e. we're iterating over a snapshot with some deltas). Unfortunately, returning a negative value from the callback means that iterate() function fails and thus we report an error: virsh # vol-clone --pool rbd_image_root coreos_2023 coreos00.disk error: Failed to clone vol from coreos_2023 error: failed to iterate RBD snapshot coreos_2023@base: Operation not permitted Therefore, report an error if and only if it doesn't originates in the callback. Reported on libvirt-users list: https://www.redhat.com/archives/libvirt-users/2019-April/msg00060.html Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- diff to v1: - Ignore error as suggested by Jason as it allows us to exit iteration early. src/storage/storage_backend_rbd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index f8c968e682..69f4fa2117 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1101,7 +1101,9 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, virStorageBackendRBDIterateCb, (void *)&diff); #endif - if (r < 0) { + /* We care for errors only if the callback wasn't called at all. If it + * was, rbd_diff_iterate() returns an error which we have to ignore. */ + if (r < 0 && diff == 0) { virReportSystemError(-r, _("failed to iterate RBD snapshot %s@%s"), imgname, snaps[i].name); goto cleanup; -- 2.21.0

2 1

[libvirt] [PATCH v1] qemu: migration: Convert to virErrorRestore/virErrorPreserveLast
by Syed Humaid 20 May '19

20 May '19

Replaced usages of virSaveLastError and virSetError/virFreeError with virErrorPreserveLast and virErrorRestore respectively. Signed-off-by: Syed Humaid <syedhumaidbinharoon(a)gmail.com> --- src/qemu/qemu_migration.c | 75 +++++++++++++-------------------------- 1 file changed, 24 insertions(+), 51 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e3ad4e52a7..b1fe79aed9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -722,7 +722,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, if (rv != 0) { if (rv < 0) { if (!err) - err = virSaveLastError(); + virErrorPreserveLast(&err); failed = true; } qemuBlockJobSyncEnd(vm, job, asyncJob); @@ -747,7 +747,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, } if (failed && !err) - err = virSaveLastError(); + virErrorPreserveLast(&err); if (virDomainObjWait(vm) < 0) goto cleanup; @@ -769,10 +769,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, ret = failed ? -1 : 0; cleanup: - if (err) { - virSetError(err); - virFreeError(err); - } + virErrorRestore(&err); return ret; } @@ -3189,16 +3186,13 @@ static void qemuMigrationSrcIOFunc(void *arg) return; abrt: - err = virSaveLastError(); + virErrorPreserveLast(&err); if (err && err->code == VIR_ERR_OK) { virFreeError(err); err = NULL; } virStreamAbort(data->st); - if (err) { - virSetError(err); - virFreeError(err); - } + virErrorRestore(&err); error: /* Let the source qemu know that the transfer cant continue anymore. @@ -3685,15 +3679,12 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, if (events) priv->signalIOError = false; - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); return ret; error: - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); if (virDomainObjIsActive(vm)) { if (cancel && @@ -3948,7 +3939,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare2 did not set uri")); cancelled = true; - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } @@ -3971,7 +3962,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver, /* Perform failed. Make sure Finish doesn't overwrite the error */ if (ret < 0) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* If Perform returns < 0, then we need to cancel the VM * startup on the destination @@ -4004,10 +3995,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver, virObjectUnref(st); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); VIR_FREE(uri_out); VIR_FREE(cookie); @@ -4181,13 +4169,13 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, if (useParams && virTypedParamsReplaceString(&params, &nparams, VIR_MIGRATE_PARAM_URI, uri_out) < 0) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } } else if (!uri && !(flags & VIR_MIGRATE_TUNNELLED)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare3 did not set uri")); - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } @@ -4220,7 +4208,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, /* Perform failed. Make sure Finish doesn't overwrite the error */ if (ret < 0) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); } else { qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE); @@ -4312,7 +4300,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, * one we need to preserve it in case confirm3 overwrites */ if (!orig_err) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* * If cancelled, then src VM will be restarted, else @@ -4344,10 +4332,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, virObjectUnref(st); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); VIR_FREE(uri_out); VIR_FREE(cookiein); VIR_FREE(cookieout); @@ -4523,15 +4508,12 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriverPtr driver, } cleanup: - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); qemuDomainObjEnterRemote(vm); virConnectUnregisterCloseCallback(dconn, qemuMigrationSrcConnectionClosed); virObjectUnref(dconn); ignore_value(qemuDomainObjExitRemote(vm, false)); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virObjectUnref(cfg); return ret; } @@ -4619,7 +4601,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, endjob: if (ret < 0) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* v2 proto has no confirm phase so we need to reset migration parameters * here @@ -4639,10 +4621,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, qemuDomainRemoveInactiveJob(driver, vm); } - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); cleanup: virObjectEventStateQueue(driver->domainEventState, event); @@ -5052,7 +5031,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, /* Need to save the current error, in case shutting * down the process overwrites it */ - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* * In v3 protocol, the source VM is still available to @@ -5181,10 +5160,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, VIR_FREE(priv->origname); virDomainObjEndAPI(&vm); qemuMigrationCookieFree(mig); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virObjectUnref(cfg); /* Set a special error if Finish is expected to return NULL as a result of @@ -5289,7 +5265,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, if (rc < 0) { if (rc == -2) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); virCommandAbort(cmd); if (virDomainObjIsActive(vm) && qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { @@ -5308,7 +5284,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, cleanup: if (ret < 0 && !orig_err) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* Restore max migration bandwidth */ if (virDomainObjIsActive(vm) && @@ -5326,10 +5302,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, virCommandFree(cmd); } - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); return ret; } -- 2.20.1

2 1