[PATCH 1/2] conf: allow display and ramfb for vfio pci hostdevs
We already allow the user to specify display="on" and ramfb="on" for
mdev host devices. But newer GPU models will no longer use the mdev
framework, so we should enable this same functionality for other
non-mdev passthrough PCI devices.
Resolves: https://issues.redhat.com/browse/RHEL-28808
Signed-off-by: Jonathon Jongsma
---
docs/formatdomain.rst | 8
src/conf/domain_conf.c| 20 +++-
src/conf/domain_conf.h| 2 ++
src/conf/domain_validate.c| 21 +
src/conf/schemas/domaincommon.rng | 25 +++--
5 files changed, 57 insertions(+), 19 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 2adc2ff968..ab44ed58a6 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -4390,6 +4390,14 @@ or:
starting the guest or hot-plugging the device and
``virNodeDeviceReAttach`` (or ``virsh nodedev-reattach``) after
hot-unplug
or stopping the guest.
+ :since:`Since 10.2.0` an optional ``display`` attribute may be used to
+ enable using a vgpu device as a display device for the guest. Supported
+ values are either ``on`` or ``off`` (default). There is also an optional
+ ``ramfb`` attribute with values of either ``on`` or ``off`` (default).
+ When enabled, the ``ramfb`` attribute provides a memory framebuffer
device
+ to the guest. This framebuffer allows the vgpu to be used as a boot
display
+ before the gpu driver is loaded within the guest. ``ramfb`` requires the
+ ``display`` attribute to be set to ``on``.
``scsi``
For SCSI devices, user is responsible to make sure the device is not used
by host. If supported by the hypervisor and OS, the optional ``sgio`` (
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 770b5fbbff..11a0b0ecda 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6306,6 +6306,16 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
VIR_XML_PROP_NONE,
>ramfb) < 0)
return -1;
+} else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+if (virXMLPropTristateSwitch(node, "display",
+ VIR_XML_PROP_NONE,
+ >display) < 0)
+return -1;
+
+if (virXMLPropTristateSwitch(node, "ramfb",
+ VIR_XML_PROP_NONE,
+ >ramfb) < 0)
+return -1;
}
switch (def->source.subsys.type) {
@@ -26251,6 +26261,7 @@ virDomainHostdevDefFormat(virBuffer *buf,
const char *mode = virDomainHostdevModeTypeToString(def->mode);
virDomainHostdevSubsysSCSI *scsisrc = >source.subsys.u.scsi;
virDomainHostdevSubsysMediatedDev *mdevsrc = >source.subsys.u.mdev;
+virDomainHostdevSubsysPCI *pcisrc = >source.subsys.u.pci;
virDomainHostdevSubsysSCSIVHost *scsihostsrc =
>source.subsys.u.scsi_host;
const char *type;
@@ -26319,7 +26330,14 @@ virDomainHostdevDefFormat(virBuffer *buf,
virBufferAsprintf(buf, " ramfb='%s'",
virTristateSwitchTypeToString(mdevsrc->ramfb));
}
-
+if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+if (pcisrc->display != VIR_TRISTATE_SWITCH_ABSENT)
+virBufferAsprintf(buf, " display='%s'",
+
virTristateSwitchTypeToString(pcisrc->display));
+if (pcisrc->ramfb != VIR_TRISTATE_SWITCH_ABSENT)
+virBufferAsprintf(buf, " ramfb='%s'",
+
virTristateSwitchTypeToString(pcisrc->ramfb));
+}
}
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 76251938b8..5925faaf1a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -236,6 +236,8 @@ struct _virDomainHostdevSubsysUSB {
struct _virDomainHostdevSubsysPCI {
virPCIDeviceAddress addr; /* host address */
virDeviceHostdevPCIDriverInfo driver;
+virTristateSwitch display;
+virTristateSwitch ramfb;
virBitmap *origstates;
};
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index faa7659f07..395e036e8f 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1291,15 +1291,20 @@ virDomainDefHostdevValidate(const virDomainDef *def)
}
}
-if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
-dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
-dev->source.subsys.u.mdev.ramfb == VIR_TRISTATE_SWITCH_ON) {
-if (ramfbEnabled) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("Only one vgpu device can have 'ramfb'
[PATCH 2/2] qemu: enable display/ramfb for vfio pci hostdevs
Implement display="on" and ramfb="on" for vfio PCI host devices in qemu.
This enables passthrough PCI devices for display just like we did for
mdevs.
Resolves: https://issues.redhat.com/browse/RHEL-28808
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_command.c | 10 -
src/qemu/qemu_validate.c | 8
...stdev-pci-display-ramfb.x86_64-latest.args | 33 ++
...ostdev-pci-display-ramfb.x86_64-latest.xml | 44 +++
.../hostdev-pci-display-ramfb.xml | 33 ++
tests/qemuxmlconftest.c | 1 +
6 files changed, 128 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args
create mode 100644
tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.xml
create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2bb1b6a0e7..62df609884 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4735,10 +4735,16 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def,
virDomainNetTeamingInfo *teaming;
g_autofree char *host = virPCIDeviceAddressAsString(>addr);
const char *failover_pair_id = NULL;
+const char *driver = NULL;
/* caller has to assign proper passthrough driver name */
switch (pcisrc->driver.name) {
case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO:
+/* ramfb support requires the nohotplug variant */
+if (pcisrc->ramfb == VIR_TRISTATE_SWITCH_ON)
+driver = "vfio-pci-nohotplug";
+else
+driver = "vfio-pci";
break;
case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_KVM:
@@ -4762,11 +4768,13 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def,
failover_pair_id = teaming->persistent;
if (virJSONValueObjectAdd(,
- "s:driver", "vfio-pci",
+ "s:driver", driver,
"s:host", host,
"s:id", dev->info->alias,
"p:bootindex", dev->info->effectiveBootIndex,
"S:failover_pair_id", failover_pair_id,
+ "S:display", qemuOnOffAuto(pcisrc->display),
+ "B:ramfb", pcisrc->ramfb,
NULL) < 0)
return NULL;
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 6a73d02050..edc7169a30 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -2477,6 +2477,14 @@ qemuValidateDomainDeviceDefHostdev(const
virDomainHostdevDef *hostdev,
_("Write filtering of PCI device configuration
space is not supported by qemu"));
return -1;
}
+
+if (hostdev->source.subsys.u.pci.display ==
VIR_TRISTATE_SWITCH_ON) {
+if (def->ngraphics == 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("graphics device is needed for attribute
value 'display=on' in "));
+return -1;
+}
+}
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:
diff --git a/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args
b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args
new file mode 100644
index 00..6a3bfbe6fb
--- /dev/null
+++ b/tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args
@@ -0,0 +1,33 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest2/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest2,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest2/master-key.aes"}'
\
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
+-accel tcg \
+-cpu qemu64 \
+-m size=219136k \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-vnc 127.0.0.1:0,audiodev=audio1 \
+-device
'{"driver":"vfio-pci-nohotplug","host":":06:12.5","id":"hostdev0","display":"on","ramfb":true,"bus":"pci.0","addr":"0x2"}'
\
[PATCH 0/2] Add 'display' and 'ramfb' option for pci hostdevs
see https://issues.redhat.com/browse/RHEL-28808 Jonathon Jongsma (2): conf: allow display and ramfb for vfio pci hostdevs qemu: enable display/ramfb for vfio pci hostdevs docs/formatdomain.rst | 8 src/conf/domain_conf.c| 20 - src/conf/domain_conf.h| 2 + src/conf/domain_validate.c| 21 + src/conf/schemas/domaincommon.rng | 25 ++- src/qemu/qemu_command.c | 10 - src/qemu/qemu_validate.c | 8 ...stdev-pci-display-ramfb.x86_64-latest.args | 33 ++ ...ostdev-pci-display-ramfb.x86_64-latest.xml | 44 +++ .../hostdev-pci-display-ramfb.xml | 33 ++ tests/qemuxmlconftest.c | 1 + 11 files changed, 185 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/hostdev-pci-display-ramfb.xml -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 0/7] qemu XML testing improvements, part 3 - xmlout->xmlout testing and fixes
On Tue, Mar 19, 2024 at 21:31:52 +0100, Michal Prívozník wrote: > On 1/12/24 17:05, Peter Krempa wrote: > > The main goal of part 3 is to add testing based on parsing of the > > libvirt-formatted files from tests/qemuxml2xmloutdata and formatting > > them back, and checking that they are identical. > > Firstly, sorry for resurrecting an old thread. > Secondly, sorry for hijacking it. BUT. > > There were patches sent to the list recently [1] and I realized, the way > we usually used to do things is disturbed. I mean, whenever new device, > device knob, ... was introduced the patch series consisted (roughly) of > the following patches: > > 1) config, XML parser & formatter, RNG, docs AND xml2xml test case, > 2) qemu caps > 3) qemu cmd line AND xml2argv test case. > > Now, since we have this one huge qemuxmlconftest.c it's not as easy. If > I try to introduce a test case in 1), the test case fails as there's no > corresponding .args file. You can add a corresponding .args file that will not yet have the commandline bits added by the patches, as the implementation is missing. This means that the corresponding patch adding the qemu code will need to contain the corresponding .args change. > Fair, but also not fair - the feature is not > finished at the time of 1) so .args shouldn't even be considered. Well, it still covers what the code does at that point, so at the very least in theory it makes sense. In practice, it possibly looks a bit awkward, but the tests can still be added in a separate commit after both the XML and qemu bits are implemented. As long as the patches test the code I don't really have a problem with that. > But if > a test case is added at step 3) - well, then anybody backporting 1) > won't get the xml2xml test case. Pity. I mean, that's the whole point we > split the change into "frontend" and "backend", right? This case would be a problem only if a patchset would add implementations in two hypervisor drivers at same time and you'd decide to backport only the non-qemu one. In such case the qemu XML coverage would be missing. Reallistically that won't happen, but if it did, the corresponding patches to the other hypervisor driver should add their own tests which you really should bacport or you can also add genericxml2xmltest cases. In quite a lot of cases the qemuxml2xmltest case was also added in a separate commit which is totally fine and the 'backport' case above would not cover that one either. > Okay, you may add step 4), which introduces new qemuxmlconftest.c test > case. But it bundles both xml2xml AND xml2argv steps rendering it yet > again unsuitable for backport. I don't see how that would be unsuitable for backport. If you are backporting a qemu feature, then the qemu* tests are relevant. Otherwise you should be backporting the corresponding tests at least for that hypervisor you are adding. > One way out might be to add new testcases to genericxml2xmltest.c, but > somehow that feels wrong. Why? Genericxml2xml test is there for all generic configs. It's just that most code usually adds a qemu implementation and it's both more convenient and sufficient to excercise all things to add just the qemuxmlconftest case. P.S: 37% qemuxml2argv test cases did not have a corresponding qemuxml2xmltest case at the time when I've converted it over. Additional few were actually testing something else due to a mismatch in capabilities. ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 0/7] qemu XML testing improvements, part 3 - xmlout->xmlout testing and fixes
On 1/12/24 17:05, Peter Krempa wrote: > The main goal of part 3 is to add testing based on parsing of the > libvirt-formatted files from tests/qemuxml2xmloutdata and formatting > them back, and checking that they are identical. Firstly, sorry for resurrecting an old thread. Secondly, sorry for hijacking it. BUT. There were patches sent to the list recently [1] and I realized, the way we usually used to do things is disturbed. I mean, whenever new device, device knob, ... was introduced the patch series consisted (roughly) of the following patches: 1) config, XML parser & formatter, RNG, docs AND xml2xml test case, 2) qemu caps 3) qemu cmd line AND xml2argv test case. Now, since we have this one huge qemuxmlconftest.c it's not as easy. If I try to introduce a test case in 1), the test case fails as there's no corresponding .args file. Fair, but also not fair - the feature is not finished at the time of 1) so .args shouldn't even be considered. But if a test case is added at step 3) - well, then anybody backporting 1) won't get the xml2xml test case. Pity. I mean, that's the whole point we split the change into "frontend" and "backend", right? Okay, you may add step 4), which introduces new qemuxmlconftest.c test case. But it bundles both xml2xml AND xml2argv steps rendering it yet again unsuitable for backport. One way out might be to add new testcases to genericxml2xmltest.c, but somehow that feels wrong. Michal 1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/LEAENTOC4GA5DJGGEII4A3BS6YYGO7IP/ ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virsysinfo: Try reading DMI table
On Tue, Mar 19, 2024 at 07:53:35PM -, brett.hol...@canonical.com wrote: > My apologies for this duplicate, please direct reviews to this thread -> > https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/G6GPDHYL7CT7MFRECAPL7ZDSXOWQUABG/ No worries, it was my oversight to blindly approve everything in the moderator queue, without checking if you had since subscribed to the list and re-sent the patches. With 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 :| ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virsysinfo: Try reading DMI table
My apologies for this duplicate, please direct reviews to this thread -> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/G6GPDHYL7CT7MFRECAPL7ZDSXOWQUABG/ ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virsysinfo: Try reading DMI table
My apologies for this duplicate, please direct reviews to this thread -> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/G6GPDHYL7CT7MFRECAPL7ZDSXOWQUABG/ ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] NEWS: Announce support for MTP filesystem driver type
Signed-off-by: Rayhan Faizel --- NEWS.rst | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 489201d3fc..16a34e8114 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,12 @@ v10.2.0 (unreleased) * **New features** + * qemu: Support for driver type ``mtp`` in devices + +The ``mtp`` driver type exposes the ``usb-mtp`` device in QEMU. The +guest can access files on this driver through the Media Transfer +Protocol (MTP). + * **Improvements** * **Bug fixes** -- 2.34.1 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 0/2] meson: Fix/improve detection of scheduler-related functionality
On Tue, Mar 19, 2024 at 05:45:10PM +0100, Michal Prívozník wrote: > On 2/27/24 19:30, Andrea Bolognani wrote: > > This applies on top of [1]. Test pipeline: [2] > > > > Upon further investigation, I have determined that not only > > we are unintentionally using the Linux APIs on FreeBSD instead > > of the native ones, but we are also accidentally skipping some > > setup steps on Linux. > > > > [1] > > https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/CX26NU7ZHD4TFDPQM3HQ4YW5ATP4LYDV/ > > [2] https://gitlab.com/abologna/libvirt/-/pipelines/1192942974 > > > > Andrea Bolognani (2): > > meson: Restore check for sched_getaffinity() > > meson: Check for sched_get_priority_min() > > > > meson.build | 4 ++-- > > src/ch/ch_process.c | 1 + > > src/util/virprocess.c | 12 ++-- > > 3 files changed, 9 insertions(+), 8 deletions(-) > > Reviewed-by: Michal Privoznik > > but let me just say that I had to dig [1] out of the archives only to > apply these cleanly (even though they were reviewed long time ago and > agreed to be pushed "soon") did not help. Yeah, sorry for the annoyance. And thanks a lot for the review! I've pushed everything now. -- Andrea Bolognani / Red Hat / Virtualization ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 0/2] meson: Fix/improve detection of scheduler-related functionality
On 2/27/24 19:30, Andrea Bolognani wrote: > This applies on top of [1]. Test pipeline: [2] > > Upon further investigation, I have determined that not only > we are unintentionally using the Linux APIs on FreeBSD instead > of the native ones, but we are also accidentally skipping some > setup steps on Linux. > > [1] > https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/CX26NU7ZHD4TFDPQM3HQ4YW5ATP4LYDV/ > [2] https://gitlab.com/abologna/libvirt/-/pipelines/1192942974 > > Andrea Bolognani (2): > meson: Restore check for sched_getaffinity() > meson: Check for sched_get_priority_min() > > meson.build | 4 ++-- > src/ch/ch_process.c | 1 + > src/util/virprocess.c | 12 ++-- > 3 files changed, 9 insertions(+), 8 deletions(-) > Reviewed-by: Michal Privoznik but let me just say that I had to dig [1] out of the archives only to apply these cleanly (even though they were reviewed long time ago and agreed to be pushed "soon") did not help. Michal ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 0/4] qemu: Add support for mtp filesystem driver
On 3/8/24 21:16, Rayhan Faizel wrote: > This patch series adds support for the mtp backed filesystem device > exposed through a virtual USB MTP device. > > Usage: > > > > > > > > Rayhan Faizel (4): > qemu_capabilities: Add QEMU_CAPS_DEVICE_MTP capability > qemu: Support for parsing usb-mtp devices > tests: Add testcases for mtp filesystem driver > docs: Add documentation for mtp filesystem driver > > 49 files changed, 220 insertions(+), 2 deletions(-) Reviewed-by: Michal Privoznik Do you mind posting a patch that mentions this new feature in NEWS.rst? It's something users might be interested in. Michal ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 2/4] qemu: Support for parsing usb-mtp devices
On 3/8/24 21:16, Rayhan Faizel wrote: > The source tag sets the rootdir property of the device, which is the directory > exposed to the guest via the MTP device. The target tag sets the desc > property. > This device supports read-only mode as well. Like virtiofs, it does not > support additional access modes. > > Signed-off-by: Rayhan Faizel > --- > src/bhyve/bhyve_command.c | 1 + > src/conf/domain_conf.c| 10 + > src/conf/domain_conf.h| 1 + > src/conf/schemas/domaincommon.rng | 5 + > src/qemu/qemu_command.c | 34 +++ > src/qemu/qemu_domain_address.c| 7 +-- > src/qemu/qemu_validate.c | 13 > 7 files changed, 69 insertions(+), 2 deletions(-) The code is okay, but for future - we tend to split changes a bit differently: in one patch we do necessary XML parsing & formatting work, update RNG and docs. Then, in another patch xml -> qemu cmd line generator is introduced. Reasoning is quite simple - easier backports. And of course, split between front & back ends. Now, in the past we used to update tests too: in the commit that introduces XML parsing & formatting an xml2xml test case was introduced, then in the other patch (xml -> qemu cmd line) an xml2argv test case was introdcued. But this is hard to do after qemuxml2xmltest and qemuxml2argvtest were merged into one. I'm not sure what our recommendation should be here. For now, I'll just merge qemu command line generation code AND test cases (patch 3/4) into one. Let me split changes in patches 2-4 into separate ones. Michal ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 1/4] qemu_capabilities: Add QEMU_CAPS_DEVICE_MTP capability
On 3/8/24 21:16, Rayhan Faizel wrote: > Signed-off-by: Rayhan Faizel > --- > src/qemu/qemu_capabilities.c | 2 ++ > src/qemu/qemu_capabilities.h | 1 + > tests/qemucapabilitiesdata/caps_4.2.0_aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_4.2.0_ppc64.xml | 1 + > tests/qemucapabilitiesdata/caps_4.2.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.0.0_aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.0.0_ppc64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.0.0_riscv64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.0.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.1.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.2.0_ppc64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.2.0_riscv64.xml | 1 + > tests/qemucapabilitiesdata/caps_5.2.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.0.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.1.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.2.0_ppc64.xml | 1 + > tests/qemucapabilitiesdata/caps_6.2.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml | 1 + > tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.0.0_ppc64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.0.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + > tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml | 1 + > tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml | 1 + > tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + > tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_8.2.0_s390x.xml | 1 + > tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml | 1 + > 37 files changed, 38 insertions(+) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index ab11a929a3..7696756c3e 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -704,6 +704,7 @@ VIR_ENUM_IMPL(virQEMUCaps, > >/* 455 */ >"blockjob.backing-mask-protocol", /* > QEMU_CAPS_BLOCKJOB_BACKING_MASK_PROTOCOL */ > + "mtp", /* QEMU_CAPS_DEVICE_MTP */ Nitpick, I'd name this "usb-mtp" and QEMU_CAPS_DEVICE_USB_MTP so that it's obvious the corresponding device is "-device usb-mtp" and not just "-device mtp". I'll do the necessary changes before pushing. Michal ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 01/28] vshCmddefHelp: Drop empty line at the end
On a Friday in 2024, Peter Krempa wrote: All virsh commands in non-quiet mode append another separator line thus having two is unnecessary and in quiet mode it still has a trailing blank line. Remove it. Signed-off-by: Peter Krempa --- tools/vsh.c | 1 - 1 file changed, 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Plans for 10.2.0 release (freeze on Monday 25 Mar)
We are getting close to 10.2.0 release of libvirt. To aim for the release on Tuesday 02 Apr I suggest entering the freeze on Monday 25 Mar and tagging RC2 on Thursday 28 Mar. I hope this works for everyone. Jirka ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virt-admin: Fix segfault when libvirtd dies
On 3/19/24 15:58, Jiri Denemark wrote:
> On Tue, Mar 19, 2024 at 14:34:08 +0100, Claudio Fontana wrote:
>> On 3/19/24 12:02, Adam Julis wrote:
>>> vshAdmCatchDisconnect requires non-NULL structure vshControl for
>>> getting connection name (stored at opaque), but
>>> virAdmConnectRegisterCloseCallback at vshAdmConnect called it
>>> with NULL.
>>>
>>> Signed-off-by: Adam Julis ---
>>> tools/virt-admin.c | 2 +- 1 file changed, 1 insertion(+), 1
>>> deletion(-)
>>>
>>> diff --git a/tools/virt-admin.c b/tools/virt-admin.c index
>>> 37bc6fe4f0..0766032e4a 100644 --- a/tools/virt-admin.c +++
>>> b/tools/virt-admin.c @@ -112,7 +112,7 @@ vshAdmConnect(vshControl
>>> *ctl, unsigned int flags) return -1; } else { if
>>> (virAdmConnectRegisterCloseCallback(priv->conn,
>>> vshAdmCatchDisconnect, -
>>> NULL, NULL) < 0) +
>>> ctl, NULL) < 0) vshError(ctl, "%s", _("Unable to register
>>> disconnect callback"));
>>>
>>> if (priv->wantReconnect)
>>
>> Hi,
>>
>> if that is the case I would then expect that
>> virAdmConnectRegisterCloseCallback() needs to also be updated
>> with:
>>
>> +virCheckNonNullArgGoto(opaque, error);
>>
>> or something like that. Is there a reason why it isn't? We better
>> catch the error early if the API is used wrongly.
>
> Well, vshAdmCatchDisconnect requires opaque to be non-NULL, but
> other callbacks registered with virAdmConnectRegisterCloseCallback
> may not need any opaque data.
>
Fair enough.
Reviewed-by: Claudio Fontana
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virt-admin: Fix segfault when libvirtd dies
On a Tuesday in 2024, Adam Julis wrote: vshAdmCatchDisconnect requires non-NULL structure vshControl for getting connection name (stored at opaque), but virAdmConnectRegisterCloseCallback at vshAdmConnect called it with NULL. Signed-off-by: Adam Julis --- tools/virt-admin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virt-admin: Fix segfault when libvirtd dies
On Tue, Mar 19, 2024 at 14:34:08 +0100, Claudio Fontana wrote:
> On 3/19/24 12:02, Adam Julis wrote:
> > vshAdmCatchDisconnect requires non-NULL structure vshControl for
> > getting connection name (stored at opaque), but
> > virAdmConnectRegisterCloseCallback at vshAdmConnect called it
> > with NULL.
> >
> > Signed-off-by: Adam Julis
> > ---
> > tools/virt-admin.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> > index 37bc6fe4f0..0766032e4a 100644
> > --- a/tools/virt-admin.c
> > +++ b/tools/virt-admin.c
> > @@ -112,7 +112,7 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
> > return -1;
> > } else {
> > if (virAdmConnectRegisterCloseCallback(priv->conn,
> > vshAdmCatchDisconnect,
> > - NULL, NULL) < 0)
> > + ctl, NULL) < 0)
> > vshError(ctl, "%s", _("Unable to register disconnect
> > callback"));
> >
> > if (priv->wantReconnect)
>
> Hi,
>
> if that is the case I would then expect that
> virAdmConnectRegisterCloseCallback() needs to also be updated with:
>
> +virCheckNonNullArgGoto(opaque, error);
>
> or something like that. Is there a reason why it isn't? We better catch the
> error early if the API is used wrongly.
Well, vshAdmCatchDisconnect requires opaque to be non-NULL, but other
callbacks registered with virAdmConnectRegisterCloseCallback may not
need any opaque data.
Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v2 1/1] remote: properly initialize objects in ACL helpers
Commit 2ecdf259299813c2c674377e22a0acbce5ccbbb2 was intended to
implement two things: reduce stack usage inside ACL helpers and
minimally initialize virDomainDef object to avoid passing garbage
inside validation framework. Though original commit has not
touched other ACL helpers.
This patch adds proper clauses to
remoteRelayNetworkEventCheckACL
remoteRelayStoragePoolEventCheckACL
remoteRelayNodeDeviceEventCheckACL
remoteRelaySecretEventCheckACL
Signed-off-by: Denis V. Lunev
CC: Peter Krempa
CC: Roman Grigoriev
---
src/remote/remote_daemon_dispatch.c | 32 ++---
1 file changed, 16 insertions(+), 16 deletions(-)
Changes from v1:
* g_autoptr is replaced with g_autofree upon reached consensus
* patch 1 in series has been dropped
diff --git a/src/remote/remote_daemon_dispatch.c
b/src/remote/remote_daemon_dispatch.c
index aaabd1e56c..b566a510b8 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -180,21 +180,21 @@ static bool
remoteRelayNetworkEventCheckACL(virNetServerClient *client,
virConnectPtr conn, virNetworkPtr net)
{
-virNetworkDef def;
+g_autofree virNetworkDef *def = g_new0(virNetworkDef, 1);
g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virNetworkDef with enough contents to
* satisfy what viraccessdriverpolkit.c references. This is a bit
* fragile, but I don't know of anything better. */
-def.name = net->name;
-memcpy(def.uuid, net->uuid, VIR_UUID_BUFLEN);
+def->name = net->name;
+memcpy(def->uuid, net->uuid, VIR_UUID_BUFLEN);
if (!(identity = virNetServerClientGetIdentity(client)))
goto cleanup;
if (virIdentitySetCurrent(identity) < 0)
goto cleanup;
-ret = virConnectNetworkEventRegisterAnyCheckACL(conn, );
+ret = virConnectNetworkEventRegisterAnyCheckACL(conn, def);
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
@@ -206,21 +206,21 @@ remoteRelayStoragePoolEventCheckACL(virNetServerClient
*client,
virConnectPtr conn,
virStoragePoolPtr pool)
{
-virStoragePoolDef def;
+g_autofree virStoragePoolDef *def = g_new0(virStoragePoolDef, 1);
g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virStoragePoolDef with enough contents to
* satisfy what viraccessdriverpolkit.c references. This is a bit
* fragile, but I don't know of anything better. */
-def.name = pool->name;
-memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN);
+def->name = pool->name;
+memcpy(def->uuid, pool->uuid, VIR_UUID_BUFLEN);
if (!(identity = virNetServerClientGetIdentity(client)))
goto cleanup;
if (virIdentitySetCurrent(identity) < 0)
goto cleanup;
-ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, );
+ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, def);
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
@@ -232,20 +232,20 @@ remoteRelayNodeDeviceEventCheckACL(virNetServerClient
*client,
virConnectPtr conn,
virNodeDevicePtr dev)
{
-virNodeDeviceDef def;
+g_autofree virNodeDeviceDef *def = g_new0(virNodeDeviceDef, 1);
g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virNodeDeviceDef with enough contents to
* satisfy what viraccessdriverpolkit.c references. This is a bit
* fragile, but I don't know of anything better. */
-def.name = dev->name;
+def->name = dev->name;
if (!(identity = virNetServerClientGetIdentity(client)))
goto cleanup;
if (virIdentitySetCurrent(identity) < 0)
goto cleanup;
-ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, );
+ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, def);
cleanup:
ignore_value(virIdentitySetCurrent(NULL));
@@ -257,22 +257,22 @@ remoteRelaySecretEventCheckACL(virNetServerClient *client,
virConnectPtr conn,
virSecretPtr secret)
{
-virSecretDef def;
+g_autofree virSecretDef *def = g_new0(virSecretDef, 1);
g_autoptr(virIdentity) identity = NULL;
bool ret = false;
/* For now, we just create a virSecretDef with enough contents to
* satisfy what viraccessdriverpolkit.c references. This is a bit
* fragile, but I don't know of anything better. */
-memcpy(def.uuid, secret->uuid, VIR_UUID_BUFLEN);
-def.usage_type = secret->usageType;
-def.usage_id = secret->usageID;
+memcpy(def->uuid, secret->uuid, VIR_UUID_BUFLEN);
+def->usage_type = secret->usageType;
+def->usage_id = secret->usageID;
if (!(identity = virNetServerClientGetIdentity(client)))
goto cleanup;
Re: [PATCH] virt-admin: Fix segfault when libvirtd dies
On 3/19/24 12:02, Adam Julis wrote:
> vshAdmCatchDisconnect requires non-NULL structure vshControl for
> getting connection name (stored at opaque), but
> virAdmConnectRegisterCloseCallback at vshAdmConnect called it
> with NULL.
>
> Signed-off-by: Adam Julis
> ---
> tools/virt-admin.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> index 37bc6fe4f0..0766032e4a 100644
> --- a/tools/virt-admin.c
> +++ b/tools/virt-admin.c
> @@ -112,7 +112,7 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
> return -1;
> } else {
> if (virAdmConnectRegisterCloseCallback(priv->conn,
> vshAdmCatchDisconnect,
> - NULL, NULL) < 0)
> + ctl, NULL) < 0)
> vshError(ctl, "%s", _("Unable to register disconnect callback"));
>
> if (priv->wantReconnect)
Hi,
if that is the case I would then expect that
virAdmConnectRegisterCloseCallback() needs to also be updated with:
+virCheckNonNullArgGoto(opaque, error);
or something like that. Is there a reason why it isn't? We better catch the
error early if the API is used wrongly.
Claudio
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 1/2] openvz_driver: use g_autofree instead of VIR_FREE in openvz_driver.c
On 3/17/24 16:19, Karim Taha wrote:
> Signed-off-by: Karim Taha
> ---
> src/openvz/openvz_driver.c | 15 ---
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 1bd96dfb56..fd3c25a393 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
> @@ -615,7 +615,7 @@ openvzGenerateVethName(int veid, char *dev_name_ve)
> static char *
> openvzGenerateContainerVethName(int veid)
> {
> -char *temp = NULL;
> +g_autofree char *temp = NULL;
> char *name = NULL;
>
> /* try to get line "^NETIF=..." from config */
> @@ -638,8 +638,6 @@ openvzGenerateContainerVethName(int veid)
> name = g_strdup_printf("eth%d", max + 1);
> }
>
> -VIR_FREE(temp);
> -
> return name;
> }
>
> @@ -750,7 +748,7 @@ openvzDomainSetNetworkConfig(virConnectPtr conn,
> {
> size_t i;
> g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> -char *param;
> +g_autofree char *param = NULL;
We can take this opportunity and move this declaration ...
> int first = 1;
> struct openvz_driver *driver = conn->privateData;
>
> @@ -774,12 +772,10 @@ openvzDomainSetNetworkConfig(virConnectPtr conn,
... here. It's only used inside this block.
> param = virBufferContentAndReset();
> if (param) {
> if (openvzWriteVPSConfigParam(strtoI(def->name), "NETIF", param)
> < 0) {
> -VIR_FREE(param);
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("cannot replace NETIF config"));
> return -1;
> }
> -VIR_FREE(param);
> }
> }
>
Michal
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 0/2] Use g_autofree annotatins instead of VIR_FREE
On 3/17/24 16:19, Karim Taha wrote: > Substitute VIR_FREE with g_autofree annotations in openvz_driver.c and > node_device_driver.c whenever possible. > > The remaining instances of VIR_FREE are used to free return values > in case of error. > > Kariiem (2): > openvz_driver: use g_autofree instead of VIR_FREE in openvz_driver.c > node_device: use g_autofree instead of VIR_FREE in > node_device_driver.c > > src/node_device/node_device_driver.c | 4 +--- > src/openvz/openvz_driver.c | 15 --- > 2 files changed, 5 insertions(+), 14 deletions(-) > Reviewed-by: Michal Privoznik and merged. Congratulations on your first libvirt contribution! Michal ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] virsysinfo: Try reading DMI table
From: Brett Holman The SMBIOS specification[1] includes RISC-V and mips, and some systems have SMBIOS info. Attempt to read dmidecode and fall back to old behavior if that fails. [1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.4.0.pdf Signed-off-by: Brett Holman --- src/util/virsysinfo.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 36a861c53f..d8d660d694 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -1248,6 +1248,8 @@ virSysinfoRead(void) #elif !defined(WIN32) && \ (defined(__x86_64__) || \ defined(__i386__) || \ + defined(__mips__) || \ + defined(__riscv__) || \ defined(__amd64__)) return virSysinfoReadDMI(); #else /* WIN32 || not supported arch */ -- 2.43.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[RFC] virsysinfo: Try reading DMI table
This patch intends to add DMI support to libvirt for RISC-V and mips. This change is based on commit ec6ce6363, which added ARM support. This is not tested on the target arches, as I've been unable to find hardware to test this on. src/util/virsysinfo.c | 2 ++ 1 file changed, 2 insertions(+) Cheers, Brett Holman PS: This is my first post on this mailing list. I believe that I've followed the documented steps, but if I've missed anything please let me know. ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] virsysinfo: Try reading DMI table
From: Brett Holman The SMBIOS specification[1] includes RISC-V and mips, and some systems have SMBIOS info. Attempt to read dmidecode and fall back to old behavior if that fails. [1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.4.0.pdf Signed-off-by: Brett Holman --- src/util/virsysinfo.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 36a861c53f..d8d660d694 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -1248,6 +1248,8 @@ virSysinfoRead(void) #elif !defined(WIN32) && \ (defined(__x86_64__) || \ defined(__i386__) || \ + defined(__mips__) || \ + defined(__riscv__) || \ defined(__amd64__)) return virSysinfoReadDMI(); #else /* WIN32 || not supported arch */ -- 2.43.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[RFC] virsysinfo: Try reading DMI table
This patch intends to add DMI support to libvirt for RISC-V and mips. This is based on commit ec6ce6363, which added ARM support. This is untested, as I've been unable to find hardware to test this on. src/util/virsysinfo.c | 2 ++ 1 file changed, 2 insertions(+) Cheers, Brett Holman P.S. This is my first post on this mailing list, I believe that I've followed ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] virt-admin: Fix segfault when libvirtd dies
vshAdmCatchDisconnect requires non-NULL structure vshControl for
getting connection name (stored at opaque), but
virAdmConnectRegisterCloseCallback at vshAdmConnect called it
with NULL.
Signed-off-by: Adam Julis
---
tools/virt-admin.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 37bc6fe4f0..0766032e4a 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -112,7 +112,7 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
return -1;
} else {
if (virAdmConnectRegisterCloseCallback(priv->conn,
vshAdmCatchDisconnect,
- NULL, NULL) < 0)
+ ctl, NULL) < 0)
vshError(ctl, "%s", _("Unable to register disconnect callback"));
if (priv->wantReconnect)
--
2.43.2
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 1/1] gitignore: add cscope files to git ignore
On 3/19/24 11:02, Martin Kletzander wrote: On Mon, Mar 18, 2024 at 09:35:32PM +0100, Denis V. Lunev wrote: On 3/18/24 15:25, Martin Kletzander wrote: On Mon, Mar 18, 2024 at 02:25:53PM +0100, Pavel Hrdina wrote: On Mon, Mar 18, 2024 at 01:31:27PM +0100, Peter Krempa wrote: On Mon, Mar 18, 2024 at 13:14:54 +0100, Michal Prívozník wrote: > On 3/18/24 12:45, Denis V. Lunev wrote: > > On 3/18/24 11:42, Michal Prívozník wrote: > >> On 3/17/24 16:00, Denis V. Lunev wrote: > >>> Signed-off-by: Denis V. Lunev > >>> --- > >>> .gitignore | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/.gitignore b/.gitignore > >>> index 4695391342..44a9b446bd 100644 > >>> --- a/.gitignore > >>> +++ b/.gitignore > >>> @@ -20,6 +20,7 @@ __pycache__/ > >>> /build/ > >>> /ci/scratch/ > >>> tags > >>> +cscope.* > >>> # clangd related ignores > >>> .clangd > >> Apparently, at some point in time this was here, but it was removed: > >> > >> https://gitlab.com/libvirt/libvirt/-/commit/8a63add283c310952b7df161b4b413e817468d01 > >> > >> Michal > >> > > This is quite strange for me: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/.gitignore > > https://gitlab.com/qemu-project/qemu/-/blob/master/.gitignore?ref_type=heads > > > > I do not see any obvious reason how big and extensive > > list of files in .gitignore could hurt us while it > > is obviously convenient. > It might be that once there is too much of it there might be some false positives and it is harder to maintain. > Yeah, it feels a bit selective. I mean, we allow vim swap files to be in > .gitignore, we allow tags file to be there too (which strictly speaking > is a tooling helper file), but not cscope? The reasoning to keep 'tags' ignored is because we ship .ctags, but generally tooling files should be ignored by "personal" .gitignores as we can't cover all of it especially if we don't configure it. Agreed, I see we have ignores for vim and emacs, but honestly these could be removed as well because everyone can just edit `.config/git/ignore` and put the files created by their favorite editor into that file and it will work for every project you participate in. Pavel > Martin, do you remember the reasoning there? No, not really, it was most probably what others said above. I don't remember having issues with those ignores, it might've been an end to some discussion we had, not sure. Even though I don't have issues with tool-specific gitignores being there I copied all the non-libvirt As I said I don't have problems with that ^^, I thought the line could be based on what we have configs or targets for, for example. But just out of curiosity I have few questions below. related ignores from our .gitignore to my existing ~/.config/git/ignore and I know I'll be fine in other projects as well. Maybe we were trying to help others learn this "trick" to help them in other repositories. For me this is not looking like a reasonable option. Rationale: * I am working on tenth of different hosts in the corporate environment and building libvirt not only locally but on those hosts too Are you building it with some script or a long command line that these files get generated for you? Default build for me does not generate those. scope files appear once I am starting to work in the same way as tags. I need them to navigate over sources. They do not have any connection to the building procedure. That is why they are special and falls into the same category as tags. Or do you need those tags to browse the code? That would suggest you want some configuration setup on those machines as well. As above. They are heavily used to navigate sources and unfortunately, I do not have such influence to bring by default .gitignore to user defaults on the system. Den * This is unavoidable as always there are several different versions involved Within the proposed way, I have EACH time to copy personal settings from the root node to all working nodes. This looks like a personal pain to me. I would say, once again, that other projects are going different way and the chosen approach does not fit the my scenario. Den ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 1/1] gitignore: add cscope files to git ignore
On Mon, Mar 18, 2024 at 09:35:32PM +0100, Denis V. Lunev wrote: On 3/18/24 15:25, Martin Kletzander wrote: On Mon, Mar 18, 2024 at 02:25:53PM +0100, Pavel Hrdina wrote: On Mon, Mar 18, 2024 at 01:31:27PM +0100, Peter Krempa wrote: On Mon, Mar 18, 2024 at 13:14:54 +0100, Michal Prívozník wrote: > On 3/18/24 12:45, Denis V. Lunev wrote: > > On 3/18/24 11:42, Michal Prívozník wrote: > >> On 3/17/24 16:00, Denis V. Lunev wrote: > >>> Signed-off-by: Denis V. Lunev > >>> --- > >>> .gitignore | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/.gitignore b/.gitignore > >>> index 4695391342..44a9b446bd 100644 > >>> --- a/.gitignore > >>> +++ b/.gitignore > >>> @@ -20,6 +20,7 @@ __pycache__/ > >>> /build/ > >>> /ci/scratch/ > >>> tags > >>> +cscope.* > >>> # clangd related ignores > >>> .clangd > >> Apparently, at some point in time this was here, but it was removed: > >> > >> https://gitlab.com/libvirt/libvirt/-/commit/8a63add283c310952b7df161b4b413e817468d01 > >> > >> Michal > >> > > This is quite strange for me: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/.gitignore > > https://gitlab.com/qemu-project/qemu/-/blob/master/.gitignore?ref_type=heads > > > > I do not see any obvious reason how big and extensive > > list of files in .gitignore could hurt us while it > > is obviously convenient. > It might be that once there is too much of it there might be some false positives and it is harder to maintain. > Yeah, it feels a bit selective. I mean, we allow vim swap files to be in > .gitignore, we allow tags file to be there too (which strictly speaking > is a tooling helper file), but not cscope? The reasoning to keep 'tags' ignored is because we ship .ctags, but generally tooling files should be ignored by "personal" .gitignores as we can't cover all of it especially if we don't configure it. Agreed, I see we have ignores for vim and emacs, but honestly these could be removed as well because everyone can just edit `.config/git/ignore` and put the files created by their favorite editor into that file and it will work for every project you participate in. Pavel > Martin, do you remember the reasoning there? No, not really, it was most probably what others said above. I don't remember having issues with those ignores, it might've been an end to some discussion we had, not sure. Even though I don't have issues with tool-specific gitignores being there I copied all the non-libvirt As I said I don't have problems with that ^^, I thought the line could be based on what we have configs or targets for, for example. But just out of curiosity I have few questions below. related ignores from our .gitignore to my existing ~/.config/git/ignore and I know I'll be fine in other projects as well. Maybe we were trying to help others learn this "trick" to help them in other repositories. For me this is not looking like a reasonable option. Rationale: * I am working on tenth of different hosts in the corporate environment and building libvirt not only locally but on those hosts too Are you building it with some script or a long command line that these files get generated for you? Default build for me does not generate those. Or do you need those tags to browse the code? That would suggest you want some configuration setup on those machines as well. * This is unavoidable as always there are several different versions involved Within the proposed way, I have EACH time to copy personal settings from the root node to all working nodes. This looks like a personal pain to me. I would say, once again, that other projects are going different way and the chosen approach does not fit the my scenario. Den signature.asc Description: PGP signature ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] qemu: Add sysusers config file for qemu & kvm user/groups
On Mon, Feb 05, 2024 at 03:10:41PM +0100, Peter Krempa wrote: > On Fri, Feb 02, 2024 at 18:59:44 -, t...@siosm.fr wrote: > > Install a systemd sysusers config file for the qemu & kvm user/groups. > > > > We can not use the sysusers_create_compat macro in the RPM specfile to > > create those users as we want to keep the specfile standalone and not > > relying on additionnal files. > > > > Update the specfile to make the commands closer to what is generated by > > the current macro. > > > > See: https://src.fedoraproject.org/rpms/libvirt/pull-request/22 > > See: https://gitlab.com/libvirt/libvirt/-/merge_requests/319 > > See: > > https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/ > > > > Based on previous work by: Peter Krempa > > Signed-off-by: Timothée Ravier > > --- > > libvirt.spec.in | 21 + > > src/qemu/libvirt-qemu.sysusers.conf | 4 > > src/qemu/meson.build| 7 +++ > > 3 files changed, 24 insertions(+), 8 deletions(-) > > create mode 100644 src/qemu/libvirt-qemu.sysusers.conf > > Reviewed-by: Peter Krempa Unfortunately I failed to notice this before it had already made it into a release... > > +++ b/src/qemu/libvirt-qemu.sysusers.conf > > @@ -0,0 +1,4 @@ > > +g kvm 36 > > +g qemu 107 > > +u qemu 107:qemu "qemu user" - - > > +m qemu kvm These values are fine for Fedora/RHEL, but not elsewhere. For example, Debian would need something like g libvirt-qemu 64055 u libvirt-qemu 64055:libvirt-qemu instead. If you look at meson.build, you will see that we detect a number of operating systems/distributions in order to integrate as smoothly as possible with them. This can potentially break that, or at the very least make things quite confusing by virtue of more than one "QEMU user" existing on the system. Additionally, it completely fails to account for the qemu_user and qemu_group meson options, which have been around forever and can take arbitrary values. At the very least, installing this file needs to be gated behind a meson option that is off by default. A more complete solution that integrates properly with the existing facilities will require further work. -- Andrea Bolognani / Red Hat / Virtualization ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
