Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support
Hi Cole, I merged crobinso/qcow2-data_file branch to 37b565c00. Reserved new capabilities introduced by these to branches to resolve conflicts. Then build and test as following: # ./autogen.sh&& ./configure --without-libssh --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-qemu --without-openvz --without-lxc --without-vbox --without-libxl --with-sasl --with-polkit --with-libvirtd --without-phyp --with-esx --without-hyperv --without-vmware --without-xenapi --without-vz --without-bhyve --with-interface --with-network --with-storage-fs --with-storage-lvm --with-storage-iscsi --with-storage-iscsi-direct --with-storage-scsi --with-storage-disk --with-storage-mpath --with-storage-rbd --without-storage-sheepdog --with-storage-gluster --without-storage-zfs --without-storage-vstorage --with-numactl --with-numad --with-capng --without-fuse --with-netcf --with-selinux --with-selinux-mount=/sys/fs/selinux --without-apparmor --without-hal --with-udev --with-yajl --with-sanlock --with-libpcap --with-macvtap --with-audit --with-dtrace --with-driver-modules --with-firewalld --with-firewalld-zone --without-wireshark-dissector --without-pm-utils --with-nss-plugin '--with-packager=Unknown, 2019-08-19-12:13:01, lab.rhel8.me' --with-packager-version=1.el8 --with-qemu-user=qemu --with-qemu-group=qemu --with-tls-priority=@LIBVIRT,SYSTEM --enable-werror --enable-expensive-tests --with-init-script=systemd --without-login-shell && make Start libvirtd and virtlogd # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd # LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" ./src/virtlogd Then try to list all domains: # virsh list --all Libvirtd exits with segment fault: [1]30104 segmentation fault (core dumped) LD_PRELOAD="$(find src -name '*.so.*'|tr '\n' ' ')" src/.libs/libvirtd Version: qemu-4.1 Backtrace: (gdb) bt #0 0x7fbe57a0d1b9 in virDomainVirtioSerialAddrSetAddControllers (def=, def=, addrs=) at conf/domain_addr.c:1656 #1 virDomainVirtioSerialAddrSetCreateFromDomain (def=def@entry=0x7fbde81cc3f0) at conf/domain_addr.c:1753 #2 0x7fbe0179897e in qemuDomainAssignVirtioSerialAddresses (def=0x7fbde81cc3f0) at qemu/qemu_domain_address.c:3174 #3 qemuDomainAssignAddresses (def=0x7fbde81cc3f0, qemuCaps=0x7fbde81d2210, driver=0x7fbde8126850, obj=0x0, newDomain=) at qemu/qemu_domain_address.c:3174 #4 0x7fbe57a39e0d in virDomainDefPostParse (def=def@entry=0x7fbde81cc3f0, caps=caps@entry=0x7fbde8154d20, parseFlags=parseFlags@entry=4610, xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0) at conf/domain_conf.c:5858 #5 0x7fbe57a525c5 in virDomainDefParseNode (xml=, root=0x7fbde83c5ff0, caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070, parseOpaque=0x0, flags=4610) at conf/domain_conf.c:21677 #6 0x7fbe57a526c8 in virDomainDefParse (xmlStr=xmlStr@entry=0x0, filename=, caps=caps@entry=0x7fbde8154d20, xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=4610) at conf/domain_conf.c:21628 #7 0x7fbe57a528f6 in virDomainDefParseFile (filename=, caps=caps@entry=0x7fbde8154d20, xmlopt=xmlopt@entry=0x7fbde83ce070, parseOpaque=parseOpaque@entry=0x0, flags=flags@entry=4610) at conf/domain_conf.c:21653 #8 0x7fbe57a5e16a in virDomainObjListLoadConfig (opaque=0x0, notify=0x0, name=0x7fbde81d7ff3 "pc", autostartDir=0x7fbde8124070 "/etc/libvirt/qemu/autostart", configDir=0x7fbde8124050 "/etc/libvirt/qemu", xmlopt=0x7fbde83ce070, caps=0x7fbde8154d20, doms=0x7fbde8126940) at conf/virdomainobjlist.c:503 #9 virDomainObjListLoadAllConfigs (doms=0x7fbde8126940, configDir=0x7fbde8124050 "/etc/libvirt/qemu", autostartDir=0x7fbde8124070 "/etc/libvirt/qemu/autostart", liveStatus=liveStatus@entry=false, caps=0x7fbde8154d20, xmlopt=0x7fbde83ce070, notify=0x0, opaque=0x0) at conf/virdomainobjlist.c:625 #10 0x7fbe017f57e2 in qemuStateInitialize (privileged=true, callback=, opaque=) at qemu/qemu_driver.c:1007 #11 0x7fbe57b8033d in virStateInitialize (privileged=true, mandatory=mandatory@entry=false, callback=callback@entry=0x55dfb702ecc0 , opaque=opaque@entry=0x55dfb8869d60) at libvirt.c:666 #12 0x55dfb702ed1d in daemonRunStateInit (opaque=0x55dfb8869d60) at remote/remote_daemon.c:846 #13 0x7fbe579f4be2 in virThreadHelper (data=) at util/virthread.c:196 #14 0x7fbe55a322de in start_thread (arg=) at pthread_create.c:486 #15 0x7fbe55763133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 Could you please check this issue? The full threads backtrace is in attachment On Tue, Oct 8, 2019 at 5:49 AM Cole Robinson wrote: > This series is the first steps to
Re: [libvirt] [PATCH v2 2/3] tests: libxl: ACPI slic table test
On 9/16/19 6:47 AM, Marek Marczykowski-Górecki wrote:
> On Mon, Sep 16, 2019 at 12:23:35PM +, Jim Fehlig wrote:
>> On 9/15/19 1:43 PM, Marek Marczykowski-Górecki wrote:
>>> Signed-off-by: Marek Marczykowski-Górecki
>>> ---
>>>tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json | 54 +-
>>>tests/libxlxml2domconfigdata/fullvirt-acpi-slic.xml | 32 -
>>>tests/libxlxml2domconfigtest.c | 2 +-
>>>3 files changed, 88 insertions(+)
>>>create mode 100644 tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json
>>>create mode 100644 tests/libxlxml2domconfigdata/fullvirt-acpi-slic.xml
>>>
>>> diff --git a/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json
>>> b/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json
>>> new file mode 100644
>>> index 000..5d85d75
>>> --- /dev/null
>>> +++ b/tests/libxlxml2domconfigdata/fullvirt-acpi-slic.json
>>> @@ -0,0 +1,54 @@
>>> +{
>>> +"c_info": {
>>> +"type": "hvm",
>>> +"name": "XenGuest2",
>>> +"uuid": "c7a5fdb2-cdaf-9455-926a-d65c16db1809"
>>> +},
>>> +"b_info": {
>>> +"max_vcpus": 1,
>>> +"avail_vcpus": [
>>> +0
>>> +],
>>> +"max_memkb": 592896,
>>> +"target_memkb": 403456,
>>> +"shadow_memkb": 5656,
>>> +"sched_params": {
>>> +},
>>> +"nested_hvm": "False",
>>
>> I had to remove the above line otherwise 'make check' fails. Did it work for
>> you
>> as is?
>
> Yes, it works for me. But what's interesting, if I remove it, it works
> too. Other modifications do cause the test to fail, so the test was
> called. Maybe it's about Xen libs version? 4.8 here.
I think so. Starting with Xen 4.10 'nested_hvm' is not included in json config
if it is set to the default value (which is consistent with other settings).
I've removed it in my local branch and will push the series soon.
BTW, the mentioning of versions got me to thinking that we should bump the
minimum supported Xen version to 4.9. According to the support matrix for older
xen versions [0], 4.8 general support ended June 2018 and security support ends
in Dec 2019 (soon). 4.9 general support ends in Jan 2020 and security support
extends until July 2020. What is your opinion on bumping the minimum version to
4.9?
Regards,
Jim
[0] https://wiki.xenproject.org/wiki/Xen_Project_Release_Features
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
On 9/16/19 5:12 AM, Michal Privoznik wrote: See 5/5 for explanation. Michal Prívozník (5): security: Pass @migrated to virSecurityManagerSetAllLabel security: Rename virSecurityManagerGetDriver() to virSecurityManagerGetVirtDriver() security: Introduce virSecurityManagerGetDriver() security_stack: Turn list of nested drivers into a doubly linked list security_stack: Perform rollback if one of stacked drivers fails src/lxc/lxc_process.c| 2 +- src/qemu/qemu_process.c | 3 +- src/qemu/qemu_security.c | 6 +- src/qemu/qemu_security.h | 3 +- src/security/security_apparmor.c | 3 +- src/security/security_dac.c | 3 +- src/security/security_driver.h | 3 +- src/security/security_manager.c | 17 ++- src/security/security_manager.h | 4 +- src/security/security_nop.c | 3 +- src/security/security_selinux.c | 9 +- src/security/security_stack.c| 220 +-- tests/qemusecuritytest.c | 2 +- tests/securityselinuxlabeltest.c | 2 +- 14 files changed, 222 insertions(+), 58 deletions(-) For the series: Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] security: Don't remember labels for TPM
On 10/1/19 11:00 AM, Michal Privoznik wrote: As it turns out, /dev/tpm0 can't be opened more than once. This doesn't fit into our seclabel remembering approach and thus disable it for TPM devices. There's also another type of files which can't be opened more than once - /dev/vfio/N. Usually, this won't be a problem unless users try to attach/detach some devices from the same IOMMU group. This will require more treatment though because it's broken on more levels. 1) we remove /dev/vfio/N in private devtmpfs on device detach, even though there is another device still attached to domain from the same IOMMU group, 2) we remove the IOMMU group from CGroups, i.e. we effectively deny access to qemu 3) we restore seclabels (regardless of seclabel remembering) Therefore, I'm only addressing TPM issue here and will continue work on hostdevs. Michal Prívozník (3): security: Try to lock only paths with remember == true security_dac: Allow selective remember/recall for chardevs security: Don't remember labels for TPM src/security/security_dac.c | 91 ++--- src/security/security_selinux.c | 16 +++--- 2 files changed, 71 insertions(+), 36 deletions(-) Reviewed-by: Cole Robinson but see comment on #3, I think the EMULATOR bits can be dropped. I verified this fixes TPM passthrough VM startup too - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] security: Don't remember labels for TPM
On 10/1/19 11:00 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1755803
The /dev/tpmN file can be opened only once, as implemented in
drivers/char/tpm/tpm-dev.c:tpm_open() from the kernel's tree. Any
other attempt to open the file fails. And since we're opening the
file ourselves and passing the FD to qemu we will not succeed
opening the file again when locking it for seclabel remembering.
Signed-off-by: Michal Privoznik
---
src/security/security_dac.c | 18 +-
src/security/security_selinux.c | 10 +-
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 2733fa664f..347a7a5f63 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1653,14 +1653,14 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-ret = virSecurityDACSetChardevLabel(mgr, def,
->data.passthrough.source,
-false);
+ret = virSecurityDACSetChardevLabelHelper(mgr, def,
+
>data.passthrough.source,
+ false, false);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
-ret = virSecurityDACSetChardevLabel(mgr, def,
->data.emulator.source,
-false);
+ret = virSecurityDACSetChardevLabelHelper(mgr, def,
+ >data.emulator.source,
+ false, false);
break;
case VIR_DOMAIN_TPM_TYPE_LAST:
break;
@@ -1679,9 +1679,9 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr
mgr,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
-ret = virSecurityDACRestoreChardevLabel(mgr, def,
->data.passthrough.source,
-false);
+ret = virSecurityDACRestoreChardevLabelHelper(mgr, def,
+
>data.passthrough.source,
+ false, false);
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
/* swtpm will have removed the Unix socket upon termination */
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e3be724a2b..0486bdd6b6 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1682,14 +1682,14 @@ virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr
mgr,
switch (tpm->type) {
case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
tpmdev = tpm->data.passthrough.source.data.file.path;
-rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel,
true);
+rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel,
false);
if (rc < 0)
return -1;
if ((cancel_path = virTPMCreateCancelPath(tpmdev)) != NULL) {
rc = virSecuritySELinuxSetFilecon(mgr,
cancel_path,
- seclabel->imagelabel, true);
+ seclabel->imagelabel, false);
VIR_FREE(cancel_path);
if (rc < 0) {
virSecuritySELinuxRestoreTPMFileLabelInt(mgr, def, tpm);
@@ -1701,7 +1701,7 @@ virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr
mgr,
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
tpmdev = tpm->data.emulator.source.data.nix.path;
-rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel,
true);
+rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel,
false);
if (rc < 0)
return -1;
break;
Are the EMULATOR changes actually required? I think it just uses a unix
socket and doesn't touch the host kernel tpm subsystem
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On 10/10/19 7:54 AM, Peter Krempa wrote: > On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote: >> On 10/10/19 1:26 PM, Peter Krempa wrote: >>> On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote: On 10/10/19 12:43 AM, John Snow wrote: > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > I'd like to refactor these some day, and getting rid of the super-object > will make that easier. > > Either way, we don't need this. > > Libvirt-checked-by: Peter Krempa Peter made a comment regarding Laszlo's Regression-tested-by tag: [...] nobody else is using this convention (there are exactly 0 instances of "Regression-tested-by" in the project git log as far as I can see), and so in practice people reading the commits won't really know what you meant by it. Everybody else on the project uses "Tested-by" to mean either of the two cases you describe above, without distinction... It probably applies to 'Libvirt-checked-by' too. >>> >>> I certainly didn't test it. So feel free to drop that line altogether. >> >> But you reviewed it, can we use your 'Reviewed-by' instead? > > To be honest, I didn't really review the code nor the documentation. > I actually reviewed only the idea itself in the context of integration > with libvirt and that's why I didn't go for 'Reviewed-by:'. > > The gist of the citation above is that we should stick to well known > tags with their well known meanings and I think that considering this a > 'review' would be a stretch of the definiton. > I wasn't aware that PMM wanted to avoid non-standard tags; I consider them to be for human use, but I can change that behavior. Peter, I'll change it to an ACK (as suggested by Kevin) is that's OK by you. --js -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] src/driver.c: remove duplicated code in virGetConnect* functions
On 10/8/19 5:02 PM, Daniel Henrique Barboza wrote: All the 6 virGetConnect* functions in driver.c shares the same code base. This patch creates a new static function virGetConnectGeneric() that contains the common code to be used with all other virGetConnect*. Signed-off-by: Daniel Henrique Barboza --- src/driver.c | 99 +--- 1 file changed, 24 insertions(+), 75 deletions(-) Reviewed-by: Cole Robinson And pushed - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/4] conf: Add XML resolution support for video models
On 10/7/19 11:35 PM, jcfara...@gmail.com wrote:
From: Julio Faracco
XML need to support both properties together. This commit adds XML
support for video model if they are set. Domain configuration is able
to parse this properties as 'x' and 'y'. Code is not using same label as
QEMU: 'xres' and 'yres'.
Compared to v2, the commit message here dropped the XML example, which I
find useful; can make it easier to grep git logs in some instances
Signed-off-by: Julio Faracco
---
src/conf/domain_conf.c | 74 -
src/conf/domain_conf.h | 5 +++
src/conf/virconftypes.h | 3 ++
3 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a53cd6a725..950c4522f9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15403,6 +15403,52 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
return def;
}
+static virDomainVideoResolutionDefPtr
+virDomainVideoResolutionDefParseXML(xmlNodePtr node)
+{
+xmlNodePtr cur;
+virDomainVideoResolutionDefPtr def;
+VIR_AUTOFREE(char *) x = NULL;
+VIR_AUTOFREE(char *) y = NULL;
+
+cur = node->children;
+while (cur != NULL) {
+if (cur->type == XML_ELEMENT_NODE) {
+if (!x && !y &&
+virXMLNodeNameEqual(cur, "resolution")) {
+x = virXMLPropString(cur, "x");
+y = virXMLPropString(cur, "y");
+}
+}
+cur = cur->next;
+}
+
+if (!x || !y)
+return NULL;
+
+if (VIR_ALLOC(def) < 0)
+goto cleanup;
+
+if (x) {
+if (virStrToLong_uip(x, NULL, 10, >x) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot parse video x-resolution '%s'"), x);
+goto cleanup;
+}
+}
+
+if (y) {
+if (virStrToLong_uip(y, NULL, 10, >y) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot parse video y-resolution '%s'"), y);
+goto cleanup;
+}
+}
+
+ cleanup:
+return def;
+}
+
static virDomainVideoDriverDefPtr
virDomainVideoDriverDefParseXML(xmlNodePtr node)
{
@@ -15482,6 +15528,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
}
def->accel = virDomainVideoAccelDefParseXML(cur);
+def->res = virDomainVideoResolutionDefParseXML(cur);
}
if (virXMLNodeNameEqual(cur, "driver")) {
if (virDomainVirtioOptionsParseXML(cur, >virtio) < 0)
@@ -15569,6 +15616,17 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
+if (def->res) {
+if (def->type != VIR_DOMAIN_VIDEO_TYPE_VGA &&
+def->type != VIR_DOMAIN_VIDEO_TYPE_QXL &&
+def->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+def->type != VIR_DOMAIN_VIDEO_TYPE_BOCHS) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("model resolution is not supported"));
+goto error;
+}
+}
+
I mentioned this in v2: IMO this check should be removed, it's just a
whitelist duplicating qemu logic which could easily go out of date. But
if you want to keep it, it should go in a Validate callback,
specifically virDomainVideoDefValidate. We are trying to avoid adding
more of these checks inline in the XML parser
You can CC me on v4 and I'll review it
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] qemu: Generate 'xres' and 'yres' for video devices
On 10/7/19 11:35 PM, jcfara...@gmail.com wrote:
From: Julio Faracco
Now, QEMU command line can define 'xres' and 'yres' if XML contains both
properties from video model. This is generetaed if resolution was set.
Code let QEMU handle if video model supports this feature.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_command.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cbf25d5f07..99b43243e3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4596,6 +4596,11 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
virBufferAsprintf(, ",vgamem=%uk", video->vram);
}
+if (video->res && video->res->x && video->res->y) {
+/* QEMU accepts resolution xres and yres. */
+virBufferAsprintf(, ",xres=%u,yres=%u", video->res->x,
video->res->y);
+}
+
if (qemuBuildDeviceAddressStr(, def, >info, qemuCaps) < 0)
return NULL;
If I apply just patch #1 and #2, the code fails to compile. This patch
depends on patch #3. This may break 'git bisect' if people are trying to
locate a regression so it should be avoided. Please ensure every
individual patch in a series is compilable and doesn't break the test
suite, or have syntax-check errors. You can write a git rebase one liner
to do roughly that.
In v2 review I also pointed to this example commit:
https://www.redhat.com/archives/libvir-list/2019-May/msg00820.html
I don't think all these changes need to be lumped together, but adding
docs/ and src/conf and a qemu test case in one commit is nice and self
containers. Then patch #2 can add the qemu_command.c bits and regenerate
the test output, to demonstrate what's actually changing. Generally
though I think it's a good idea to put the conf and docs/ changes in the
same commit, unless there's a large amount of doc additions
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dockerfiles PATCH] Refresh after forcing an UTF-8 locale
The corresponding libvirt-jenkins-ci commit is 7ee840dc17ca. Signed-off-by: Andrea Bolognani --- Pushed under the Dockerfile refresh rule. Plain-text diff below. buildenv-libosinfo-centos-7.zip | Bin 546 -> 575 bytes buildenv-libosinfo-debian-10.zip | Bin 646 -> 663 bytes buildenv-libosinfo-debian-9.zip | Bin 666 -> 683 bytes buildenv-libosinfo-debian-sid.zip | Bin 646 -> 663 bytes buildenv-libosinfo-fedora-29.zip | Bin 513 -> 544 bytes buildenv-libosinfo-fedora-30.zip | Bin 575 -> 605 bytes buildenv-libosinfo-fedora-rawhide.zip | Bin 533 -> 564 bytes buildenv-libosinfo-ubuntu-16.zip | Bin 669 -> 686 bytes buildenv-libosinfo-ubuntu-18.zip | Bin 669 -> 686 bytes buildenv-libvirt-centos-7.zip | Bin 729 -> 759 bytes buildenv-libvirt-debian-10-cross-aarch64.zip | Bin 998 -> 1010 bytes buildenv-libvirt-debian-10-cross-armv6l.zip | Bin 989 -> 1003 bytes buildenv-libvirt-debian-10-cross-armv7l.zip | Bin 993 -> 1008 bytes buildenv-libvirt-debian-10-cross-i686.zip | Bin 993 -> 1001 bytes buildenv-libvirt-debian-10-cross-mips.zip | Bin 987 -> 997 bytes buildenv-libvirt-debian-10-cross-mips64el.zip | Bin 1000 -> 1011 bytes buildenv-libvirt-debian-10-cross-mipsel.zip | Bin 992 -> 1001 bytes buildenv-libvirt-debian-10-cross-ppc64le.zip | Bin 999 -> 1012 bytes buildenv-libvirt-debian-10-cross-s390x.zip| Bin 991 -> 1000 bytes buildenv-libvirt-debian-10.zip| Bin 879 -> 898 bytes buildenv-libvirt-debian-9-cross-aarch64.zip | Bin 1028 -> 1042 bytes buildenv-libvirt-debian-9-cross-armv6l.zip| Bin 1021 -> 1033 bytes buildenv-libvirt-debian-9-cross-armv7l.zip| Bin 1025 -> 1039 bytes buildenv-libvirt-debian-9-cross-mips.zip | Bin 1020 -> 1033 bytes buildenv-libvirt-debian-9-cross-mips64el.zip | Bin 1032 -> 1041 bytes buildenv-libvirt-debian-9-cross-mipsel.zip| Bin 1023 -> 1036 bytes buildenv-libvirt-debian-9-cross-ppc64le.zip | Bin 1032 -> 1045 bytes buildenv-libvirt-debian-9-cross-s390x.zip | Bin 1022 -> 1034 bytes buildenv-libvirt-debian-9.zip | Bin 908 -> 927 bytes buildenv-libvirt-debian-sid-cross-aarch64.zip | Bin 997 -> 1010 bytes buildenv-libvirt-debian-sid-cross-armv6l.zip | Bin 989 -> 1003 bytes buildenv-libvirt-debian-sid-cross-armv7l.zip | Bin 994 -> 1008 bytes buildenv-libvirt-debian-sid-cross-i686.zip| Bin 992 -> 1001 bytes buildenv-libvirt-debian-sid-cross-mips.zip| Bin 987 -> 1000 bytes ...denv-libvirt-debian-sid-cross-mips64el.zip | Bin 999 -> 1014 bytes buildenv-libvirt-debian-sid-cross-mipsel.zip | Bin 992 -> 1005 bytes buildenv-libvirt-debian-sid-cross-ppc64le.zip | Bin 1000 -> 1012 bytes buildenv-libvirt-debian-sid-cross-s390x.zip | Bin 991 -> 1000 bytes buildenv-libvirt-debian-sid.zip | Bin 879 -> 898 bytes buildenv-libvirt-fedora-29.zip| Bin 738 -> 768 bytes buildenv-libvirt-fedora-30.zip| Bin 862 -> 891 bytes buildenv-libvirt-fedora-rawhide.zip | Bin 758 -> 788 bytes buildenv-libvirt-ubuntu-16.zip| Bin 915 -> 933 bytes buildenv-libvirt-ubuntu-18.zip| Bin 916 -> 935 bytes 44 files changed, 0 insertions(+), 0 deletions(-) diff --git a/buildenv-libosinfo-centos-7.zip b/buildenv-libosinfo-centos-7.zip index 4da4c52..c9dd791 100644 --- a/buildenv-libosinfo-centos-7.zip +++ b/buildenv-libosinfo-centos-7.zip @@ -54,3 +54,5 @@ RUN yum update -y && \ RUN pip3 install \ meson==0.49.0 + +ENV LANG "en_US.UTF-8" diff --git a/buildenv-libosinfo-debian-10.zip b/buildenv-libosinfo-debian-10.zip index e3b9a99..2592910 100644 --- a/buildenv-libosinfo-debian-10.zip +++ b/buildenv-libosinfo-debian-10.zip @@ -54,3 +54,5 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ apt-get autoclean -y && \ sed -Ei 's,^# (en_US\.UTF-8 .*)$,\1,' /etc/locale.gen && \ dpkg-reconfigure locales + +ENV LANG "en_US.UTF-8" diff --git a/buildenv-libosinfo-debian-9.zip b/buildenv-libosinfo-debian-9.zip index 47d4c59..72b04a2 100644 --- a/buildenv-libosinfo-debian-9.zip +++ b/buildenv-libosinfo-debian-9.zip @@ -57,3 +57,5 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ RUN pip3 install \ meson==0.49.0 + +ENV LANG "en_US.UTF-8" diff --git a/buildenv-libosinfo-debian-sid.zip b/buildenv-libosinfo-debian-sid.zip index de8128f..f81f758 100644 --- a/buildenv-libosinfo-debian-sid.zip +++ b/buildenv-libosinfo-debian-sid.zip @@ -54,3 +54,5 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ apt-get autoclean -y && \ sed -Ei 's,^# (en_US\.UTF-8 .*)$,\1,' /etc/locale.gen && \ dpkg-reconfigure locales + +ENV LANG "en_US.UTF-8" diff --git a/buildenv-libosinfo-fedora-29.zip b/buildenv-libosinfo-fedora-29.zip index a0fbbb2..c3cb705 100644 --- a/buildenv-libosinfo-fedora-29.zip +++ b/buildenv-libosinfo-fedora-29.zip @@ -51,3 +51,5 @@ RUN dnf update -y && \
Re: [libvirt] [PATCH 00/30] storagefile, security: qcow2 data_file support
Hi,
ACKed basically everything, perhaps one or two patches I found something
worth talking about but nothing too gamebreaker. Logic-wise everything made
sense to me, but I believe someone else with a deeper understanding of the
storage backend in Libvirt might know better.
I am not sure how hard it is to test the changes you're making here, but it
would be good to have new stuff in virstoragetest.c (seems like the best
place)
to cover this new data_file support as well.
On a side note: from patches 20+ I got an impression that I was reviewing
the same patches over and over again. I think it's a good idea to have a
look
at the code repetition between the files in src/security dir, or at the
very least
between security_dac.c and security_selinux.c files.
Thanks,
DHB
On 10/7/19 6:49 PM, Cole Robinson wrote:
This series is the first steps to teaching libvirt about qcow2
data_file support, aka external data files or qcow2 external metadata.
A bit about the feature: it was added in qemu 4.0. It essentially
creates a two part image file: a qcow2 layer that just tracks the
image metadata, and a separate data file which is stores the VM
disk contents. AFAICT the driving use case is to keep a fully coherent
raw disk image on disk, and only use qcow2 as an intermediate metadata
layer when necessary, for things like incremental backup support.
The original qemu patch posting is here:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07496.html
For testing, you can create a new qcow2+raw data_file image from an
existing image, like:
qemu-img convert -O qcow2 \
-o data_file=NEW.raw,data_file_raw=yes
EXISTING.raw NEW.qcow2
The goal of this series is to teach libvirt enough about this case
so that we can correctly relabel the data_file on VM startup/shutdown.
The main functional changes are
* Teach storagefile how to parse out data_file from the qcow2 header
* Store the raw string as virStorageSource->externalDataStoreRaw
* Track that as its out virStorageSource in externalDataStore
* dac/selinux relabel externalDataStore as needed
>From libvirt's perspective, externalDataStore is conceptually pretty
close to a backingStore, but the main difference is its read/write
permissions should match its parent image, rather than being readonly
like backingStore.
This series has only been tested on top of the -blockdev enablement
series, but I don't think it actually interacts with that work at
the moment.
Future work:
* Exposing this in the runtime XML. We need to figure out an XML
schema. It will reuse virStorageSource obviously, but the main
thing to figure out is probably 1) what the top element name
should be ('dataFile' maybe?), 2) where it sits in the XML
hierarchy (under or under I guess)
* Exposing this on the qemu -blockdev command line. Similar to how
in the blockdev world we are explicitly putting the disk backing
chain on the command line, we can do that for data_file too. Then
like persistent XML the user will have the power
to overwrite the data_file location for an individual VM run.
* Figure out how we expect ovirt/rhev to be using this at runtime.
Possibly taking a running VM using a raw image, doing blockdev-*
magic to pivot it to qcow2+raw data_file, so it can initiate
incremental backup on top of a previously raw only VM?
Known issues:
* In the qemu driver, the qcow2 image metadata is only parsed
in -blockdev world if no is specified in the
persistent XML. So basically if there's a listed,
we never parse the qcow2 header and detect the presence of
data_file. Fixable I'm sure but I didn't look into it much yet.
Most of this is cleanups and refactorings to simplify the actual
functional changes.
Cole Robinson (30):
storagefile: Make GetMetadataInternal static
storagefile: qcow1: Check for BACKING_STORE_OK
storagefile: qcow1: Fix check for empty backing file
storagefile: qcow1: Let qcowXGetBackingStore fill in format
storagefile: Check version to determine if qcow2 or not
storagefile: Drop now unused isQCow2 argument
storagefile: Use qcowXGetBackingStore directly
storagefile: Push 'start' into qcow2GetBackingStoreFormat
storagefile: Push extension_end calc to qcow2GetBackingStoreFormat
storagefile: Rename qcow2GetBackingStoreFormat
storagefile: Rename qcow2GetExtensions 'format' argument
storagefile: Fix backing format \0 check
storagefile: Add externalDataStoreRaw member
storagefile: Parse qcow2 external data file
storagefile: Fill in meta->externalDataStoreRaw
storagefile: Don't access backingStoreRaw directly in
FromBackingRelative
storagefile: Split out virStorageSourceNewFromChild
storagefile: Add externalDataStore member
storagefile: Fill in meta->externalDataStore
security: dac: Drop !parent handling in SetImageLabelInternal
security: dac: Add is_toplevel to
Re: [libvirt] [jenkins-ci PATCH] lcitool: Force LANG=en_US.UTF-8 for the containers
On Thu, 2019-10-10 at 17:13 +0200, Fabiano Fidêncio wrote:
> As we cannot and should not rely on how the containers were generated,
> let's force the container LANG to be en_US.UTF-8 otherwise some
> containers (Debian 9, Ubuntu 16, and Ubuntu 18) would simply bail when
> dealing with environment variables inherited from Gitlab CI which
> contains non-POSIX characteres, such as "Fidêncio".
>
> Unfortunately, there's no standard way to do this accross different
> distros, leaving us with this "happy little accident" of setting up LANG
> in the way it's done right now.
>
> Signed-off-by: Fabiano Fidêncio
> ---
> guests/lcitool | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/guests/lcitool b/guests/lcitool
> index 49bb50b..a630971 100755
> --- a/guests/lcitool
> +++ b/guests/lcitool
> @@ -735,6 +735,10 @@ class Application:
> RUN pip3 install {pip_pkgs}
> """).format(**varmap))
>
> +sys.stdout.write(textwrap.dedent("""
> +ENV LANG "en_US.UTF-8"
> +""").format(**varmap))
The approach we ultimately want to take is probably to store this
variable, along with others like $NINJA, in the global shell
profile / initialization scripts, both for containers and for
virtual machines.
But it's a lot of work to get there and this solution, although
somewhat gross, fixes the build failures you're seeing in GitLab CI,
so it gets a reluctant
Reviewed-by: Andrea Bolognani
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 30/30] security: selinux: Label externalDataStore
On 10/7/19 6:49 PM, Cole Robinson wrote: We mirror the labeling strategy that was used for its top image Signed-off-by: Cole Robinson --- Reviewed-by: Daniel Henrique Barboza src/security/security_selinux.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index feb703d325..2a3b7fc10d 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1844,7 +1844,7 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr, virSecurityDeviceLabelDefPtr parent_seclabel = NULL; char *use_label = NULL; bool remember; -bool is_toplevel = parent == src; +bool is_toplevel = parent == src || parent->externalDataStore == src; int ret; if (!src->path || !virStorageSourceIsLocalStorage(src)) @@ -1931,6 +1931,14 @@ virSecuritySELinuxSetImageLabelRelative(virSecurityManagerPtr mgr, if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent) < 0) return -1; +if (n->externalDataStore && +virSecuritySELinuxSetImageLabelRelative(mgr, +def, +n->externalDataStore, +parent, +flags) < 0) +return -1; + if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) break; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 29/30] security: selinux: break out SetImageLabelRelative
On 10/7/19 6:49 PM, Cole Robinson wrote:
This will be used for recursing into externalDataStore
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/security/security_selinux.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index c0bfb581e3..feb703d325 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1919,15 +1919,16 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
static int
-virSecuritySELinuxSetImageLabel(virSecurityManagerPtr mgr,
-virDomainDefPtr def,
-virStorageSourcePtr src,
-virSecurityDomainImageLabelFlags flags)
+virSecuritySELinuxSetImageLabelRelative(virSecurityManagerPtr mgr,
+virDomainDefPtr def,
+virStorageSourcePtr src,
+virStorageSourcePtr parent,
+virSecurityDomainImageLabelFlags flags)
{
virStorageSourcePtr n;
for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
-if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, src) < 0)
+if (virSecuritySELinuxSetImageLabelInternal(mgr, def, n, parent) < 0)
return -1;
if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
@@ -1938,6 +1939,15 @@ virSecuritySELinuxSetImageLabel(virSecurityManagerPtr
mgr,
}
+static int
+virSecuritySELinuxSetImageLabel(virSecurityManagerPtr mgr,
+virDomainDefPtr def,
+virStorageSourcePtr src,
+virSecurityDomainImageLabelFlags flags)
+{
+return virSecuritySELinuxSetImageLabelRelative(mgr, def, src, src, flags);
+}
+
struct virSecuritySELinuxMoveImageMetadataData {
virSecurityManagerPtr mgr;
const char *src;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 28/30] security: selinux: Restore image label for externalDataStore
On 10/7/19 6:49 PM, Cole Robinson wrote:
Rename the existing virSecuritySELinuxRestoreImageLabelInt
to virSecuritySELinuxRestoreImageLabelSingle, and extend the new
ImageLabelInt handle externalDataStore
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/security/security_selinux.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index fd7dd080c1..c0bfb581e3 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1747,10 +1747,10 @@
virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr,
static int
-virSecuritySELinuxRestoreImageLabelInt(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virStorageSourcePtr src,
- bool migrated)
+virSecuritySELinuxRestoreImageLabelSingle(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virStorageSourcePtr src,
+ bool migrated)
{
virSecurityLabelDefPtr seclabel;
virSecurityDeviceLabelDefPtr disk_seclabel;
@@ -1802,6 +1802,26 @@
virSecuritySELinuxRestoreImageLabelInt(virSecurityManagerPtr mgr,
}
+static int
+virSecuritySELinuxRestoreImageLabelInt(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virStorageSourcePtr src,
+ bool migrated)
+{
+if (virSecuritySELinuxRestoreImageLabelSingle(mgr, def, src, migrated) < 0)
+return -1;
+
+if (src->externalDataStore &&
+virSecuritySELinuxRestoreImageLabelSingle(mgr,
+ def,
+ src->externalDataStore,
+ migrated) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
virSecuritySELinuxRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 27/30] security: selinux: Add is_toplevel to SetImageLabelInternal
On 10/7/19 6:49 PM, Cole Robinson wrote:
This will simplify future patches and make the logic easier to follow
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/security/security_selinux.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e384542c49..fd7dd080c1 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1824,6 +1824,7 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
virSecurityDeviceLabelDefPtr parent_seclabel = NULL;
char *use_label = NULL;
bool remember;
+bool is_toplevel = parent == src;
int ret;
if (!src->path || !virStorageSourceIsLocalStorage(src))
@@ -1845,7 +1846,7 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
* but the top layer, or read only image, or disk explicitly
* marked as shared.
*/
-remember = src == parent && !src->readonly && !src->shared;
+remember = is_toplevel && !src->readonly && !src->shared;
disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
SECURITY_SELINUX_NAME);
@@ -1862,7 +1863,7 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
return 0;
use_label = parent_seclabel->label;
-} else if (parent == src) {
+} else if (is_toplevel) {
if (src->shared) {
use_label = data->file_context;
} else if (src->readonly) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 26/30] security: selinux: Drop !parent handling in SetImageLabelInternal
On 10/7/19 6:49 PM, Cole Robinson wrote:
The only caller always passes in a non-null parent
Signed-off-by: Cole Robinson
---
A replay from patch 20. I wonder how much common code there are
between security_dac.c and security_selinux.c.
Reviewed-by: Daniel Henrique Barboza
src/security/security_selinux.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 9d28bc5773..e384542c49 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1849,9 +1849,8 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
SECURITY_SELINUX_NAME);
-if (parent)
-parent_seclabel = virStorageSourceGetSecurityLabelDef(parent,
-
SECURITY_SELINUX_NAME);
+parent_seclabel = virStorageSourceGetSecurityLabelDef(parent,
+
SECURITY_SELINUX_NAME);
if (disk_seclabel && (!disk_seclabel->relabel || disk_seclabel->label)) {
if (!disk_seclabel->relabel)
@@ -1863,7 +1862,7 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
return 0;
use_label = parent_seclabel->label;
-} else if (!parent || parent == src) {
+} else if (parent == src) {
if (src->shared) {
use_label = data->file_context;
} else if (src->readonly) {
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 25/30] security: selinux: Simplify SetImageLabelInternal
On 10/7/19 6:49 PM, Cole Robinson wrote:
All the SetFileCon calls only differ by the label they pass in.
Rework the conditionals to track what label we need, and use a
single SetFileCon call
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/security/security_selinux.c | 31 ++-
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e879fa39ab..9d28bc5773 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1822,6 +1822,7 @@
virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
virSecurityLabelDefPtr secdef;
virSecurityDeviceLabelDefPtr disk_seclabel;
virSecurityDeviceLabelDefPtr parent_seclabel = NULL;
+char *use_label = NULL;
bool remember;
int ret;
@@ -1856,40 +1857,28 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManagerPtr mgr,
if (!disk_seclabel->relabel)
return 0;
-ret = virSecuritySELinuxSetFilecon(mgr, src->path,
- disk_seclabel->label, remember);
+use_label = disk_seclabel->label;
} else if (parent_seclabel && (!parent_seclabel->relabel ||
parent_seclabel->label)) {
if (!parent_seclabel->relabel)
return 0;
-ret = virSecuritySELinuxSetFilecon(mgr, src->path,
- parent_seclabel->label, remember);
+use_label = parent_seclabel->label;
} else if (!parent || parent == src) {
if (src->shared) {
-ret = virSecuritySELinuxSetFilecon(mgr,
- src->path,
- data->file_context,
- remember);
+use_label = data->file_context;
} else if (src->readonly) {
-ret = virSecuritySELinuxSetFilecon(mgr,
- src->path,
- data->content_context,
- remember);
+use_label = data->content_context;
} else if (secdef->imagelabel) {
-ret = virSecuritySELinuxSetFilecon(mgr,
- src->path,
- secdef->imagelabel,
- remember);
+use_label = secdef->imagelabel;
} else {
-ret = 0;
+return 0;
}
} else {
-ret = virSecuritySELinuxSetFilecon(mgr,
- src->path,
- data->content_context,
- remember);
+use_label = data->content_context;
}
+ret = virSecuritySELinuxSetFilecon(mgr, src->path, use_label, remember);
+
if (ret == 1 && !disk_seclabel) {
/* If we failed to set a label, but virt_use_nfs let us
* proceed anyway, then we don't need to relabel later. */
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 23/30] security: dac: break out SetImageLabelRelative
On 10/7/19 6:49 PM, Cole Robinson wrote:
This will be used for recursing into externalDataStore
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/security/security_dac.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index befa388791..326b9b1a3c 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -934,15 +934,16 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr
mgr,
static int
-virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
-virDomainDefPtr def,
-virStorageSourcePtr src,
-virSecurityDomainImageLabelFlags flags)
+virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr,
+virDomainDefPtr def,
+virStorageSourcePtr src,
+virStorageSourcePtr parent,
+virSecurityDomainImageLabelFlags flags)
{
virStorageSourcePtr n;
for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
-if (virSecurityDACSetImageLabelInternal(mgr, def, n, src) < 0)
+if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent) < 0)
return -1;
if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN))
@@ -952,6 +953,14 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
return 0;
}
+static int
+virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
+virDomainDefPtr def,
+virStorageSourcePtr src,
+virSecurityDomainImageLabelFlags flags)
+{
+return virSecurityDACSetImageLabelRelative(mgr, def, src, src, flags);
+}
static int
virSecurityDACRestoreImageLabelSingle(virSecurityManagerPtr mgr,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 24/30] security: dac: Label externalDataStore
On 10/7/19 6:49 PM, Cole Robinson wrote: We mirror the labeling strategy that was used for its sibling image Signed-off-by: Cole Robinson --- Reviewed-by: Daniel Henrique Barboza src/security/security_dac.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 326b9b1a3c..2bbf773dd3 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -882,7 +882,7 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr, virSecurityDeviceLabelDefPtr parent_seclabel = NULL; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); bool remember; -bool is_toplevel = parent == src; +bool is_toplevel = parent == src || parent->externalDataStore == src; uid_t user; gid_t group; @@ -946,6 +946,14 @@ virSecurityDACSetImageLabelRelative(virSecurityManagerPtr mgr, if (virSecurityDACSetImageLabelInternal(mgr, def, n, parent) < 0) return -1; +if (n->externalDataStore && +virSecurityDACSetImageLabelRelative(mgr, +def, +n->externalDataStore, +parent, +flags) < 0) +return -1; + if (!(flags & VIR_SECURITY_DOMAIN_IMAGE_LABEL_BACKING_CHAIN)) break; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 22/30] security: dac: Restore image label for externalDataStore
On 10/7/19 6:49 PM, Cole Robinson wrote:
Rename the existing virSecurityDACRestoreImageLabelInt
to virSecurityDACRestoreImageLabelSingle, and extend the new
ImageLabelInt handle externalDataStore
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/security/security_dac.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 6d0c8a9b1c..befa388791 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -954,10 +954,10 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
static int
-virSecurityDACRestoreImageLabelInt(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virStorageSourcePtr src,
- bool migrated)
+virSecurityDACRestoreImageLabelSingle(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virStorageSourcePtr src,
+ bool migrated)
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
virSecurityLabelDefPtr secdef;
@@ -1008,6 +1008,26 @@ virSecurityDACRestoreImageLabelInt(virSecurityManagerPtr
mgr,
}
+static int
+virSecurityDACRestoreImageLabelInt(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virStorageSourcePtr src,
+ bool migrated)
+{
+if (virSecurityDACRestoreImageLabelSingle(mgr, def, src, migrated) < 0)
+return -1;
+
+if (src->externalDataStore &&
+virSecurityDACRestoreImageLabelSingle(mgr,
+ def,
+ src->externalDataStore,
+ migrated) < 0)
+return -1;
+
+return 0;
+}
+
+
static int
virSecurityDACRestoreImageLabel(virSecurityManagerPtr mgr,
virDomainDefPtr def,
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 21/30] security: dac: Add is_toplevel to SetImageLabelInternal
On 10/7/19 6:49 PM, Cole Robinson wrote: This will simplify future patches and make the logic easier to follow Signed-off-by: Cole Robinson --- Reviewed-by: Daniel Henrique Barboza src/security/security_dac.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index bcc781213e..6d0c8a9b1c 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -882,6 +882,7 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr, virSecurityDeviceLabelDefPtr parent_seclabel = NULL; virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr); bool remember; +bool is_toplevel = parent == src; uid_t user; gid_t group; @@ -926,7 +927,7 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr, * but the top layer, or read only image, or disk explicitly * marked as shared. */ -remember = src == parent && !src->readonly && !src->shared; +remember = is_toplevel && !src->readonly && !src->shared; return virSecurityDACSetOwnership(mgr, src, NULL, user, group, remember); } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 20/30] security: dac: Drop !parent handling in SetImageLabelInternal
On 10/7/19 6:49 PM, Cole Robinson wrote:
The only caller always passes in a non-null parent
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/security/security_dac.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 4b4afef18a..bcc781213e 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -893,9 +893,8 @@ virSecurityDACSetImageLabelInternal(virSecurityManagerPtr
mgr,
return 0;
disk_seclabel = virStorageSourceGetSecurityLabelDef(src, SECURITY_DAC_NAME);
-if (parent)
-parent_seclabel = virStorageSourceGetSecurityLabelDef(parent,
-
SECURITY_DAC_NAME);
+parent_seclabel = virStorageSourceGetSecurityLabelDef(parent,
+ SECURITY_DAC_NAME);
if (disk_seclabel && (!disk_seclabel->relabel || disk_seclabel->label)) {
if (!disk_seclabel->relabel)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 19/30] storagefile: Fill in meta->externalDataStore
On 10/7/19 6:49 PM, Cole Robinson wrote:
Add virStorageSourceNewFromExternalData, similar to
virStorageSourceNewFromBacking and use it to fill in a
virStorageSource for externalDataStore
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/util/virstoragefile.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index ce669b6e0b..4aa70d71b1 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3799,6 +3799,24 @@ virStorageSourceNewFromBacking(virStorageSourcePtr
parent,
}
+static int
+virStorageSourceNewFromExternalData(virStorageSourcePtr parent,
+virStorageSourcePtr *externalDataStore)
+{
+int rc;
+
+if ((rc = virStorageSourceNewFromChild(parent,
+ parent->externalDataStoreRaw,
+ externalDataStore)) < 0)
+return rc;
+
+/* qcow2 data_file can only be raw */
+(*externalDataStore)->format = VIR_STORAGE_FILE_RAW;
+(*externalDataStore)->readonly = parent->readonly;
+return rc;
+}
+
+
/**
* @src: disk source definition structure
* @fd: file descriptor
@@ -5007,6 +5025,23 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
}
VIR_STEAL_PTR(src->backingStore, backingStore);
+
+if (src->externalDataStoreRaw) {
+VIR_AUTOUNREF(virStorageSourcePtr) externalDataStore = NULL;
+
+if ((rv = virStorageSourceNewFromExternalData(src,
+ )) < 0)
+goto cleanup;
+
+if (rv == 1) {
+/* the file would not be usable for VM usage */
+ret = 0;
+goto cleanup;
+}
+
+VIR_STEAL_PTR(src->externalDataStore, externalDataStore);
+}
+
ret = 0;
cleanup:
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] lcitool: Force LANG=en_US.UTF-8 for the containers
As we cannot and should not rely on how the containers were generated,
let's force the container LANG to be en_US.UTF-8 otherwise some
containers (Debian 9, Ubuntu 16, and Ubuntu 18) would simply bail when
dealing with environment variables inherited from Gitlab CI which
contains non-POSIX characteres, such as "Fidêncio".
Unfortunately, there's no standard way to do this accross different
distros, leaving us with this "happy little accident" of setting up LANG
in the way it's done right now.
Signed-off-by: Fabiano Fidêncio
---
guests/lcitool | 4
1 file changed, 4 insertions(+)
diff --git a/guests/lcitool b/guests/lcitool
index 49bb50b..a630971 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -735,6 +735,10 @@ class Application:
RUN pip3 install {pip_pkgs}
""").format(**varmap))
+sys.stdout.write(textwrap.dedent("""
+ENV LANG "en_US.UTF-8"
+""").format(**varmap))
+
if args.cross_arch:
sys.stdout.write(textwrap.dedent("""
ENV ABI "{cross_abi}"
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 18/30] storagefile: Add externalDataStore member
On 10/7/19 6:49 PM, Cole Robinson wrote:
Add the plumbing to track a externalDataStoreRaw as a virStorageSource
Signed-off-by: Cole Robinson
---
src/util/virstoragefile.c | 9 +
src/util/virstoragefile.h | 3 +++
2 files changed, 12 insertions(+)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 7ae6719dd6..ce669b6e0b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2339,6 +2339,12 @@ virStorageSourceCopy(const virStorageSource *src,
return NULL;
}
+if (src->externalDataStore) {
+if (!(def->externalDataStore =
virStorageSourceCopy(src->externalDataStore,
+true)))
+return NULL;
+}
+
VIR_STEAL_PTR(ret, def);
return ret;
}
@@ -2560,6 +2566,9 @@ virStorageSourceClear(virStorageSourcePtr def)
VIR_FREE(def->timestamps);
VIR_FREE(def->externalDataStoreRaw);
+virObjectUnref(def->externalDataStore);
+def->externalDataStore = NULL;
Is this assign to NULL necessary? virObjectUnref will call VIR_FREE() in
def->externalDataStore if there is no more references to it (which will
assign it to NULL in the end).
LGTM otherwise
Reviewed-by: Daniel Henrique Barboza
+
virStorageNetHostDefFree(def->nhosts, def->hosts);
virStorageAuthDefFree(def->auth);
virObjectUnref(def->privateData);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index bbff511657..d84dad052d 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -292,6 +292,9 @@ struct _virStorageSource {
/* backing chain of the storage source */
virStorageSourcePtr backingStore;
+/* external data store storage source */
+virStorageSourcePtr externalDataStore;
+
/* metadata for storage driver access to remote and local volumes */
virStorageDriverDataPtr drv;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] tests: Add capabilitie for QEMU 4.2.0 on ppc64
Signed-off-by: Andrea Bolognani
---
.../caps_4.2.0.ppc64.replies | 24406
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1087 +
2 files changed, 25493 insertions(+)
create mode 100644 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.replies
create mode 100644 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.replies
b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.replies
new file mode 100644
index 00..d1c998f767
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.replies
@@ -0,0 +1,24406 @@
+{
+ "execute": "qmp_capabilities",
+ "id": "libvirt-1"
+}
+
+{
+ "return": {
+ },
+ "id": "libvirt-1"
+}
+
+{
+ "execute": "query-version",
+ "id": "libvirt-2"
+}
+
+{
+ "return": {
+"qemu": {
+ "micro": 50,
+ "minor": 1,
+ "major": 4
+},
+"package": "v4.1.0-1378-g98b2e3c9ab"
+ },
+ "id": "libvirt-2"
+}
[...]
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
new file mode 100644
index 00..a3d7ab59f5
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml
@@ -0,0 +1,1087 @@
+
+ 4001050
+ 0
+ 42900760
+ v4.1.0-1378-g98b2e3c9ab
+ ppc64
[...]
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] tests: Add capabilitie for QEMU 4.2.0 on aarch64
Signed-off-by: Andrea Bolognani
---
.../caps_4.2.0.aarch64.replies| 20684
.../caps_4.2.0.aarch64.xml| 321 +
2 files changed, 21005 insertions(+)
create mode 100644 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.replies
create mode 100644 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.replies
b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.replies
new file mode 100644
index 00..08987b91d3
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.replies
@@ -0,0 +1,20684 @@
+{
+ "execute": "qmp_capabilities",
+ "id": "libvirt-1"
+}
+
+{
+ "return": {
+ },
+ "id": "libvirt-1"
+}
+
+{
+ "execute": "query-version",
+ "id": "libvirt-2"
+}
+
+{
+ "return": {
+"qemu": {
+ "micro": 50,
+ "minor": 1,
+ "major": 4
+},
+"package": "v4.1.0-1378-g98b2e3c9ab"
+ },
+ "id": "libvirt-2"
+}
[...]
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
new file mode 100644
index 00..e75b39ff16
--- /dev/null
+++ b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
@@ -0,0 +1,321 @@
+
+ 4001050
+ 0
+ 61700760
+ v4.1.0-1378-g98b2e3c9ab
+ aarch64
[...]
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] tests: Add capabilities for QEMU 4.2.0 on ppc64 and aarch64
This will be useful to test Jirka's changes to how we handle default CPU models on more architectures. The version posted to the list is heavily snipped, but you can grab the full one with $ git fetch https://gitlab.com/abologna/libvirt caps-4.2.0 Andrea Bolognani (2): tests: Add capabilitie for QEMU 4.2.0 on ppc64 tests: Add capabilitie for QEMU 4.2.0 on aarch64 .../caps_4.2.0.aarch64.replies| 20684 + .../caps_4.2.0.aarch64.xml| 321 + .../caps_4.2.0.ppc64.replies | 24406 .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1087 + 4 files changed, 46498 insertions(+) create mode 100644 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.replies create mode 100644 tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml create mode 100644 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.replies create mode 100644 tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/30] storagefile: Split out virStorageSourceNewFromChild
On 10/7/19 6:49 PM, Cole Robinson wrote:
Future patches will use this for external data file handling
Signed-off-by: Cole Robinson
---
Reviewed-by: Daniel Henrique Barboza
src/util/virstoragefile.c | 47 ++-
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index c47df6c200..7ae6719dd6 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -3716,38 +3716,38 @@ virStorageSourceNewFromBackingAbsolute(const char *path,
/**
- * virStorageSourceNewFromBacking:
+ * virStorageSourceNewFromChild:
* @parent: storage source parent
- * @backing: returned backing store definition
+ * @child: returned child/backing store definition
+ * @parentRaw: raw child string (backingStoreRaw)
*
* Creates a storage source which describes the backing image of @parent and
- * fills it into @backing depending on the 'backingStoreRaw' property of
@parent
+ * fills it into @backing depending on the passed parentRaw (backingStoreRaw)
* and other data. Note that for local storage this function accesses the file
- * to update the actual type of the backing store.
+ * to update the actual type of the child store.
*
- * Returns 0 on success, 1 if we could parse all location data but the backinig
+ * Returns 0 on success, 1 if we could parse all location data but the child
* store specification contained other data unrepresentable by libvirt (e.g.
* inline authentication).
* In both cases @src is filled. On error -1 is returned @src is NULL and an
* error is reported.
*/
-int
-virStorageSourceNewFromBacking(virStorageSourcePtr parent,
- virStorageSourcePtr *backing)
+static int
+virStorageSourceNewFromChild(virStorageSourcePtr parent,
+ const char *parentRaw,
+ virStorageSourcePtr *child)
{
struct stat st;
VIR_AUTOUNREF(virStorageSourcePtr) def = NULL;
int rc = 0;
-*backing = NULL;
+*child = NULL;
-if (virStorageIsRelative(parent->backingStoreRaw)) {
-if (!(def = virStorageSourceNewFromBackingRelative(parent,
-
parent->backingStoreRaw)))
+if (virStorageIsRelative(parentRaw)) {
+if (!(def = virStorageSourceNewFromBackingRelative(parent, parentRaw)))
return -1;
} else {
-if ((rc =
virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw,
- )) < 0)
+if ((rc = virStorageSourceNewFromBackingAbsolute(parentRaw, )) < 0)
return -1;
}
@@ -3767,10 +3767,25 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent,
if (virStorageSourceInitChainElement(def, parent, true) < 0)
return -1;
-def->readonly = true;
def->detected = true;
-VIR_STEAL_PTR(*backing, def);
+VIR_STEAL_PTR(*child, def);
+return rc;
+}
+
+
+int
+virStorageSourceNewFromBacking(virStorageSourcePtr parent,
+ virStorageSourcePtr *backing)
+{
+int rc;
+
+if ((rc = virStorageSourceNewFromChild(parent,
+ parent->backingStoreRaw,
+ backing)) < 0)
+return rc;
+
+(*backing)->readonly = true;
return rc;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/3] qemu: Implement the ccf-assist pSeries feature
On 10/9/19 7:06 PM, Cole Robinson wrote:
On 10/8/19 4:06 PM, Daniel Henrique Barboza wrote:
This patch adds the implementation of the ccf-assist pSeries
feature, based on the QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST
capability that was added in the previous patch.
Signed-off-by: Daniel Henrique Barboza
---
docs/formatdomain.html.in | 9 +
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c | 4
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 20 +++
src/qemu/qemu_domain.c | 1 +
tests/qemuxml2argvdata/pseries-features.args | 2 +-
tests/qemuxml2argvdata/pseries-features.xml | 1 +
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmloutdata/pseries-features.xml | 1 +
tests/qemuxml2xmltest.c | 1 +
11 files changed, 45 insertions(+), 1 deletion(-)
Reviewed-by: Cole Robinson
And pushed.
One general comment, I find it helpful to put the XML addition in the
commit message, and the cover letter. Makes it easier for reviewers,
and also easier for grepping commit logs for an XML property which is
occasionally useful.
A couple comments below though
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 647f96f651..059781a0c3 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2059,6 +2059,7 @@
tseg unit='MiB'48/tseg
/smm
htm state='on'/
+ ccf-assist state='on'/
msrs unknown='ignore'/
/features
...
@@ -2357,6 +2358,14 @@
will not be ignored.
Since 5.1.0 (bhyve only)
+ ccf-assist
+ Configure ccf-assist (Count Cache Flush Assist)
availability for
+ pSeries guests.
+ Possible values for the state attribute
+ are on and off. If the attribute
is not
+ defined, the hypervisor default will be used.
+ Since 5.8.0 (QEMU/KVM only)
+
I changed the version reference to 5.9.0 too
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cbf25d5f07..3bd841919d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7409,6 +7409,26 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
virBufferAsprintf(, ",cap-nested-hv=%s", str);
}
+ if (def->features[VIR_DOMAIN_FEATURE_CCF_ASSIST] !=
VIR_TRISTATE_SWITCH_ABSENT) {
+ const char *str;
+
I know you were just follow the pattern of the rest of the function
here so I didn't object. But, these two error checks should not be here.
+ if (!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_MACHINE_PSERIES_CAP_CCF_ASSIST)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("ccf-assist configuration is not
supported by this "
+ "QEMU binary"));
+ return -1;
+ }
+
This is a validation check, throwing an error if an unsupported qemu
config was requested. This should be part of the
qemuDomainDefValidateFeatures in qemu_domain.c, which already has
several other feature validation checks.
Basically every qemuCaps validation check in qemu_command.c CLI
building is a candidate for moving to something triggered from
qemuDomainDefValidate.
The benefits of moving to validate time is that XML is rejected by
'virsh define' rather than at 'virsh start' time. It also makes it
easier to follow the cli building code, and makes it easier to verify
qemu_command.c test suite code coverage for the important stuff like
covering every CLI option. It's also a good intermediate step for
sharing validation with domain capabilities building, like is done
presently for rng models.
The features stuff here is a good place to start. I know you've been
sending cleanup patches lately; if working on this is something you
want to do, I'm happy to provide timely reviews (that's my sales pitch
:) )
hehehe yeah, I use these cleanups as an opportunity to try to learn more
about the
code and so on.
I'll follow your suggestion and send patches to move these verifications
from
qemu_command.c to qemu_domain:qemuDomainDefValidate, hopefully
soon(C).
Thanks,
DHB
+ str =
virTristateSwitchTypeToString(def->features[VIR_DOMAIN_FEATURE_CCF_ASSIST]);
+ if (!str) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Invalid setting for ccf-assist state"));
+ return -1;
+ }
+
+ virBufferAsprintf(, ",cap-ccf-assist=%s", str);
+ }
+
This check isn't really required IMO. The only time we should hit !str
is if some there's some internal bug which doesn't deserve an explicit
error message. Better to just use NULLSTR(str) and let the (null) hit
qemu command line which will hopefully fail. Either way it's a bug
that shouldn't happen in practice, so the
Re: [libvirt] [PATCH v4 00/26] scripts: convert most perl scripts to python
On Thu, Oct 10, 2019 at 03:28:39PM +0200, Ján Tomko wrote:
> On Wed, Oct 09, 2019 at 12:37:15PM +0100, Daniel P. Berrangé wrote:
> > Note that the check-spacing.py script is significantly
> > slower in Python than in Perl. After researching this
> > it appears there is nothing that can be done. The Perl
> > regex engine is simply much better optimized than the
> > Python one.
>
> I'm afraid immutability of strings does not help either.
> I tried speeding it up a bit by running the regexes conditionally, e.g.:
>
> +if '"' in data:
> +data = quotedstringprog.sub('"XXX"', data)
>
> That got it down from 0m6.368s to 0m3.802s (compared to perl's 1.5s),
> but even before that change, I don't consider the Python version of this
> one script to be more readable.
>
> Note that perl's optimizations aren't perfect either - merely
> moving all the checks into a subroutine resulted in a significant
> slowdown.
>
> > As previously discussed we need to loook
> > at uncrustify or clang-format or some other tool to
> > validate whitespace formatting. This is ongoing. We
> > can either accept the slow down in the short term or
> > keep the Perl version in the short term.
> >
>
> Just to set the expectations, running clang-format on all the
> C files took me ~40 s. Not sure how much that can be efficiently
> parallelized or if meson could be convinced to start clang-format
> as the first thing for the 'check' target, even before starting
> to compile anything.
Not entirely surprising since it is actually doing proper parsing
of the C syntax, which is quite a bit of work.
FWIW, when I'm writing Go code I have used emacs 'go-mode' and
this runs 'go fmt' to automatically fix your style problems
every time you save the file. So your code is basically always
correct, and you don't need a separate job to check it later.
For this approach to be viable though, you need to have an
automated CI check that is run at time of pushing to git master
to block pushes of code which is non-compliant. It is the kind
of thing projects do when using pull requests & gating CI in
gitlab/github though. So this is itself another can of worms.
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 02/26] build-aux: rewrite augeas test generator in Python
On Wed, Oct 09, 2019 at 12:37:17PM +0100, Daniel P. Berrangé wrote: As part of an goal to eliminate Perl from libvirt build tools, rewrite the augeas-gentest.pl tool in Python. This was a straight conversion, manually going line-by-line to change the syntax from Perl to Python. Thus the overall structure of the file and approach is the same. The use of $(AUG_GENTEST) as a dependancy in the makefiles needed to be fixed, because this was assumed to be the filename of the script, but is in fact a full shell command line. Signed-off-by: Daniel P. Berrangé --- Makefile.am | 2 +- build-aux/augeas-gentest.pl | 60 - scripts/augeas-gentest.py | 67 + src/Makefile.am | 4 +-- 4 files changed, 70 insertions(+), 63 deletions(-) delete mode 100755 build-aux/augeas-gentest.pl create mode 100755 scripts/augeas-gentest.py Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 00/26] scripts: convert most perl scripts to python
On Wed, Oct 09, 2019 at 12:37:15PM +0100, Daniel P. Berrangé wrote:
Note that the check-spacing.py script is significantly
slower in Python than in Perl. After researching this
it appears there is nothing that can be done. The Perl
regex engine is simply much better optimized than the
Python one.
I'm afraid immutability of strings does not help either.
I tried speeding it up a bit by running the regexes conditionally, e.g.:
+if '"' in data:
+data = quotedstringprog.sub('"XXX"', data)
That got it down from 0m6.368s to 0m3.802s (compared to perl's 1.5s),
but even before that change, I don't consider the Python version of this
one script to be more readable.
Note that perl's optimizations aren't perfect either - merely
moving all the checks into a subroutine resulted in a significant
slowdown.
As previously discussed we need to loook
at uncrustify or clang-format or some other tool to
validate whitespace formatting. This is ongoing. We
can either accept the slow down in the short term or
keep the Perl version in the short term.
Just to set the expectations, running clang-format on all the
C files took me ~40 s. Not sure how much that can be efficiently
parallelized or if meson could be convinced to start clang-format
as the first thing for the 'check' target, even before starting
to compile anything.
Jano
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 06/19] util: rewrite auto cleanup macros to use glib's equivalent
Daniel P. Berrangé [2019-10-10, 11:54AM +0100]: > diff --git a/docs/hacking.html.in b/docs/hacking.html.in > index 3f1542b6de..6e62b2d4ff 100644 > --- a/docs/hacking.html.in > +++ b/docs/hacking.html.in > @@ -1028,6 +1028,24 @@ BAD: >The GLib APIs g_strdup_printf / g_strdup_vprint should be used > instead. Don't use g_vasprintf unless having the string length > returned is unavoidable. > + > + VIR_AUTOPTR, VIR_AUTOCLEAN, VIR_AUTOFREE > + The GLib macros g_autoptr, g_auto and g_autofree must be used > +instead in all new code. In existing code, the GLib macros must > +never be mixed with libvirt macros within a method, nor should > +they be mixed with VIR_FREE. If introducing GLib macros to an > +existing method, any use of libvirt macros must be converted > +in an independent commit. > + > + > + VIR_DEFINE_AUTOPTR_FUNC, VIR_DEFINE_AUTOCLEAN_FUNC > + The Gib macros G_DEFINE_AUTOPTR_CLEANUP_FUNC and ^ GLib -- IBM Systems Linux on Z & Virtualization Development -- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -- Vorsitzende des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: drop support for stack traces with logging
On Thu, Oct 10, 2019 at 01:05:43PM +0100, Daniel P. Berrangé wrote: The log filters have supported the use of a "+" before the source match string to request that a stack trace be emitted for every log message: commit 548563956e484e0e43e9a66a89bdda0f95930108 Author: Daniel P. Berrange Date: Wed May 9 15:18:56 2012 +0100 Allow stack traces to be included with log messages Sometimes it is useful to see the callpath for log messages. This change enhances the log filter syntax so that stack traces can be show by setting '1:+NAME' instead of '1:NAME'. With the huge & ever increasing number of logging statements per file, this will be incredibly verbose and have a major performance penalty. This makes the feature impractical to use widely and as such it is not worth the code maint cost. Removing this seldom used feature allows us to drop the 'execinfo' module in gnulib which provides the backtrace() function which doesn't exist on non-Linux. Users who want to get stack traces of parts of libvirt can use GDB, or systemtap for live tracing with minimal perf impact. Signed-off-by: Daniel P. Berrangé --- bootstrap.conf | 1 - docs/logging.html.in| 7 ++-- src/locking/virtlockd.conf | 4 --- src/logging/virtlogd.conf | 4 --- src/remote/libvirtd.conf.in | 4 --- src/util/virlog.c | 68 ++--- src/util/virlog.h | 11 +- tests/testutils.c | 2 -- tests/virtestmock.c | 1 - 9 files changed, 14 insertions(+), 88 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 358d783a6b..55997b018f 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -35,7 +35,6 @@ connect configmake dirname-lgpl environ -execinfo fclose fcntl fcntl-h diff --git a/docs/logging.html.in b/docs/logging.html.in index be2fd4ab5b..d62220a9c1 100644 --- a/docs/logging.html.in +++ b/docs/logging.html.in @@ -102,16 +102,13 @@ variables. The format for a filter is one of: d/ one of/ -x:name (log message only) -x:+name (log message + stack trace) +x:name (log message only) I guess "log message only" no longer makes sense here either where name is a string which is matched against the category given in the VIR_LOG_INIT() at the top of each libvirt source file, e.g., remote, qemu, or util.json (the name in the filter can be a substring of the full category name, in order to match multiple -similar categories), the optional + prefix tells -libvirt to log stack trace for each message -matching name, and x is the minimal +similar categories), and x is the minimal level where matching messages should be logged: 1: DEBUG diff --git a/src/locking/virtlockd.conf b/src/locking/virtlockd.conf index b65110fc3e..b75eb3b279 100644 --- a/src/locking/virtlockd.conf +++ b/src/locking/virtlockd.conf @@ -26,7 +26,6 @@ # of logs. The format for a filter is one of: # d/ one of/ #level:match -#level:+match # # where 'match' is a string which is matched against the category # given in the VIR_LOG_INIT() at the top of each libvirt source @@ -35,9 +34,6 @@ # The 'match' is always treated as a substring match. IOW a match # string 'foo' is equivalent to '*foo*'. # -# If 'match' contains the optional "+" prefix, it tells libvirt -# to log stack trace for each message matching name. -# # 'level' is the minimal level where matching messages should # be logged: # diff --git a/src/logging/virtlogd.conf b/src/logging/virtlogd.conf index bc41edbc6b..9f2d36c382 100644 --- a/src/logging/virtlogd.conf +++ b/src/logging/virtlogd.conf @@ -26,7 +26,6 @@ # of logs. The format for a filter is one of: # d/ one of/ #level:match -#level:+match # # where 'match' is a string which is matched against the category # given in the VIR_LOG_INIT() at the top of each libvirt source @@ -35,9 +34,6 @@ # The 'match' is always treated as a substring match. IOW a match # string 'foo' is equivalent to '*foo*'. # -# If 'match' contains the optional "+" prefix, it tells libvirt -# to log stack trace for each message matching name. -# # 'level' is the minimal level where matching messages should # be logged: # diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index fdef97f371..50e3a00854 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -372,7 +372,6 @@ # of logs. The format for a filter is one of: # d/ one of/ #level:match -#level:+match # # where 'match' is a string which is matched against the category # given in the VIR_LOG_INIT() at the top of each libvirt source @@ -381,9 +380,6 @@ # The 'match' is always treated as a substring match. IOW a match # string 'foo' is equivalent to '*foo*'. # -# If 'match' contains the optional "+" prefix, it tells libvirt -# to log stack trace for each message matching name. -# # 'level' is the minimal level where matching messages should # be logged: # diff --git a/src/util/virlog.c
Re: [libvirt] [PATCH v3 19/19] build: remove use of usleep gnulib module in favour of g_usleep
On Thu, Oct 10, 2019 at 11:54:13AM +0100, Daniel P. Berrangé wrote: The usleep function was missing on older mingw versions, but we can rely on it existing everywhere these days. It may only support times upto 1 second in duration though, so we'll prefer to use g_usleep instead. The commandhelper program is not changed since that can't link to glib. Fortunately it doesn't need to build on Windows platforms either. Signed-off-by: Daniel P. Berrangé --- bootstrap.conf | 1 - src/hyperv/hyperv_driver.c | 2 +- src/hyperv/hyperv_wmi.c | 4 ++-- src/locking/lock_daemon.c | 2 +- src/locking/lock_driver_sanlock.c | 2 +- src/lxc/lxc_controller.c| 2 +- src/lxc/lxc_driver.c| 2 +- src/lxc/lxc_process.c | 2 +- src/network/bridge_driver.c | 2 +- src/nwfilter/nwfilter_dhcpsnoop.c | 4 ++-- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/qemu/qemu_monitor_json.c| 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_tpm.c | 2 +- src/rpc/virnetsocket.c | 2 +- src/security/security_manager.c | 2 +- src/storage/storage_util.c | 4 ++-- src/util/vircgroup.c| 2 +- src/util/virfile.c | 2 +- src/util/virnetdev.c| 2 +- src/util/virnetdevip.c | 2 +- src/util/virnetdevmacvlan.c | 2 +- src/util/virnetdevvportprofile.c| 2 +- src/util/virpci.c | 8 src/util/virprocess.c | 4 ++-- src/util/virtime.c | 2 +- src/vbox/vbox_common.c | 2 +- tests/commandtest.c | 6 +++--- tests/eventtest.c | 4 ++-- tests/fdstreamtest.c| 4 ++-- tools/virsh-domain.c| 2 +- 31 files changed, 41 insertions(+), 42 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 18/19] util: replace strerror/strerror_r with g_strerror
On Thu, Oct 10, 2019 at 11:54:12AM +0100, Daniel P. Berrangé wrote: g_strerror is offers the safety/correctness benefits of strerror_r, with the API design convenience of strerror. Use of virStrerror should be eliminated through the codebase in favour of g_strerror. commandhelper.c is a special case as its a tiny single threaded test program, not linked to glib, so it just uses traditional strerror(). Signed-off-by: Daniel P. Berrangé --- bootstrap.conf | 2 -- docs/hacking.html.in | 4 src/util/virerror.c | 9 + src/util/virerror.h | 1 + tests/commandtest.c | 10 +- tests/qemumonitortestutils.c | 2 +- tests/seclabeltest.c | 4 ++-- tests/testutils.c| 6 +++--- tests/virhostcputest.c | 4 ++-- tests/virtestmock.c | 4 ++-- 10 files changed, 25 insertions(+), 21 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 01/19] build: probe for glib-2 library in configure
On Thu, Oct 10, 2019 at 11:53:55AM +0100, Daniel P. Berrangé wrote: Prepare for linking with glib by probing for it at configure time. Per supported platforms target, the min glib versions on relevant distros are: RHEL-8: 2.56.1 RHEL-7: 2.50.3 Debian (Buster): 2.58.3 Debian (Stretch): 2.50.3 OpenBSD (Ports): 2.58.3 FreeBSD (Ports): 2.56.3 OpenSUSE Leap 15: 2.54.3 SLE12-SP2: 2.48.2 Ubuntu (Xenial): 2.48.0 macOS (Homebrew): 2.56.0 This suggests that a minimum glib of 2.48 is a reasonable target. This aligns with the minimum version required by qemu too. We must disable the bad-function-cast warning as various GLib APIs and macros will trigger this. Reviewed-by: Ján Tomko Reviewed-by: Pavel Hrdina Signed-off-by: Daniel P. Berrangé --- .travis.yml | 1 + configure.ac| 2 ++ libvirt.spec.in | 1 + m4/virt-compile-warnings.m4 | 2 ++ m4/virt-glib.m4 | 36 mingw-libvirt.spec.in | 2 ++ 6 files changed, 44 insertions(+) create mode 100644 m4/virt-glib.m4 Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 02/19] build: link to glib library
On Thu, Oct 10, 2019 at 11:53:56AM +0100, Daniel P. Berrangé wrote: Add the main glib.h to internal.h so that all common code can use it. Historically glib allowed applications to register an alternative memory allocator, so mixing g_malloc/g_free with malloc/free was not safe. This was feature was dropped in 2.46.0 with: commit 3be6ed60aa58095691bd697344765e715a327fc1 Author: Alexander Larsson Date: Sat Jun 27 18:38:42 2015 +0200 Deprecate and drop support for memory vtables Applications are still encourged to match g_malloc/g_free, but it is no longer a mandatory requirement for correctness, just stylistic. This is explicitly clarified in commit 1f24b36607bf708f037396014b2cdbc08d67b275 Author: Daniel P. Berrangé Date: Thu Sep 5 14:37:54 2019 +0100 gmem: clarify that g_malloc always uses the system allocator Applications can still use custom allocators in general, but they must do this by linking to a library that replaces the core malloc/free implemenentation entirely, instead of via a glib specific call. This means that libvirt does not need to be concerned about use of g_malloc/g_free causing an ABI change in the public libary, and can avoid memory copying when talking to external libraries. This patch probes for glib, which provides the foundation layer with a collection of data structures, helper APIs, and platform portability logic. Later patches will introduce linkage to gobject which provides the object type system, built on glib, and gio which providing objects for various interesting tasks, most notably including DBus client and server support and portable sockets APIs, but much more too. Signed-off-by: Daniel P. Berrangé --- docs/hacking.html.in| 21 + src/Makefile.am | 3 +++ src/access/Makefile.inc.am | 4 +++- src/bhyve/Makefile.inc.am | 1 + src/interface/Makefile.inc.am | 1 + src/internal.h | 1 + src/libxl/Makefile.inc.am | 1 + src/locking/Makefile.inc.am | 9 - src/logging/Makefile.inc.am | 1 + src/lxc/Makefile.inc.am | 4 src/network/Makefile.inc.am | 2 ++ src/node_device/Makefile.inc.am | 5 - src/nwfilter/Makefile.inc.am| 1 + src/qemu/Makefile.inc.am| 1 + src/remote/Makefile.inc.am | 2 ++ src/secret/Makefile.inc.am | 1 + src/security/Makefile.inc.am| 1 + src/storage/Makefile.inc.am | 16 src/vbox/Makefile.inc.am| 1 + src/vz/Makefile.inc.am | 1 + tests/Makefile.am | 7 +-- tools/Makefile.am | 4 22 files changed, 83 insertions(+), 5 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: drop support for stack traces with logging
The log filters have supported the use of a "+" before the source match string to request that a stack trace be emitted for every log message: commit 548563956e484e0e43e9a66a89bdda0f95930108 Author: Daniel P. Berrange Date: Wed May 9 15:18:56 2012 +0100 Allow stack traces to be included with log messages Sometimes it is useful to see the callpath for log messages. This change enhances the log filter syntax so that stack traces can be show by setting '1:+NAME' instead of '1:NAME'. With the huge & ever increasing number of logging statements per file, this will be incredibly verbose and have a major performance penalty. This makes the feature impractical to use widely and as such it is not worth the code maint cost. Removing this seldom used feature allows us to drop the 'execinfo' module in gnulib which provides the backtrace() function which doesn't exist on non-Linux. Users who want to get stack traces of parts of libvirt can use GDB, or systemtap for live tracing with minimal perf impact. Signed-off-by: Daniel P. Berrangé --- bootstrap.conf | 1 - docs/logging.html.in| 7 ++-- src/locking/virtlockd.conf | 4 --- src/logging/virtlogd.conf | 4 --- src/remote/libvirtd.conf.in | 4 --- src/util/virlog.c | 68 ++--- src/util/virlog.h | 11 +- tests/testutils.c | 2 -- tests/virtestmock.c | 1 - 9 files changed, 14 insertions(+), 88 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 358d783a6b..55997b018f 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -35,7 +35,6 @@ connect configmake dirname-lgpl environ -execinfo fclose fcntl fcntl-h diff --git a/docs/logging.html.in b/docs/logging.html.in index be2fd4ab5b..d62220a9c1 100644 --- a/docs/logging.html.in +++ b/docs/logging.html.in @@ -102,16 +102,13 @@ variables. The format for a filter is one of: -x:name (log message only) -x:+name (log message + stack trace) +x:name (log message only) where name is a string which is matched against the category given in the VIR_LOG_INIT() at the top of each libvirt source file, e.g., remote, qemu, or util.json (the name in the filter can be a substring of the full category name, in order to match multiple -similar categories), the optional + prefix tells -libvirt to log stack trace for each message -matching name, and x is the minimal +similar categories), and x is the minimal level where matching messages should be logged: 1: DEBUG diff --git a/src/locking/virtlockd.conf b/src/locking/virtlockd.conf index b65110fc3e..b75eb3b279 100644 --- a/src/locking/virtlockd.conf +++ b/src/locking/virtlockd.conf @@ -26,7 +26,6 @@ # of logs. The format for a filter is one of: # #level:match -#level:+match # # where 'match' is a string which is matched against the category # given in the VIR_LOG_INIT() at the top of each libvirt source @@ -35,9 +34,6 @@ # The 'match' is always treated as a substring match. IOW a match # string 'foo' is equivalent to '*foo*'. # -# If 'match' contains the optional "+" prefix, it tells libvirt -# to log stack trace for each message matching name. -# # 'level' is the minimal level where matching messages should # be logged: # diff --git a/src/logging/virtlogd.conf b/src/logging/virtlogd.conf index bc41edbc6b..9f2d36c382 100644 --- a/src/logging/virtlogd.conf +++ b/src/logging/virtlogd.conf @@ -26,7 +26,6 @@ # of logs. The format for a filter is one of: # #level:match -#level:+match # # where 'match' is a string which is matched against the category # given in the VIR_LOG_INIT() at the top of each libvirt source @@ -35,9 +34,6 @@ # The 'match' is always treated as a substring match. IOW a match # string 'foo' is equivalent to '*foo*'. # -# If 'match' contains the optional "+" prefix, it tells libvirt -# to log stack trace for each message matching name. -# # 'level' is the minimal level where matching messages should # be logged: # diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index fdef97f371..50e3a00854 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -372,7 +372,6 @@ # of logs. The format for a filter is one of: # #level:match -#level:+match # # where 'match' is a string which is matched against the category # given in the VIR_LOG_INIT() at the top of each libvirt source @@ -381,9 +380,6 @@ # The 'match' is always treated as a substring match. IOW a match # string 'foo' is equivalent to '*foo*'. # -# If 'match' contains the optional "+" prefix, it tells libvirt -# to log stack trace for each message matching name. -# # 'level' is the minimal level where matching messages should # be logged: # diff --git a/src/util/virlog.c b/src/util/virlog.c index 4c76fbc5a4..611475c8d7 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -28,7 +28,6 @@ #include
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On Thu, Oct 10, 2019 at 13:42:26 +0200, Philippe Mathieu-Daudé wrote: > On 10/10/19 1:26 PM, Peter Krempa wrote: > > On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote: > > > On 10/10/19 12:43 AM, John Snow wrote: > > > > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > > > > I'd like to refactor these some day, and getting rid of the super-object > > > > will make that easier. > > > > > > > > Either way, we don't need this. > > > > > > > > Libvirt-checked-by: Peter Krempa > > > > > > Peter made a comment regarding Laszlo's Regression-tested-by tag: > > > > > >[...] nobody else is using > > >this convention (there are exactly 0 instances of > > >"Regression-tested-by" in the project git log as far as > > >I can see), and so in practice people reading the commits > > >won't really know what you meant by it. Everybody else > > >on the project uses "Tested-by" to mean either of the > > >two cases you describe above, without distinction... > > > > > > It probably applies to 'Libvirt-checked-by' too. > > > > I certainly didn't test it. So feel free to drop that line altogether. > > But you reviewed it, can we use your 'Reviewed-by' instead? To be honest, I didn't really review the code nor the documentation. I actually reviewed only the idea itself in the context of integration with libvirt and that's why I didn't go for 'Reviewed-by:'. The gist of the citation above is that we should stick to well known tags with their well known meanings and I think that considering this a 'review' would be a stretch of the definiton. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On 10/10/19 1:26 PM, Peter Krempa wrote: On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote: On 10/10/19 12:43 AM, John Snow wrote: It's an old compatibility shim that just delegates to ide-cd or ide-hd. I'd like to refactor these some day, and getting rid of the super-object will make that easier. Either way, we don't need this. Libvirt-checked-by: Peter Krempa Peter made a comment regarding Laszlo's Regression-tested-by tag: [...] nobody else is using this convention (there are exactly 0 instances of "Regression-tested-by" in the project git log as far as I can see), and so in practice people reading the commits won't really know what you meant by it. Everybody else on the project uses "Tested-by" to mean either of the two cases you describe above, without distinction... It probably applies to 'Libvirt-checked-by' too. I certainly didn't test it. So feel free to drop that line altogether. But you reviewed it, can we use your 'Reviewed-by' instead? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-dbus] [PATCH 0/2] Implement snapshots
Implement snapshot interface and its APIs. Simon Kobyda (2): Introduce Domain Snapshot Interface Implement snapshots APIs data/org.libvirt.Domain.xml | 26 +++ data/org.libvirt.DomainSnapshot.xml | 41 src/connect.c | 6 + src/connect.h | 1 + src/domain.c| 158 src/domainsnapshot.c| 282 src/domainsnapshot.h| 9 + src/meson.build | 1 + src/util.c | 49 + src/util.h | 16 ++ tests/libvirttest.py| 14 ++ tests/meson.build | 1 + tests/test_domain.py| 8 + tests/test_snapshot.py | 43 + tests/xmldata.py| 6 + 15 files changed, 661 insertions(+) create mode 100644 data/org.libvirt.DomainSnapshot.xml create mode 100644 src/domainsnapshot.c create mode 100644 src/domainsnapshot.h create mode 100755 tests/test_snapshot.py -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-dbus] [PATCH 2/2] Implement snapshots APIs
Signed-off-by: Simon Kobyda
---
data/org.libvirt.Domain.xml | 26
data/org.libvirt.DomainSnapshot.xml | 34 +
src/domain.c| 158 +
src/domainsnapshot.c| 205 +++-
tests/libvirttest.py| 14 ++
tests/meson.build | 1 +
tests/test_domain.py| 8 ++
tests/test_snapshot.py | 43 ++
tests/xmldata.py| 6 +
9 files changed, 494 insertions(+), 1 deletion(-)
create mode 100755 tests/test_snapshot.py
diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index b4ed495..f03faf8 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -350,6 +350,12 @@
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainListAllSnapshots"/>
+
+
+
https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainManagedSave"/>
@@ -589,6 +595,26 @@
value="See
https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainShutdownFlags"/>
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotCurrent"/>
+
+
+
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotCreateXML"/>
+
+
+
+
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotLookupByName"/>
+
+
+
+
https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSuspend"/>
diff --git a/data/org.libvirt.DomainSnapshot.xml
b/data/org.libvirt.DomainSnapshot.xml
index 80f61c6..463654f 100644
--- a/data/org.libvirt.DomainSnapshot.xml
+++ b/data/org.libvirt.DomainSnapshot.xml
@@ -3,5 +3,39 @@
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotDelete"/>
+
+
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotGetParent"/>
+
+
+
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotGetXMLDesc"/>
+
+
+
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotIsCurrent"/>
+
+
+
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainSnapshotListAllChildren"/>
+
+
+
+
+ https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html#virDomainRevertToSnapshot"/>
+
+
diff --git a/src/domain.c b/src/domain.c
index 10fa8de..3fc5bab 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -1857,6 +1857,50 @@ virtDBusDomainInjectNMI(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusDomainListDomainSnapshots(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath,
+ gpointer userData,
+ GVariant **outArgs G_GNUC_UNUSED,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virDomain) domain = NULL;
+g_autoptr(virDomainSnapshotPtr) domainSnapshots = NULL;
+guint flags;
+GVariantBuilder builder;
+GVariant *gdomainSnapshots;
+
+g_variant_get(inArgs, "(u)", );
+
+domain = virtDBusDomainGetVirDomain(connect, objectPath,
+error);
+if (!domain)
+return;
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+if (virDomainListAllSnapshots(domain, , flags) < 0)
+return virtDBusUtilSetLastVirtError(error);
+
+g_variant_builder_init(, G_VARIANT_TYPE("ao"));
+
+for (gint i = 0; domainSnapshots[i]; i++) {
+g_autofree gchar *path = NULL;
+path = virtDBusUtilBusPathForVirDomainSnapshot(domain,
+ domainSnapshots[i],
+
connect->domainSnapshotPath);
+
+g_variant_builder_add(, "o", path);
+}
+
+gdomainSnapshots = g_variant_builder_end();
+*outArgs = g_variant_new_tuple(, 1);
+}
+
static void
virtDBusDomainInterfaceAddresses(GVariant *inArgs,
GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -2966,6 +3010,116 @@ virtDBusDomainShutdown(GVariant *inArgs,
virtDBusUtilSetLastVirtError(error);
}
+static void
+virtDBusDomainSnapshotCurrent(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath,
+ gpointer userData,
+ GVariant **outArgs G_GNUC_UNUSED,
+ GUnixFDList
[libvirt] [libvirt-dbus] [PATCH 1/2] Introduce Domain Snapshot Interface
Signed-off-by: Simon Kobyda
---
data/org.libvirt.DomainSnapshot.xml | 7 +++
src/connect.c | 6 +++
src/connect.h | 1 +
src/domainsnapshot.c| 79 +
src/domainsnapshot.h| 9
src/meson.build | 1 +
src/util.c | 49 ++
src/util.h | 16 ++
8 files changed, 168 insertions(+)
create mode 100644 data/org.libvirt.DomainSnapshot.xml
create mode 100644 src/domainsnapshot.c
create mode 100644 src/domainsnapshot.h
diff --git a/data/org.libvirt.DomainSnapshot.xml
b/data/org.libvirt.DomainSnapshot.xml
new file mode 100644
index 000..80f61c6
--- /dev/null
+++ b/data/org.libvirt.DomainSnapshot.xml
@@ -0,0 +1,7 @@
+http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd;>
+
+
+
+
+
diff --git a/src/connect.c b/src/connect.c
index f8f76a2..b40b0ba 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -1,5 +1,6 @@
#include "connect.h"
#include "domain.h"
+#include "domainsnapshot.h"
#include "events.h"
#include "interface.h"
#include "network.h"
@@ -2002,6 +2003,7 @@ virtDBusConnectFree(virtDBusConnect *connect)
virtDBusConnectClose(connect, TRUE);
g_free(connect->domainPath);
+g_free(connect->domainSnapshotPath);
g_free(connect->interfacePath);
g_free(connect->networkPath);
g_free(connect->nodeDevPath);
@@ -2062,6 +2064,10 @@ virtDBusConnectNew(virtDBusConnect **connectp,
if (error && *error)
return;
+virtDBusDomainSnapshotRegister(connect, error);
+if (error && *error)
+return;
+
virtDBusInterfaceRegister(connect, error);
if (error && *error)
return;
diff --git a/src/connect.h b/src/connect.h
index 829bd57..74c89cf 100644
--- a/src/connect.h
+++ b/src/connect.h
@@ -13,6 +13,7 @@ struct virtDBusConnect {
const gchar *uri;
const gchar *connectPath;
gchar *domainPath;
+gchar *domainSnapshotPath;
gchar *interfacePath;
gchar *networkPath;
gchar *nodeDevPath;
diff --git a/src/domainsnapshot.c b/src/domainsnapshot.c
new file mode 100644
index 000..590cbef
--- /dev/null
+++ b/src/domainsnapshot.c
@@ -0,0 +1,79 @@
+#include "domainsnapshot.h"
+#include "util.h"
+
+#include
+
+static virtDBusGDBusPropertyTable virtDBusDomainSnapshotPropertyTable[] = {
+{ 0 }
+};
+
+static virtDBusGDBusMethodTable virtDBusDomainSnapshotMethodTable[] = {
+{ 0 }
+};
+
+static gchar **
+virtDBusDomainSnapshotEnumerate(gpointer userData)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virDomainPtr) domains = NULL;
+gint numDoms = 0;
+GPtrArray *list = NULL;
+
+if (!virtDBusConnectOpen(connect, NULL))
+return NULL;
+
+numDoms = virConnectListAllDomains(connect->connection, , 0);
+if (numDoms <= 0)
+return NULL;
+
+list = g_ptr_array_new();
+
+for (gint i = 0; i < numDoms; i++) {
+g_autoptr(virDomainSnapshotPtr) domainSnapshots = NULL;
+gint numSnaps;
+
+numSnaps = virDomainListAllSnapshots(domains[i], , 0);
+if (numSnaps <= 0)
+continue;
+
+for (gint j = 0; j < numSnaps; j++) {
+gchar *snapPath =
virtDBusUtilBusPathForVirDomainSnapshot(domains[i],
+
domainSnapshots[j],
+
connect->domainSnapshotPath);
+g_ptr_array_add(list, snapPath);
+}
+}
+
+gchar **ret = g_new0(gchar *, numDoms + 1);
+
+for (gint i = 0; i < numDoms; i++) {
+ret[i] = virtDBusUtilBusPathForVirDomain(domains[i],
+ connect->domainPath);
+}
+
+return ret;
+}
+
+static GDBusInterfaceInfo *interfaceInfo = NULL;
+
+void
+virtDBusDomainSnapshotRegister(virtDBusConnect *connect,
+ GError **error)
+{
+connect->domainSnapshotPath = g_strdup_printf("%s/snapshot",
connect->connectPath);
+
+if (!interfaceInfo) {
+interfaceInfo =
virtDBusGDBusLoadIntrospectData(VIRT_DBUS_DOMAIN_SNAPSHOT_INTERFACE,
+error);
+if (!interfaceInfo)
+return;
+}
+
+virtDBusGDBusRegisterSubtree(connect->bus,
+ connect->domainSnapshotPath,
+ interfaceInfo,
+ virtDBusDomainSnapshotEnumerate,
+ virtDBusDomainSnapshotMethodTable,
+ virtDBusDomainSnapshotPropertyTable,
+ connect);
+}
diff --git a/src/domainsnapshot.h b/src/domainsnapshot.h
new file mode 100644
index 000..7d8a938
--- /dev/null
+++ b/src/domainsnapshot.h
@@ -0,0 +1,9 @@
+#pragma once
+
+#include "connect.h"
+
+#define
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On Thu, Oct 10, 2019 at 13:22:37 +0200, Philippe Mathieu-Daudé wrote: > On 10/10/19 12:43 AM, John Snow wrote: > > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > > I'd like to refactor these some day, and getting rid of the super-object > > will make that easier. > > > > Either way, we don't need this. > > > > Libvirt-checked-by: Peter Krempa > > Peter made a comment regarding Laszlo's Regression-tested-by tag: > > [...] nobody else is using > this convention (there are exactly 0 instances of > "Regression-tested-by" in the project git log as far as > I can see), and so in practice people reading the commits > won't really know what you meant by it. Everybody else > on the project uses "Tested-by" to mean either of the > two cases you describe above, without distinction... > > It probably applies to 'Libvirt-checked-by' too. I certainly didn't test it. So feel free to drop that line altogether. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 12/19] admin: convert admin server code to use auto free macros
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
src/admin/admin_server.c | 204 ---
1 file changed, 85 insertions(+), 119 deletions(-)
diff --git a/src/admin/admin_server.c b/src/admin/admin_server.c
index 0d6091937d..ba87f701c3 100644
--- a/src/admin/admin_server.c
+++ b/src/admin/admin_server.c
@@ -75,15 +75,13 @@ adminServerGetThreadPoolParameters(virNetServerPtr srv,
int *nparams,
unsigned int flags)
{
-int ret = -1;
-int maxparams = 0;
size_t minWorkers;
size_t maxWorkers;
size_t nWorkers;
size_t freeWorkers;
size_t nPrioWorkers;
size_t jobQueueDepth;
-virTypedParameterPtr tmpparams = NULL;
+g_autoptr(virTypedParamList) paramlist = g_new0(virTypedParamList, 1);
virCheckFlags(0, -1);
@@ -93,46 +91,36 @@ adminServerGetThreadPoolParameters(virNetServerPtr srv,
) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Unable to retrieve threadpool parameters"));
-goto cleanup;
+return -1;
}
-if (virTypedParamsAddUInt(, nparams,
- , VIR_THREADPOOL_WORKERS_MIN,
- minWorkers) < 0)
-goto cleanup;
+if (virTypedParamListAddUInt(paramlist, minWorkers,
+ "%s", VIR_THREADPOOL_WORKERS_MIN) < 0)
+return -1;
-if (virTypedParamsAddUInt(, nparams,
- , VIR_THREADPOOL_WORKERS_MAX,
- maxWorkers) < 0)
-goto cleanup;
+if (virTypedParamListAddUInt(paramlist, maxWorkers,
+ "%s", VIR_THREADPOOL_WORKERS_MAX) < 0)
+return -1;
-if (virTypedParamsAddUInt(, nparams,
- , VIR_THREADPOOL_WORKERS_CURRENT,
- nWorkers) < 0)
-goto cleanup;
+if (virTypedParamListAddUInt(paramlist, nWorkers,
+ "%s", VIR_THREADPOOL_WORKERS_CURRENT) < 0)
+return -1;
-if (virTypedParamsAddUInt(, nparams,
- , VIR_THREADPOOL_WORKERS_FREE,
- freeWorkers) < 0)
-goto cleanup;
+if (virTypedParamListAddUInt(paramlist, freeWorkers,
+ "%s", VIR_THREADPOOL_WORKERS_FREE) < 0)
+return -1;
-if (virTypedParamsAddUInt(, nparams,
- , VIR_THREADPOOL_WORKERS_PRIORITY,
- nPrioWorkers) < 0)
-goto cleanup;
+if (virTypedParamListAddUInt(paramlist, nPrioWorkers,
+ "%s", VIR_THREADPOOL_WORKERS_PRIORITY) < 0)
+return -1;
-if (virTypedParamsAddUInt(, nparams,
- , VIR_THREADPOOL_JOB_QUEUE_DEPTH,
- jobQueueDepth) < 0)
-goto cleanup;
+if (virTypedParamListAddUInt(paramlist, jobQueueDepth,
+ "%s", VIR_THREADPOOL_JOB_QUEUE_DEPTH) < 0)
+return -1;
-*params = tmpparams;
-tmpparams = NULL;
-ret = 0;
+*nparams = virTypedParamListStealParams(paramlist, params);
- cleanup:
-virTypedParamsFree(tmpparams, *nparams);
-return ret;
+return 0;
}
int
@@ -215,106 +203,90 @@ adminClientGetInfo(virNetServerClientPtr client,
int *nparams,
unsigned int flags)
{
-int ret = -1;
-int maxparams = 0;
bool readonly;
-char *sock_addr = NULL;
+g_autofree char *sock_addr = NULL;
const char *attr = NULL;
-virTypedParameterPtr tmpparams = NULL;
-virIdentityPtr identity = NULL;
+g_autoptr(virTypedParamList) paramlist = g_new0(virTypedParamList, 1);
+g_autoptr(virIdentity) identity = NULL;
int rc;
virCheckFlags(0, -1);
if (virNetServerClientGetInfo(client, ,
_addr, ) < 0)
-goto cleanup;
+return -1;
-if (virTypedParamsAddBoolean(, nparams, ,
- VIR_CLIENT_INFO_READONLY,
- readonly) < 0)
-goto cleanup;
+if (virTypedParamListAddBoolean(paramlist, readonly,
+"%s", VIR_CLIENT_INFO_READONLY) < 0)
+return -1;
if ((rc = virIdentityGetSASLUserName(identity, )) < 0)
-goto cleanup;
+return -1;
if (rc == 1 &&
-virTypedParamsAddString(, nparams, ,
-VIR_CLIENT_INFO_SASL_USER_NAME,
-attr) < 0)
-goto cleanup;
+virTypedParamListAddString(paramlist, attr,
+ "%s", VIR_CLIENT_INFO_SASL_USER_NAME) < 0)
+return -1;
if (!virNetServerClientIsLocal(client)) {
-if (virTypedParamsAddString(, nparams, ,
-
[libvirt] [PATCH v3 19/19] build: remove use of usleep gnulib module in favour of g_usleep
The usleep function was missing on older mingw versions, but we can rely
on it existing everywhere these days. It may only support times upto 1
second in duration though, so we'll prefer to use g_usleep instead.
The commandhelper program is not changed since that can't link to glib.
Fortunately it doesn't need to build on Windows platforms either.
Signed-off-by: Daniel P. Berrangé
---
bootstrap.conf | 1 -
src/hyperv/hyperv_driver.c | 2 +-
src/hyperv/hyperv_wmi.c | 4 ++--
src/locking/lock_daemon.c | 2 +-
src/locking/lock_driver_sanlock.c | 2 +-
src/lxc/lxc_controller.c| 2 +-
src/lxc/lxc_driver.c| 2 +-
src/lxc/lxc_process.c | 2 +-
src/network/bridge_driver.c | 2 +-
src/nwfilter/nwfilter_dhcpsnoop.c | 4 ++--
src/nwfilter/nwfilter_learnipaddr.c | 2 +-
src/qemu/qemu_monitor_json.c| 2 +-
src/qemu/qemu_process.c | 2 +-
src/qemu/qemu_tpm.c | 2 +-
src/rpc/virnetsocket.c | 2 +-
src/security/security_manager.c | 2 +-
src/storage/storage_util.c | 4 ++--
src/util/vircgroup.c| 2 +-
src/util/virfile.c | 2 +-
src/util/virnetdev.c| 2 +-
src/util/virnetdevip.c | 2 +-
src/util/virnetdevmacvlan.c | 2 +-
src/util/virnetdevvportprofile.c| 2 +-
src/util/virpci.c | 8
src/util/virprocess.c | 4 ++--
src/util/virtime.c | 2 +-
src/vbox/vbox_common.c | 2 +-
tests/commandtest.c | 6 +++---
tests/eventtest.c | 4 ++--
tests/fdstreamtest.c| 4 ++--
tools/virsh-domain.c| 2 +-
31 files changed, 41 insertions(+), 42 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 241dce50c2..1b5a68b873 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -90,7 +90,6 @@ timegm
ttyname_r
uname
unsetenv
-usleep
verify
vsnprintf
waitpid
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 0e2c6c55ef..ceaf528dd3 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1415,7 +1415,7 @@ hypervDomainSendKey(virDomainPtr domain, unsigned int
codeset,
/* simulate holdtime by sleeping */
if (holdtime > 0)
-usleep(holdtime * 1000);
+g_usleep(holdtime * 1000);
/* release the keys */
for (i = 0; i < nkeycodes; i++) {
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index 0f39bd4431..c2c1f082e1 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -909,7 +909,7 @@ hypervInvokeMethod(hypervPrivate *priv,
hypervInvokeParamsListPtr params,
case MSVM_CONCRETEJOB_JOBSTATE_SHUTTING_DOWN:
hypervFreeObject(priv, (hypervObject *)job);
job = NULL;
-usleep(100 * 1000); /* sleep 100 ms */
+g_usleep(100 * 1000); /* sleep 100 ms */
timeout -= 100;
continue;
case MSVM_CONCRETEJOB_JOBSTATE_COMPLETED:
@@ -1418,7 +1418,7 @@
hypervInvokeMsvmComputerSystemRequestStateChange(virDomainPtr domain,
hypervFreeObject(priv, (hypervObject *)concreteJob);
concreteJob = NULL;
-usleep(100 * 1000);
+g_usleep(100 * 1000);
continue;
case MSVM_CONCRETEJOB_JOBSTATE_COMPLETED:
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index a5a3a97e99..c12cb4ea0f 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -657,7 +657,7 @@ virLockDaemonClientFree(void *opaque)
VIR_WARN("Failed to kill off pid %lld",
(unsigned long long)priv->clientPid);
}
-usleep(200 * 1000);
+g_usleep(200 * 1000);
}
}
}
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index 85a23c7642..7ebd63913e 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -376,7 +376,7 @@
virLockManagerSanlockSetupLockspace(virLockManagerSanlockDriverPtr driver)
#else
/* fall back to polling */
VIR_DEBUG("Sleeping for %dms", LOCKSPACE_SLEEP);
-usleep(LOCKSPACE_SLEEP * 1000);
+g_usleep(LOCKSPACE_SLEEP * 1000);
#endif
VIR_DEBUG("Retrying to add lockspace (left %d)", retries);
goto retry;
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 37851bf284..9097655b4d 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -574,7 +574,7 @@ static int
virLXCControllerAppendNBDPids(virLXCControllerPtr ctrl,
while (!virFileExists(pidpath)) {
/* wait for 100ms before checking again, but don't do it for ever */
if
[libvirt] [PATCH v3 15/19] util: convert virIdentity class to use GObject
Converting from virObject to GObject is reasonably straightforward,
as illustrated by this patch for virIdentity
In the header file
- Remove
typedef struct _virIdentity virIdentity
- Add
#define VIR_TYPE_IDENTITY virIdentity_get_type ()
G_DECLARE_FINAL_TYPE (virIdentity, vir_identity, VIR, IDENTITY, GObject);
Which provides the typedef we just removed, and class
declaration boilerplate and various other constants/macros.
In the source file
- Change 'virObject parent' to 'GObject parent' in the struct
- Remove the virClass variable and its initializing call
- Add
G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT)
which declares the instance & class constructor functions
- Add an impl of the instance & class constructors
wiring up the finalize method to point to our dispose impl
In all files
- Replace VIR_AUTOUNREF(virIdentityPtr) with g_autoptr(virIdentity)
- Replace virObjectRef/Unref with g_object_ref/unref. Note
the latter functions do *NOT* accept a NULL object where as
libvirt's do. If you replace g_object_unref with g_clear_object
it is NULL safe, but also clears the pointer.
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
m4/virt-glib.m4 | 4 +--
src/qemu/qemu_process.c | 4 +--
src/rpc/virnetserverclient.c | 10 +++
src/util/viridentity.c | 56 ++--
src/util/viridentity.h | 9 +++---
tests/viridentitytest.c | 5 +---
6 files changed, 49 insertions(+), 39 deletions(-)
diff --git a/m4/virt-glib.m4 b/m4/virt-glib.m4
index 5a5bc19660..eb2c77b25b 100644
--- a/m4/virt-glib.m4
+++ b/m4/virt-glib.m4
@@ -24,10 +24,10 @@ AC_DEFUN([LIBVIRT_ARG_GLIB], [
AC_DEFUN([LIBVIRT_CHECK_GLIB],[
GLIB_REQUIRED=2.48.0
- LIBVIRT_CHECK_PKG([GLIB], [glib-2.0], [$GLIB_REQUIRED])
+ LIBVIRT_CHECK_PKG([GLIB], [glib-2.0 gobject-2.0], [$GLIB_REQUIRED])
if test "$with_glib" = "no" ; then
-AC_MSG_ERROR([glib-2.0 >= $GLIB_REQUIRED is required for libvirt])
+AC_MSG_ERROR([glib-2.0, gobject-2.0 >= $GLIB_REQUIRED are required for
libvirt])
fi
])
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c14c09da11..3b45b2f641 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8040,7 +8040,7 @@ qemuProcessReconnect(void *opaque)
bool tryMonReconn = false;
virIdentitySetCurrent(data->identity);
-virObjectUnref(data->identity);
+g_clear_object(>identity);
VIR_FREE(data);
qemuDomainObjRestoreJob(obj, );
@@ -8353,7 +8353,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
virDomainObjEndAPI();
virNWFilterUnlockFilterUpdates();
-virObjectUnref(data->identity);
+g_clear_object(>identity);
VIR_FREE(data);
return -1;
}
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 79287572b6..8482c5c29c 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -829,7 +829,7 @@ virIdentityPtr
virNetServerClientGetIdentity(virNetServerClientPtr client)
if (!client->identity)
client->identity = virNetServerClientCreateIdentity(client);
if (client->identity)
-ret = virObjectRef(client->identity);
+ret = g_object_ref(client->identity);
virObjectUnlock(client);
return ret;
}
@@ -839,10 +839,10 @@ void virNetServerClientSetIdentity(virNetServerClientPtr
client,
virIdentityPtr identity)
{
virObjectLock(client);
-virObjectUnref(client->identity);
+g_clear_object(>identity);
client->identity = identity;
if (client->identity)
-virObjectRef(client->identity);
+g_object_ref(client->identity);
virObjectUnlock(client);
}
@@ -979,7 +979,7 @@ void virNetServerClientDispose(void *obj)
if (client->privateData)
client->privateDataFreeFunc(client->privateData);
-virObjectUnref(client->identity);
+g_clear_object(>identity);
#if WITH_SASL
virObjectUnref(client->sasl);
@@ -1674,7 +1674,7 @@ virNetServerClientGetInfo(virNetServerClientPtr client,
goto cleanup;
}
-*identity = virObjectRef(client->identity);
+*identity = g_object_ref(client->identity);
ret = 0;
cleanup:
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 6636077161..8cc2db2568 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -43,25 +43,29 @@
VIR_LOG_INIT("util.identity");
struct _virIdentity {
-virObject parent;
+GObject parent;
int nparams;
int maxparams;
virTypedParameterPtr params;
};
-static virClassPtr virIdentityClass;
+G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT)
+
static virThreadLocal virIdentityCurrent;
-static void virIdentityDispose(void *obj);
+static void virIdentityFinalize(GObject *obj);
-static int virIdentityOnceInit(void)
+static void virIdentityCurrentCleanup(void
[libvirt] [PATCH v3 06/19] util: rewrite auto cleanup macros to use glib's equivalent
To facilitate porting over to glib, this rewrites the auto cleanup
macros to use glib's equivalent.
As a result it is now possible to use g_autoptr/VIR_AUTOPTR, and
g_auto/VIR_AUTOCLEAN, g_autofree/VIR_AUTOFREE interchangably, regardless
of which macros were used to declare the cleanup types.
Within the scope of any single method, code must remain consistent
using either GLib or Libvirt macros, never mixing both. New code
must preferentially use the GLib macros, and old code will be
converted incrementally.
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
build-aux/syntax-check.mk | 2 +-
docs/hacking.html.in| 18 ++
m4/virt-compile-warnings.m4 | 21
src/util/viralloc.h | 5 -
src/util/virautoclean.h | 38 +++--
5 files changed, 64 insertions(+), 20 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 8345703b3e..f4712c24c3 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -1128,7 +1128,7 @@ sc_prohibit_backslash_alignment:
# Rule to ensure that variables declared using a cleanup macro are
# always initialized.
sc_require_attribute_cleanup_initialization:
- @prohibit='VIR_AUTO((FREE|PTR|UNREF|CLEAN)\(.+\)|CLOSE|STRINGLIST)
*[^=]+;' \
+
@prohibit='((g_auto(ptr|free)?)|(VIR_AUTO((FREE|PTR|UNREF|CLEAN)\(.+\)|CLOSE|STRINGLIST)))
*[^=]+;' \
in_vc_files='\.[chx]$$' \
halt='variable declared with a cleanup macro must be initialized' \
$(_sc_search_regexp)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 3f1542b6de..6e62b2d4ff 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1028,6 +1028,24 @@ BAD:
The GLib APIs g_strdup_printf / g_strdup_vprint should be used
instead. Don't use g_vasprintf unless having the string length
returned is unavoidable.
+
+ VIR_AUTOPTR, VIR_AUTOCLEAN, VIR_AUTOFREE
+ The GLib macros g_autoptr, g_auto and g_autofree must be used
+instead in all new code. In existing code, the GLib macros must
+never be mixed with libvirt macros within a method, nor should
+they be mixed with VIR_FREE. If introducing GLib macros to an
+existing method, any use of libvirt macros must be converted
+in an independent commit.
+
+
+ VIR_DEFINE_AUTOPTR_FUNC, VIR_DEFINE_AUTOCLEAN_FUNC
+ The Gib macros G_DEFINE_AUTOPTR_CLEANUP_FUNC and
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC must be used in all
+new code. Existing code should be converted to the
+new macros where relevant. It is permissible to use
+g_autoptr, g_auto on an object whose cleanup function
+is declared with the libvirt macros and vice-verca.
+
File handling
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index 1dbe1abe27..7c86fdd3c6 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -104,6 +104,20 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
dontwarn="$dontwarn -Wdouble-promotion"
fi
+# Clang complains about unused static inline functions
+# which are common with G_DEFINE_AUTOPTR_CLEANUP_FUNC
+AC_CACHE_CHECK([whether clang gives bogus warnings for -Wunused-function],
+ [lv_cv_clang_unused_function_broken], [
+save_CFLAGS="$CFLAGS"
+CFLAGS="-Wunused-function -Werror"
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+ static inline void foo(void) {}
+]], [[
+ return 0]])],
+[lv_cv_clang_unused_function_broken=no],
+[lv_cv_clang_unused_function_broken=yes])
+CFLAGS="$save_CFLAGS"])
+
# We might fundamentally need some of these disabled forever, but
# ideally we'd turn many of them on
dontwarn="$dontwarn -Wfloat-equal"
@@ -119,6 +133,13 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
# Remove the ones we don't want, blacklisted earlier
gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn])
+# -Wunused-functin is implied by -Wall we must turn it
+# off explicitly.
+if test "$lv_cv_clang_unused_function_broken" = "yes";
+then
+ wantwarn="$wantwarn -Wno-unused-function"
+fi
+
# GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff.
# Unfortunately, this means you can't simply use '-Wsign-compare'
# with gl_MANYWARN_COMPLEMENT
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 517f9aada6..49bf2b86e7 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -494,8 +494,11 @@ void virDisposeString(char **strptr)
* VIR_AUTOFREE:
* @type: type of the variable to be freed automatically
*
+ * DEPRECATED: use g_autofree for new code. See HACKING
+ * for further guidance.
+ *
* Macro to automatically free the memory allocated to
* the variable declared with it by calling virFree
* when the variable goes out of scope.
*/
-#define
[libvirt] [PATCH v3 08/19] conf: convert virSecretObj APIs to use autofree
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
src/conf/virsecretobj.c | 46 +++--
1 file changed, 17 insertions(+), 29 deletions(-)
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 7800912bff..aeae82332b 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -678,43 +678,33 @@ virSecretObjDeleteData(virSecretObjPtr obj)
int
virSecretObjSaveConfig(virSecretObjPtr obj)
{
-char *xml = NULL;
-int ret = -1;
+g_autofree char *xml = NULL;
if (!(xml = virSecretDefFormat(obj->def)))
-goto cleanup;
+return -1;
if (virFileRewriteStr(obj->configFile, S_IRUSR | S_IWUSR, xml) < 0)
-goto cleanup;
-
-ret = 0;
+return -1;
- cleanup:
-VIR_FREE(xml);
-return ret;
+return 0;
}
int
virSecretObjSaveData(virSecretObjPtr obj)
{
-char *base64 = NULL;
-int ret = -1;
+g_autofree char *base64 = NULL;
if (!obj->value)
return 0;
if (!(base64 = virStringEncodeBase64(obj->value, obj->value_size)))
-goto cleanup;
+return -1;
if (virFileRewriteStr(obj->base64File, S_IRUSR | S_IWUSR, base64) < 0)
-goto cleanup;
-
-ret = 0;
+return -1;
- cleanup:
-VIR_FREE(base64);
-return ret;
+return 0;
}
@@ -762,7 +752,8 @@ virSecretObjSetValue(virSecretObjPtr obj,
size_t value_size)
{
virSecretDefPtr def = obj->def;
-unsigned char *old_value, *new_value;
+g_autofree unsigned char *old_value = NULL;
+g_autofree unsigned char *new_value = NULL;
size_t old_value_size;
if (VIR_ALLOC_N(new_value, value_size) < 0)
@@ -772,26 +763,24 @@ virSecretObjSetValue(virSecretObjPtr obj,
old_value_size = obj->value_size;
memcpy(new_value, value, value_size);
-obj->value = new_value;
+obj->value = g_steal_pointer(_value);
obj->value_size = value_size;
if (!def->isephemeral && virSecretObjSaveData(obj) < 0)
goto error;
/* Saved successfully - drop old value */
-if (old_value) {
+if (old_value)
memset(old_value, 0, old_value_size);
-VIR_FREE(old_value);
-}
return 0;
error:
/* Error - restore previous state and free new value */
-obj->value = old_value;
+new_value = g_steal_pointer(>value);
+obj->value = g_steal_pointer(_value);
obj->value_size = old_value_size;
memset(new_value, 0, value_size);
-VIR_FREE(new_value);
return -1;
}
@@ -835,7 +824,8 @@ virSecretLoadValue(virSecretObjPtr obj)
{
int ret = -1, fd = -1;
struct stat st;
-char *contents = NULL, *value = NULL;
+g_autofree char *contents = NULL;
+char *value = NULL;
size_t value_size;
if ((fd = open(obj->base64File, O_RDONLY)) == -1) {
@@ -892,10 +882,8 @@ virSecretLoadValue(virSecretObjPtr obj)
memset(value, 0, value_size);
VIR_FREE(value);
}
-if (contents != NULL) {
+if (contents != NULL)
memset(contents, 0, st.st_size);
-VIR_FREE(contents);
-}
VIR_FORCE_CLOSE(fd);
return ret;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 07/19] src: add support for g_autoptr with virObject instances
Libvirt currently uses the VIR_AUTOUNREF macro for auto cleanup of
virObject instances. GLib approaches things differently with GObject,
reusing their g_autoptr() concept.
This introduces support for g_autoptr() with virObject, to facilitate
the conversion to GObject.
Only virObject classes which are currently used with VIR_AUTOREF are
updated. Any others should be converted to GObject before introducing
use of autocleanup.
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
docs/hacking.html.in| 5 +
src/conf/capabilities.h | 3 +++
src/conf/domain_capabilities.h | 3 +++
src/conf/domain_conf.h | 3 +++
src/conf/snapshot_conf.h| 3 +++
src/conf/storage_capabilities.h | 3 +++
src/datatypes.h | 15 +++
src/libxl/libxl_conf.h | 2 ++
src/qemu/qemu_blockjob.h| 1 +
src/qemu/qemu_capabilities.h| 2 ++
src/qemu/qemu_conf.h| 3 +++
src/util/virhostdev.h | 3 +++
src/util/viridentity.h | 2 ++
src/util/virmdev.h | 3 +++
src/util/virobject.h| 4
src/util/virpci.h | 3 +++
src/util/virresctrl.h | 4
src/util/virscsi.h | 3 +++
src/util/virscsivhost.h | 3 +++
src/util/virstoragefile.h | 2 ++
src/util/virusb.h | 3 +++
21 files changed, 73 insertions(+)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 6e62b2d4ff..a92608cd3c 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1046,6 +1046,11 @@ BAD:
g_autoptr, g_auto on an object whose cleanup function
is declared with the libvirt macros and vice-verca.
+
+ VIR_AUTOUNREF
+ The GLib macros g_autoptr and G_DEFINE_AUTOPTR_CLEANUP_FUNC
+should be used to manage autoclean of virObject classes.
+This matches usage with GObject classes.
File handling
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index d6a4e79d77..4abd3dcabd 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -195,6 +195,9 @@ struct _virCaps {
virCapsStoragePoolPtr *pools;
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCaps, virObjectUnref);
+
+
struct _virCapsDomainData {
int ostype;
int arch;
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 4756af38e9..f5571b2188 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -185,6 +185,9 @@ struct _virDomainCaps {
/* add new domain features here */
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainCaps, virObjectUnref);
+
+
virDomainCapsPtr virDomainCapsNew(const char *path,
const char *machine,
virArch arch,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b7ae57aa9d..f7404b814f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2594,6 +2594,9 @@ struct _virDomainObj {
* restore will be required later */
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainObj, virObjectUnref);
+
+
typedef bool (*virDomainObjListACLFilter)(virConnectPtr conn,
virDomainDefPtr def);
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 17d614a7e1..7e2ffa9d60 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -87,6 +87,9 @@ struct _virDomainSnapshotDef {
virObjectPtr cookie;
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSnapshotDef, virObjectUnref);
+
+
typedef enum {
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE = 1 << 0,
VIR_DOMAIN_SNAPSHOT_PARSE_DISKS= 1 << 1,
diff --git a/src/conf/storage_capabilities.h b/src/conf/storage_capabilities.h
index 948e5bed5b..788ea227ea 100644
--- a/src/conf/storage_capabilities.h
+++ b/src/conf/storage_capabilities.h
@@ -30,6 +30,9 @@ struct _virStoragePoolCaps {
virCapsPtr driverCaps;
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStoragePoolCaps, virObjectUnref);
+
+
virStoragePoolCapsPtr
virStoragePoolCapsNew(virCapsPtr driverCaps);
diff --git a/src/datatypes.h b/src/datatypes.h
index 87e77fff79..16ab5b7282 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -548,6 +548,9 @@ struct _virConnect {
void *userData; /* the user data */
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virConnect, virObjectUnref);
+
+
/**
* _virAdmConnect:
*
@@ -616,6 +619,9 @@ struct _virNetwork {
unsigned char uuid[VIR_UUID_BUFLEN]; /* the network unique identifier */
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetwork, virObjectUnref);
+
+
/**
* _virNetworkPort:
*
@@ -627,6 +633,9 @@ struct _virNetworkPort {
unsigned char uuid[VIR_UUID_BUFLEN]; /* the network unique identifier */
};
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetworkPort, virObjectUnref);
+
+
/**
* _virInterface:
*
@@ -658,6 +667,9 @@ struct _virStoragePool {
virFreeCallback privateDataFreeFunc;
[libvirt] [PATCH v3 13/19] rpc: convert methods using virIdentityPtr to auto free macros
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
src/rpc/virnetserverclient.c | 47 ++
src/rpc/virnetserverprogram.c | 13 +++---
tests/virnetserverclienttest.c | 3 +--
3 files changed, 23 insertions(+), 40 deletions(-)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 171ee636dd..79287572b6 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -759,13 +759,13 @@ int
virNetServerClientGetUNIXIdentity(virNetServerClientPtr client,
static virIdentityPtr
virNetServerClientCreateIdentity(virNetServerClientPtr client)
{
-char *username = NULL;
-char *groupname = NULL;
-char *seccontext = NULL;
-virIdentityPtr ret = NULL;
+g_autofree char *username = NULL;
+g_autofree char *groupname = NULL;
+g_autofree char *seccontext = NULL;
+g_autoptr(virIdentity) ret = NULL;
if (!(ret = virIdentityNew()))
-goto error;
+return NULL;
if (client->sock && virNetSocketIsLocal(client->sock)) {
gid_t gid;
@@ -775,59 +775,50 @@ virNetServerClientCreateIdentity(virNetServerClientPtr
client)
if (virNetSocketGetUNIXIdentity(client->sock,
, , ,
) < 0)
-goto error;
+return NULL;
if (!(username = virGetUserName(uid)))
-goto error;
+return NULL;
if (virIdentitySetUserName(ret, username) < 0)
-goto error;
+return NULL;
if (virIdentitySetUNIXUserID(ret, uid) < 0)
-goto error;
+return NULL;
if (!(groupname = virGetGroupName(gid)))
-goto error;
+return NULL;
if (virIdentitySetGroupName(ret, groupname) < 0)
-goto error;
+return NULL;
if (virIdentitySetUNIXGroupID(ret, gid) < 0)
-goto error;
+return NULL;
if (virIdentitySetProcessID(ret, pid) < 0)
-goto error;
+return NULL;
if (virIdentitySetProcessTime(ret, timestamp) < 0)
-goto error;
+return NULL;
}
#if WITH_SASL
if (client->sasl) {
const char *identity = virNetSASLSessionGetIdentity(client->sasl);
if (virIdentitySetSASLUserName(ret, identity) < 0)
-goto error;
+return NULL;
}
#endif
if (client->tls) {
const char *identity = virNetTLSSessionGetX509DName(client->tls);
if (virIdentitySetX509DName(ret, identity) < 0)
-goto error;
+return NULL;
}
if (client->sock &&
virNetSocketGetSELinuxContext(client->sock, ) < 0)
-goto error;
+return NULL;
if (seccontext &&
virIdentitySetSELinuxContext(ret, seccontext) < 0)
-goto error;
-
- cleanup:
-VIR_FREE(username);
-VIR_FREE(groupname);
-VIR_FREE(seccontext);
-return ret;
+return NULL;
- error:
-virObjectUnref(ret);
-ret = NULL;
-goto cleanup;
+return g_steal_pointer();
}
diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
index 7ae1d2e955..cca820ca5a 100644
--- a/src/rpc/virnetserverprogram.c
+++ b/src/rpc/virnetserverprogram.c
@@ -370,13 +370,13 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr
prog,
virNetServerClientPtr client,
virNetMessagePtr msg)
{
-char *arg = NULL;
-char *ret = NULL;
+g_autofree char *arg = NULL;
+g_autofree char *ret = NULL;
int rv = -1;
virNetServerProgramProcPtr dispatcher;
virNetMessageError rerr;
size_t i;
-virIdentityPtr identity = NULL;
+g_autoptr(virIdentity) identity = NULL;
memset(, 0, sizeof(rerr));
@@ -484,10 +484,7 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr
prog,
}
xdr_free(dispatcher->ret_filter, ret);
-VIR_FREE(arg);
-VIR_FREE(ret);
-virObjectUnref(identity);
/* Put reply on end of tx queue to send out */
return virNetServerClientSendMessage(client, msg);
@@ -496,10 +493,6 @@ virNetServerProgramDispatchCall(virNetServerProgramPtr
prog,
* RPC error message we can send back to the client */
rv = virNetServerProgramSendReplyError(prog, client, msg, ,
>header);
-VIR_FREE(arg);
-VIR_FREE(ret);
-virObjectUnref(identity);
-
return rv;
}
diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c
index d094de9840..42393d7dbe 100644
--- a/tests/virnetserverclienttest.c
+++ b/tests/virnetserverclienttest.c
@@ -51,7 +51,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED)
int ret = -1;
virNetSocketPtr sock = NULL;
virNetServerClientPtr client = NULL;
-virIdentityPtr ident = NULL;
+g_autoptr(virIdentity) ident = NULL;
const char *gotUsername = NULL;
[libvirt] [PATCH v3 18/19] util: replace strerror/strerror_r with g_strerror
g_strerror is offers the safety/correctness benefits of strerror_r, with
the API design convenience of strerror.
Use of virStrerror should be eliminated through the codebase in favour
of g_strerror.
commandhelper.c is a special case as its a tiny single threaded test
program, not linked to glib, so it just uses traditional strerror().
Signed-off-by: Daniel P. Berrangé
---
bootstrap.conf | 2 --
docs/hacking.html.in | 4
src/util/virerror.c | 9 +
src/util/virerror.h | 1 +
tests/commandtest.c | 10 +-
tests/qemumonitortestutils.c | 2 +-
tests/seclabeltest.c | 4 ++--
tests/testutils.c| 6 +++---
tests/virhostcputest.c | 4 ++--
tests/virtestmock.c | 4 ++--
10 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index bb40e978aa..241dce50c2 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -79,8 +79,6 @@ snprintf
socket
stat-time
strchrnul
-strerror
-strerror_r-posix
strptime
strsep
strtok_r
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index a92608cd3c..6cf796a175 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1051,6 +1051,10 @@ BAD:
The GLib macros g_autoptr and G_DEFINE_AUTOPTR_CLEANUP_FUNC
should be used to manage autoclean of virObject classes.
This matches usage with GObject classes.
+
+ virStrerror
+ The GLib g_strerror() function should be used instead,
+which has a simpler calling convention as an added benefit.
File handling
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 3bb9d1d32c..5d69e4e972 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1322,8 +1322,11 @@ const char *virStrerror(int theerrno, char *errBuf,
size_t errBufLen)
{
int save_errno = errno;
const char *ret;
+const char *str = g_strerror(theerrno);
+size_t len = strlen(str);
-strerror_r(theerrno, errBuf, errBufLen);
+memcpy(errBuf, str, MIN(len, errBufLen));
+errBuf[errBufLen-1] = '\0';
ret = errBuf;
errno = save_errno;
return ret;
@@ -1349,11 +1352,9 @@ void virReportSystemErrorFull(int domcode,
const char *fmt, ...)
{
int save_errno = errno;
-char strerror_buf[VIR_ERROR_MAX_LENGTH];
char msgDetailBuf[VIR_ERROR_MAX_LENGTH];
-const char *errnoDetail = virStrerror(theerrno, strerror_buf,
- sizeof(strerror_buf));
+const char *errnoDetail = g_strerror(theerrno);
const char *msg = virErrorMsg(VIR_ERR_SYSTEM_ERROR, fmt);
const char *msgDetail = NULL;
diff --git a/src/util/virerror.h b/src/util/virerror.h
index fa88217b27..201195d660 100644
--- a/src/util/virerror.h
+++ b/src/util/virerror.h
@@ -193,6 +193,7 @@ void virReportOOMErrorFull(int domcode,
int virSetError(virErrorPtr newerr);
virErrorPtr virErrorCopyNew(virErrorPtr err);
void virDispatchError(virConnectPtr conn);
+/* DEPRECATED: use g_strerror() directly */
const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
typedef int (*virErrorLogPriorityFunc)(virErrorPtr, int);
diff --git a/tests/commandtest.c b/tests/commandtest.c
index c6fd826003..2aaddef3d1 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -636,12 +636,12 @@ static int test16(const void *unused ATTRIBUTE_UNUSED)
}
if ((fd = open(abs_builddir "/commandhelper.log",
O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0) {
-printf("Cannot open log file: %s\n", strerror(errno));
+printf("Cannot open log file: %s\n", g_strerror(errno));
goto cleanup;
}
virCommandWriteArgLog(cmd, fd);
if (VIR_CLOSE(fd) < 0) {
-printf("Cannot close log file: %s\n", strerror(errno));
+printf("Cannot close log file: %s\n", g_strerror(errno));
goto cleanup;
}
@@ -1116,12 +1116,12 @@ static int test26(const void *unused ATTRIBUTE_UNUSED)
}
if ((fd = open(abs_builddir "/commandhelper.log",
O_CREAT | O_TRUNC | O_WRONLY, 0600)) < 0) {
-printf("Cannot open log file: %s\n", strerror(errno));
+printf("Cannot open log file: %s\n", g_strerror(errno));
goto cleanup;
}
virCommandWriteArgLog(cmd, fd);
if (VIR_CLOSE(fd) < 0) {
-printf("Cannot close log file: %s\n", strerror(errno));
+printf("Cannot close log file: %s\n", g_strerror(errno));
goto cleanup;
}
@@ -1186,7 +1186,7 @@ static int test27(const void *unused ATTRIBUTE_UNUSED)
}
if (pipe(pipe1) < 0 || pipe(pipe2) < 0) {
-printf("Could not create pipe: %s\n", strerror(errno));
+printf("Could not create pipe: %s\n", g_strerror(errno));
goto cleanup;
}
diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index c7580c5f28..9f2594b09a 100644
--- a/tests/qemumonitortestutils.c
+++
[libvirt] [PATCH v3 03/19] util: use glib memory allocation functions
Convert the VIR_ALLOC family of APIs with use of the g_malloc family of
APIs. Use of VIR_ALLOC related functions should be incrementally phased
out over time, allowing return value checks to be dropped. Use of
VIR_FREE should be replaced with auto-cleanup whenever possible.
We previously used the 'calloc-posix' gnulib module because mingw does
not set errno to ENOMEM on failure.
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
bootstrap.conf | 1 -
docs/hacking.html.in | 106 +--
src/util/viralloc.c | 29 +++-
src/util/viralloc.h | 9
4 files changed, 27 insertions(+), 118 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 358d783a6b..7d73584809 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -26,7 +26,6 @@ byteswap
c-ctype
c-strcase
c-strcasestr
-calloc-posix
canonicalize-lgpl
chown
clock-time
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 2e064ced5e..8072796312 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1008,102 +1008,20 @@ BAD:
+ VIR_ALLOC, VIR_REALLOC, VIR_RESIZE_N, VIR_EXPAND_N,
+VIR_SHRINK_N, VIR_FREE, VIR_APPEND_ELEMENT, VIR_INSERT_ELEMENT,
+VIR_DELETE_ELEMENT
+ Prefer the GLib APIs g_new0/g_renew/g_free in most cases.
+There should rarely be a need to use g_malloc/g_realloc.
+Instead of using plain C arrays, it is preferrable to use
+one of the GLib types, GArray, GPtrArray or GByteArray. These
+all use a struct to track the array memory and size together
+and efficiently resize. NEVER MIX use of the
+classic libvirt memory allocation APIs and GLib APIs within
+a single method. Keep the style consistent, converting existing
+code to GLib style in a separate, prior commit.
-Low level memory management
-
-
- Use of the malloc/free/realloc/calloc APIs is deprecated in the libvirt
- codebase, because they encourage a number of serious coding bugs and do
- not enable compile time verification of checks for NULL. Instead of these
- routines, use the macros from viralloc.h.
-
-
-
- To allocate a single object:
-
-
- virDomainPtr domain;
-
- if (VIR_ALLOC(domain) 0)
- return NULL;
-
-
-
- To allocate an array of objects:
-
- virDomainPtr domains;
- size_t ndomains = 10;
-
- if (VIR_ALLOC_N(domains, ndomains) 0)
- return NULL;
-
-
-
- To allocate an array of object pointers:
-
- virDomainPtr *domains;
- size_t ndomains = 10;
-
- if (VIR_ALLOC_N(domains, ndomains) 0)
- return NULL;
-
-
-
- To re-allocate the array of domains to be 1 element
- longer (however, note that repeatedly expanding an array by 1
- scales quadratically, so this is recommended only for smaller
- arrays):
-
- virDomainPtr domains;
- size_t ndomains = 0;
-
- if (VIR_EXPAND_N(domains, ndomains, 1) 0)
- return NULL;
- domains[ndomains - 1] = domain;
-
-
- To ensure an array has room to hold at least one more
- element (this approach scales better, but requires tracking
- allocation separately from usage)
-
-
- virDomainPtr domains;
- size_t ndomains = 0;
- size_t ndomains_max = 0;
-
- if (VIR_RESIZE_N(domains, ndomains_max, ndomains, 1) 0)
- return NULL;
- domains[ndomains++] = domain;
-
-
-
- To trim an array of domains from its allocated size down
- to the actual used size:
-
-
- virDomainPtr domains;
- size_t ndomains = x;
- size_t ndomains_max = y;
-
- VIR_SHRINK_N(domains, ndomains_max, ndomains_max - ndomains);
-
-
- To free an array of domains:
-
- virDomainPtr domains;
- size_t ndomains = x;
- size_t ndomains_max = y;
- size_t i;
-
- for (i = 0; i ndomains; i++)
- VIR_FREE(domains[i]);
- VIR_FREE(domains);
- ndomains_max = ndomains = 0;
-
-
-
-
File handling
diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index 10a8d0fb73..b8ca850764 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
@@ -45,10 +45,7 @@ VIR_LOG_INIT("util.alloc");
int virAlloc(void *ptrptr,
size_t size)
{
-*(void **)ptrptr = calloc(1, size);
-if (*(void **)ptrptr == NULL)
-abort();
-
+*(void **)ptrptr = g_malloc0(size);
return 0;
}
@@ -69,10 +66,7 @@ int virAllocN(void *ptrptr,
size_t size,
size_t count)
{
-*(void**)ptrptr = calloc(count, size);
-if (*(void**)ptrptr == NULL)
-abort();
-
+*(void**)ptrptr = g_malloc0_n(count, size);
return 0;
}
@@ -94,16 +88,7 @@ int virReallocN(void *ptrptr,
size_t size,
size_t count)
{
-void *tmp;
-
-if (xalloc_oversized(count, size))
-abort();
-
-tmp = realloc(*(void**)ptrptr, size * count);
-if (!tmp && ((size * count) != 0))
-abort();
-
-*(void**)ptrptr = tmp;
+*(void **)ptrptr =
[libvirt] [PATCH v3 14/19] remote: convert methods using virIdentityPtr to auto free macros
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
src/remote/remote_daemon.c | 3 +--
src/remote/remote_daemon_dispatch.c | 35 ++---
2 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 9281e226c4..7fcaa31c49 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -826,7 +826,7 @@ handleSystemMessageFunc(DBusConnection *connection
ATTRIBUTE_UNUSED,
static void daemonRunStateInit(void *opaque)
{
virNetDaemonPtr dmn = opaque;
-virIdentityPtr sysident = virIdentityGetSystem();
+g_autoptr(virIdentity) sysident = virIdentityGetSystem();
#ifdef MODULE_NAME
bool mandatory = true;
#else /* ! MODULE_NAME */
@@ -879,7 +879,6 @@ static void daemonRunStateInit(void *opaque)
cleanup:
daemonInhibitCallback(false, dmn);
virObjectUnref(dmn);
-virObjectUnref(sysident);
virIdentitySetCurrent(NULL);
}
diff --git a/src/remote/remote_daemon_dispatch.c
b/src/remote/remote_daemon_dispatch.c
index dbd2985c38..9fc5dc3998 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -159,7 +159,7 @@ remoteRelayDomainEventCheckACL(virNetServerClientPtr client,
virConnectPtr conn, virDomainPtr dom)
{
virDomainDef def;
-virIdentityPtr identity = NULL;
+g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virDomainDef with enough contents to
@@ -177,7 +177,6 @@ remoteRelayDomainEventCheckACL(virNetServerClientPtr client,
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
-virObjectUnref(identity);
return ret;
}
@@ -187,7 +186,7 @@ remoteRelayNetworkEventCheckACL(virNetServerClientPtr
client,
virConnectPtr conn, virNetworkPtr net)
{
virNetworkDef def;
-virIdentityPtr identity = NULL;
+g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virNetworkDef with enough contents to
@@ -204,7 +203,6 @@ remoteRelayNetworkEventCheckACL(virNetServerClientPtr
client,
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
-virObjectUnref(identity);
return ret;
}
@@ -214,7 +212,7 @@ remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr
client,
virStoragePoolPtr pool)
{
virStoragePoolDef def;
-virIdentityPtr identity = NULL;
+g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virStoragePoolDef with enough contents to
@@ -231,7 +229,6 @@ remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr
client,
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
-virObjectUnref(identity);
return ret;
}
@@ -241,7 +238,7 @@ remoteRelayNodeDeviceEventCheckACL(virNetServerClientPtr
client,
virNodeDevicePtr dev)
{
virNodeDeviceDef def;
-virIdentityPtr identity = NULL;
+g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virNodeDeviceDef with enough contents to
@@ -257,7 +254,6 @@ remoteRelayNodeDeviceEventCheckACL(virNetServerClientPtr
client,
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
-virObjectUnref(identity);
return ret;
}
@@ -267,7 +263,7 @@ remoteRelaySecretEventCheckACL(virNetServerClientPtr client,
virSecretPtr secret)
{
virSecretDef def;
-virIdentityPtr identity = NULL;
+g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virSecretDef with enough contents to
@@ -285,7 +281,6 @@ remoteRelaySecretEventCheckACL(virNetServerClientPtr client,
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
-virObjectUnref(identity);
return ret;
}
@@ -294,7 +289,7 @@
remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client,
virConnectPtr conn, virDomainPtr dom)
{
virDomainDef def;
-virIdentityPtr identity = NULL;
+g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virDomainDef with enough contents to
@@ -311,7 +306,6 @@
remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client,
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
-virObjectUnref(identity);
return ret;
}
@@ -1869,7 +1863,7 @@ void remoteRelayConnectionClosedEvent(virConnectPtr conn
ATTRIBUTE_UNUSED, int r
static void
remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv)
{
-virIdentityPtr sysident = virIdentityGetSystem();
+g_autoptr(virIdentity) sysident = virIdentityGetSystem();
virIdentitySetCurrent(sysident);
DEREG_CB(priv->conn, priv->domainEventCallbacks,
@@ -1898,7 +1892,6 @@ remoteClientFreePrivateCallbacks(struct
[libvirt] [PATCH v3 01/19] build: probe for glib-2 library in configure
Prepare for linking with glib by probing for it at configure
time. Per supported platforms target, the min glib versions on
relevant distros are:
RHEL-8: 2.56.1
RHEL-7: 2.50.3
Debian (Buster): 2.58.3
Debian (Stretch): 2.50.3
OpenBSD (Ports): 2.58.3
FreeBSD (Ports): 2.56.3
OpenSUSE Leap 15: 2.54.3
SLE12-SP2: 2.48.2
Ubuntu (Xenial): 2.48.0
macOS (Homebrew): 2.56.0
This suggests that a minimum glib of 2.48 is a reasonable target.
This aligns with the minimum version required by qemu too.
We must disable the bad-function-cast warning as various GLib APIs
and macros will trigger this.
Reviewed-by: Ján Tomko
Reviewed-by: Pavel Hrdina
Signed-off-by: Daniel P. Berrangé
---
.travis.yml | 1 +
configure.ac| 2 ++
libvirt.spec.in | 1 +
m4/virt-compile-warnings.m4 | 2 ++
m4/virt-glib.m4 | 36
mingw-libvirt.spec.in | 2 ++
6 files changed, 44 insertions(+)
create mode 100644 m4/virt-glib.m4
diff --git a/.travis.yml b/.travis.yml
index e475af34cf..478909d3bb 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -13,6 +13,7 @@ addons:
- rpcgen
- xz
- yajl
+ - glib
matrix:
include:
diff --git a/configure.ac b/configure.ac
index f6bf4fb60a..9b4e6fdd6d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -311,6 +311,7 @@ LIBVIRT_CHECK_DLOPEN
LIBVIRT_CHECK_FIREWALLD
LIBVIRT_CHECK_FIREWALLD_ZONE
LIBVIRT_CHECK_FUSE
+LIBVIRT_CHECK_GLIB
LIBVIRT_CHECK_GLUSTER
LIBVIRT_CHECK_GNUTLS
LIBVIRT_CHECK_HAL
@@ -1007,6 +1008,7 @@ LIBVIRT_RESULT_DLOPEN
LIBVIRT_RESULT_FIREWALLD
LIBVIRT_RESULT_FIREWALLD_ZONE
LIBVIRT_RESULT_FUSE
+LIBVIRT_RESULT_GLIB
LIBVIRT_RESULT_GLUSTER
LIBVIRT_RESULT_GNUTLS
LIBVIRT_RESULT_HAL
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 7f5183f341..dcad08cb5f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -273,6 +273,7 @@ BuildRequires: systemd-units
%if %{with_libxl}
BuildRequires: xen-devel
%endif
+BuildRequires: glib2-devel >= 2.48
BuildRequires: libxml2-devel
BuildRequires: libxslt
BuildRequires: readline-devel
diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
index 4f9eee121c..1dbe1abe27 100644
--- a/m4/virt-compile-warnings.m4
+++ b/m4/virt-compile-warnings.m4
@@ -67,6 +67,8 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
# > to handle the code effectively.
# Source: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
dontwarn="$dontwarn -Wdisabled-optimization"
+# Various valid glib APIs/macros trigger this warning
+dontwarn="$dontwarn -Wbad-function-cast"
# Broken in 6.0 and later
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602
diff --git a/m4/virt-glib.m4 b/m4/virt-glib.m4
new file mode 100644
index 00..5a5bc19660
--- /dev/null
+++ b/m4/virt-glib.m4
@@ -0,0 +1,36 @@
+dnl The glib.so library
+dnl
+dnl Copyright (C) 2016 Red Hat, Inc.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library. If not, see
+dnl
[libvirt] [PATCH v3 04/19] util: use glib string allocation/formatting functions
Convert the string duplication APIs to use the g_strdup family of APIs.
We previously used the 'strdup-posix' gnulib module because mingw does
not set errno to ENOMEM on failure
We previously used the 'strndup' gnulib module because this function
does not exist on mingw.
We previously used the 'vasprintf' gnulib module because of many GNU
supported format specifiers not working on non-Linux platforms. glib's
own equivalent standardizes on GNU format specifiers too.
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
bootstrap.conf | 3 ---
docs/hacking.html.in | 8
src/util/virstring.c | 28 ++--
src/util/virstring.h | 8
4 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 7d73584809..7e264b63ad 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -80,8 +80,6 @@ snprintf
socket
stat-time
strchrnul
-strdup-posix
-strndup
strerror
strerror_r-posix
strptime
@@ -96,7 +94,6 @@ ttyname_r
uname
unsetenv
usleep
-vasprintf
verify
vsnprintf
waitpid
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 8072796312..3f1542b6de 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1020,6 +1020,14 @@ BAD:
classic libvirt memory allocation APIs and GLib APIs within
a single method. Keep the style consistent, converting existing
code to GLib style in a separate, prior commit.
+
+ VIR_STRDUP, VIR_STRNDUP
+ Prefer the GLib APIs g_strdup and g_strndup.
+
+ virAsprintf, virVasprintf
+ The GLib APIs g_strdup_printf / g_strdup_vprint should be used
+instead. Don't use g_vasprintf unless having the string length
+returned is unavoidable.
File handling
diff --git a/src/util/virstring.c b/src/util/virstring.c
index a4cc7e9c0a..6b2b6ed24e 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -18,6 +18,7 @@
#include
+#include
#include
#include
@@ -730,10 +731,21 @@ virVasprintfInternal(char **strp,
const char *fmt,
va_list list)
{
+char *str = NULL;
int ret;
-if ((ret = vasprintf(strp, fmt, list)) == -1)
+ret = g_vasprintf(, fmt, list);
+
+/* GLib is supposed to abort() on OOM, but a mistake meant
+ * it did not. Delete this once our min glib is at 2.64.0
+ * which includes the fix:
+ * https://gitlab.gnome.org/GNOME/glib/merge_requests/1145
+ */
+#if !GLIB_CHECK_VERSION(2, 64, 0)
+if (!str)
abort();
+#endif
+*strp = str;
return ret;
}
@@ -743,11 +755,17 @@ virAsprintfInternal(char **strp,
const char *fmt, ...)
{
va_list ap;
+char *str = NULL;
int ret;
va_start(ap, fmt);
-ret = virVasprintfInternal(strp, fmt, ap);
+ret = g_vasprintf(, fmt, ap);
va_end(ap);
+
+if (!*str)
+abort();
+*strp = str;
+
return ret;
}
@@ -936,8 +954,7 @@ virStrdup(char **dest,
*dest = NULL;
if (!src)
return 0;
-if (!(*dest = strdup(src)))
-abort();
+*dest = g_strdup(src);
return 1;
}
@@ -965,8 +982,7 @@ virStrndup(char **dest,
return 0;
if (n < 0)
n = strlen(src);
-if (!(*dest = strndup(src, n)))
-abort();
+*dest = g_strndup(src, n);
return 1;
}
diff --git a/src/util/virstring.h b/src/util/virstring.h
index f537f3472e..3ffe51f7b8 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -145,6 +145,8 @@ int virVasprintfInternal(char **strp, const char *fmt,
va_list list)
* @dst: variable to hold result (char*, not char**)
* @src: string to duplicate
*
+ * DEPRECATED: use g_strdup instead
+ *
* Duplicate @src string and store it into @dst.
*
* This macro is safe to use on arguments with side effects.
@@ -158,6 +160,8 @@ int virVasprintfInternal(char **strp, const char *fmt,
va_list list)
* @dst: variable to hold result (char*, not char**)
* @src: string to duplicate
*
+ * DEPRECATED: use g_strdup instead
+ *
* Duplicate @src string and store it into @dst.
*
* This macro is safe to use on arguments with side effects.
@@ -172,6 +176,8 @@ int virVasprintfInternal(char **strp, const char *fmt,
va_list list)
* @src: string to duplicate
* @n: the maximum number of bytes to copy
*
+ * DEPRECATED: use g_strndup instead
+ *
* Duplicate @src string and store it into @dst. If @src is longer than @n,
* only @n bytes are copied and terminating null byte '\0' is added. If @n
* is a negative number, then the whole @src string is copied. That is,
@@ -189,6 +195,8 @@ int virVasprintfInternal(char **strp, const char *fmt,
va_list list)
* @src: string to duplicate
* @n: the maximum number of bytes to copy
*
+ * DEPRECATED: use g_strndup instead
+ *
* Duplicate @src string and store it into @dst. If @src is longer than @n,
* only @n bytes are copied and terminating null byte '\0' is added. If @n
* is a
[libvirt] [PATCH v3 16/19] libxl: convert over to use GRegex for regular expressions
Reviewed-by: Ján Tomko
Reviewed-by: Pavel Hrdina
Signed-off-by: Daniel P. Berrangé
---
src/libxl/libxl_capabilities.c | 42 +++---
1 file changed, 19 insertions(+), 23 deletions(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 73ae0b3fa1..65c68ffb52 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -21,7 +21,6 @@
#include
#include
-#include
#include "internal.h"
#include "virlog.h"
@@ -374,10 +373,10 @@ static int
libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
{
const libxl_version_info *ver_info;
-int err;
-regex_t regex;
+g_autoptr(GRegex) regex = NULL;
+g_autoptr(GError) err = NULL;
+g_autoptr(GMatchInfo) info = NULL;
char *str, *token;
-regmatch_t subs[4];
char *saveptr = NULL;
size_t i;
@@ -398,12 +397,10 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
return -1;
}
-err = regcomp(, XEN_CAP_REGEX, REG_EXTENDED);
-if (err != 0) {
-char error[100];
-regerror(err, , error, sizeof(error));
+regex = g_regex_new(XEN_CAP_REGEX, G_REGEX_EXTENDED, 0, );
+if (!regex) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to compile regex %s"), error);
+ _("Failed to compile regex %s"), err->message);
return -1;
}
@@ -436,31 +433,31 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
nr_guest_archs < sizeof(guest_archs) / sizeof(guest_archs[0])
&& (token = strtok_r(str, " ", )) != NULL;
str = NULL) {
-if (regexec(, token, sizeof(subs) / sizeof(subs[0]),
-subs, 0) == 0) {
-int hvm = STRPREFIX([subs[1].rm_so], "hvm");
+if (g_regex_match(regex, token, 0, )) {
+g_autofree char *modestr = g_match_info_fetch(info, 1);
+g_autofree char *archstr = g_match_info_fetch(info, 2);
+g_autofree char *suffixstr = g_match_info_fetch(info, 3);
+int hvm = STRPREFIX(modestr, "hvm");
virArch arch;
int pae = 0, nonpae = 0, ia64_be = 0;
-if (STRPREFIX([subs[2].rm_so], "x86_32")) {
+if (STRPREFIX(archstr, "x86_32")) {
arch = VIR_ARCH_I686;
-if (subs[3].rm_so != -1 &&
-STRPREFIX([subs[3].rm_so], "p"))
+if (suffixstr != NULL && STRPREFIX(suffixstr, "p"))
pae = 1;
else
nonpae = 1;
-} else if (STRPREFIX([subs[2].rm_so], "x86_64")) {
+} else if (STRPREFIX(archstr, "x86_64")) {
arch = VIR_ARCH_X86_64;
-} else if (STRPREFIX([subs[2].rm_so], "ia64")) {
+} else if (STRPREFIX(archstr, "ia64")) {
arch = VIR_ARCH_ITANIUM;
-if (subs[3].rm_so != -1 &&
-STRPREFIX([subs[3].rm_so], "be"))
+if (suffixstr != NULL && STRPREFIX(suffixstr, "be"))
ia64_be = 1;
-} else if (STRPREFIX([subs[2].rm_so], "powerpc64")) {
+} else if (STRPREFIX(archstr, "powerpc64")) {
arch = VIR_ARCH_PPC64;
-} else if (STRPREFIX([subs[2].rm_so], "armv7l")) {
+} else if (STRPREFIX(archstr, "armv7l")) {
arch = VIR_ARCH_ARMV7L;
-} else if (STRPREFIX([subs[2].rm_so], "aarch64")) {
+} else if (STRPREFIX(archstr, "aarch64")) {
arch = VIR_ARCH_AARCH64;
} else {
continue;
@@ -515,7 +512,6 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
#endif
}
}
-regfree();
for (i = 0; i < nr_guest_archs; ++i) {
virCapsGuestPtr guest;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 05/19] util: convert virSystemdActivation to use VIR_DEFINE_AUTOPTR_FUNC
Using the standard macro will facilitate the conversion to glib's
auto cleanup macros.
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
src/util/virsystemd.c | 10 +-
src/util/virsystemd.h | 5 +++--
tests/virsystemdtest.c | 2 +-
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 2efc0dd72c..c2e4c3df51 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -917,7 +917,7 @@ virSystemdActivationNew(virSystemdActivationMap *map,
return act;
error:
-virSystemdActivationFree();
+virSystemdActivationFree(act);
return NULL;
}
@@ -1046,12 +1046,12 @@ virSystemdActivationClaimFDs(virSystemdActivationPtr
act,
* associated with the activation object
*/
void
-virSystemdActivationFree(virSystemdActivationPtr *act)
+virSystemdActivationFree(virSystemdActivationPtr act)
{
-if (!*act)
+if (!act)
return;
-virHashFree((*act)->fds);
+virHashFree(act->fds);
-VIR_FREE(*act);
+VIR_FREE(act);
}
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
index 5f1a4413fe..2c0a0d8dc0 100644
--- a/src/util/virsystemd.h
+++ b/src/util/virsystemd.h
@@ -22,6 +22,7 @@
#pragma once
#include "internal.h"
+#include "virautoclean.h"
typedef struct _virSystemdActivation virSystemdActivation;
typedef virSystemdActivation *virSystemdActivationPtr;
@@ -81,6 +82,6 @@ void virSystemdActivationClaimFDs(virSystemdActivationPtr act,
int **fds,
size_t *nfds);
-void virSystemdActivationFree(virSystemdActivationPtr *act);
+void virSystemdActivationFree(virSystemdActivationPtr act);
-#define virSystemdActivationAutoPtrFree virSystemdActivationFree
+VIR_DEFINE_AUTOPTR_FUNC(virSystemdActivation, virSystemdActivationFree);
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index 3add1ab56f..d33a7c192f 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -650,7 +650,7 @@ testActivationEmpty(const void *opaque ATTRIBUTE_UNUSED)
if (act != NULL) {
fprintf(stderr, "Unexpectedly got activation object");
-virSystemdActivationFree();
+virSystemdActivationFree(act);
return -1;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 11/19] access: convert polkit driver to auto free memory
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
src/access/viraccessdriverpolkit.c | 38 +++---
1 file changed, 14 insertions(+), 24 deletions(-)
diff --git a/src/access/viraccessdriverpolkit.c
b/src/access/viraccessdriverpolkit.c
index e61ac6fa19..fbe7789ccd 100644
--- a/src/access/viraccessdriverpolkit.c
+++ b/src/access/viraccessdriverpolkit.c
@@ -78,8 +78,7 @@ virAccessDriverPolkitGetCaller(const char *actionid,
unsigned long long *startTime,
uid_t *uid)
{
-virIdentityPtr identity = virIdentityGetCurrent();
-int ret = -1;
+g_autoptr(virIdentity) identity = virIdentityGetCurrent();
int rc;
if (!identity) {
@@ -90,37 +89,33 @@ virAccessDriverPolkitGetCaller(const char *actionid,
}
if ((rc = virIdentityGetProcessID(identity, pid)) < 0)
-goto cleanup;
+return -1;
if (rc == 0) {
virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No process ID available"));
-goto cleanup;
+return -1;
}
if ((rc = virIdentityGetProcessTime(identity, startTime)) < 0)
-goto cleanup;
+return -1;
if (rc == 0) {
virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No process start time available"));
-goto cleanup;
+return -1;
}
if ((rc = virIdentityGetUNIXUserID(identity, uid)) < 0)
-goto cleanup;
+return -1;
if (rc == 0) {
virAccessError(VIR_ERR_INTERNAL_ERROR, "%s",
_("No UNIX caller UID available"));
-goto cleanup;
+return -1;
}
-ret = 0;
-
- cleanup:
-virObjectUnref(identity);
-return ret;
+return 0;
}
@@ -130,21 +125,20 @@ virAccessDriverPolkitCheck(virAccessManagerPtr manager
ATTRIBUTE_UNUSED,
const char *permname,
const char **attrs)
{
-char *actionid = NULL;
-int ret = -1;
+g_autofree char *actionid = NULL;
pid_t pid;
uid_t uid;
unsigned long long startTime;
int rv;
if (!(actionid = virAccessDriverPolkitFormatAction(typename, permname)))
-goto cleanup;
+return -1;
if (virAccessDriverPolkitGetCaller(actionid,
,
,
) < 0)
-goto cleanup;
+return -1;
VIR_DEBUG("Check action '%s' for process '%lld' time %lld uid %d",
actionid, (long long)pid, startTime, uid);
@@ -157,18 +151,14 @@ virAccessDriverPolkitCheck(virAccessManagerPtr manager
ATTRIBUTE_UNUSED,
false);
if (rv == 0) {
-ret = 1; /* Allowed */
+return 1; /* Allowed */
} else {
if (rv == -2) {
-ret = 0; /* Denied */
+return 0; /* Denied */
} else {
-ret = -1; /* Error */
+return -1; /* Error */
}
}
-
- cleanup:
-VIR_FREE(actionid);
-return ret;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 09/19] util: use glib base64 encoding/decoding APIs
Replace use of the gnulib base64 module with glib's own base64 API family.
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
bootstrap.conf| 1 -
configure.ac | 5 -
src/conf/virsecretobj.c | 26 --
src/libvirt_private.syms | 1 -
src/libxl/libxl_conf.c| 3 +--
src/qemu/qemu_agent.c | 6 ++
src/qemu/qemu_command.c | 5 ++---
src/qemu/qemu_domain.c| 8 +++-
src/secret/secret_driver.c| 1 -
src/storage/storage_backend_rbd.c | 4 +---
src/util/virstring.c | 21 -
src/util/virstring.h | 2 --
tools/virsh-secret.c | 17 -
13 files changed, 17 insertions(+), 83 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 7e264b63ad..bb40e978aa 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -20,7 +20,6 @@
gnulib_modules='
accept
areadlink
-base64
bind
byteswap
c-ctype
diff --git a/configure.ac b/configure.ac
index 9b4e6fdd6d..acac1bed41 100644
--- a/configure.ac
+++ b/configure.ac
@@ -911,11 +911,6 @@ test "x$lv_cv_static_analysis" = xyes && t=1
AC_DEFINE_UNQUOTED([STATIC_ANALYSIS], [$t],
[Define to 1 when performing static analysis.])
-# Some GNULIB base64 symbols clash with a kerberos library
-AC_DEFINE_UNQUOTED([isbase64],[libvirt_gl_isbase64],[Hack to avoid symbol
clash])
-AC_DEFINE_UNQUOTED([base64_encode],[libvirt_gl_base64_encode],[Hack to avoid
symbol clash])
-AC_DEFINE_UNQUOTED([base64_encode_alloc],[libvirt_gl_base64_encode_alloc],[Hack
to avoid symbol clash])
-
GNUmakefile=GNUmakefile
m4_if(m4_version_compare([2.61a.100],
m4_defn([m4_PACKAGE_VERSION])), [1], [],
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index aeae82332b..5bd84d82ed 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -31,7 +31,6 @@
#include "virhash.h"
#include "virlog.h"
#include "virstring.h"
-#include "base64.h"
#define VIR_FROM_THIS VIR_FROM_SECRET
@@ -698,8 +697,7 @@ virSecretObjSaveData(virSecretObjPtr obj)
if (!obj->value)
return 0;
-if (!(base64 = virStringEncodeBase64(obj->value, obj->value_size)))
-return -1;
+base64 = g_base64_encode(obj->value, obj->value_size);
if (virFileRewriteStr(obj->base64File, S_IRUSR | S_IWUSR, base64) < 0)
return -1;
@@ -825,8 +823,6 @@ virSecretLoadValue(virSecretObjPtr obj)
int ret = -1, fd = -1;
struct stat st;
g_autofree char *contents = NULL;
-char *value = NULL;
-size_t value_size;
if ((fd = open(obj->base64File, O_RDONLY)) == -1) {
if (errno == ENOENT) {
@@ -851,7 +847,7 @@ virSecretLoadValue(virSecretObjPtr obj)
goto cleanup;
}
-if (VIR_ALLOC_N(contents, st.st_size) < 0)
+if (VIR_ALLOC_N(contents, st.st_size + 1) < 0)
goto cleanup;
if (saferead(fd, contents, st.st_size) != st.st_size) {
@@ -859,29 +855,15 @@ virSecretLoadValue(virSecretObjPtr obj)
obj->base64File);
goto cleanup;
}
+contents[st.st_size] = '\0';
VIR_FORCE_CLOSE(fd);
-if (!base64_decode_alloc(contents, st.st_size, , _size)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("invalid base64 in '%s'"),
- obj->base64File);
-goto cleanup;
-}
-if (value == NULL)
-goto cleanup;
-
-obj->value = (unsigned char *)value;
-value = NULL;
-obj->value_size = value_size;
+obj->value = g_base64_decode(contents, >value_size);
ret = 0;
cleanup:
-if (value != NULL) {
-memset(value, 0, value_size);
-VIR_FREE(value);
-}
if (contents != NULL)
memset(contents, 0, st.st_size);
VIR_FORCE_CLOSE(fd);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5949cba08d..e88518a1ce 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3069,7 +3069,6 @@ virSkipSpacesBackwards;
virStrcpy;
virStrdup;
virStringBufferIsPrintable;
-virStringEncodeBase64;
virStringFilterChars;
virStringHasCaseSuffix;
virStringHasChars;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index c76704a11d..de56567cf0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -999,8 +999,7 @@ libxlMakeNetworkDiskSrc(virStorageSourcePtr src, char
**srcstr)
goto cleanup;
/* RBD expects an encoded secret */
-if (!(base64secret = virStringEncodeBase64(secret, secretlen)))
-goto cleanup;
+base64secret = g_base64_encode(secret, secretlen);
}
if (!(*srcstr = libxlMakeNetworkDiskSrcStr(src, username, base64secret)))
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 34e1a85d64..0ef8b563f5 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -39,7 +39,6 @@
#include "virtime.h"
#include
[libvirt] [PATCH v3 17/19] conf: convert over to use GRegex for regular expressions
Reviewed-by: Ján Tomko
Reviewed-by: Pavel Hrdina
Signed-off-by: Daniel P. Berrangé
---
src/conf/domain_event.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index b33589f472..40031a46c8 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -22,8 +22,6 @@
#include
-#include
-
#include "domain_event.h"
#include "object_event.h"
#include "object_event_private.h"
@@ -2009,7 +2007,7 @@ virDomainQemuMonitorEventNew(int id,
* deregisters. */
struct virDomainQemuMonitorEventData {
char *event;
-regex_t regex;
+GRegex *regex;
unsigned int flags;
void *opaque;
virFreeCallback freecb;
@@ -2241,7 +2239,7 @@ virDomainQemuMonitorEventFilter(virConnectPtr conn
ATTRIBUTE_UNUSED,
if (data->flags == -1)
return true;
if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX)
-return regexec(>regex, monitorEvent->event, 0, NULL, 0) == 0;
+return g_regex_match(data->regex, monitorEvent->event, 0, NULL) ==
TRUE;
if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)
return STRCASEEQ(monitorEvent->event, data->event);
return STREQ(monitorEvent->event, data->event);
@@ -2255,7 +2253,7 @@ virDomainQemuMonitorEventCleanup(void *opaque)
VIR_FREE(data->event);
if (data->flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX)
-regfree(>regex);
+g_regex_unref(data->regex);
if (data->freecb)
(data->freecb)(data->opaque);
VIR_FREE(data);
@@ -2306,20 +2304,17 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr
conn,
return -1;
data->flags = flags;
if (event && flags != -1) {
-int rflags = REG_NOSUB;
-
-if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)
-rflags |= REG_ICASE;
if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) {
-int err = regcomp(>regex, event, rflags);
+int cflags = G_REGEX_OPTIMIZE;
+g_autoptr(GError) err = NULL;
-if (err) {
-char error[100];
-regerror(err, >regex, error, sizeof(error));
+if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)
+cflags |= G_REGEX_CASELESS;
+data->regex = g_regex_new(event, cflags, 0, );
+if (!data->regex) {
virReportError(VIR_ERR_INVALID_ARG,
_("failed to compile regex '%s': %s"),
- event, error);
-regfree(>regex);
+ event, err->message);
VIR_FREE(data);
return -1;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 10/19] util: convert virIdentity implementation and test suite to g_autoptr
To simplify the later conversion from virObject to GObject, introduce
the use of g_autoptr to the virIdentity implementnation and test suite.
Reviewed-by: Ján Tomko
Signed-off-by: Daniel P. Berrangé
---
src/util/viridentity.c | 36 ++---
tests/viridentitytest.c | 44 +
2 files changed, 33 insertions(+), 47 deletions(-)
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 22e2644c19..6636077161 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -105,7 +105,7 @@ virIdentityPtr virIdentityGetCurrent(void)
*/
int virIdentitySetCurrent(virIdentityPtr ident)
{
-virIdentityPtr old;
+g_autoptr(virIdentity) old = NULL;
if (virIdentityInitialize() < 0)
return -1;
@@ -120,8 +120,6 @@ int virIdentitySetCurrent(virIdentityPtr ident)
return -1;
}
-virObjectUnref(old);
-
return 0;
}
@@ -136,60 +134,56 @@ int virIdentitySetCurrent(virIdentityPtr ident)
*/
virIdentityPtr virIdentityGetSystem(void)
{
-VIR_AUTOFREE(char *) username = NULL;
-VIR_AUTOFREE(char *) groupname = NULL;
+g_autofree char *username = NULL;
+g_autofree char *groupname = NULL;
unsigned long long startTime;
-virIdentityPtr ret = NULL;
+g_autoptr(virIdentity) ret = NULL;
#if WITH_SELINUX
security_context_t con;
#endif
if (!(ret = virIdentityNew()))
-goto error;
+return NULL;
if (virIdentitySetProcessID(ret, getpid()) < 0)
-goto error;
+return NULL;
if (virProcessGetStartTime(getpid(), ) < 0)
-goto error;
+return NULL;
if (startTime != 0 &&
virIdentitySetProcessTime(ret, startTime) < 0)
-goto error;
+return NULL;
if (!(username = virGetUserName(geteuid(
return ret;
if (virIdentitySetUserName(ret, username) < 0)
-goto error;
+return NULL;
if (virIdentitySetUNIXUserID(ret, getuid()) < 0)
-goto error;
+return NULL;
if (!(groupname = virGetGroupName(getegid(
return ret;
if (virIdentitySetGroupName(ret, groupname) < 0)
-goto error;
+return NULL;
if (virIdentitySetUNIXGroupID(ret, getgid()) < 0)
-goto error;
+return NULL;
#if WITH_SELINUX
if (is_selinux_enabled() > 0) {
if (getcon() < 0) {
virReportSystemError(errno, "%s",
_("Unable to lookup SELinux process
context"));
-return ret;
+return NULL;
}
if (virIdentitySetSELinuxContext(ret, con) < 0) {
freecon(con);
-goto error;
+return NULL;
}
freecon(con);
}
#endif
-return ret;
-
- error:
-virObjectUnref(ret);
-return NULL;
+return g_steal_pointer();
}
diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c
index 1eadd6173a..db041a98a8 100644
--- a/tests/viridentitytest.c
+++ b/tests/viridentitytest.c
@@ -38,58 +38,53 @@ VIR_LOG_INIT("tests.identitytest");
static int testIdentityAttrs(const void *data ATTRIBUTE_UNUSED)
{
-int ret = -1;
-virIdentityPtr ident;
+g_autoptr(virIdentity) ident = NULL;
const char *val;
int rc;
if (!(ident = virIdentityNew()))
-goto cleanup;
+return -1;
if (virIdentitySetUserName(ident, "fred") < 0)
-goto cleanup;
+return -1;
if ((rc = virIdentityGetUserName(ident, )) < 0)
-goto cleanup;
+return -1;
if (STRNEQ_NULLABLE(val, "fred") || rc != 1) {
VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val));
-goto cleanup;
+return -1;
}
if ((rc = virIdentityGetGroupName(ident, )) < 0)
-goto cleanup;
+return -1;
if (val != NULL || rc != 0) {
VIR_DEBUG("Unexpected groupname attribute");
-goto cleanup;
+return -1;
}
if (virIdentitySetUserName(ident, "joe") >= 0) {
VIR_DEBUG("Unexpectedly overwrote attribute");
-goto cleanup;
+return -1;
}
if ((rc = virIdentityGetUserName(ident, )) < 0)
-goto cleanup;
+return -1;
if (STRNEQ_NULLABLE(val, "fred") || rc != 1) {
VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val));
-goto cleanup;
+return -1;
}
-ret = 0;
- cleanup:
-virObjectUnref(ident);
-return ret;
+return 0;
}
static int testIdentityGetSystem(const void *data)
{
const char *context = data;
-int ret = -1;
-virIdentityPtr ident = NULL;
+g_autoptr(virIdentity) ident = NULL;
const char *val;
int rc;
@@ -97,35 +92,32 @@ static int testIdentityGetSystem(const void *data)
if (context) {
VIR_DEBUG("libvirt not compiled with SELinux, skipping this test");
ret = EXIT_AM_SKIP;
-goto cleanup;
+return -1;
}
#endif
if
[libvirt] [PATCH v3 00/19] Integrate usage of glib into libvirt
This is a followup to a previous patch series: v0: https://www.redhat.com/archives/libvir-list/2019-August/msg01374.html v1: https://www.redhat.com/archives/libvir-list/2019-September/msg01331.html v2: https://www.redhat.com/archives/libvir-list/2019-October/msg00271.html The focus in this glib series is - Wire up pieces to facilitate interoperability around memory allocation/cleanup. - Deprecate & document existing libvirt APIs that are targetted for removal/conversion. - Illustrate some conversions and/or usage of new APIs. - Convert code that allows us to eliminate more gnulib modules There's obviously alot of conversion work that could be done, especially around memory allocation, auto cleanup and virObject stuff. I'm not intending to do that myself in the short term, as my immediate focus is on eliminating gnulib modules. Once this series is merged though, anyone is able to take on conversion jobs, so this allows the work to be spread out across the contributors. changed in v2: - First patch gets an addition to .travis.yml to add glib2 on macOS - First patch blacklists -Wbad-function-cast warning flag - Second patch updates many makefiles to refer to $(GLIB_LIBS) to fix build on platforms with stricter linkers (Ubuntu) - virIdentity conversion uses _ consistently in methods - GOptionContext conversions are dropped Daniel P. Berrangé (19): build: probe for glib-2 library in configure build: link to glib library util: use glib memory allocation functions util: use glib string allocation/formatting functions util: convert virSystemdActivation to use VIR_DEFINE_AUTOPTR_FUNC util: rewrite auto cleanup macros to use glib's equivalent src: add support for g_autoptr with virObject instances conf: convert virSecretObj APIs to use autofree util: use glib base64 encoding/decoding APIs util: convert virIdentity implementation and test suite to g_autoptr access: convert polkit driver to auto free memory admin: convert admin server code to use auto free macros rpc: convert methods using virIdentityPtr to auto free macros remote: convert methods using virIdentityPtr to auto free macros util: convert virIdentity class to use GObject libxl: convert over to use GRegex for regular expressions conf: convert over to use GRegex for regular expressions util: replace strerror/strerror_r with g_strerror build: remove use of usleep gnulib module in favour of g_usleep .travis.yml | 1 + bootstrap.conf | 8 -- build-aux/syntax-check.mk | 2 +- configure.ac| 7 +- docs/hacking.html.in| 144 libvirt.spec.in | 1 + m4/virt-compile-warnings.m4 | 23 m4/virt-glib.m4 | 36 + mingw-libvirt.spec.in | 2 + src/Makefile.am | 3 + src/access/Makefile.inc.am | 4 +- src/access/viraccessdriverpolkit.c | 38 ++ src/admin/admin_server.c| 204 src/bhyve/Makefile.inc.am | 1 + src/conf/capabilities.h | 3 + src/conf/domain_capabilities.h | 3 + src/conf/domain_conf.h | 3 + src/conf/domain_event.c | 25 ++-- src/conf/snapshot_conf.h| 3 + src/conf/storage_capabilities.h | 3 + src/conf/virsecretobj.c | 68 +++--- src/datatypes.h | 15 ++ src/hyperv/hyperv_driver.c | 2 +- src/hyperv/hyperv_wmi.c | 4 +- src/interface/Makefile.inc.am | 1 + src/internal.h | 1 + src/libvirt_private.syms| 1 - src/libxl/Makefile.inc.am | 1 + src/libxl/libxl_capabilities.c | 42 +++--- src/libxl/libxl_conf.c | 3 +- src/libxl/libxl_conf.h | 2 + src/locking/Makefile.inc.am | 9 +- src/locking/lock_daemon.c | 2 +- src/locking/lock_driver_sanlock.c | 2 +- src/logging/Makefile.inc.am | 1 + src/lxc/Makefile.inc.am | 4 + src/lxc/lxc_controller.c| 2 +- src/lxc/lxc_driver.c| 2 +- src/lxc/lxc_process.c | 2 +- src/network/Makefile.inc.am | 2 + src/network/bridge_driver.c | 2 +- src/node_device/Makefile.inc.am | 5 +- src/nwfilter/Makefile.inc.am| 1 + src/nwfilter/nwfilter_dhcpsnoop.c | 4 +- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/qemu/Makefile.inc.am| 1 + src/qemu/qemu_agent.c | 6 +- src/qemu/qemu_blockjob.h| 1 + src/qemu/qemu_capabilities.h| 2 + src/qemu/qemu_command.c | 5 +- src/qemu/qemu_conf.h| 3 + src/qemu/qemu_domain.c | 8 +- src/qemu/qemu_monitor_json.c| 2 +- src/qemu/qemu_process.c | 6 +-
[libvirt] [PATCH v3 02/19] build: link to glib library
Add the main glib.h to internal.h so that all common code can use it. Historically glib allowed applications to register an alternative memory allocator, so mixing g_malloc/g_free with malloc/free was not safe. This was feature was dropped in 2.46.0 with: commit 3be6ed60aa58095691bd697344765e715a327fc1 Author: Alexander Larsson Date: Sat Jun 27 18:38:42 2015 +0200 Deprecate and drop support for memory vtables Applications are still encourged to match g_malloc/g_free, but it is no longer a mandatory requirement for correctness, just stylistic. This is explicitly clarified in commit 1f24b36607bf708f037396014b2cdbc08d67b275 Author: Daniel P. Berrangé Date: Thu Sep 5 14:37:54 2019 +0100 gmem: clarify that g_malloc always uses the system allocator Applications can still use custom allocators in general, but they must do this by linking to a library that replaces the core malloc/free implemenentation entirely, instead of via a glib specific call. This means that libvirt does not need to be concerned about use of g_malloc/g_free causing an ABI change in the public libary, and can avoid memory copying when talking to external libraries. This patch probes for glib, which provides the foundation layer with a collection of data structures, helper APIs, and platform portability logic. Later patches will introduce linkage to gobject which provides the object type system, built on glib, and gio which providing objects for various interesting tasks, most notably including DBus client and server support and portable sockets APIs, but much more too. Signed-off-by: Daniel P. Berrangé --- docs/hacking.html.in| 21 + src/Makefile.am | 3 +++ src/access/Makefile.inc.am | 4 +++- src/bhyve/Makefile.inc.am | 1 + src/interface/Makefile.inc.am | 1 + src/internal.h | 1 + src/libxl/Makefile.inc.am | 1 + src/locking/Makefile.inc.am | 9 - src/logging/Makefile.inc.am | 1 + src/lxc/Makefile.inc.am | 4 src/network/Makefile.inc.am | 2 ++ src/node_device/Makefile.inc.am | 5 - src/nwfilter/Makefile.inc.am| 1 + src/qemu/Makefile.inc.am| 1 + src/remote/Makefile.inc.am | 2 ++ src/secret/Makefile.inc.am | 1 + src/security/Makefile.inc.am| 1 + src/storage/Makefile.inc.am | 16 src/vbox/Makefile.inc.am| 1 + src/vz/Makefile.inc.am | 1 + tests/Makefile.am | 7 +-- tools/Makefile.am | 4 22 files changed, 83 insertions(+), 5 deletions(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index edf2f54ce3..2e064ced5e 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -989,6 +989,27 @@ BAD: it points to, or it is aliased to another pointer that is. +Adoption of GLib APIs + + + Libvirt has adopted use of the + https://developer.gnome.org/glib/stable/;>GLib library. + Due to libvirt's long history of development, there are many APIs + in libvirt, for which GLib provides an alternative solution. The + general rule to follow is that the standard GLib solution will be + preferred over historical libvirt APIs. Existing code will be + ported over to use GLib APIs over time, but new code should use + the GLib APIs straight away where possible. + + + + The following is a list of libvirt APIs that should no longer be + used in new code, and their suggested GLib replacements: + + + + + Low level memory management diff --git a/src/Makefile.am b/src/Makefile.am index bd03b09cb2..e646e954a4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -34,6 +34,7 @@ AM_CPPFLAGS = -I../gnulib/lib \ WARN_CFLAGS += $(STRICT_FRAME_LIMIT_CFLAGS) AM_CFLAGS =$(LIBXML_CFLAGS) \ + $(GLIB_CFLAGS) \ $(WARN_CFLAGS) \ $(LOCK_CHECKING_CFLAGS) \ $(WIN32_EXTRA_CFLAGS) \ @@ -558,6 +559,7 @@ libvirt_admin_la_LIBADD += \ $(YAJL_LIBS) \ $(DEVMAPPER_LIBS) \ $(LIBXML_LIBS) \ + $(GLIB_LIBS) \ $(SSH2_LIBS) \ $(SASL_LIBS) \ $(GNUTLS_LIBS) \ @@ -773,6 +775,7 @@ libvirt_iohelper_LDFLAGS = \ $(NULL) libvirt_iohelper_LDADD = \ libvirt.la \ + $(GLIB_LIBS) \ ../gnulib/lib/libgnu.la if WITH_DTRACE_PROBES libvirt_iohelper_LDADD += libvirt_probes.lo diff --git a/src/access/Makefile.inc.am b/src/access/Makefile.inc.am index 4dc742f4e5..ea27adbe0b 100644 --- a/src/access/Makefile.inc.am +++ b/src/access/Makefile.inc.am @@ -59,7 +59,9 @@ libvirt_driver_access_la_CFLAGS = \ $(AM_CFLAGS) \ $(NULL) libvirt_driver_access_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_access_la_LIBADD = +libvirt_driver_access_la_LIBADD
Re: [libvirt] [PATCH v2 17/23] conf: convert over to use GRegex for regular expressions
On Wed, Oct 09, 2019 at 10:08:32AM +0200, Bjoern Walk wrote:
> Daniel P. Berrangé [2019-10-07, 06:14PM +0100]:
> > @@ -2306,20 +2304,17 @@
> > virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn,
> > return -1;
> > data->flags = flags;
> > if (event && flags != -1) {
> > -int rflags = REG_NOSUB;
> > -
> > -if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_NOCASE)
> > -rflags |= REG_ICASE;
> > if (flags & VIR_CONNECT_DOMAIN_QEMU_MONITOR_EVENT_REGISTER_REGEX) {
> > -int err = regcomp(>regex, event, rflags);
> > +int cflags = 0;
>
> Do we want to use G_REGEX_OPTIMIZE here?
Yes, makes sense for this one as we compile it once and then us it
to match every single event emitted on every guest. So the optimization
cost will be mitigated by improved matching performance.
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
On Thu, Oct 10, 2019 at 11:29:17AM +0100, Richard W.M. Jones wrote: > On Wed, Oct 09, 2019 at 07:49:29PM -0400, Cole Robinson wrote: > > In that bug, I see that rjones (cc'd) said that libvirt not > > remembering labels/uid causes issues for libguestfs that requires > > workarounds. Rich, do you have links to threads or bug reports where > > this is described in more detail? > > I think there are two problems (which I often confuse) and they are > possibly related. This one where libvirt doesn't restore permissions > afterwards, and the other one where qemu:///session cannot be used as > root which implies that when you run libguestfs as root it doesn't > have access to things that root would normally have access to (bug 890291 > / 1045069). > > In answer to your question this is the only one I could find which is > definitely related to this bug: > > https://www.redhat.com/archives/libguestfs/2013-May/msg00115.html Anything related to device nodes & permissions/ownership shouldn't be an issue any more. We switched to create a private mount namespace for each QEMU and setup a custom /dev populated with only the devices QEMU is allowed. Thus we should no longer be touching permisisons/owners in the real /dev > Here's another one, but I think this is related to the other bug: > > https://bugs.launchpad.net/nova/+bug/1241659/comments/6 > > I suspect there are cases where openstack sets LIBGUESTFS_BACKEND=direct > to workaround one of these two bugs. > > Is fixing the qemu:///session as root problem going to also solve this? If we had a real qemu:///session mode running QEMU itself as root, then we would never change permissions/ownership. We would still need to be changing SELinux labels & so the label restore logic is needd there. We should be able to use qemu:///system & the DAC driver to run QEMU as root though. There was previously a problem wrt monitor sockets that you hit when trying this with libguestfs, but I believe that should now be fixed: https://bugzilla.redhat.com/show_bug.cgi?id=890291#c30 If using the DAC driver to request running as root, the only remaining difference in terms of permissions is that we clear CAP_DAC_OVERRIDE, so the root user will only be able to access files which explicitly grant root access. We could fix this limitation in the DAC driver I believe to allow capabilities to be retained. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] security_stack: Perform rollback if one of stacked drivers fails
On Wed, Oct 09, 2019 at 07:49:29PM -0400, Cole Robinson wrote: > In that bug, I see that rjones (cc'd) said that libvirt not > remembering labels/uid causes issues for libguestfs that requires > workarounds. Rich, do you have links to threads or bug reports where > this is described in more detail? I think there are two problems (which I often confuse) and they are possibly related. This one where libvirt doesn't restore permissions afterwards, and the other one where qemu:///session cannot be used as root which implies that when you run libguestfs as root it doesn't have access to things that root would normally have access to (bug 890291 / 1045069). In answer to your question this is the only one I could find which is definitely related to this bug: https://www.redhat.com/archives/libguestfs/2013-May/msg00115.html Here's another one, but I think this is related to the other bug: https://bugs.launchpad.net/nova/+bug/1241659/comments/6 I suspect there are cases where openstack sets LIBGUESTFS_BACKEND=direct to workaround one of these two bugs. Is fixing the qemu:///session as root problem going to also solve this? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 18/23] virsh: convert command line parsing to use GOptionContext
On Mon, Oct 07, 2019 at 06:14:20PM +0100, Daniel P. Berrangé wrote: > The GOptionContext API has the benefit over getopt_long that it will > automatically handle --help output formatting. > > Signed-off-by: Daniel P. Berrangé > --- > tools/virsh.c | 304 +++--- > 1 file changed, 137 insertions(+), 167 deletions(-) This patch is broken. Unfortunately the GOptionContext parsing is not compatible enough with getopt_long. In particular you can't pass the value to a short option as a single flag. ie getopt_long allows '-k0' but GOptionContext requires to use '-k 0'. This is a backwards incompatible change so I don't think we can do this conversion. I might investigate improving GOptionContext, but for now this & the following patches are dropped. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 00/26] scripts: convert most perl scripts to python
On Thu, Oct 10, 2019 at 09:22:29AM +0200, Andrea Bolognani wrote: > On Thu, 2019-10-10 at 08:55 +0200, Erik Skultety wrote: > > On Wed, Oct 09, 2019 at 02:01:37PM +0200, Peter Krempa wrote: > > > On Wed, Oct 09, 2019 at 12:37:15 +0100, Daniel Berrange wrote: > > > > Note that the check-spacing.py script is significantly > > > > slower in Python than in Perl. After researching this > > > > it appears there is nothing that can be done. The Perl > > > > regex engine is simply much better optimized than the > > > > Python one. As previously discussed we need to loook > > > > at uncrustify or clang-format or some other tool to > > > > validate whitespace formatting. This is ongoing. We > > > > can either accept the slow down in the short term or > > > > keep the Perl version in the short term. > > > > > > I vote for keeping the perl version then. I don't see much value in > > > rewriting it into a much slower form just to drop it soon after. > > > > I second ^this opinion. > > Same, especially if it puts more pressure on us finally adopting > something like clang-format. Since all the patches in this series are essentially independant, I'm not going to repost this huge series just to drop that one patch. Just consider it dropped locally. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
On 10/10/2019 00.43, John Snow wrote: > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > I'd like to refactor these some day, and getting rid of the super-object > will make that easier. > > Either way, we don't need this. > > Libvirt-checked-by: Peter Krempa > Signed-off-by: John Snow > --- > qemu-deprecated.texi | 5 + > hw/ide/qdev.c | 3 +++ > tests/qemu-iotests/051.pc.out | 6 -- > 3 files changed, 12 insertions(+), 2 deletions(-) Reviewed-by: Thomas Huth -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] qemu: Add support for host-model pseries machine option
On Wed, Oct 09, 2019 at 02:29:37PM -0300, Fabiano Rosas wrote: > Daniel P. Berrangé writes: > > > What userspace tool is broken, and in what way ? > > The major use case, as far as I know, is from software license > managers which use this to determine how much to charge for > software. I would have to ask around to know exactly which ones and > how they operate. Mostly such code should not need to know the real host model / serial information. It should be sufficient to pass through fake data, as long it is consistently fake for all VMs on the same host. Still license managers like this are doomed in a virtualized world as it is trivial to fake the information they're relying on. > > Re-introducing the host passthrough to satisfy a broken tool is not > > very attractive because it reintroduces the security flaw that the > > QEMU change was fixing. > > Sure, this is reasonable. I'm just trying to make it less painful for > those that depend on the old behavior for some reason. =) > > Determined folks will probably just use anyway. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] IDE: deprecate ide-drive
John Snow writes: > It's an old compatibility shim that just delegates to ide-cd or ide-hd. > I'd like to refactor these some day, and getting rid of the super-object > will make that easier. > > Either way, we don't need this. > > Libvirt-checked-by: Peter Krempa > Signed-off-by: John Snow Reviewed-by: Markus Armbruster -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu_process: Initialize domain definition for QMP query
When constructing QMP capabilities we allocate a dummy domain object to pass to qemuMonitorOpen(). However, after 75dd595861 the function also expects domain definition to be allocated for the domain object. The referenced commit already fixed qemumonitortestutils.c but forgot to fix the other caller: qemuProcessQMPConnectMonitor(). Signed-off-by: Michal Privoznik --- Pushed under trivial rule. src/qemu/qemu_process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ca2a5cab5b..c14c09da11 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8654,7 +8654,8 @@ qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc) monConfig.data.nix.listen = false; if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL, NULL, NULL)) || -!(proc->vm = virDomainObjNew(xmlopt))) +!(proc->vm = virDomainObjNew(xmlopt)) || +!(proc->vm->def = virDomainDefNew())) goto cleanup; proc->vm->pid = proc->pid; -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Fix @vm locking issue when connecting to the monitor
On 10/9/19 11:42 PM, Jonathon Jongsma wrote: This change gives me a segfault at startup when running libvirtd from my working directory. Reverting these two patches solves the issue. Oops, yes. The qemuProcessQMPConnectMonitor() needs the same treatement as qemuMonitorCommonTestNew(). Since it's a trivial fix, I'll push it shortly. Thanks for reporting this. We should probably teach virDomainObjIsActive() to rely on vm->state rather than vm->def->id. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Limiting old version back compat for language bindings ?
On Mon, 2019-10-07 at 16:29 +0100, Daniel P. Berrangé wrote: > On Mon, Oct 07, 2019 at 04:58:34PM +0200, Andrea Bolognani wrote: > > It seems to me that people who want to run the latest version of > > whatever application will also use a non-obsolete operating system, > > and conversely people stuck with an old OS will rather also stick to > > the vendor-provided (and -supported) versions of the various > > components rather than installing newer ones from source. > > > > It's basically the same argument we used to justify libvirt dropping > > support for old operating systems and old QEMU versions, and I think > > it still applies when you take it one layer up the stack. > > It is the difference between the OS infrastructure layer and > the application layer. Essentially what I'm saying is that > libvirt is part of the OS infrastructure, and the libvirt > language bindings are part of the application infrastructure. > > It is valid to deploy on an old OS with vendor supplied libvirt, > while still using brand new libvirt python/Go bundled with the > application, not using the OS vendor provided version. > > The latter is in fact the recommended approach for application > developers in RHEL these days. We ship libvirt-python in RHEL-8 > built against the system python for use by virt tools we ship > like virt-install, virt-manager. From a support POV 3rd party > application developers are not supposed to use system python, > instead they must pick a python module stream. If they're lucky > their python module is the same version as system python and > they can use the system libvirt-python, but in general they > are expected to ship libvirt-python themselves as part of their > application install. I expected that to happen for languages like Rust and Go, but I thought for Python the common behavior would be to either used the packaged version (in the RHEL 8 case perhaps from a stream rather than the system one, doesn't change much) or perhaps bring in the necessary dependency with pip, not bundle it. > > If anything, the higher you go up the stack and the more developers > > are okay with having tighter coupling between their application and > > libvirt/QEMU versions, so I'd say it actually applies even more to > > them. > > I don't believe you can make such a blanket assertion about tight > coupling. I've seen both from apps developing against a very > specific min libvirt version only, vs apps developing against a > range of libvirt versions. I don't know. The latest version of oVirt requires RHEL 7.7, virt-manager is Python 3 only these days so it would be quite a pain to get it running on RHEL 6, if at all possible... The libvirt support matrix is fairly reasonable IMHO, and if you're stuck with a base OS which is outside of it I think your best bet is to stick with older versions of everything else as well. At some point, even applications that try to support arbitrarily old operating systems will end up in a situation where they're not actually testing on them (we have no CI test for libvirt-python or libvirt-go on CentOS 6, for example) and issues will either be ignored or a massive pain for developers to address. With all that said, I don't actually do any work on bindings so my own take only matters so much :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 00/26] scripts: convert most perl scripts to python
On Thu, 2019-10-10 at 08:55 +0200, Erik Skultety wrote: > On Wed, Oct 09, 2019 at 02:01:37PM +0200, Peter Krempa wrote: > > On Wed, Oct 09, 2019 at 12:37:15 +0100, Daniel Berrange wrote: > > > Note that the check-spacing.py script is significantly > > > slower in Python than in Perl. After researching this > > > it appears there is nothing that can be done. The Perl > > > regex engine is simply much better optimized than the > > > Python one. As previously discussed we need to loook > > > at uncrustify or clang-format or some other tool to > > > validate whitespace formatting. This is ongoing. We > > > can either accept the slow down in the short term or > > > keep the Perl version in the short term. > > > > I vote for keeping the perl version then. I don't see much value in > > rewriting it into a much slower form just to drop it soon after. > > I second ^this opinion. Same, especially if it puts more pressure on us finally adopting something like clang-format. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 01/12] conf: add replug option for usb hostdev
On 09.10.2019 20:02, Jonathon Jongsma wrote: > On Tue, 2019-09-17 at 16:47 +0300, Nikolay Shirokovskiy wrote: >> If usb device attached to a domain is unplugged from host and >> then plugged back then it will no longer be available in guest. >> We are going to support this case so that device will be detached >> from qemu on unplug and attached back on replug. As sometimes >> this behaviour is not desirable and for backcompat too let's >> add 'replug' option for usb hostdev. >> >> Signed-off-by: Nikolay Shirokovskiy >> --- >> docs/formatdomain.html.in | 10 - >> docs/schemas/domaincommon.rng | 5 +++ >> src/conf/domain_conf.c| 30 ++ >> src/conf/domain_conf.h| 2 + >> tests/qemuxml2argvdata/hostdev-usb-replug.xml | 36 + >> .../qemuxml2xmloutdata/hostdev-usb-replug.xml | 40 >> +++ >> tests/qemuxml2xmltest.c | 1 + >> 7 files changed, 122 insertions(+), 2 deletions(-) >> create mode 100644 tests/qemuxml2argvdata/hostdev-usb-replug.xml >> create mode 100644 tests/qemuxml2xmloutdata/hostdev-usb-replug.xml >> >> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in >> index 86a5261e47..5b0d41760b 100644 >> --- a/docs/formatdomain.html.in >> +++ b/docs/formatdomain.html.in >> @@ -4678,7 +4678,7 @@ >> >> ... >> devices >> - hostdev mode='subsystem' type='usb' >> + hostdev mode='subsystem' type='usb' replug='yes' >> source startupPolicy='optional' >>vendor id='0x1234'/ >>product id='0xbeef'/ >> @@ -4777,7 +4777,13 @@ >>usb >>USB devices are detached from the host on guest >> startup >> and reattached after the guest exits or the device is >> -hot-unplugged. >> +hot-unplugged. If optional replug >> +(since 5.8.0) is "yes" then >> libvirt >> +tracks USB device unplug/plug on host. On unplug the >> correspondent > > "corresponding" is a bit better here > >> +QEMU device will be be deleted but device stays in >> libvirt config. > > Do we want to mention qemu explicitly here? Presumably this > configuration option could be hypervisor indepedendant? Maybe it's > better to refer generically to the "guest" or "hypervisor" rather than > qemu? > Yeah, makes sense. Nikolay -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 00/26] scripts: convert most perl scripts to python
On Wed, Oct 09, 2019 at 02:01:37PM +0200, Peter Krempa wrote: > On Wed, Oct 09, 2019 at 12:37:15 +0100, Daniel Berrange wrote: > > [...] > > > - src/rpc/gendispatch.pl > > - src/rpc/genprotocol.pl > > - tools/wireshark/util/genxdrstub.pl > > [...] > > > Note that the check-spacing.py script is significantly > > slower in Python than in Perl. After researching this > > it appears there is nothing that can be done. The Perl > > regex engine is simply much better optimized than the > > Python one. As previously discussed we need to loook > > at uncrustify or clang-format or some other tool to > > validate whitespace formatting. This is ongoing. We > > can either accept the slow down in the short term or > > keep the Perl version in the short term. > > I vote for keeping the perl version then. I don't see much value in > rewriting it into a much slower form just to drop it soon after. I second ^this opinion. Regards, Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
