Re: [libvirt] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
On Thu, May 09, 2019 at 11:24:49PM +0800, Cornelia Huck wrote: > On Wed, 8 May 2019 07:57:05 -0400 > Yan Zhao wrote: > > > On Tue, May 07, 2019 at 05:19:54PM +0800, Cornelia Huck wrote: > > > On Sun, 5 May 2019 21:49:04 -0400 > > > Yan Zhao wrote: > > > > > > > version attribute is used to check two mdev devices' compatibility. > > > > > > > > The key point of this version attribute is that it's rw. > > > > User space has no need to understand internal of device version and no > > > > need to compare versions by itself. > > > > Compared to reading version strings from both two mdev devices being > > > > checked, user space only reads from one mdev device's version attribute. > > > > After getting its version string, user space writes this string into the > > > > other mdev device's version attribute. Vendor driver of mdev device > > > > whose version attribute being written will check device compatibility of > > > > the two mdev devices for user space and return success for compatibility > > > > or errno for incompatibility. > > > > > > I'm still missing a bit _what_ is actually supposed to be > > > compatible/incompatible. I'd assume some internal state descriptions > > > (even if this is not actually limited to migration). > > > > > right. > > originally, I thought this attribute should only contain a device's hardware > > compatibility info. But seems also including vendor specific software > > migration > > version is more reasonable, because general VFIO migration code cannot know > > version of vendor specific software migration code until migration data is > > transferring to the target vm. Then renaming it to migration_version is more > > appropriate. > > :) > > Nod. > > (...) > > > > > @@ -246,6 +249,143 @@ Directories and files under the sysfs for Each > > > > Physical Device > > > >This attribute should show the number of devices of type > > > > that can be > > > >created. > > > > > > > > +* version > > > > + > > > > + This attribute is rw, and is optional. > > > > + It is used to check device compatibility between two mdev devices > > > > and is > > > > + accessed in pairs between the two mdev devices being checked. > > > > + The intent of this attribute is to make an mdev device's version > > > > opaque to > > > > + user space, so instead of reading two mdev devices' version strings > > > > and > > > > + comparing in userspace, user space should only read one mdev > > > > device's version > > > > + attribute, and writes this version string into the other mdev > > > > device's version > > > > + attribute. Then vendor driver of mdev device whose version attribute > > > > being > > > > + written would check the incoming version string and tell user space > > > > whether > > > > + the two mdev devices are compatible via return value. That's why this > > > > + attribute is writable. > > > > > > I would reword this a bit: > > > > > > "This attribute provides a way to check device compatibility between > > > two mdev devices from userspace. The intended usage is for userspace to > > > read the version attribute from one mdev device and then writing that > > > value to the version attribute of the other mdev device. The second > > > mdev device indicates compatibility via the return code of the write > > > operation. This makes compatibility between mdev devices completely > > > vendor-defined and opaque to userspace." > > > > > > We still should explain _what_ compatibility we're talking about here, > > > though. > > > > > Thanks. It's much better than mine:) > > Then I'll change compatibility --> migration compatibility. > > Ok, with that it should be clear enough. > > > > > > > + > > > > + when reading this attribute, it should show device version string of > > > > + the device of type . > > > > + > > > > + This string is private to vendor driver itself. Vendor driver is > > > > able to > > > > + freely define format and length of device version string. > > > > + e.g. It can use a combination of pciid of parent device + mdev type. > > > > + > > > > + When writing a string to this attribute, vendor driver should > > > > analyze this > > > > + string and check whether the mdev device being identified by this > > > > string is > > > > + compatible with the mdev device for this attribute. vendor driver > > > > should then > > > > + return written string's length if it regards the two mdev devices are > > > > + compatible; vendor driver should return negative errno if it regards > > > > the two > > > > + mdev devices are not compatible. > > > > + > > > > + User space should treat ANY of below conditions as two mdev devices > > > > not > > > > + compatible: > > > > + (1) any one of the two mdev devices does not have a version attribute > > > > + (2) error when read from one mdev device's version attribute > > > > > > s/read/reading/ > > > > > > > + (3) error when write one mdev device's version string to the other >
Re: [libvirt] [PATCHv2] build: restore support for libyajl 2.0.1
On 5/9/19 9:18 AM, Andrea Bolognani wrote: On Thu, 2019-05-09 at 16:07 +0200, Ján Tomko wrote: Commit 105756660f944e7db02de3b55b98bb7c11cd03bf was too eager and did not consider SLE 12 which still has 2.0.1 that does not ship "SLE" is no longer a thing, so either s/SLE/SUSE Linux Enterprise Server/ (preferred) or s/SLE/SLES/ [...] + PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0]) + + if test $use_pkgconfig = 1; then +dnl 2.0.3 was the version where the pkg-config file was first added +LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3]) + else +dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1 "SUSE SLE" was never a thing, so either s/SUSE SLE/SUSE Linux Enterprise Server/ That's a lot of typing :-). I think SLES is fine. And to be super pedantic, s/OpenSUSE/openSUSE/. Regards, Jim (preferred) or s/SUSE SLE/SLES/ +dnl TODO: delete this in July 2020 +LIBVIRT_CHECK_LIB([YAJL], [yajl], + [yajl_tree_parse], [yajl/yajl_common.h]) + + fi + Please drop the empty lines right before and right after the 'if'. With the above nits addressed, Reviewed-by: Andrea Bolognani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Avoid issues due to qemu dropping osxsave and ospke
This is a VM with 'osxsave' capability declared, when there is no 'osxsave' guest support, using this series on top of current master: $ sudo ./run tools/virsh start ub1810-osxsave error: Failed to start domain ub1810-osxsave error: internal error: process exited while connecting to monitor: 2019-05-09T17:10:41.077568Z qemu-system-x86_64: can't apply global Skylake-Client-IBRS-x86_64-cpu.osxsave=off: Property '.osxsave' not found Applying this series makes the VM boot up, as intended. I believe there is room for discussion whether the VM should boot up or fail to boot with a "feature not found" error like the case below, in a sense of "you can't disable a feature that does not exist": $ sudo ./run tools/virsh start ub1810-unknown-cap error: Failed to start domain ub1810-unknown-cap error: unsupported configuration: unknown CPU feature: not_a_cap But the alternative presented in this series does the trick and it's less annoying for existing VMs. For all the patch series: Reviewed-by: Daniel Henrique Barboza Tested-by: Daniel Henrique Barboza On 4/25/19 9:50 AM, Christian Ehrhardt wrote: Hi, this series tries to address a drop of commandline options by qemu in regard to osxsave [1] and ospke [2]. This was already discussed in [3] late last year but got forgotten afterwards. The Ubuntu bug is at [4] and an older Fedora bug is at [5]. TL;DR: - osxsave/ospke features were never really configurable - KVM never returned the bits on GET_SUPPORTED_CPUID - very rare to be seen in the wild - avoid issues with newer qemu and old/odd XMLs to be sure Details: I checked various use cases from virt-install to openstack and some in between. The only cases I found that would define osxsave/ospke is virt-install pior to version 2.0 and even there only when used with --cpu=host-model or --cpu=host-copy. If you ever really enabled the feature you'd have got: error: the CPU is incompatible with host CPU: Host CPU does not provide required features: ospke The problem lies in domain XMLs that explicitly disable it. That would be But due to almost (or actually none) no host exposing this the following also triggers: This will make libvirt add it to the qemu commandline like: -cpu ...,osxsave=off,ospke=off And that will crash when qemu starts with: error: internal error: process exited while connecting to monitor: 2019-04-25T12:12:01.698646Z qemu-system-x86_64: can't apply global core2duo-x86_64-cpu.osxsave=off: Property '.osxsave' not found There are much more long term discussions about demoting and dropping qemu features and I'd like to avoid those discussions being mixed. The reason to drop it more or less without notice was that it never did anything to begin with. Due to that our solution might in a similar fashion be more trivial - just stop defining those two features to qemu commandline. [1]: https://git.qemu.org/?p=qemu.git;a=commit;h=f1a23522b03a569f13aad49294bb4c4b1a9500c7 [2]: https://git.qemu.org/?p=qemu.git;a=commit;h=9ccb9784b57804f5c74434ad6ccb66650a015ffc [3]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg561877.html [4]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1825195 [5]: https://bugzilla.redhat.com/show_bug.cgi?id=1644848 Christian Ehrhardt (2): qemu: do not define known no-op features qemuxml2argvtest: add test for remove cpu features src/qemu/qemu_command.c | 23 +++ .../qemuxml2argvdata/cpu-host-model-cmt.args | 2 +- .../cpu-no-removed-features.args | 29 +++ .../cpu-no-removed-features.xml | 23 +++ tests/qemuxml2argvdata/cpu-tsc-frequency.args | 4 +-- tests/qemuxml2argvtest.c | 1 + 6 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/cpu-no-removed-features.args create mode 100644 tests/qemuxml2argvdata/cpu-no-removed-features.xml -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] test_driver: implement virDomainGetDiskErrors
On Thu, May 9, 2019 at 4:36 PM Michal Privoznik wrote:
>
> On 5/7/19 10:23 PM, Ilias Stamatis wrote:
> > Return the number of disks present in the configuration of the fake
> > driver when called with @errors as NULL and @maxerrors as 0.
> >
> > Otherwise return 0 as the number of errors encountered.
> >
> > Signed-off-by: Ilias Stamatis
> > ---
> > src/test/test_driver.c | 27 +++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index 460c896ef6..5fa9ab30f1 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -3046,6 +3046,32 @@ static int testDomainSetAutostart(virDomainPtr
> > domain,
> > return 0;
> > }
> >
> > +static int testDomainGetDiskErrors(virDomainPtr dom,
> > + virDomainDiskErrorPtr errors,
> > + unsigned int maxerrors ATTRIBUTE_UNUSED,
> > + unsigned int flags)
> > +{
> > +int ret = -1;
> > +virDomainObjPtr vm = NULL;
> > +
> > +virCheckFlags(0, -1);
> > +
> > +if (!(vm = testDomObjFromDomain(dom)))
> > +goto cleanup;
> > +
> > +if (virDomainObjCheckActive(vm) < 0)
> > +goto cleanup;
> > +
> > +if (!errors)
> > +ret = vm->def->ndisks;
> > +else
> > +ret = 0;
>
> Don't we want to actually set some errors? That might be more helpful
> because mgmt app can actually test if it reports disk errors properly.
>
> Michal
Right, that makes sense.
The number of disks can actually vary because the test connection can
be configured differently, plus I'll implement virDomainAttachDevice
for the test driver too.
The possible error codes are just 2: VIR_DOMAIN_DISK_ERROR_UNSPEC and
VIR_DOMAIN_DISK_ERROR_NO_SPACE
So, should it be randomly decided at runtime whether or not to report
an error for each disk present and which error code will it actually
be?
Or would it be better to take a deterministic approach and report
let's say an error for the first 2 disks (if there are that many at
all), each with a different error code, and return no errors for all
the remaining disks?
Thanks,
Ilias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Avoid issues due to qemu dropping osxsave and ospke
On 5/9/19 6:31 AM, Christian Ehrhardt wrote: On Tue, May 7, 2019 at 11:03 PM Daniel Henrique Barboza wrote: On 4/25/19 9:50 AM, Christian Ehrhardt wrote: Hi, this series tries to address a drop of commandline options by qemu in regard to osxsave [1] and ospke [2]. This was already discussed in [3] late last year but got forgotten afterwards. The Ubuntu bug is at [4] and an older Fedora bug is at [5]. TL;DR: - osxsave/ospke features were never really configurable - KVM never returned the bits on GET_SUPPORTED_CPUID - very rare to be seen in the wild - avoid issues with newer qemu and old/odd XMLs to be sure Details: I checked various use cases from virt-install to openstack and some in between. The only cases I found that would define osxsave/ospke is virt-install pior to version 2.0 and even there only when used with --cpu=host-model or --cpu=host-copy. If you ever really enabled the feature you'd have got: error: the CPU is incompatible with host CPU: Host CPU does not provide required features: ospke The problem lies in domain XMLs that explicitly disable it. That would be But due to almost (or actually none) no host exposing this the following also triggers: So, it happened that this notebook (ThinkPad T480) has the feature you're handling in this patch series: Mine as well actually, but obviously only on the host capabilities. [...] And that changes everything I said in my previous email ... not only I messed up checking for instead of capabilities, but also I could've checked "domcapabilities". In the docs: "While the Driver Capabilities provides the host capabilities (...), the Domain Capabilities provides the hypervisor specific capabilities for Management Applications to query and make decisions regarding what to utilize." Thus, it doesn't matter if osxsave is being reported in the host capabilities. What matters if whether osxsave is declared in the domcapabilities (or the element in capabilities) - which would mean that guests can utilize it. I apologize for this confusion. With your approach, what happens is that a feature that is declared to be effective in the capabilities is, in fact, ignored. It is an upgrade of what would happen without it, of course. Some bikeshedding on "declared to be effective in the capabilities" needed to bring me up to date on this. See below... But I am wondering here, fully aware that you mentioned that you wanted this discussion to be avoided, Sorry my comment was misleading then - I'm absolutely fine having that part of the discussion in this context. I only wanted to avoid the discussion about removing features that "actually did something" which naturally is a way more complex topic. that we should simply exterminate both osxsave and ospke from Libvirt capabilities. I mean, what's the point of even reporting them if they will end up being ignored? You have both QEMU commits [1] and [2] mentioning that these flags were never configurable in the first place, so it's not like we need to keep them for backward compatibility either. Maybe it is my current lack of understanding how "exterminating both" would be exactly done and I see potential issues where there are none. I wonder would that just be dropping the mappings in src/cpu_map/x86_features.xml? If not then I'd appreciate a hint where exactly you'd suggest we would do the mentioned further extermination of ospke/osxsave. I was afraid of side effects or when no more being detected. And even more so it seems the definition of "capabilities" is not clear enough to me (I never thought too much about it so far): - "The capabilities of the hypervisor" (man page) - "The host CPU architecture and features." (web page) For the former definition yes, we might want to drop such non passable features then. But for the latter definition it feels right to continue reporting that the host has it. It is valid to report that the host has the feature - even thou it can't pass it to a guest. After all it is in the host and not the guest (hypervisor dependent) section of capabilities right? Yes, I agree. Also we might (later) use the mapping for other things down the road where being an tunable feature (or not) is not important either. Or users compare hosts with same hardware but different libvirt versions and wonder why they lost a cpu feature. Note that this does not discard this patch series - I think we'll need a solution like this to deal with older VMs that define these features in their XMLs. Thanks, DHB This will make libvirt add it to the qemu commandline like: -cpu ...,osxsave=off,ospke=off And that will crash when qemu starts with: error: internal error: process exited while connecting to monitor: 2019-04-25T12:12:01.698646Z qemu-system-x86_64: can't apply global core2duo-x86_64-cpu.osxsave=off: Property '.osxsave' not found There are much more long term discussions about demoting and dropping qemu features and I'd
Re: [libvirt] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
* Cornelia Huck (coh...@redhat.com) wrote: > On Thu, 9 May 2019 16:48:57 +0100 > "Dr. David Alan Gilbert" wrote: > > > * Cornelia Huck (coh...@redhat.com) wrote: > > > On Tue, 7 May 2019 15:18:26 -0600 > > > Alex Williamson wrote: > > > > > > > On Sun, 5 May 2019 21:49:04 -0400 > > > > Yan Zhao wrote: > > > > > > > > + Errno: > > > > > + If vendor driver wants to claim a mdev device incompatible to all > > > > > other mdev > > > > > + devices, it should not register version attribute for this mdev > > > > > device. But if > > > > > + a vendor driver has already registered version attribute and it > > > > > wants to claim > > > > > + a mdev device incompatible to all other mdev devices, it needs to > > > > > return > > > > > + -ENODEV on access to this mdev device's version attribute. > > > > > + If a mdev device is only incompatible to certain mdev devices, > > > > > write of > > > > > + incompatible mdev devices's version strings to its version > > > > > attribute should > > > > > + return -EINVAL; > > > > > > > > I think it's best not to define the specific errno returned for a > > > > specific situation, let the vendor driver decide, userspace simply > > > > needs to know that an errno on read indicates the device does not > > > > support migration version comparison and that an errno on write > > > > indicates the devices are incompatible or the target doesn't support > > > > migration versions. > > > > > > I think I have to disagree here: It's probably valuable to have an > > > agreed error for 'cannot migrate at all' vs 'cannot migrate between > > > those two particular devices'. Userspace might want to do different > > > things (e.g. trying with different device pairs). > > > > Trying to stuff these things down an errno seems a bad idea; we can't > > get much information that way. > > So, what would be a reasonable approach? Userspace should first read > the version attributes on both devices (to find out whether migration > is supported at all), and only then figure out via writing whether they > are compatible? > > (Or just go ahead and try, if it does not care about the reason.) Well, I'm OK with something like writing to test whether it's compatible, it's just we need a better way of saying 'no'. I'm not sure if that involves reading back from somewhere after the write or what. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
On Thu, 9 May 2019 16:48:57 +0100 "Dr. David Alan Gilbert" wrote: > * Cornelia Huck (coh...@redhat.com) wrote: > > On Tue, 7 May 2019 15:18:26 -0600 > > Alex Williamson wrote: > > > > > On Sun, 5 May 2019 21:49:04 -0400 > > > Yan Zhao wrote: > > > > > > + Errno: > > > > + If vendor driver wants to claim a mdev device incompatible to all > > > > other mdev > > > > + devices, it should not register version attribute for this mdev > > > > device. But if > > > > + a vendor driver has already registered version attribute and it > > > > wants to claim > > > > + a mdev device incompatible to all other mdev devices, it needs to > > > > return > > > > + -ENODEV on access to this mdev device's version attribute. > > > > + If a mdev device is only incompatible to certain mdev devices, write > > > > of > > > > + incompatible mdev devices's version strings to its version attribute > > > > should > > > > + return -EINVAL; > > > > > > I think it's best not to define the specific errno returned for a > > > specific situation, let the vendor driver decide, userspace simply > > > needs to know that an errno on read indicates the device does not > > > support migration version comparison and that an errno on write > > > indicates the devices are incompatible or the target doesn't support > > > migration versions. > > > > I think I have to disagree here: It's probably valuable to have an > > agreed error for 'cannot migrate at all' vs 'cannot migrate between > > those two particular devices'. Userspace might want to do different > > things (e.g. trying with different device pairs). > > Trying to stuff these things down an errno seems a bad idea; we can't > get much information that way. So, what would be a reasonable approach? Userspace should first read the version attributes on both devices (to find out whether migration is supported at all), and only then figure out via writing whether they are compatible? (Or just go ahead and try, if it does not care about the reason.) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
* Cornelia Huck (coh...@redhat.com) wrote: > On Tue, 7 May 2019 15:18:26 -0600 > Alex Williamson wrote: > > > On Sun, 5 May 2019 21:49:04 -0400 > > Yan Zhao wrote: > > > > + Errno: > > > + If vendor driver wants to claim a mdev device incompatible to all > > > other mdev > > > + devices, it should not register version attribute for this mdev > > > device. But if > > > + a vendor driver has already registered version attribute and it wants > > > to claim > > > + a mdev device incompatible to all other mdev devices, it needs to > > > return > > > + -ENODEV on access to this mdev device's version attribute. > > > + If a mdev device is only incompatible to certain mdev devices, write of > > > + incompatible mdev devices's version strings to its version attribute > > > should > > > + return -EINVAL; > > > > I think it's best not to define the specific errno returned for a > > specific situation, let the vendor driver decide, userspace simply > > needs to know that an errno on read indicates the device does not > > support migration version comparison and that an errno on write > > indicates the devices are incompatible or the target doesn't support > > migration versions. > > I think I have to disagree here: It's probably valuable to have an > agreed error for 'cannot migrate at all' vs 'cannot migrate between > those two particular devices'. Userspace might want to do different > things (e.g. trying with different device pairs). Trying to stuff these things down an errno seems a bad idea; we can't get much information that way. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/6] mappings: add pulseaudio libs for native packages
On Thu, May 09, 2019 at 04:25:03PM +0200, Andrea Bolognani wrote: > On Fri, 2019-05-03 at 19:02 +0100, Daniel P. Berrangé wrote: > > Subject: mappings: add pulseaudio libs for native packages > > Again, this can be > > mappings: Add PulseAudio > > [...] > > + pulseaudio: > > +rpm: pulseaudio-libs-devel > > +deb: libpulse-dev > > Lines are not in alphabetical order. > > More importantly, shouldn't this have > > cross-policy-deb: foreign > > as well? > > > Unrelated to this patch, but does our default of 'native' for > cross-policy really make sense? Unless I'm mistaken, that's the > value we need for tools rather than libraries, and in general > projects depend on way more libraries than they do on tools, so > perhaps it would make sense to make 'foreign' the default and > use 'native' explicitly for tools only. 'native' by default was done as it is the safe option. ie if you don't mark a package for foreign install, the container image will still build fine. If it was the reverse, then we'd get either errors from apt failing to resolve deps, or worse yet, it can even uninstall the native tool & add the foreign build which then won't work at runtime. 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/2] vfio/mdev: add version attribute for mdev device
On Tue, 7 May 2019 15:18:26 -0600 Alex Williamson wrote: > On Sun, 5 May 2019 21:49:04 -0400 > Yan Zhao wrote: > > + Errno: > > + If vendor driver wants to claim a mdev device incompatible to all other > > mdev > > + devices, it should not register version attribute for this mdev device. > > But if > > + a vendor driver has already registered version attribute and it wants to > > claim > > + a mdev device incompatible to all other mdev devices, it needs to return > > + -ENODEV on access to this mdev device's version attribute. > > + If a mdev device is only incompatible to certain mdev devices, write of > > + incompatible mdev devices's version strings to its version attribute > > should > > + return -EINVAL; > > I think it's best not to define the specific errno returned for a > specific situation, let the vendor driver decide, userspace simply > needs to know that an errno on read indicates the device does not > support migration version comparison and that an errno on write > indicates the devices are incompatible or the target doesn't support > migration versions. I think I have to disagree here: It's probably valuable to have an agreed error for 'cannot migrate at all' vs 'cannot migrate between those two particular devices'. Userspace might want to do different things (e.g. trying with different device pairs). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Running libvirtd out of source directory connection reset error
Hi all, I'm getting started with hacking around with libvirt and am trying to familiarize myself with launching and running an instance of libvirtd I built from source on Centos 7.6. Following the instructions from https://libvirt.org/compiling.html to launch my built versions of libvirtd and virsh, I get the following error with no other context when trying to start a domain using "virsh start mydomain": error: Cannot recv data: Connection reset by peer Despite this error, I am able to run commands list virsh list. Are there additional parameters needed to launch libvirtd or additional services I need to start up alongside it? Thanks, Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] src: remove HAL node device driver
On Thu, May 09, 2019 at 05:16:16PM +0200, Pavel Hrdina wrote: > The HAL driver is not used by any supported Linux, the only possible > user could be FreeBSD, but the official libvirt port disables HAL > driver unconditionally which means probably nobody is using it there. Last time we asked, Roman said he at least tested HAL on FreeBSD to see if basics were working, so I think we want his agreement before deciding to remove this https://www.redhat.com/archives/libvir-list/2016-April/msg00991.html > Signed-off-by: Pavel Hrdina > --- > > HAL was deprecated on FreeBSD as well and the current supported device > manager is *devd*. If there will be need to implement node device > support on FreeBSD we can add a new DEVD driver or we can use > libudev-devd project to support UDEV driver on FreeBSD. 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] [jenkins-ci PATCH 6/6] jobs: allow test-suite.log to exist anywhere
On Fri, 2019-05-03 at 19:02 +0100, Daniel P. Berrangé wrote: > Don't assume that all projects have a dedicated tests/ subdirectory > for unit tests. > > Signed-off-by: Daniel P. Berrangé > --- > guests/playbooks/build/jobs/autotools-check-job.yml | 2 +- > jenkins/jobs/autotools.yaml | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) 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] [PATCHv2] build: restore support for libyajl 2.0.1
On Thu, 2019-05-09 at 16:07 +0200, Ján Tomko wrote: > Commit 105756660f944e7db02de3b55b98bb7c11cd03bf was too eager and did > not consider SLE 12 which still has 2.0.1 that does not ship "SLE" is no longer a thing, so either s/SLE/SUSE Linux Enterprise Server/ (preferred) or s/SLE/SLES/ [...] > + PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0]) > + > + if test $use_pkgconfig = 1; then > +dnl 2.0.3 was the version where the pkg-config file was first added > +LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3]) > + else > +dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1 "SUSE SLE" was never a thing, so either s/SUSE SLE/SUSE Linux Enterprise Server/ (preferred) or s/SUSE SLE/SLES/ > +dnl TODO: delete this in July 2020 > +LIBVIRT_CHECK_LIB([YAJL], [yajl], > + [yajl_tree_parse], [yajl/yajl_common.h]) > + > + fi > + Please drop the empty lines right before and right after the 'if'. With the above nits addressed, 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 v2 1/2] vfio/mdev: add version attribute for mdev device
On Wed, 8 May 2019 07:57:05 -0400 Yan Zhao wrote: > On Tue, May 07, 2019 at 05:19:54PM +0800, Cornelia Huck wrote: > > On Sun, 5 May 2019 21:49:04 -0400 > > Yan Zhao wrote: > > > > > version attribute is used to check two mdev devices' compatibility. > > > > > > The key point of this version attribute is that it's rw. > > > User space has no need to understand internal of device version and no > > > need to compare versions by itself. > > > Compared to reading version strings from both two mdev devices being > > > checked, user space only reads from one mdev device's version attribute. > > > After getting its version string, user space writes this string into the > > > other mdev device's version attribute. Vendor driver of mdev device > > > whose version attribute being written will check device compatibility of > > > the two mdev devices for user space and return success for compatibility > > > or errno for incompatibility. > > > > I'm still missing a bit _what_ is actually supposed to be > > compatible/incompatible. I'd assume some internal state descriptions > > (even if this is not actually limited to migration). > > > right. > originally, I thought this attribute should only contain a device's hardware > compatibility info. But seems also including vendor specific software > migration > version is more reasonable, because general VFIO migration code cannot know > version of vendor specific software migration code until migration data is > transferring to the target vm. Then renaming it to migration_version is more > appropriate. > :) Nod. (...) > > > @@ -246,6 +249,143 @@ Directories and files under the sysfs for Each > > > Physical Device > > >This attribute should show the number of devices of type > > > that can be > > >created. > > > > > > +* version > > > + > > > + This attribute is rw, and is optional. > > > + It is used to check device compatibility between two mdev devices and > > > is > > > + accessed in pairs between the two mdev devices being checked. > > > + The intent of this attribute is to make an mdev device's version > > > opaque to > > > + user space, so instead of reading two mdev devices' version strings and > > > + comparing in userspace, user space should only read one mdev device's > > > version > > > + attribute, and writes this version string into the other mdev device's > > > version > > > + attribute. Then vendor driver of mdev device whose version attribute > > > being > > > + written would check the incoming version string and tell user space > > > whether > > > + the two mdev devices are compatible via return value. That's why this > > > + attribute is writable. > > > > I would reword this a bit: > > > > "This attribute provides a way to check device compatibility between > > two mdev devices from userspace. The intended usage is for userspace to > > read the version attribute from one mdev device and then writing that > > value to the version attribute of the other mdev device. The second > > mdev device indicates compatibility via the return code of the write > > operation. This makes compatibility between mdev devices completely > > vendor-defined and opaque to userspace." > > > > We still should explain _what_ compatibility we're talking about here, > > though. > > > Thanks. It's much better than mine:) > Then I'll change compatibility --> migration compatibility. Ok, with that it should be clear enough. > > > > + > > > + when reading this attribute, it should show device version string of > > > + the device of type . > > > + > > > + This string is private to vendor driver itself. Vendor driver is able > > > to > > > + freely define format and length of device version string. > > > + e.g. It can use a combination of pciid of parent device + mdev type. > > > + > > > + When writing a string to this attribute, vendor driver should analyze > > > this > > > + string and check whether the mdev device being identified by this > > > string is > > > + compatible with the mdev device for this attribute. vendor driver > > > should then > > > + return written string's length if it regards the two mdev devices are > > > + compatible; vendor driver should return negative errno if it regards > > > the two > > > + mdev devices are not compatible. > > > + > > > + User space should treat ANY of below conditions as two mdev devices not > > > + compatible: > > > + (1) any one of the two mdev devices does not have a version attribute > > > + (2) error when read from one mdev device's version attribute > > > > s/read/reading/ > > > > > + (3) error when write one mdev device's version string to the other > > > mdev > > > > s/write/writing/ > > > > > + device's version attribute > > > + > > > + User space should regard two mdev devices compatible when ALL of below > > > + conditions are met: > > > + (1) success when read from one mdev device's version attribute. > > > > s/read/reading/ > >
[libvirt] [PATCH 1/2] node_device_udev: remove deprecated logging function
The function was deprecated in udev 219 and all the supported OSes
don't have older version of udev or systemd.
Signed-off-by: Pavel Hrdina
---
m4/virt-udev.m4| 5
src/node_device/node_device_udev.c | 41 --
2 files changed, 46 deletions(-)
diff --git a/m4/virt-udev.m4 b/m4/virt-udev.m4
index be7dba5d2d..cf977c650b 100644
--- a/m4/virt-udev.m4
+++ b/m4/virt-udev.m4
@@ -30,11 +30,6 @@ AC_DEFUN([LIBVIRT_CHECK_UDEV],[
fi
if test "$with_udev" = "yes" ; then
- PKG_CHECK_EXISTS([libudev >= 218], [with_udev_logging=no],
[with_udev_logging=yes])
- if test "$with_udev_logging" = "yes" ; then
-AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1, [whether libudev logging
can be used])
- fi
-
old_CFLAGS="$CFLAGS"
old_LIBS="$LIBS"
CFLAGS="$CFLAGS $UDEV_CFLAGS"
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 7dd9804a0e..5df2fd72f3 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -318,43 +318,6 @@ udevGenerateDeviceName(struct udev_device *device,
}
-#if HAVE_UDEV_LOGGING
-typedef void
-(*udevLogFunctionPtr)(struct udev *udev,
- int priority,
- const char *file,
- int line,
- const char *fn,
- const char *format,
- va_list args);
-
-static void
-ATTRIBUTE_FMT_PRINTF(6, 0)
-udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED,
-int priority,
-const char *file,
-int line,
-const char *fn,
-const char *fmt,
-va_list args)
-{
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *format = NULL;
-
-virBufferAdd(, fmt, -1);
-virBufferTrim(, "\n", -1);
-
-format = virBufferContentAndReset();
-
-virLogVMessage(,
- virLogPriorityFromSyslog(priority),
- file, line, fn, NULL, format ? format : fmt, args);
-
-VIR_FREE(format);
-}
-#endif
-
-
static int
udevTranslatePCIIds(unsigned int vendor,
unsigned int product,
@@ -1872,10 +1835,6 @@ nodeStateInitialize(bool privileged,
_("failed to create udev context"));
goto cleanup;
}
-#if HAVE_UDEV_LOGGING
-/* cast to get rid of missing-format-attribute warning */
-udev_set_log_fn(udev, (udevLogFunctionPtr) udevLogFunction);
-#endif
virObjectLock(priv);
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] node device cleanup
Pavel Hrdina (2): node_device_udev: remove deprecated logging function src: remove HAL node device driver configure.ac | 3 +- docs/drvnodedev.html.in | 3 +- docs/hvsupport.pl| 2 +- libvirt.spec.in | 1 - m4/virt-hal.m4 | 10 +- m4/virt-udev.m4 | 5 - po/POTFILES | 1 - src/node_device/Makefile.inc.am | 12 - src/node_device/node_device_driver.c | 10 +- src/node_device/node_device_driver.h | 5 - src/node_device/node_device_hal.c| 804 --- src/node_device/node_device_hal.h| 22 - src/node_device/node_device_udev.c | 41 -- 13 files changed, 9 insertions(+), 910 deletions(-) delete mode 100644 src/node_device/node_device_hal.c delete mode 100644 src/node_device/node_device_hal.h -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] src: remove HAL node device driver
The HAL driver is not used by any supported Linux, the only possible
user could be FreeBSD, but the official libvirt port disables HAL
driver unconditionally which means probably nobody is using it there.
Signed-off-by: Pavel Hrdina
---
HAL was deprecated on FreeBSD as well and the current supported device
manager is *devd*. If there will be need to implement node device
support on FreeBSD we can add a new DEVD driver or we can use
libudev-devd project to support UDEV driver on FreeBSD.
configure.ac | 3 +-
docs/drvnodedev.html.in | 3 +-
docs/hvsupport.pl| 2 +-
libvirt.spec.in | 1 -
m4/virt-hal.m4 | 10 +-
po/POTFILES | 1 -
src/node_device/Makefile.inc.am | 12 -
src/node_device/node_device_driver.c | 10 +-
src/node_device/node_device_driver.h | 5 -
src/node_device/node_device_hal.c| 804 ---
src/node_device/node_device_hal.h| 22 -
11 files changed, 9 insertions(+), 864 deletions(-)
delete mode 100644 src/node_device/node_device_hal.c
delete mode 100644 src/node_device/node_device_hal.h
diff --git a/configure.ac b/configure.ac
index fabec815db..cbec6b9e14 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,7 +794,7 @@ AC_SUBST([LV_LIBTOOL_OBJDIR])
with_nodedev=no;
-if test "$with_hal" = "yes" || test "$with_udev" = "yes";
+if test "$with_udev" = "yes";
then
with_nodedev=yes
AC_DEFINE_UNQUOTED([WITH_NODE_DEVICES], 1, [with node device driver])
@@ -995,7 +995,6 @@ LIBVIRT_RESULT_FIREWALLD_ZONE
LIBVIRT_RESULT_FUSE
LIBVIRT_RESULT_GLUSTER
LIBVIRT_RESULT_GNUTLS
-LIBVIRT_RESULT_HAL
LIBVIRT_RESULT_LIBISCSI
LIBVIRT_RESULT_LIBNL
LIBVIRT_RESULT_LIBPCAP
diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in
index 71a8a57b0c..8582f18655 100644
--- a/docs/drvnodedev.html.in
+++ b/docs/drvnodedev.html.in
@@ -23,8 +23,7 @@
(http://wiki.libvirt.org/page/NPIV_in_libvirt;>more info about
NPIV)).
Devices on the host system are arranged in a tree-like hierarchy, with
the root node being called computer. The node device driver
- supports two backends to manage the devices, HAL and udev, with the
former
- being deprecated in favour of the latter.
+ supports one backend to manage the devices, udev.
diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl
index a2b980c502..1554e9985e 100755
--- a/docs/hvsupport.pl
+++ b/docs/hvsupport.pl
@@ -38,7 +38,7 @@ my %groupheaders = (
my @srcs;
find({
wanted => sub {
-if (m!$srcdir/.*/\w+_(driver|common|tmpl|monitor|hal|udev)\.c$!) {
+if (m!$srcdir/.*/\w+_(driver|common|tmpl|monitor|udev)\.c$!) {
push @srcs, $_ if $_ !~ /vbox_driver\.c/;
}
}, no_chdir => 1}, $srcdir);
diff --git a/libvirt.spec.in b/libvirt.spec.in
index e07041c0b9..c90c1a0f2c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1198,7 +1198,6 @@ rm -f po/stamp-po
--with-selinux \
%{?arg_selinux_mount} \
--without-apparmor \
- --without-hal \
--with-udev \
--with-yajl \
%{?arg_sanlock} \
diff --git a/m4/virt-hal.m4 b/m4/virt-hal.m4
index e37bbf979b..35391f0d2b 100644
--- a/m4/virt-hal.m4
+++ b/m4/virt-hal.m4
@@ -18,13 +18,11 @@ dnl
Re: [libvirt] [jenkins-ci PATCH 5/6] mappings: remove gtk-vnc2 native and mingw packages
On Fri, 2019-05-03 at 19:02 +0100, Daniel P. Berrangé wrote: > Subject: mappings: remove gtk-vnc2 native and mingw packages This can be simply mappings: Remove gtk-vnc2 Either way 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] [jenkins-ci PATCH 4/6] projects: make virt-viewer depend on gtk-vnc jobs
On Fri, 2019-05-03 at 19:02 +0100, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > guests/playbooks/build/jobs/defaults.yml | 1 + > guests/vars/projects/virt-viewer+mingw32.yml | 1 - > guests/vars/projects/virt-viewer+mingw64.yml | 1 - > guests/vars/projects/virt-viewer.yml | 1 - > jenkins/jobs/defaults.yaml | 1 + > jenkins/projects/virt-viewer+mingw32.yaml| 4 +++- > jenkins/projects/virt-viewer+mingw64.yaml| 4 +++- > jenkins/projects/virt-viewer.yaml| 4 +++- > 8 files changed, 11 insertions(+), 6 deletions(-) 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] [jenkins-ci PATCH 3/6] projects: add gtk-vnc project
On Fri, 2019-05-03 at 19:02 +0100, Daniel P. Berrangé wrote:
> +++ b/jenkins/projects/gtk-vnc.yaml
> @@ -0,0 +1,15 @@
> +---
> +- project:
> +name: gtk-vnc
> +machines: '{all_machines}'
> +title: GTK-VNC
> +archive_format: gz
> +git_url: '{git_urls[gtk-vnc][default]}'
> +jobs:
> + - autotools-build-job:
> + - autotools-syntax-check-job:
> + parent_jobs: 'gtk-vnc-build'
> + - autotools-check-job:
> + parent_jobs: 'gtk-vnc-syntax-check'
> + - autotools-rpm-job:
> + parent_jobs: 'gtk-vnc-check'
Actually you also need to make sure you have
- autotools-build-job:
parent_jobs:
here and in the MinGW jobs, otherwise you'll make Jenkins Job
Builder very unhappy.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 3/6] projects: add gtk-vnc project
On Fri, 2019-05-03 at 19:02 +0100, Daniel P. Berrangé wrote:
[...]
> +++ b/guests/playbooks/build/projects/gtk-vnc.yml
> @@ -0,0 +1,12 @@
> +---
> +- set_fact:
> +name: gtk-vnc
> +machines: '{{ all_machines }}'
> +archive_format: gz
> +git_url: '{{ git_urls["gtk-vnc"][git_remote] }}'
> +
> +- include: '{{ playbook_base }}/jobs/prepare.yml'
> +- include: '{{ playbook_base }}/jobs/autotools-build-job.yml'
> +- include: '{{ playbook_base }}/jobs/autotools-syntax-check-job.yml'
> +- include: '{{ playbook_base }}/jobs/autotools-check-job.yml'
> +- include: '{{ playbook_base }}/jobs/autotools-rpm-job.yml'
You need to do
- include: '{{ playbook_base }}/jobs/autotools-rpm-job.yml'
vars:
machines: '{{ rpm_machines }}'
here, or it will try (and obviously fail) to build RPMs on Debian,
Ubuntu and FreeBSD.
Same for the Jenkins equivalent, of course.
[...]
> +++ b/guests/vars/projects/gtk-vnc.yml
> @@ -0,0 +1,11 @@
> +---
> +packages:
> + - cyrus-sasl
> + - glib2
> + - gnutls
> + - gobject-introspection
> + - gtk3
> + - intltool
> + - libgcrypt
> + - pulseaudio
> + - vala
Please split all changes to guests/vars/ and guests/host_vars/ off
to a separate preparatory commit, similar to eg. 56c9e4a4b506.
Everything else looks good.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: restore support for libyajl 2.0.1
On Thu, May 09, 2019 at 04:10:36PM +0200, Olaf Hering wrote: > Am Thu, 9 May 2019 13:31:45 +0200 > schrieb Ján Tomko : > > > +dnl TODO: delete this in July 2020 > > Does this come with a cost? I mean, SLE_12 is not going away any time soon. > Just dropping things because we can seems to be the wrong approach. Historically we almost never dropped stuff, and when we did it was completely unpredictable and arbitrary each time. Maintaining support for old software has a maint cost so it is desirable to drop things after a period of time. Thus we defined a platform support rules for when we will drop distros so that downstream vendors/ users have clear expectations: https://libvirt.org/platforms.html SLE is falls under the long life distros rule, so we have at most 2 major versions supported at any time, and the older version is dropped 2 years after the newer version is released. Our belief is that this cut off point for the old major version is long enough that people still using this older version are doing so becasue they want unchanging stable versions, not the bleeding edge. IOW, after SLE 15 has been released for 2 years, it is increasingly unlikely that most people will want to run new libvirt on SLE 12. There will always be people who are the exception to the rule, but on balance this is a good tradeoff between maint cost for libvirt vs likely usage by downstream. This same policy has also been now adopted by QEMU. 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] test_driver: implement virDomainGetDiskErrors
On 5/7/19 10:23 PM, Ilias Stamatis wrote:
Return the number of disks present in the configuration of the fake
driver when called with @errors as NULL and @maxerrors as 0.
Otherwise return 0 as the number of errors encountered.
Signed-off-by: Ilias Stamatis
---
src/test/test_driver.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 460c896ef6..5fa9ab30f1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3046,6 +3046,32 @@ static int testDomainSetAutostart(virDomainPtr domain,
return 0;
}
+static int testDomainGetDiskErrors(virDomainPtr dom,
+ virDomainDiskErrorPtr errors,
+ unsigned int maxerrors ATTRIBUTE_UNUSED,
+ unsigned int flags)
+{
+int ret = -1;
+virDomainObjPtr vm = NULL;
+
+virCheckFlags(0, -1);
+
+if (!(vm = testDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto cleanup;
+
+if (!errors)
+ret = vm->def->ndisks;
+else
+ret = 0;
Don't we want to actually set some errors? That might be more helpful
because mgmt app can actually test if it reports disk errors properly.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/6] mappings: add pulseaudio libs for native packages
On Fri, 2019-05-03 at 19:02 +0100, Daniel P. Berrangé wrote: > Subject: mappings: add pulseaudio libs for native packages Again, this can be mappings: Add PulseAudio [...] > + pulseaudio: > +rpm: pulseaudio-libs-devel > +deb: libpulse-dev Lines are not in alphabetical order. More importantly, shouldn't this have cross-policy-deb: foreign as well? Unrelated to this patch, but does our default of 'native' for cross-policy really make sense? Unless I'm mistaken, that's the value we need for tools rather than libraries, and in general projects depend on way more libraries than they do on tools, so perhaps it would make sense to make 'foreign' the default and use 'native' explicitly for tools only. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am
On Thu, May 09, 2019 at 03:56:20PM +0200, Martin Kletzander wrote: > On Thu, May 09, 2019 at 01:54:42PM +0100, Daniel P. Berrangé wrote: > > On Tue, May 07, 2019 at 05:45:30PM +0200, Martin Kletzander wrote: > > > The way it works now the Makefile needs to be both make valid and automake > > > valid. That is fine for now, but if we want to use anything more > > > advanced, like > > > conditionals, we cannot have it like that any more. > > > > > > So instead forward all ci-* rules to that file. > > > > > > Signed-off-by: Martin Kletzander > > > --- > > > Makefile.am | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/Makefile.am b/Makefile.am > > > index 0d8bb733e6d2..442bae511828 100644 > > > --- a/Makefile.am > > > +++ b/Makefile.am > > > @@ -35,6 +35,7 @@ EXTRA_DIST = \ > > >libvirt-qemu.pc.in \ > > >libvirt-lxc.pc.in \ > > >libvirt-admin.pc.in \ > > > + Makefile.ci \ > > >Makefile.nonreentrant \ > > >autogen.sh \ > > >cfg.mk \ > > > > Indentation is not consistent here - tabs vs non-tabs. > > > > > @@ -107,4 +108,5 @@ gen-AUTHORS: > > > rm -f all.list maint.list contrib.list; \ > > > fi > > > > > > -include Makefile.ci > > > +ci-%: > > > + $(MAKE) -f Makefile.ci $@ > > > > Will this cause all variables to be forwarded ? > > > > eg will > > > > make ci-build@fedora-29 CI_IMAGE_TAG=:latest > > > > result in > > > > make -f Makefile.ci ci-build@fedora-29 CI_IMAGE_TAG=:latest > > > > It worked for me with CI_CENGINE, I thing this is forwarded thanks to $(MAKE). I don't think it is $(MAKE), as that's just a variable that expands to the string "make". It looks like any variables in the makefile and turned into environment variables that are inherited by the child make process. I did a quick test and it all worked as desired. $ cat bar.mak FISH=food fish: $(MAKE) -f foo.mak $@ $ cat foo.mak FISH=cake fish: @echo ">>$(FISH)<<" $ make -f bar.mak fish FISH=pond make -f foo.mak fish make[1]: Entering directory '/home/berrange/tmp' >>pond<< make[1]: Leaving directory '/home/berrange/tmp' So on that basis, with the indent fix: Reviewed-by: Daniel P. Berrangé 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] [jenkins-ci PATCH 1/6] mappings: add libgcrypt native & mingw packages
On Fri, 2019-05-03 at 19:02 +0100, Daniel P. Berrangé wrote: > Subject: mappings: add libgcrypt native & mingw packages This can be simply mappings: Add libgcrypt [...] > + libgcrypt: > +rpm: libgcrypt-devel > +deb: libgcrypt20-dev > +cross-policy-deb: foreign This should be libgcrypt: deb: libgcrypt20-dev pkg: libgcrypt rpm: libgcrypt-devel cross-policy-deb: foreign with FreeBSD taken into account and entries in the correct order. With that fixed, 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] build: restore support for libyajl 2.0.1
Am Thu, 9 May 2019 13:31:45 +0200 schrieb Ján Tomko : > +dnl TODO: delete this in July 2020 Does this come with a cost? I mean, SLE_12 is not going away any time soon. Just dropping things because we can seems to be the wrong approach. There might be other new things that must be used in libvirt, and none of the currently supported systems may have them. Olaf pgpFX7HVNIZC1.pgp Description: Digitale Signatur von OpenPGP -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] build: restore support for libyajl 2.0.1
Commit 105756660f944e7db02de3b55b98bb7c11cd03bf was too eager and did not consider SLE 12 which still has 2.0.1 that does not ship a pkg-config file. Similar to how we check for readline, prefer pkg-config if available and fall back to the old detection code if not found. NB: this is not a clean revert because we're not reintroducing support for YAJL 1. Signed-off-by: Ján Tomko Reported-by: Olaf Hering --- m4/virt-yajl.m4 | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/m4/virt-yajl.m4 b/m4/virt-yajl.m4 index 494e722963..3caf0c5d07 100644 --- a/m4/virt-yajl.m4 +++ b/m4/virt-yajl.m4 @@ -24,7 +24,19 @@ AC_DEFUN([LIBVIRT_ARG_YAJL],[ AC_DEFUN([LIBVIRT_CHECK_YAJL],[ dnl YAJL JSON library http://lloyd.github.com/yajl/ - LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3]) + PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0]) + + if test $use_pkgconfig = 1; then +dnl 2.0.3 was the version where the pkg-config file was first added +LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3]) + else +dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1 +dnl TODO: delete this in July 2020 +LIBVIRT_CHECK_LIB([YAJL], [yajl], + [yajl_tree_parse], [yajl/yajl_common.h]) + + fi + ]) AC_DEFUN([LIBVIRT_RESULT_YAJL],[ -- 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: restore support for libyajl 2.0.1
On Thu, 2019-05-09 at 13:31 +0200, Ján Tomko wrote: > + PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0]) > + > + if test $use_pkgconfig = 1; then > +dnl 2.0.3 was the version where the pkg-config file was first added > +LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3]) > + else > +dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1 > +dnl TODO: delete this in July 2020 > +LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl], > + [yajl_tree_parse], [yajl/yajl_common.h]) > + > + fi We're only concerned with YAJL 2 nowadays, so we should be able to use the non-ALT version of the macro here, right? Other than that, the change looks good. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am
On Thu, May 09, 2019 at 01:54:42PM +0100, Daniel P. Berrangé wrote: On Tue, May 07, 2019 at 05:45:30PM +0200, Martin Kletzander wrote: The way it works now the Makefile needs to be both make valid and automake valid. That is fine for now, but if we want to use anything more advanced, like conditionals, we cannot have it like that any more. So instead forward all ci-* rules to that file. Signed-off-by: Martin Kletzander --- Makefile.am | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index 0d8bb733e6d2..442bae511828 100644 --- a/Makefile.am +++ b/Makefile.am @@ -35,6 +35,7 @@ EXTRA_DIST = \ libvirt-qemu.pc.in \ libvirt-lxc.pc.in \ libvirt-admin.pc.in \ + Makefile.ci \ Makefile.nonreentrant \ autogen.sh \ cfg.mk \ Indentation is not consistent here - tabs vs non-tabs. @@ -107,4 +108,5 @@ gen-AUTHORS: rm -f all.list maint.list contrib.list; \ fi -include Makefile.ci +ci-%: + $(MAKE) -f Makefile.ci $@ Will this cause all variables to be forwarded ? eg will make ci-build@fedora-29 CI_IMAGE_TAG=:latest result in make -f Makefile.ci ci-build@fedora-29 CI_IMAGE_TAG=:latest It worked for me with CI_CENGINE, I thing this is forwarded thanks to $(MAKE). 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 :| signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/4] guests: Use package_manager everywhere
On Thu, 2019-05-09 at 13:08 +0100, Daniel P. Berrangé wrote:
> On Tue, May 07, 2019 at 03:17:40PM +0200, Andrea Bolognani wrote:
> > - name: Bootstrap Ansible
> > - raw: 'yum install -y {{ python }}'
> > - when:
> > -- package_format == 'rpm'
> > -
> > -- name: Bootstrap Ansible
> > - raw: 'apt-get install -y {{ python }}'
> > - when:
> > -- package_format == 'deb'
> > -
> > -- name: Bootstrap Ansible
> > - raw: 'pkg install -y {{ python }}'
> > - when:
> > -- package_format == 'pkg'
> > + raw: '{{ package_manager }} install -y {{ python }}'
>
> Heh, rather amuzing that all three package managers happen to support
> the same "install -y" command syntax.
"We don't make mistakes, just happy little accidents." :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/12] qemu: Refactor/simplify qemuDomainStorageSourceAccessAllow
On Thu, Apr 18, 2019 at 04:43:07PM +0200, Peter Krempa wrote: Use qemuDomainStorageSourceAccessModify with correct flags to do the job. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 30 ++ 1 file changed, 6 insertions(+), 24 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 11/12] qemu: Mark when modifying access to existing source in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:06PM +0200, Peter Krempa wrote: Some operations e.g. namespace setup are not necessary when modifying access to a file which the VM can already access. Add a flag which allows to skip them. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 11 --- 1 file changed, 8 insertions(+), 3 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 10/12] qemu: Allow skipping the revoke step in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:05PM +0200, Peter Krempa wrote: In some cases when we need to modify access permissions for a storage source which is already used by the VM we should not revoke all permissions on a failure. Allow this in qemuDomainStorageSourceAccessModify by adding a new flag. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 5 + 1 file changed, 5 insertions(+) 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 09/12] qemu: Use bools rather than labels in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:04PM +0200, Peter Krempa wrote: Rather than jumping to the correct label use a set of booleans to determine which operation needs to be rolled back. This will allow more flexibility when e.g. rollback after a failed operation will not be necessary/desired. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 68 -- 1 file changed, 45 insertions(+), 23 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 08/12] qemu: Allow forcing read-only mode in qemuDomainStorageSourceAccessModify
On Thu, Apr 18, 2019 at 04:43:03PM +0200, Peter Krempa wrote: Add a new flag which will set the image as read-only even if the image data allows writing. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 7 +++ 1 file changed, 7 insertions(+) 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 2/2] Add support for podman in Makefile.ci
On Tue, May 07, 2019 at 05:45:31PM +0200, Martin Kletzander wrote:
> This way more users can run our CI builds locally.
>
> Signed-off-by: Martin Kletzander
> ---
> Makefile.ci | 125 ++--
> 1 file changed, 93 insertions(+), 32 deletions(-)
>
> diff --git a/Makefile.ci b/Makefile.ci
> index 12a62167cc67..e2989ada313c 100644
> --- a/Makefile.ci
> +++ b/Makefile.ci
> @@ -17,7 +17,7 @@ CI_GIT_ROOT = $(shell git rev-parse --show-toplevel)
> CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src
>
> # The directory holding the source inside the
> -# container. ie where we told Docker to expose
> +# container. ie where we want to expose
> # the $(CI_HOST_SRCDIR) directory from the host
> CI_CONT_SRCDIR = /src
>
> @@ -46,14 +46,13 @@ CI_CONFIGURE_ARGS =
> # cloning them
> CI_SUBMODULES = $(shell git submodule | awk '{ print $$2 }')
>
> -# Location of the Docker images we're going to pull
> +# Location of the container images we're going to pull
> # Can be useful to overridde to use a locally built
> # image instead
> CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-
>
> -# Docker defaults to pulling the ':latest' tag but
> -# if the Docker repo above uses different conventions
> -# this can override it
> +# The default tag is ':latest' but if the container
> +# repo above uses different conventions this can override it
> CI_IMAGE_TAG = :master
>
> # We delete the virtual root after completion, set
> @@ -71,24 +70,82 @@ CI_REUSE = 0
> CI_UID = $(shell id -u)
> CI_GID = $(shell id -g)
>
> -# Docker doesn't require the IDs you run as to exist in
> +# Container engine runtime we are going to use, can be overridden per make
> +# invocation, if it is not, we try podman and then default to docker.
> +ifeq ($(CI_CENGINE),)
> + CI_CENGINE = $(shell podman version >/dev/null && echo podman || echo
> docker)
> +endif
> +
> +# IDs you run as do not need to exist in
> # the container's /etc/passwd & /etc/group files, but
> -# if they do not, then libvirt's 'make check' will fail
> +# if they do not, then libvirt's 'make check' will fail
> # many tests.
> -#
> -# We do not directly mount /etc/{passwd,group} as Docker
> -# is liable to mess with SELinux labelling which will
> -# then prevent the host accessing them. Copying them
> -# first is safer.
> -CI_PWDB_MOUNTS = \
> - --volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
> - --volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
> - $(NULL)
> +ifeq ($(CI_CENGINE),podman)
> + CI_PWDB_MOUNTS = \
> + --volume /etc/group:/etc/group:ro,z \
> + --volume /etc/passwd:/etc/passwd:ro,z \
> + $(NULL)
> +else
> + # We do not directly mount /etc/{passwd,group} as Docker
> + # is liable to mess with SELinux labelling which will
> + # then prevent the host accessing them. Copying them
> + # first is safer.
> + CI_PWDB_MOUNTS = \
> + --volume $(CI_SCRATCHDIR)/group:/etc/group:ro,z \
> + --volume $(CI_SCRATCHDIR)/passwd:/etc/passwd:ro,z \
> + $(NULL)
> +endif
Does this need to be conditionalized ? Wouldn't podman just
work with the existing code. How does podman end up giving
access to these files if it isn't changing the SELinux label
on them ?
> +
> +ifeq ($(CI_CENGINE),docker)
> + # Docker containers can have very large ulimits
> + # for nofiles - as much as 1048576. This makes
> + # libvirt very slow at exec'ing programs.
> + CI_ULIMIT_FILES = 1024
> +endif
Again, does this really need to be conditionalized ?
>
> -# Docker containers can have very large ulimits
> -# for nofiles - as much as 1048576. This makes
> -# libvirt very slow at exec'ing programs.
> -CI_ULIMIT_FILES = 1024
> +ifeq ($(CI_CENGINE),podman)
> + # Podman cannot reuse host namespace when running non-root containers.
> Until
> + # support for --keep-uid is added we can just create another mapping
> that will
> + # do that for us. Beware, that in
> {uid,git}map=container_id:host_id:range,
> + # the host_id does actually refer to the uid in the first mapping where > 0
> + # (root) is mapped to the current user and rest is offset.
> +
> + # In order to set up this mapping, we need to keep all the user IDs to
> prevent
> + # possible errors as some images might expect UIDs up to 9 (looking
> at you
> + # fedora), so we don't want the overflowuid to be used for them. For
> mapping
> + # all the other users properly ther eneeds to be some math done. Don't
> worry,
> + # it's just addition and subtraction.
> +
> + # 65536 ought to be enough (tm), but for really rare cases the maximums
> might
> + # need to be higher, but that only happens when your /etc/sub{u,g}id
> allow
> + # users to have more IDs. Unless --keep-uid is supported, let's do
> this in a
> + # way that should work for everyone.
> + CI_MAX_UID = $(shell sed -n "s/^$USER:[^:]\+://p" /etc/subuid)
>
Re: [libvirt] [PATCH 4/4] snapshot: Make virDomainSnapshotDef a virObject
On Wed, May 08, 2019 at 17:24:12 -0500, Eric Blake wrote: > This brings about a couple of benefits: > - use of VIR_AUTOUNREF() simplifies several callers > - Fixes a todo about virDomainMomentObjList not being polymorphic enough > > Signed-off-by: Eric Blake > --- > src/conf/moment_conf.h| 5 - > src/conf/snapshot_conf.h | 1 - > cfg.mk| 2 -- > src/conf/moment_conf.c| 28 - > src/conf/snapshot_conf.c | 34 ++- > src/conf/virdomainmomentobjlist.c | 3 +-- > src/esx/esx_driver.c | 3 +-- > src/libvirt_private.syms | 1 - > src/qemu/qemu_driver.c| 5 ++--- > src/test/test_driver.c| 5 ++--- > src/vbox/vbox_common.c| 6 ++ > src/vz/vz_driver.c| 3 +-- > tests/domainsnapshotxml2xmltest.c | 3 +-- > 13 files changed, 65 insertions(+), 34 deletions(-) ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] snapshot: Add virDomainSnapshotDefNew
On Wed, May 08, 2019 at 17:24:11 -0500, Eric Blake wrote:
> In preparation for making virDomainSnapshotDef a descendant of
> virObject, it is time to fix all callers that allocate an object to
> use virDomainSnapshotDefNew() instead of VIR_ALLOC(). Fortunately,
> there aren't very many :)
>
> Signed-off-by: Eric Blake
> ---
> src/conf/snapshot_conf.h | 1 +
> src/conf/snapshot_conf.c | 16 +---
> src/libvirt_private.syms | 1 +
> src/vbox/vbox_common.c | 3 ++-
> 4 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
> index f54be11619..0ce9dda355 100644
> --- a/src/conf/snapshot_conf.h
> +++ b/src/conf/snapshot_conf.h
> @@ -114,6 +114,7 @@ virDomainSnapshotDefPtr
> virDomainSnapshotDefParseNode(xmlDocPtr xml,
>virDomainXMLOptionPtr
> xmlopt,
>bool *current,
>unsigned int flags);
> +virDomainSnapshotDefPtr virDomainSnapshotDefNew(void);
> void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def);
> char *virDomainSnapshotDefFormat(const char *uuidstr,
> virDomainSnapshotDefPtr def,
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index dd281d57fe..e5771ae635 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -81,7 +81,17 @@ virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr
> disk)
> disk->src = NULL;
> }
>
> -void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
> +virDomainSnapshotDefPtr
> +virDomainSnapshotDefNew(void)
> +{
> +virDomainSnapshotDefPtr def;
> +
> +ignore_value(VIR_ALLOC(def));
> +return def;
> +}
> +
> +void
> +virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
This is changed/deleted again in next patch so this patch shouldn't
tweak the style.
ACK
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] snapshot: s/current/parent/ as prep for virObject
On Wed, May 08, 2019 at 17:24:10 -0500, Eric Blake wrote: > VIR_CLASS_NEW insists that descendents of virObject have 'parent' as > the name of their inherited member at offset 0. While it would be > possible to write a new class-creation macro that takes the actual > field name 'current', and rewrite VIR_CLASS_NEW to call the new macro > with the hard-coded name 'parent', it seems less confusing if all > object code uses similar naming. Thus, this is a mechanical rename in > preparation of making virDomainSnapshotDef a descendent of virObject. > > Signed-off-by: Eric Blake > --- > src/conf/snapshot_conf.h| 2 +- > src/conf/snapshot_conf.c| 120 ++-- > src/conf/virdomainsnapshotobjlist.c | 2 +- > src/esx/esx_driver.c| 16 ++-- > src/qemu/qemu_domain.c | 2 +- > src/qemu/qemu_driver.c | 12 +-- > src/test/test_driver.c | 2 +- > src/vbox/vbox_common.c | 88 ++-- > src/vz/vz_driver.c | 2 +- > src/vz/vz_sdk.c | 10 +-- > 10 files changed, 128 insertions(+), 128 deletions(-) ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am
On Tue, May 07, 2019 at 05:45:30PM +0200, Martin Kletzander wrote: > The way it works now the Makefile needs to be both make valid and automake > valid. That is fine for now, but if we want to use anything more advanced, > like > conditionals, we cannot have it like that any more. > > So instead forward all ci-* rules to that file. > > Signed-off-by: Martin Kletzander > --- > Makefile.am | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/Makefile.am b/Makefile.am > index 0d8bb733e6d2..442bae511828 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -35,6 +35,7 @@ EXTRA_DIST = \ >libvirt-qemu.pc.in \ >libvirt-lxc.pc.in \ >libvirt-admin.pc.in \ > + Makefile.ci \ >Makefile.nonreentrant \ >autogen.sh \ >cfg.mk \ Indentation is not consistent here - tabs vs non-tabs. > @@ -107,4 +108,5 @@ gen-AUTHORS: > rm -f all.list maint.list contrib.list; \ > fi > > -include Makefile.ci > +ci-%: > + $(MAKE) -f Makefile.ci $@ Will this cause all variables to be forwarded ? eg will make ci-build@fedora-29 CI_IMAGE_TAG=:latest result in make -f Makefile.ci ci-build@fedora-29 CI_IMAGE_TAG=:latest 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] spec: Bump minimum supported Fedora version to 29
On Tue, May 07, 2019 at 12:15:32PM +0200, Andrea Bolognani wrote: > Fedora 30 is out, which means that Fedora 28 is going to be > EOL very soon. Let's get ahead of the game and drop support > for it right now. > > Signed-off-by: Andrea Bolognani > --- > libvirt.spec.in | 2 +- > mingw-libvirt.spec.in | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé 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] [jenkins-ci PATCH 4/4] guests: Drop Fedora 28
On Thu, May 02, 2019 at 06:28:13PM +0200, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- > guests/host_vars/libvirt-fedora-28/docker.yml | 2 - > .../host_vars/libvirt-fedora-28/install.yml | 2 - > guests/host_vars/libvirt-fedora-28/main.yml | 24 -- > guests/inventory | 1 - > guests/vars/mappings.yml | 1 - > guests/vars/vault.yml | 78 +-- > 6 files changed, 37 insertions(+), 71 deletions(-) > delete mode 100644 guests/host_vars/libvirt-fedora-28/docker.yml > delete mode 100644 guests/host_vars/libvirt-fedora-28/install.yml > delete mode 100644 guests/host_vars/libvirt-fedora-28/main.yml Reviewed-by: Daniel P. Berrangé 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] [jenkins-ci PATCH 3/4] Stop building on Fedora 28
On Thu, May 02, 2019 at 06:28:12PM +0200, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- > guests/playbooks/build/jobs/defaults.yml| 2 -- > guests/playbooks/build/projects/libvirt-dbus.yml| 3 --- > guests/playbooks/build/projects/libvirt-go-xml.yml | 1 - > guests/playbooks/build/projects/libvirt-go.yml | 1 - > guests/playbooks/build/projects/libvirt-ocaml.yml | 1 - > guests/playbooks/build/projects/libvirt-sandbox.yml | 2 -- > guests/playbooks/build/projects/libvirt-tck.yml | 2 -- > guests/playbooks/build/projects/libvirt.yml | 1 - > guests/playbooks/build/projects/osinfo-db-tools.yml | 1 - > guests/playbooks/build/projects/osinfo-db.yml | 1 - > guests/playbooks/build/projects/virt-manager.yml| 3 --- > guests/playbooks/build/projects/virt-viewer.yml | 1 - > jenkins/jobs/defaults.yaml | 2 -- > jenkins/projects/libvirt-dbus.yaml | 3 --- > jenkins/projects/libvirt-go-xml.yaml| 1 - > jenkins/projects/libvirt-go.yaml| 1 - > jenkins/projects/libvirt-ocaml.yaml | 1 - > jenkins/projects/libvirt-sandbox.yaml | 2 -- > jenkins/projects/libvirt-tck.yaml | 2 -- > jenkins/projects/libvirt.yaml | 1 - > jenkins/projects/osinfo-db-tools.yaml | 1 - > jenkins/projects/osinfo-db.yaml | 1 - > jenkins/projects/virt-manager.yaml | 3 --- > jenkins/projects/virt-viewer.yaml | 1 - > 24 files changed, 38 deletions(-) Reviewed-by: Daniel P. Berrangé 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] [jenkins-ci PATCH 2/4] Start building on Fedora 30
On Thu, May 02, 2019 at 06:28:11PM +0200, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- > guests/playbooks/build/jobs/defaults.yml| 2 ++ > guests/playbooks/build/projects/libvirt-dbus.yml| 3 +++ > guests/playbooks/build/projects/libvirt-go-xml.yml | 1 + > guests/playbooks/build/projects/libvirt-go.yml | 1 + > guests/playbooks/build/projects/libvirt-ocaml.yml | 1 + > guests/playbooks/build/projects/libvirt-sandbox.yml | 2 ++ > guests/playbooks/build/projects/libvirt-tck.yml | 2 ++ > guests/playbooks/build/projects/libvirt.yml | 1 + > guests/playbooks/build/projects/osinfo-db-tools.yml | 1 + > guests/playbooks/build/projects/osinfo-db.yml | 1 + > guests/playbooks/build/projects/virt-manager.yml| 3 +++ > guests/playbooks/build/projects/virt-viewer.yml | 1 + > jenkins/jobs/defaults.yaml | 2 ++ > jenkins/projects/libvirt-dbus.yaml | 3 +++ > jenkins/projects/libvirt-go-xml.yaml| 1 + > jenkins/projects/libvirt-go.yaml| 1 + > jenkins/projects/libvirt-ocaml.yaml | 1 + > jenkins/projects/libvirt-sandbox.yaml | 2 ++ > jenkins/projects/libvirt-tck.yaml | 2 ++ > jenkins/projects/libvirt.yaml | 1 + > jenkins/projects/osinfo-db-tools.yaml | 1 + > jenkins/projects/osinfo-db.yaml | 1 + > jenkins/projects/virt-manager.yaml | 3 +++ > jenkins/projects/virt-viewer.yaml | 1 + > 24 files changed, 38 insertions(+) Reviewed-by: Daniel P. Berrangé 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] [jenkins-ci PATCH 1/4] guests: Add Fedora 30
On Thu, May 02, 2019 at 06:28:10PM +0200, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- > guests/host_vars/libvirt-fedora-30/docker.yml | 2 + > .../host_vars/libvirt-fedora-30/install.yml | 2 + > guests/host_vars/libvirt-fedora-30/main.yml | 24 ++ > guests/inventory | 1 + > guests/vars/vault.yml | 78 ++- > 5 files changed, 70 insertions(+), 37 deletions(-) > create mode 100644 guests/host_vars/libvirt-fedora-30/docker.yml > create mode 100644 guests/host_vars/libvirt-fedora-30/install.yml > create mode 100644 guests/host_vars/libvirt-fedora-30/main.yml Reviewed-by: Daniel P. Berrangé 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 1/4] snapshot: s/parent/parent_name/ as prep for virObject
On Wed, May 08, 2019 at 17:24:09 -0500, Eric Blake wrote: > VIR_CLASS_NEW insists that descendents of virObject have 'parent' as > the name of their inherited member at offset 0. While it would be > possible to write a new class-creation macro that takes the actual > field name, and rewrite VIR_CLASS_NEW to call the new macro with the > hard-coded name 'parent', so that we could make virDomainMomentDef use > a custom name for its base class, it seems less confusing if all > object code uses similar naming. Thus, this is a mechanical rename in > preparation of making virDomainSnapshotDef a descendent of virObject, > when we can no longer use 'parent' for a different purpose than the > base class. > > Signed-off-by: Eric Blake > --- > src/conf/moment_conf.h| 2 +- > src/conf/moment_conf.c| 2 +- > src/conf/snapshot_conf.c | 22 -- > src/conf/virdomainmomentobjlist.c | 4 ++-- > src/esx/esx_driver.c | 2 +- > src/qemu/qemu_domain.c| 8 > src/qemu/qemu_driver.c| 12 ++-- > src/test/test_driver.c| 18 +- > src/vbox/vbox_common.c| 14 +++--- > src/vz/vz_sdk.c | 2 +- > 10 files changed, 44 insertions(+), 42 deletions(-) ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 2/4] guests: Use package_manager everywhere
On Tue, May 07, 2019 at 03:17:40PM +0200, Andrea Bolognani wrote:
> Instead of hardcoding the name of the package manager in
> commands, use the value obtained from the inventory.
>
> In some cases this is necessary, eg. when RPM-based
> distributions are involved; for most other cases we could
> get away with keepking the hardcoded names, but it's better
> to be completely consistent to hopefully avoid usage of the
> wrong package manager slipping with further changes.
>
> Signed-off-by: Andrea Bolognani
> ---
> guests/lcitool | 40 +++--
> guests/playbooks/update/tasks/base.yml | 12 +++
> guests/playbooks/update/tasks/bootstrap.yml | 16 ++---
> 3 files changed, 29 insertions(+), 39 deletions(-)
Reviewed-by: Daniel P. Berrangé
> - name: Bootstrap Ansible
> - raw: 'yum install -y {{ python }}'
> - when:
> -- package_format == 'rpm'
> -
> -- name: Bootstrap Ansible
> - raw: 'apt-get install -y {{ python }}'
> - when:
> -- package_format == 'deb'
> -
> -- name: Bootstrap Ansible
> - raw: 'pkg install -y {{ python }}'
> - when:
> -- package_format == 'pkg'
> + raw: '{{ package_manager }} install -y {{ python }}'
Heh, rather amuzing that all three package managers happen to support
the same "install -y" command syntax.
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] [jenkins-ci PATCH 3/4] guests: Don't call out to the shell twice
On Tue, May 07, 2019 at 03:17:41PM +0200, Andrea Bolognani wrote: > We're already doing this for FreeBSD, so do it for CentOS > and Fedora as well. > > Signed-off-by: Andrea Bolognani > --- > guests/playbooks/update/tasks/base.yml | 11 +-- > 1 file changed, 1 insertion(+), 10 deletions(-) Reviewed-by: Daniel P. Berrangé 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] [jenkins-ci PATCH 4/4] lcitool: Fix Dockerfile alignment
On Tue, May 07, 2019 at 03:17:42PM +0200, Andrea Bolognani wrote: > Now that we are using package_manager everywhere instead of > hardcoding the names, it's finally possible to make the > alignment of the resulting Dockerfiles perfect. > > Signed-off-by: Andrea Bolognani > --- > guests/lcitool | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Daniel P. Berrangé 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
[libvirt] [PATCH] build: restore support for libyajl 2.0.1
Commit 105756660f944e7db02de3b55b98bb7c11cd03bf was too eager and did not consider SLE 12 which still has 2.0.1 that does not ship a pkg-config file. Similar to how we check for readline, prefer pkg-config if available and fall back to the old detection code if not found. NB: this is not a clean revert because we're not reintroducing support for YAJL 1. Signed-off-by: Ján Tomko Reported-by: Olaf Hering --- m4/virt-yajl.m4 | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/m4/virt-yajl.m4 b/m4/virt-yajl.m4 index 494e722963..05a8f8b9bd 100644 --- a/m4/virt-yajl.m4 +++ b/m4/virt-yajl.m4 @@ -24,7 +24,19 @@ AC_DEFUN([LIBVIRT_ARG_YAJL],[ AC_DEFUN([LIBVIRT_CHECK_YAJL],[ dnl YAJL JSON library http://lloyd.github.com/yajl/ - LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3]) + PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0]) + + if test $use_pkgconfig = 1; then +dnl 2.0.3 was the version where the pkg-config file was first added +LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3]) + else +dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1 +dnl TODO: delete this in July 2020 +LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl], + [yajl_tree_parse], [yajl/yajl_common.h]) + + fi + ]) AC_DEFUN([LIBVIRT_RESULT_YAJL],[ -- 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 1/4] guests: Introduce package_manager
On Tue, May 07, 2019 at 03:17:39PM +0200, Andrea Bolognani wrote: > Platforms that share the same package format might still want > to use different package managers, a good example being CentOS > with yum and Fedora with dnf. > > Signed-off-by: Andrea Bolognani > --- > guests/host_vars/libvirt-centos-7/main.yml| 1 + > guests/host_vars/libvirt-debian-9/main.yml| 1 + > guests/host_vars/libvirt-debian-sid/main.yml | 1 + > guests/host_vars/libvirt-fedora-29/main.yml | 1 + > guests/host_vars/libvirt-fedora-30/main.yml | 1 + > guests/host_vars/libvirt-fedora-rawhide/main.yml | 1 + > guests/host_vars/libvirt-freebsd-11/main.yml | 1 + > guests/host_vars/libvirt-freebsd-12/main.yml | 1 + > guests/host_vars/libvirt-freebsd-current/main.yml | 1 + > guests/host_vars/libvirt-ubuntu-18/main.yml | 1 + > 10 files changed, 10 insertions(+) Reviewed-by: Daniel P. Berrangé 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 2/3] docs: hacking: Add good practices for shortening conditional expressions
On Thu, May 09, 2019 at 12:53:07 +0200, Martin Kletzander wrote: > On Thu, May 09, 2019 at 12:43:33PM +0200, Peter Krempa wrote: > > Document that checking if a integer is (non-)zero should (not must) > > avoid the shortened form that C allows as it may confuse readers into > > overlooking the other possible values which might be interresting to > > handle. > > > > While pointers have distinct values from the point of view of the code > > we only care whether it's non-NULL and thus it's documented it's okay > > to shorten those. > > > > Signed-off-by: Peter Krempa > > --- > > docs/hacking.html.in | 22 ++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/docs/hacking.html.in b/docs/hacking.html.in > > index 081d793360..a2800853ef 100644 > > --- a/docs/hacking.html.in > > +++ b/docs/hacking.html.in > > @@ -826,6 +826,28 @@ > > } > > > > > > +Conditional expressions > > + For readability reasons new code should avoid shortening > > comparisons > > +to 0 for numeric types. Boolean and pointer comparisions may be > > +shortened. All long forms are okay: > > + > > + > > + virFooPtr foos = NULL; > > + size nfoos = 0; > > + bool hasFoos = false; > > + > > +GOOD: > > +if (!foos) > > +if (!hasFoos) > > +if (nfoos == 0) > > +if (foos == NULL) > > +if (hasFoos == true) > > + > > +BAD: > > +if (!nfoos) > > +if (foos) > > why is this bad when it is a pointer? Typo? Oops, yes. This was supposed to be "if (nfoos)" signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] docs: hacking: Add good practices for shortening conditional expressions
On Thu, May 09, 2019 at 12:43:33PM +0200, Peter Krempa wrote: Document that checking if a integer is (non-)zero should (not must) avoid the shortened form that C allows as it may confuse readers into overlooking the other possible values which might be interresting to handle. While pointers have distinct values from the point of view of the code we only care whether it's non-NULL and thus it's documented it's okay to shorten those. Signed-off-by: Peter Krempa --- docs/hacking.html.in | 22 ++ 1 file changed, 22 insertions(+) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 081d793360..a2800853ef 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -826,6 +826,28 @@ } +Conditional expressions + For readability reasons new code should avoid shortening comparisons +to 0 for numeric types. Boolean and pointer comparisions may be +shortened. All long forms are okay: + + + virFooPtr foos = NULL; + size nfoos = 0; + bool hasFoos = false; + +GOOD: +if (!foos) +if (!hasFoos) +if (nfoos == 0) +if (foos == NULL) +if (hasFoos == true) + +BAD: +if (!nfoos) +if (foos) why is this bad when it is a pointer? Typo? + + Preprocessor Macros defined with an ALL_CAPS name should generally be -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] docs: hacking: Discourage some patterns to help code readability
Peter Krempa (3): docs: hacking: Document few practices for creating error messages docs: hacking: Add good practices for shortening conditional expressions docs: hacking: Discourage use of the ternary operator and ban it's abuse docs/hacking.html.in | 61 1 file changed, 61 insertions(+) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] docs: hacking: Discourage use of the ternary operator and ban it's abuse
Forbid breaking lines inside the two branches of the ternary operator and nesting them. Using it in these instances does not help readability. Signed-off-by: Peter Krempa --- docs/hacking.html.in | 11 +++ 1 file changed, 11 insertions(+) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index a2800853ef..fc4adae354 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -847,6 +847,17 @@ BAD: if (!nfoos) if (foos) + New code should avoid the ternary operator as much as possible. +Specifically it must never span more than one line or nest: + + +BAD: +char *foo = baz ? +virDoSomethingReallyComplex(driver, vm, something, baz->foo) : +NULL; + +char *foo = bar ? bar->baz ? bar->baz->foo : "nobaz" : "nobar"; + Preprocessor -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] docs: hacking: Document few practices for creating error messages
State that error messages should not be broken into multiple lines for
programmer friendliness and should not be concatenated on the fly for
translator friendliness and few other details.
Signed-off-by: Peter Krempa
---
docs/hacking.html.in | 28
1 file changed, 28 insertions(+)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 902d05430f..081d793360 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1284,6 +1284,34 @@
does for snprintf.
+Error message format
+
+
+ Error messages visible to the user should be short and descriptive. All
+ error messages are translated using gettext and thus must be wrapped in
+ _() macro. To simplify the translation work, the error
message
+ must not be concatenated from various parts. To simplify searching for
+ the error message in the code the strings should not be broken even
+ if they result into a line longer than 80 columns and any formatting
+ modifier should be enclosed by quotes or other obvious separator.
+ If a string used with %s can be NULL the NULLSTR macro must
+ be used.
+
+
+
+ GOOD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to connect to remote host '%s'"), hostname)
+
+ BAD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to %s to remote host '%s'"),
+ "connect", hostname);
+
+ BAD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to connect "
+ "to remote host '%s'),
+ hostname);
+
+
Use of goto
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] docs: hacking: Add good practices for shortening conditional expressions
Document that checking if a integer is (non-)zero should (not must) avoid the shortened form that C allows as it may confuse readers into overlooking the other possible values which might be interresting to handle. While pointers have distinct values from the point of view of the code we only care whether it's non-NULL and thus it's documented it's okay to shorten those. Signed-off-by: Peter Krempa --- docs/hacking.html.in | 22 ++ 1 file changed, 22 insertions(+) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 081d793360..a2800853ef 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -826,6 +826,28 @@ } +Conditional expressions + For readability reasons new code should avoid shortening comparisons +to 0 for numeric types. Boolean and pointer comparisions may be +shortened. All long forms are okay: + + + virFooPtr foos = NULL; + size nfoos = 0; + bool hasFoos = false; + +GOOD: +if (!foos) +if (!hasFoos) +if (nfoos == 0) +if (foos == NULL) +if (hasFoos == true) + +BAD: +if (!nfoos) +if (foos) + + Preprocessor Macros defined with an ALL_CAPS name should generally be -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v6] tests: perform cross compiler builds on GitLab CI
On Tue, May 07, 2019 at 11:54:20AM +0200, Andrea Bolognani wrote: > On Tue, 2019-04-16 at 11:19 +0100, Daniel P. Berrangé wrote: > > +.job_template: _definition > > + script: > > +- mkdir vpath > > +- cd vpath > > We use build/ for CentOS CI and Travis CI builds, so let's stick > with that here as well. Ok > > +- ../autogen.sh $CONFIGURE_OPTS > > We have some code to show the contents of config.log when this > steps fails in other CI environments, would it be a good idea to > have it here as well? Make it say: - ../autogen.sh $CONFIGURE_OPTS || (cat config.log && exit 1) 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/2] Avoid issues due to qemu dropping osxsave and ospke
On Tue, May 7, 2019 at 11:03 PM Daniel Henrique Barboza wrote: > > > > On 4/25/19 9:50 AM, Christian Ehrhardt wrote: > > Hi, > > this series tries to address a drop of commandline options by qemu in > > regard to > > osxsave [1] and ospke [2]. > > This was already discussed in [3] late last year but got forgotten > > afterwards. > > The Ubuntu bug is at [4] and an older Fedora bug is at [5]. > > > > TL;DR: > > - osxsave/ospke features were never really configurable > > - KVM never returned the bits on GET_SUPPORTED_CPUID > > - very rare to be seen in the wild > > - avoid issues with newer qemu and old/odd XMLs to be sure > > > > Details: > > > > I checked various use cases from virt-install to openstack and some in > > between. > > The only cases I found that would define osxsave/ospke is virt-install pior > > to version 2.0 and even there only when used with --cpu=host-model or > > --cpu=host-copy. > > If you ever really enabled the feature you'd have got: > >error: the CPU is incompatible with host CPU: > >Host CPU does not provide required features: ospke > > > > The problem lies in domain XMLs that explicitly disable it. That would be > > > > But due to almost (or actually none) no host exposing this the following > > also triggers: > > > > So, it happened that this notebook (ThinkPad T480) has the feature you're > handling in this patch series: Mine as well actually, but obviously only on the host capabilities. [...] > > With your approach, what happens is that a feature that is declared to be > effective in the capabilities is, in fact, ignored. It is an upgrade of > what would happen without it, of course. Some bikeshedding on "declared to be effective in the capabilities" needed to bring me up to date on this. See below... > But I am wondering here, fully aware that you mentioned that you wanted this > discussion to be avoided, Sorry my comment was misleading then - I'm absolutely fine having that part of the discussion in this context. I only wanted to avoid the discussion about removing features that "actually did something" which naturally is a way more complex topic. > that we should simply exterminate both osxsave and > ospke from Libvirt capabilities. I mean, what's the point of even > reporting them > if they will end up being ignored? You have both QEMU commits [1] and [2] > mentioning that these flags were never configurable in the first place, > so it's not > like we need to keep them for backward compatibility either. Maybe it is my current lack of understanding how "exterminating both" would be exactly done and I see potential issues where there are none. I wonder would that just be dropping the mappings in src/cpu_map/x86_features.xml? If not then I'd appreciate a hint where exactly you'd suggest we would do the mentioned further extermination of ospke/osxsave. I was afraid of side effects or when no more being detected. And even more so it seems the definition of "capabilities" is not clear enough to me (I never thought too much about it so far): - "The capabilities of the hypervisor" (man page) - "The host CPU architecture and features." (web page) For the former definition yes, we might want to drop such non passable features then. But for the latter definition it feels right to continue reporting that the host has it. It is valid to report that the host has the feature - even thou it can't pass it to a guest. After all it is in the host and not the guest (hypervisor dependent) section of capabilities right? Also we might (later) use the mapping for other things down the road where being an tunable feature (or not) is not important either. Or users compare hosts with same hardware but different libvirt versions and wonder why they lost a cpu feature. > > Note that this does not discard this patch series - I think we'll need a > solution > like this to deal with older VMs that define these features in their XMLs. > > > Thanks, > > > DHB > > > > > > This will make libvirt add it to the qemu commandline like: > >-cpu ...,osxsave=off,ospke=off > > > > And that will crash when qemu starts with: > >error: internal error: process exited while connecting to monitor: > >2019-04-25T12:12:01.698646Z qemu-system-x86_64: can't apply global > >core2duo-x86_64-cpu.osxsave=off: Property '.osxsave' not found > > > > There are much more long term discussions about demoting and dropping qemu > > features and I'd like to avoid those discussions being mixed. > > The reason to drop it more or less without notice was that it never did > > anything to begin with. Due to that our solution might in a similar fashion > > be more trivial - just stop defining those two features to qemu commandline. > > > > [1]: > > https://git.qemu.org/?p=qemu.git;a=commit;h=f1a23522b03a569f13aad49294bb4c4b1a9500c7 > > [2]: > > https://git.qemu.org/?p=qemu.git;a=commit;h=9ccb9784b57804f5c74434ad6ccb66650a015ffc > > [3]:
Re: [libvirt] [PATCH 04/10] build: require yajl >= 2.0.3
On Thu, May 09, 2019 at 10:30:25AM +0200, Andrea Bolognani wrote: > On Wed, 2019-05-08 at 17:29 +0100, Daniel P. Berrangé wrote: > > On Wed, May 08, 2019 at 06:21:21PM +0200, Olaf Hering wrote: > > > On Wed, Apr 03, Ján Tomko wrote: > > > > > > > Since all our supported platforms include at least yajl 2.0.4, > > > > use pkg-config to detect the library and set the minimum to 2.0.3. > > > > > > In case SLE_12 is on the list of supported platforms, this kicked it > > > off of the list. SLE_12 (and Leap 42.3) comes with version 2.0.1. > > > SLE_15 has version 2.1.0. > > > > Yes, this is a mistake. > > > > https://libvirt.org/platforms.html > > > > "For distributions with long-lifetime releases, the project will aim > > to support the most recent major version at all times. Support for > > the previous major version will be dropped 2 years after the new > > major version is released. For the purposes of identifying supported > > software versions, the project will look at RHEL, Debian, Ubuntu LTS, > > and SLES distros. " > > > > SLE 15 was released July 2018, so we should be continuing to support > > SLE 12 until July 2020. > > > > So we need to fix this to allow fallback to the non-pkg-config detection > > for yajl. > > Well that's a bummer, but at least we don't have to re-introduce > support for YAJL 1 - just fix YAJL 2 detection. > > We should really add support for SLES, or whatever its CentOS > equivalent is, to libvirt-jenkins-ci, though. Yeah, openSUSE Leap is the one we'd want to add support for. 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 04/10] build: require yajl >= 2.0.3
On Thu, May 09, 2019 at 10:40:51AM +0200, Ján Tomko wrote: > On Thu, May 09, 2019 at 10:30:25AM +0200, Andrea Bolognani wrote: > > On Wed, 2019-05-08 at 17:29 +0100, Daniel P. Berrangé wrote: > > > On Wed, May 08, 2019 at 06:21:21PM +0200, Olaf Hering wrote: > > > > On Wed, Apr 03, Ján Tomko wrote: > > > > > > > > > Since all our supported platforms include at least yajl 2.0.4, > > > > > use pkg-config to detect the library and set the minimum to 2.0.3. > > > > > > > > In case SLE_12 is on the list of supported platforms, this kicked it > > > > off of the list. SLE_12 (and Leap 42.3) comes with version 2.0.1. > > > > SLE_15 has version 2.1.0. > > Oops, so 'Leap 42.3' is the one I should be looking at to see what's in > SLE_12? > https://repology.org/project/yajl/versions Yes, there's an explanation of the versioning scheme here: https://lists.opensuse.org/opensuse-project/2017-04/msg00014.html openSUSE Leap 42.3 is eqiuv to SLE 12 sp3 So we can use Leap versions in repoology to track requirements for SLE, which I didn't realize was possible in the past. 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 04/10] build: require yajl >= 2.0.3
On Thu, May 09, 2019 at 10:30:25AM +0200, Andrea Bolognani wrote: On Wed, 2019-05-08 at 17:29 +0100, Daniel P. Berrangé wrote: On Wed, May 08, 2019 at 06:21:21PM +0200, Olaf Hering wrote: > On Wed, Apr 03, Ján Tomko wrote: > > > Since all our supported platforms include at least yajl 2.0.4, > > use pkg-config to detect the library and set the minimum to 2.0.3. > > In case SLE_12 is on the list of supported platforms, this kicked it > off of the list. SLE_12 (and Leap 42.3) comes with version 2.0.1. > SLE_15 has version 2.1.0. Oops, so 'Leap 42.3' is the one I should be looking at to see what's in SLE_12? https://repology.org/project/yajl/versions I will look into it after lunch. Jano Yes, this is a mistake. https://libvirt.org/platforms.html "For distributions with long-lifetime releases, the project will aim to support the most recent major version at all times. Support for the previous major version will be dropped 2 years after the new major version is released. For the purposes of identifying supported software versions, the project will look at RHEL, Debian, Ubuntu LTS, and SLES distros. " SLE 15 was released July 2018, so we should be continuing to support SLE 12 until July 2020. So we need to fix this to allow fallback to the non-pkg-config detection for yajl. Well that's a bummer, but at least we don't have to re-introduce support for YAJL 1 - just fix YAJL 2 detection. We should really add support for SLES, or whatever its CentOS equivalent is, to libvirt-jenkins-ci, though. -- Andrea Bolognani / Red Hat / Virtualization signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/10] build: require yajl >= 2.0.3
On Wed, 2019-05-08 at 17:29 +0100, Daniel P. Berrangé wrote: > On Wed, May 08, 2019 at 06:21:21PM +0200, Olaf Hering wrote: > > On Wed, Apr 03, Ján Tomko wrote: > > > > > Since all our supported platforms include at least yajl 2.0.4, > > > use pkg-config to detect the library and set the minimum to 2.0.3. > > > > In case SLE_12 is on the list of supported platforms, this kicked it > > off of the list. SLE_12 (and Leap 42.3) comes with version 2.0.1. > > SLE_15 has version 2.1.0. > > Yes, this is a mistake. > > https://libvirt.org/platforms.html > > "For distributions with long-lifetime releases, the project will aim > to support the most recent major version at all times. Support for > the previous major version will be dropped 2 years after the new > major version is released. For the purposes of identifying supported > software versions, the project will look at RHEL, Debian, Ubuntu LTS, > and SLES distros. " > > SLE 15 was released July 2018, so we should be continuing to support > SLE 12 until July 2020. > > So we need to fix this to allow fallback to the non-pkg-config detection > for yajl. Well that's a bummer, but at least we don't have to re-introduce support for YAJL 1 - just fix YAJL 2 detection. We should really add support for SLES, or whatever its CentOS equivalent is, to libvirt-jenkins-ci, though. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
On Wed, May 08, 2019 at 11:27:47PM +0800, Boris Fiuczynski wrote: > On 5/8/19 11:22 PM, Alex Williamson wrote: > >>> I thought there was a request to make this more specific to migration > >>> by renaming it to something like migration_version. Also, as an > >>> > >> so this attribute may not only include a mdev device's parent device info > >> and > >> mdev type, but also include numeric software version of vendor specific > >> migration code, right? > > It's a vendor defined string, it should be considered opaque to the > > user, the vendor can include whatever they feel is relevant. > > > Would a vendor also be allowed to provide a string expressing required > features as well as containing backend resource requirements which need > to be compatible for a successful migration? Somehow a bit like a cpu > model... maybe even as json or xml... > I am asking this with vfio-ap in mind. In that context checking > compatibility of two vfio-ap mdev devices is not as simple as checking > if version A is smaller or equal to version B. > I think so. vendor driver is allowed to put whatever content into the migration_version string as long as it thinks it's necessary. vendor driver only needs ensure in the target mdev device, the write(2) operation on its migration_version attribute would correctly fail or succeeed based on the input string. Thanks Yan > -- > Mit freundlichen Grüßen/Kind regards > Boris Fiuczynski > > IBM Deutschland Research & Development GmbH > Vorsitzender des Aufsichtsrats: Matthias Hartmann > Geschäftsführung: Dirk Wittkopp > Sitz der Gesellschaft: Böblingen > Registergericht: Amtsgericht Stuttgart, HRB 243294 > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device
On 5/8/19 11:22 PM, Alex Williamson wrote: I thought there was a request to make this more specific to migration by renaming it to something like migration_version. Also, as an so this attribute may not only include a mdev device's parent device info and mdev type, but also include numeric software version of vendor specific migration code, right? It's a vendor defined string, it should be considered opaque to the user, the vendor can include whatever they feel is relevant. Would a vendor also be allowed to provide a string expressing required features as well as containing backend resource requirements which need to be compatible for a successful migration? Somehow a bit like a cpu model... maybe even as json or xml... I am asking this with vfio-ap in mind. In that context checking compatibility of two vfio-ap mdev devices is not as simple as checking if version A is smaller or equal to version B. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
