Re: [libvirt] [RFC PATCH 0/9] Introduce mediate ops in vfio-pci
On Thu, Dec 12, 2019 at 11:48:25AM +0800, Jason Wang wrote: > > On 2019/12/6 下午8:49, Yan Zhao wrote: > > On Fri, Dec 06, 2019 at 05:40:02PM +0800, Jason Wang wrote: > >> On 2019/12/6 下午4:22, Yan Zhao wrote: > >>> On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote: > On 2019/12/5 下午4:51, Yan Zhao wrote: > > On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote: > >> Hi: > >> > >> On 2019/12/5 上午11:24, Yan Zhao wrote: > >>> For SRIOV devices, VFs are passthroughed into guest directly without > >>> host > >>> driver mediation. However, when VMs migrating with passthroughed VFs, > >>> dynamic host mediation is required to (1) get device states, (2) get > >>> dirty pages. Since device states as well as other critical information > >>> required for dirty page tracking for VFs are usually retrieved from > >>> PFs, > >>> it is handy to provide an extension in PF driver to centralizingly > >>> control > >>> VFs' migration. > >>> > >>> Therefore, in order to realize (1) passthrough VFs at normal time, (2) > >>> dynamically trap VFs' bars for dirty page tracking and > >> A silly question, what's the reason for doing this, is this a must for > >> dirty > >> page tracking? > >> > > For performance consideration. VFs' bars should be passthoughed at > > normal time and only enter into trap state on need. > Right, but how does this matter for the case of dirty page tracking? > > >>> Take NIC as an example, to trap its VF dirty pages, software way is > >>> required to trap every write of ring tail that resides in BAR0. > >> > >> Interesting, but it looks like we need: > >> - decode the instruction > >> - mediate all access to BAR0 > >> All of which seems a great burden for the VF driver. I wonder whether or > >> not doing interrupt relay and tracking head is better in this case. > >> > > hi Jason > > > > not familiar with the way you mentioned. could you elaborate more? > > > It looks to me that you want to intercept the bar that contains the > head. Then you can figure out the buffers submitted from driver and you > still need to decide a proper time to mark them as dirty. > Not need to be accurate, right? just a superset of real dirty bitmap is enough. > What I meant is, intercept the interrupt, then you can figure still > figure out the buffers which has been modified by the device and make > them as dirty. > > Then there's no need to trap BAR and do decoding/emulation etc. > > But it will still be tricky to be correct... > intercept the interrupt is a little hard if post interrupt is enabled.. I think what you worried about here is the timing to mark dirty pages, right? upon interrupt receiving, you regard DMAs are finished and safe to make them dirty. But with BAR trap way, we at least can keep those dirtied pages as dirty until device stop. Of course we have other methods to optimize it. > > >>>There's > >>> still no IOMMU Dirty bit available. > >>> (3) centralizing > >>> VF critical states retrieving and VF controls into one driver, we > >>> propose > >>> to introduce mediate ops on top of current vfio-pci device driver. > >>> > >>> > >>>_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ > >>> _ _ > >>> __ register mediate ops| ___ ___ > >>> | > >>> | |<---| VF| | | > >>> | vfio-pci | | | mediate | | PF driver | | > >>> |__|--->| driver | |___| > >>> |open(pdev) | --- | > >>> | > >>> || > >>> ||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ > >>> _ _| > >>> \|/ \|/ > >>> --- > >>> |VF | |PF| > >>> --- > >>> > >>> > >>> VF mediate driver could be a standalone driver that does not bind to > >>> any devices (as in demo code in patches 5-6) or it could be a built-in > >>> extension of PF driver (as in patches 7-9) . > >>> > >>> Rather than directly bind to VF, VF mediate driver register a mediate > >>> ops into vfio-pci in driver init. vfio-pci maintains a list of such > >>> mediate ops. > >>> (Note that: VF mediate driver can register mediate ops into vfio-pci > >>> before vfio-pci binding to any devices. And VF mediate driver can > >>> support mediating multiple devices.) > >>> > >>> When opening a device (e.g. a VF), vfio-pci goes through the mediate > >>> ops > >>> list and calls each
Re: [libvirt] [RFC PATCH 0/9] Introduce mediate ops in vfio-pci
On 2019/12/7 上午1:42, Alex Williamson wrote: On Fri, 6 Dec 2019 17:40:02 +0800 Jason Wang wrote: On 2019/12/6 下午4:22, Yan Zhao wrote: On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote: On 2019/12/5 下午4:51, Yan Zhao wrote: On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote: Hi: On 2019/12/5 上午11:24, Yan Zhao wrote: For SRIOV devices, VFs are passthroughed into guest directly without host driver mediation. However, when VMs migrating with passthroughed VFs, dynamic host mediation is required to (1) get device states, (2) get dirty pages. Since device states as well as other critical information required for dirty page tracking for VFs are usually retrieved from PFs, it is handy to provide an extension in PF driver to centralizingly control VFs' migration. Therefore, in order to realize (1) passthrough VFs at normal time, (2) dynamically trap VFs' bars for dirty page tracking and A silly question, what's the reason for doing this, is this a must for dirty page tracking? For performance consideration. VFs' bars should be passthoughed at normal time and only enter into trap state on need. Right, but how does this matter for the case of dirty page tracking? Take NIC as an example, to trap its VF dirty pages, software way is required to trap every write of ring tail that resides in BAR0. Interesting, but it looks like we need: - decode the instruction - mediate all access to BAR0 All of which seems a great burden for the VF driver. I wonder whether or not doing interrupt relay and tracking head is better in this case. This sounds like a NIC specific solution, I believe the goal here is to allow any device type to implement a partial mediation solution, in this case to sufficiently track the device while in the migration saving state. I suspect there's a solution that can work for any device type. E.g for virtio, avail index (head) doesn't belongs to any BAR and device may decide to disable doorbell from guest. So did interrupt relay since driver may choose to disable interrupt from device. In this case, the only way to track dirty pages correctly is to switch to software datapath. There's still no IOMMU Dirty bit available. (3) centralizing VF critical states retrieving and VF controls into one driver, we propose to introduce mediate ops on top of current vfio-pci device driver. _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ __ register mediate ops| ___ ___| | |<---| VF| | | | vfio-pci | | | mediate | | PF driver | | |__|--->| driver | |___| |open(pdev) | --- | | || ||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _| \|/ \|/ --- |VF | |PF| --- VF mediate driver could be a standalone driver that does not bind to any devices (as in demo code in patches 5-6) or it could be a built-in extension of PF driver (as in patches 7-9) . Rather than directly bind to VF, VF mediate driver register a mediate ops into vfio-pci in driver init. vfio-pci maintains a list of such mediate ops. (Note that: VF mediate driver can register mediate ops into vfio-pci before vfio-pci binding to any devices. And VF mediate driver can support mediating multiple devices.) When opening a device (e.g. a VF), vfio-pci goes through the mediate ops list and calls each vfio_pci_mediate_ops->open() with pdev of the opening device as a parameter. VF mediate driver should return success or failure depending on it supports the pdev or not. E.g. VF mediate driver would compare its supported VF devfn with the devfn of the passed-in pdev. Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will stop querying other mediate ops and bind the opening device with this mediate ops using the returned mediate handle. Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the VF will be intercepted into VF mediate driver as vfio_pci_mediate_ops->get_region_info(), vfio_pci_mediate_ops->rw, vfio_pci_mediate_ops->mmap, and get customized. For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will further return 'pt' to indicate whether vfio-pci should further passthrough data to hw. when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release() with a mediate handle as parameter. The mediate handle returned from vfio_pci_mediate_ops->open() lets VF mediate driver be able to differentiate two opening VFs of the same device id and vendor id. When VF mediate driver exits, it unregisters its mediate ops
Re: [libvirt] [RFC PATCH 0/9] Introduce mediate ops in vfio-pci
On 2019/12/6 下午8:49, Yan Zhao wrote: On Fri, Dec 06, 2019 at 05:40:02PM +0800, Jason Wang wrote: On 2019/12/6 下午4:22, Yan Zhao wrote: On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote: On 2019/12/5 下午4:51, Yan Zhao wrote: On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote: Hi: On 2019/12/5 上午11:24, Yan Zhao wrote: For SRIOV devices, VFs are passthroughed into guest directly without host driver mediation. However, when VMs migrating with passthroughed VFs, dynamic host mediation is required to (1) get device states, (2) get dirty pages. Since device states as well as other critical information required for dirty page tracking for VFs are usually retrieved from PFs, it is handy to provide an extension in PF driver to centralizingly control VFs' migration. Therefore, in order to realize (1) passthrough VFs at normal time, (2) dynamically trap VFs' bars for dirty page tracking and A silly question, what's the reason for doing this, is this a must for dirty page tracking? For performance consideration. VFs' bars should be passthoughed at normal time and only enter into trap state on need. Right, but how does this matter for the case of dirty page tracking? Take NIC as an example, to trap its VF dirty pages, software way is required to trap every write of ring tail that resides in BAR0. Interesting, but it looks like we need: - decode the instruction - mediate all access to BAR0 All of which seems a great burden for the VF driver. I wonder whether or not doing interrupt relay and tracking head is better in this case. hi Jason not familiar with the way you mentioned. could you elaborate more? It looks to me that you want to intercept the bar that contains the head. Then you can figure out the buffers submitted from driver and you still need to decide a proper time to mark them as dirty. What I meant is, intercept the interrupt, then you can figure still figure out the buffers which has been modified by the device and make them as dirty. Then there's no need to trap BAR and do decoding/emulation etc. But it will still be tricky to be correct... There's still no IOMMU Dirty bit available. (3) centralizing VF critical states retrieving and VF controls into one driver, we propose to introduce mediate ops on top of current vfio-pci device driver. _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ __ register mediate ops| ___ ___| | |<---| VF| | | | vfio-pci | | | mediate | | PF driver | | |__|--->| driver | |___| |open(pdev) | --- | | || ||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _| \|/ \|/ --- |VF | |PF| --- VF mediate driver could be a standalone driver that does not bind to any devices (as in demo code in patches 5-6) or it could be a built-in extension of PF driver (as in patches 7-9) . Rather than directly bind to VF, VF mediate driver register a mediate ops into vfio-pci in driver init. vfio-pci maintains a list of such mediate ops. (Note that: VF mediate driver can register mediate ops into vfio-pci before vfio-pci binding to any devices. And VF mediate driver can support mediating multiple devices.) When opening a device (e.g. a VF), vfio-pci goes through the mediate ops list and calls each vfio_pci_mediate_ops->open() with pdev of the opening device as a parameter. VF mediate driver should return success or failure depending on it supports the pdev or not. E.g. VF mediate driver would compare its supported VF devfn with the devfn of the passed-in pdev. Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will stop querying other mediate ops and bind the opening device with this mediate ops using the returned mediate handle. Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the VF will be intercepted into VF mediate driver as vfio_pci_mediate_ops->get_region_info(), vfio_pci_mediate_ops->rw, vfio_pci_mediate_ops->mmap, and get customized. For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will further return 'pt' to indicate whether vfio-pci should further passthrough data to hw. when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release() with a mediate handle as parameter. The mediate handle returned from vfio_pci_mediate_ops->open() lets VF mediate driver be able to differentiate two opening VFs of the same device id and vendor id. When VF mediate driver exits, it unregisters its mediate ops from vfio-pci. In this patchset, we
Re: [libvirt] [RFC PATCH 4/9] vfio-pci: register default dynamic-trap-bar-info region
On Thu, Dec 12, 2019 at 11:07:42AM +0800, Alex Williamson wrote: > On Wed, 11 Dec 2019 21:02:40 -0500 > Yan Zhao wrote: > > > On Thu, Dec 12, 2019 at 02:56:55AM +0800, Alex Williamson wrote: > > > On Wed, 11 Dec 2019 01:25:55 -0500 > > > Yan Zhao wrote: > > > > > > > On Wed, Dec 11, 2019 at 12:38:05AM +0800, Alex Williamson wrote: > > > > > On Tue, 10 Dec 2019 02:44:44 -0500 > > > > > Yan Zhao wrote: > > > > > > > > > > > On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: > > > > > > > On Mon, 9 Dec 2019 01:22:12 -0500 > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson > > > > > > > > wrote: > > > > > > > > > On Fri, 6 Dec 2019 01:04:07 -0500 > > > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > > > > > On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson > > > > > > > > > > wrote: > > > > > > > > > > > On Wed, 4 Dec 2019 22:26:50 -0500 > > > > > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > > > > > > > > > Dynamic trap bar info region is a channel for QEMU and > > > > > > > > > > > > vendor driver to > > > > > > > > > > > > communicate dynamic trap info. It is of type > > > > > > > > > > > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > > > > > > > > > > > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > > > > > > > > > > > > > > > > > > > > > > > This region has two fields: dt_fd and trap. > > > > > > > > > > > > When QEMU detects a device regions of this type, it > > > > > > > > > > > > will create an > > > > > > > > > > > > eventfd and write its eventfd id to dt_fd field. > > > > > > > > > > > > When vendor drivre signals this eventfd, QEMU reads > > > > > > > > > > > > trap field of this > > > > > > > > > > > > info region. > > > > > > > > > > > > - If trap is true, QEMU would search the device's PCI > > > > > > > > > > > > BAR > > > > > > > > > > > > regions and disable all the sparse mmaped subregions > > > > > > > > > > > > (if the sparse > > > > > > > > > > > > mmaped subregion is disablable). > > > > > > > > > > > > - If trap is false, QEMU would re-enable those > > > > > > > > > > > > subregions. > > > > > > > > > > > > > > > > > > > > > > > > A typical usage is > > > > > > > > > > > > 1. vendor driver first cuts its bar 0 into several > > > > > > > > > > > > sections, all in a > > > > > > > > > > > > sparse mmap array. So initally, all its bar 0 are > > > > > > > > > > > > passthroughed. > > > > > > > > > > > > 2. vendor driver specifys part of bar 0 sections to be > > > > > > > > > > > > disablable. > > > > > > > > > > > > 3. on migration starts, vendor driver signals dt_fd and > > > > > > > > > > > > set trap to true > > > > > > > > > > > > to notify QEMU disabling the bar 0 sections of > > > > > > > > > > > > disablable flags on. > > > > > > > > > > > > 4. QEMU disables those bar 0 section and hence let > > > > > > > > > > > > vendor driver be able > > > > > > > > > > > > to trap access of bar 0 registers and make dirty page > > > > > > > > > > > > tracking possible. > > > > > > > > > > > > 5. on migration failure, vendor driver signals dt_fd to > > > > > > > > > > > > QEMU again. > > > > > > > > > > > > QEMU reads trap field of this info region which is > > > > > > > > > > > > false and QEMU > > > > > > > > > > > > re-passthrough the whole bar 0 region. > > > > > > > > > > > > > > > > > > > > > > > > Vendor driver specifies whether it supports > > > > > > > > > > > > dynamic-trap-bar-info region > > > > > > > > > > > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > > > > > > > > > > > > vfio_pci_mediate_ops->open(). > > > > > > > > > > > > > > > > > > > > > > > > If vfio-pci detects this cap, it will create a default > > > > > > > > > > > > dynamic_trap_bar_info region on behalf of vendor driver > > > > > > > > > > > > with region len=0 > > > > > > > > > > > > and region->ops=null. > > > > > > > > > > > > Vvendor driver should override this region's len, > > > > > > > > > > > > flags, rw, mmap in its > > > > > > > > > > > > vfio_pci_mediate_ops. > > > > > > > > > > > > > > > > > > > > > > TBH, I don't like this interface at all. Userspace > > > > > > > > > > > doesn't pass data > > > > > > > > > > > to the kernel via INFO ioctls. We have a SET_IRQS ioctl > > > > > > > > > > > for > > > > > > > > > > > configuring user signaling with eventfds. I think we > > > > > > > > > > > only need to > > > > > > > > > > > define an IRQ type that tells the user to re-evaluate the > > > > > > > > > > > sparse mmap > > > > > > > > > > > information for a region. The user would enumerate the > > > > > > > > > > > device IRQs via > > > > > > > > > > > GET_IRQ_INFO, find one of this type where the IRQ info > > > > > > > > > > > would also > > > > > > > > > > > indicate which region(s) should be re-evaluated on > > > > > > > > > > > signaling. The user > > > > > > > > > > > would
Re: [libvirt] [RFC PATCH 4/9] vfio-pci: register default dynamic-trap-bar-info region
On Wed, 11 Dec 2019 21:02:40 -0500 Yan Zhao wrote: > On Thu, Dec 12, 2019 at 02:56:55AM +0800, Alex Williamson wrote: > > On Wed, 11 Dec 2019 01:25:55 -0500 > > Yan Zhao wrote: > > > > > On Wed, Dec 11, 2019 at 12:38:05AM +0800, Alex Williamson wrote: > > > > On Tue, 10 Dec 2019 02:44:44 -0500 > > > > Yan Zhao wrote: > > > > > > > > > On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: > > > > > > On Mon, 9 Dec 2019 01:22:12 -0500 > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote: > > > > > > > > > > > > > > > On Fri, 6 Dec 2019 01:04:07 -0500 > > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > > > On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson > > > > > > > > > wrote: > > > > > > > > > > On Wed, 4 Dec 2019 22:26:50 -0500 > > > > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > > > > > > > Dynamic trap bar info region is a channel for QEMU and > > > > > > > > > > > vendor driver to > > > > > > > > > > > communicate dynamic trap info. It is of type > > > > > > > > > > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > > > > > > > > > > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > > > > > > > > > > > > > > > > > > > > > This region has two fields: dt_fd and trap. > > > > > > > > > > > When QEMU detects a device regions of this type, it will > > > > > > > > > > > create an > > > > > > > > > > > eventfd and write its eventfd id to dt_fd field. > > > > > > > > > > > When vendor drivre signals this eventfd, QEMU reads trap > > > > > > > > > > > field of this > > > > > > > > > > > info region. > > > > > > > > > > > - If trap is true, QEMU would search the device's PCI BAR > > > > > > > > > > > regions and disable all the sparse mmaped subregions (if > > > > > > > > > > > the sparse > > > > > > > > > > > mmaped subregion is disablable). > > > > > > > > > > > - If trap is false, QEMU would re-enable those subregions. > > > > > > > > > > > > > > > > > > > > > > A typical usage is > > > > > > > > > > > 1. vendor driver first cuts its bar 0 into several > > > > > > > > > > > sections, all in a > > > > > > > > > > > sparse mmap array. So initally, all its bar 0 are > > > > > > > > > > > passthroughed. > > > > > > > > > > > 2. vendor driver specifys part of bar 0 sections to be > > > > > > > > > > > disablable. > > > > > > > > > > > 3. on migration starts, vendor driver signals dt_fd and > > > > > > > > > > > set trap to true > > > > > > > > > > > to notify QEMU disabling the bar 0 sections of disablable > > > > > > > > > > > flags on. > > > > > > > > > > > 4. QEMU disables those bar 0 section and hence let vendor > > > > > > > > > > > driver be able > > > > > > > > > > > to trap access of bar 0 registers and make dirty page > > > > > > > > > > > tracking possible. > > > > > > > > > > > 5. on migration failure, vendor driver signals dt_fd to > > > > > > > > > > > QEMU again. > > > > > > > > > > > QEMU reads trap field of this info region which is false > > > > > > > > > > > and QEMU > > > > > > > > > > > re-passthrough the whole bar 0 region. > > > > > > > > > > > > > > > > > > > > > > Vendor driver specifies whether it supports > > > > > > > > > > > dynamic-trap-bar-info region > > > > > > > > > > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > > > > > > > > > > > vfio_pci_mediate_ops->open(). > > > > > > > > > > > > > > > > > > > > > > If vfio-pci detects this cap, it will create a default > > > > > > > > > > > dynamic_trap_bar_info region on behalf of vendor driver > > > > > > > > > > > with region len=0 > > > > > > > > > > > and region->ops=null. > > > > > > > > > > > Vvendor driver should override this region's len, flags, > > > > > > > > > > > rw, mmap in its > > > > > > > > > > > vfio_pci_mediate_ops. > > > > > > > > > > > > > > > > > > > > TBH, I don't like this interface at all. Userspace doesn't > > > > > > > > > > pass data > > > > > > > > > > to the kernel via INFO ioctls. We have a SET_IRQS ioctl for > > > > > > > > > > configuring user signaling with eventfds. I think we only > > > > > > > > > > need to > > > > > > > > > > define an IRQ type that tells the user to re-evaluate the > > > > > > > > > > sparse mmap > > > > > > > > > > information for a region. The user would enumerate the > > > > > > > > > > device IRQs via > > > > > > > > > > GET_IRQ_INFO, find one of this type where the IRQ info > > > > > > > > > > would also > > > > > > > > > > indicate which region(s) should be re-evaluated on > > > > > > > > > > signaling. The user > > > > > > > > > > would enable that signaling via SET_IRQS and simply > > > > > > > > > > re-evaluate the > > > > > > > > > ok. I'll try to switch to this way. Thanks for this > > > > > > > > > suggestion. > > > > > > > > > > > > > > > > > > > sparse mmap capability for the associated regions when > > > > > > > > > >
[libvirt] [PATCH RESEND 3/4] cputest: Add CPUID data for Hygon Dhyana 7185 32-core Processor
Add Hygon Dhyana CPU data test case related files.
Signed-off-by: Yingle Hou
---
tests/cputest.c|1 +
...86_64-cpuid-Hygon-C86-7185-32-core-disabled.xml |7 +
...x86_64-cpuid-Hygon-C86-7185-32-core-enabled.xml |9 +
.../x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml | 16 +
.../x86_64-cpuid-Hygon-C86-7185-32-core-host.xml | 17 +
.../x86_64-cpuid-Hygon-C86-7185-32-core-json.xml | 12 +
.../x86_64-cpuid-Hygon-C86-7185-32-core.json | 1631
.../x86_64-cpuid-Hygon-C86-7185-32-core.sig|4 +
.../x86_64-cpuid-Hygon-C86-7185-32-core.xml| 54 +
9 files changed, 1751 insertions(+)
create mode 100644
tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-disabled.xml
create mode 100644
tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-enabled.xml
create mode 100644
tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml
create mode 100644
tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml
create mode 100644
tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml
create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.json
create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.sig
create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.xml
diff --git a/tests/cputest.c b/tests/cputest.c
index fd86344..b44b184 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -1238,6 +1238,7 @@ mymain(void)
DO_TEST_CPUID(VIR_ARCH_X86_64, "Core-i7-8700", JSON_MODELS);
DO_TEST_CPUID(VIR_ARCH_X86_64, "Core2-E6850", JSON_HOST);
DO_TEST_CPUID(VIR_ARCH_X86_64, "Core2-Q9500", JSON_NONE);
+DO_TEST_CPUID(VIR_ARCH_X86_64, "Hygon-C86-7185-32-core", JSON_HOST);
DO_TEST_CPUID(VIR_ARCH_X86_64, "EPYC-7601-32-Core", JSON_HOST);
DO_TEST_CPUID(VIR_ARCH_X86_64, "EPYC-7601-32-Core-ibpb",
JSON_MODELS_REQUIRED);
DO_TEST_CPUID(VIR_ARCH_X86_64, "FX-8150", JSON_NONE);
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-disabled.xml
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-disabled.xml
new file mode 100644
index 000..747d725
--- /dev/null
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-disabled.xml
@@ -0,0 +1,7 @@
+
+
+
+
+
+
+
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-enabled.xml
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-enabled.xml
new file mode 100644
index 000..fcefcf7
--- /dev/null
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-enabled.xml
@@ -0,0 +1,9 @@
+
+
+
+
+
+
+
+
+
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml
new file mode 100644
index 000..c2541b3
--- /dev/null
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml
@@ -0,0 +1,16 @@
+
+ Dhyana
+ Hygon
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml
new file mode 100644
index 000..5199708
--- /dev/null
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml
@@ -0,0 +1,17 @@
+
+ x86_64
+ Dhyana
+ Hygon
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml
new file mode 100644
index 000..d3003b6
--- /dev/null
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml
@@ -0,0 +1,12 @@
+
+ Dhyana
+ Hygon
+
+
+
+
+
+
+
+
+
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.json
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.json
new file mode 100644
index 000..1d06d05
--- /dev/null
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.json
@@ -0,0 +1,1631 @@
+{
+ "return": {
+"model": {
+ "name": "base",
+ "props": {
+"phys-bits": 0,
+"core-id": -1,
+"xlevel": 2147483674,
+"cmov": true,
+"ia64": false,
+"ssb-no": false,
+"aes": false,
+"mmx": true,
+"rdpid": false,
+"arat": true,
+"gfni": false,
+"ibrs-all": false,
+"pause-filter": false,
+"xsavec": true,
+"intel-pt": false,
+"hv-frequencies": false,
+"tsc-frequency": 0,
+"xd": true,
+"x-intel-pt-auto-level": true,
+"hv-vendor-id": "",
+"kvm-asyncpf": true,
+"kvm_asyncpf": true,
+"perfctr_core": false,
+"perfctr-core": false,
+"mpx": false,
+"pbe": false,
+"decodeassists": false,
+"avx512cd": false,
+"sse4_1": true,
+"sse4.1": true,
+"sse4-1": true,
+"family": 24,
+"legacy-cache": true,
+"host-phys-bits-limit": 0,
+
[libvirt] [PATCH RESEND 4/4] domaincapstest: Add CPU model Dhyana to QEMU
Add CPU model Dhyana to QEMU 4.1.0 and QEMU 4.2.0 in tests/domaincapstest/. Signed-off-by: Yingle Hou --- tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.1.0.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_4.2.0.x86_64.xml | 1 + 6 files changed, 6 insertions(+) diff --git a/tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml b/tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml index 6363aa4..f4ddb66 100644 --- a/tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml +++ b/tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml @@ -87,6 +87,7 @@ Haswell EPYC-IBPB EPYC + Dhyana Conroe Cascadelake-Server Broadwell-noTSX-IBRS diff --git a/tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml b/tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml index b6168e6..5bfd065 100644 --- a/tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml +++ b/tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml @@ -97,6 +97,7 @@ Haswell EPYC-IBPB EPYC + Dhyana Conroe Cascadelake-Server Broadwell-noTSX-IBRS diff --git a/tests/domaincapsdata/qemu_4.1.0.x86_64.xml b/tests/domaincapsdata/qemu_4.1.0.x86_64.xml index 54cb76e..bcc8bbc 100644 --- a/tests/domaincapsdata/qemu_4.1.0.x86_64.xml +++ b/tests/domaincapsdata/qemu_4.1.0.x86_64.xml @@ -86,6 +86,7 @@ Haswell EPYC-IBPB EPYC + Dhyana Conroe Cascadelake-Server Broadwell-noTSX-IBRS diff --git a/tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml b/tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml index 0842a75..c4c6bfb 100644 --- a/tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml +++ b/tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml @@ -87,6 +87,7 @@ Haswell EPYC-IBPB EPYC + Dhyana Conroe Cascadelake-Server Broadwell-noTSX-IBRS diff --git a/tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml b/tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml index c415535..a7f8d9c 100644 --- a/tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml +++ b/tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml @@ -97,6 +97,7 @@ Haswell EPYC-IBPB EPYC + Dhyana Conroe Cascadelake-Server Broadwell-noTSX-IBRS diff --git a/tests/domaincapsdata/qemu_4.2.0.x86_64.xml b/tests/domaincapsdata/qemu_4.2.0.x86_64.xml index 212e0a5..f0e0f18 100644 --- a/tests/domaincapsdata/qemu_4.2.0.x86_64.xml +++ b/tests/domaincapsdata/qemu_4.2.0.x86_64.xml @@ -86,6 +86,7 @@ Haswell EPYC-IBPB EPYC + Dhyana Conroe Cascadelake-Server Broadwell-noTSX-IBRS -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RESEND 1/4] cpu: Remove the verification conditions of the model in the x86 signatures
The x86ModelParseSignatures function makes an assumption that CPU signature
model equals 0 as an invalid case. While in Hygon processor definition, A1
version (model 0, stepping 1) is mass production version, to support Hygon
Dhyana A1 version, we have removed CPU signature model zero checking condition.
Signed-off-by: Yingle Hou
---
src/cpu/cpu_x86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 1e913cc..9b7981d 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1418,7 +1418,7 @@ x86ModelParseSignatures(virCPUx86ModelPtr model,
}
rc = virXPathUInt("string(@model)", ctxt, );
-if (rc < 0 || sigModel == 0) {
+if (rc < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid CPU signature model in model %s"),
model->name);
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RESEND 0/4] Add support for Hygon Dhyana CPU
As a Joint Venture between AMD and Haiguang Information Technology Co., Ltd., Hygon aims to provide x86 server processor in China market. The first generation processor Dhyana (family 18h) shares similar architecture with AMD family 17h. As Dhyana support in QEMU already have been merged in qemu-4.1.0 [1], to add Dhyana support in libvirt, we have added a new Dhyana CPU model file x86_Dhyana.xml in cpu_map directory, and also we have added a series of Dhyana test files. We have tested the patches on Hygon Dhyana machine with the result that it has successfully worked as expected. Reference: [1] https://patchwork.kernel.org/patch/10902889/ Yingle Hou (4): cpu: Remove the verification conditions of the model in the x86 signatures cpu: Add new Dhyana CPU model cputest: Add CPUID data for Hygon Dhyana 7185 32-core Processor domaincapstest: Add CPU model Dhyana to QEMU src/cpu/cpu_x86.c |2 +- src/cpu_map/Makefile.inc.am|1 + src/cpu_map/index.xml |3 + src/cpu_map/x86_Dhyana.xml | 70 + src/cpu_map/x86_vendors.xml|1 + tests/cputest.c|1 + ...86_64-cpuid-Hygon-C86-7185-32-core-disabled.xml |7 + ...x86_64-cpuid-Hygon-C86-7185-32-core-enabled.xml |9 + .../x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml | 16 + .../x86_64-cpuid-Hygon-C86-7185-32-core-host.xml | 17 + .../x86_64-cpuid-Hygon-C86-7185-32-core-json.xml | 12 + .../x86_64-cpuid-Hygon-C86-7185-32-core.json | 1631 .../x86_64-cpuid-Hygon-C86-7185-32-core.sig|4 + .../x86_64-cpuid-Hygon-C86-7185-32-core.xml| 54 + tests/domaincapsdata/qemu_4.1.0-q35.x86_64.xml |1 + tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml |1 + tests/domaincapsdata/qemu_4.1.0.x86_64.xml |1 + tests/domaincapsdata/qemu_4.2.0-q35.x86_64.xml |1 + tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml |1 + tests/domaincapsdata/qemu_4.2.0.x86_64.xml |1 + 20 files changed, 1833 insertions(+), 1 deletion(-) create mode 100644 src/cpu_map/x86_Dhyana.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-disabled.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-enabled.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.json create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.sig create mode 100644 tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core.xml -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RESEND 2/4] cpu: Add new Dhyana CPU model
Add Hygon Dhyana CPU model to the processor model. Signed-off-by: Yingle Hou --- src/cpu_map/Makefile.inc.am | 1 + src/cpu_map/index.xml | 3 ++ src/cpu_map/x86_Dhyana.xml | 70 + src/cpu_map/x86_vendors.xml | 1 + 4 files changed, 75 insertions(+) create mode 100644 src/cpu_map/x86_Dhyana.xml diff --git a/src/cpu_map/Makefile.inc.am b/src/cpu_map/Makefile.inc.am index 7eb86c8..e935178 100644 --- a/src/cpu_map/Makefile.inc.am +++ b/src/cpu_map/Makefile.inc.am @@ -25,6 +25,7 @@ cpumap_DATA = \ cpu_map/x86_coreduo.xml \ cpu_map/x86_cpu64-rhel5.xml \ cpu_map/x86_cpu64-rhel6.xml \ + cpu_map/x86_Dhyana.xml \ cpu_map/x86_EPYC.xml \ cpu_map/x86_EPYC-IBPB.xml \ cpu_map/x86_Haswell.xml \ diff --git a/src/cpu_map/index.xml b/src/cpu_map/index.xml index ed45083..ffb2f6f 100644 --- a/src/cpu_map/index.xml +++ b/src/cpu_map/index.xml @@ -60,6 +60,9 @@ + + + diff --git a/src/cpu_map/x86_Dhyana.xml b/src/cpu_map/x86_Dhyana.xml new file mode 100644 index 000..cbc8020 --- /dev/null +++ b/src/cpu_map/x86_Dhyana.xml @@ -0,0 +1,70 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/cpu_map/x86_vendors.xml b/src/cpu_map/x86_vendors.xml index 418712a..840179d 100644 --- a/src/cpu_map/x86_vendors.xml +++ b/src/cpu_map/x86_vendors.xml @@ -1,4 +1,5 @@ + -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 4/9] vfio-pci: register default dynamic-trap-bar-info region
On Thu, Dec 12, 2019 at 02:56:55AM +0800, Alex Williamson wrote: > On Wed, 11 Dec 2019 01:25:55 -0500 > Yan Zhao wrote: > > > On Wed, Dec 11, 2019 at 12:38:05AM +0800, Alex Williamson wrote: > > > On Tue, 10 Dec 2019 02:44:44 -0500 > > > Yan Zhao wrote: > > > > > > > On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: > > > > > On Mon, 9 Dec 2019 01:22:12 -0500 > > > > > Yan Zhao wrote: > > > > > > > > > > > On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote: > > > > > > > On Fri, 6 Dec 2019 01:04:07 -0500 > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson > > > > > > > > wrote: > > > > > > > > > On Wed, 4 Dec 2019 22:26:50 -0500 > > > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > > > > > Dynamic trap bar info region is a channel for QEMU and > > > > > > > > > > vendor driver to > > > > > > > > > > communicate dynamic trap info. It is of type > > > > > > > > > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > > > > > > > > > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > > > > > > > > > > > > > > > > > > > This region has two fields: dt_fd and trap. > > > > > > > > > > When QEMU detects a device regions of this type, it will > > > > > > > > > > create an > > > > > > > > > > eventfd and write its eventfd id to dt_fd field. > > > > > > > > > > When vendor drivre signals this eventfd, QEMU reads trap > > > > > > > > > > field of this > > > > > > > > > > info region. > > > > > > > > > > - If trap is true, QEMU would search the device's PCI BAR > > > > > > > > > > regions and disable all the sparse mmaped subregions (if > > > > > > > > > > the sparse > > > > > > > > > > mmaped subregion is disablable). > > > > > > > > > > - If trap is false, QEMU would re-enable those subregions. > > > > > > > > > > > > > > > > > > > > A typical usage is > > > > > > > > > > 1. vendor driver first cuts its bar 0 into several > > > > > > > > > > sections, all in a > > > > > > > > > > sparse mmap array. So initally, all its bar 0 are > > > > > > > > > > passthroughed. > > > > > > > > > > 2. vendor driver specifys part of bar 0 sections to be > > > > > > > > > > disablable. > > > > > > > > > > 3. on migration starts, vendor driver signals dt_fd and set > > > > > > > > > > trap to true > > > > > > > > > > to notify QEMU disabling the bar 0 sections of disablable > > > > > > > > > > flags on. > > > > > > > > > > 4. QEMU disables those bar 0 section and hence let vendor > > > > > > > > > > driver be able > > > > > > > > > > to trap access of bar 0 registers and make dirty page > > > > > > > > > > tracking possible. > > > > > > > > > > 5. on migration failure, vendor driver signals dt_fd to > > > > > > > > > > QEMU again. > > > > > > > > > > QEMU reads trap field of this info region which is false > > > > > > > > > > and QEMU > > > > > > > > > > re-passthrough the whole bar 0 region. > > > > > > > > > > > > > > > > > > > > Vendor driver specifies whether it supports > > > > > > > > > > dynamic-trap-bar-info region > > > > > > > > > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > > > > > > > > > > vfio_pci_mediate_ops->open(). > > > > > > > > > > > > > > > > > > > > If vfio-pci detects this cap, it will create a default > > > > > > > > > > dynamic_trap_bar_info region on behalf of vendor driver > > > > > > > > > > with region len=0 > > > > > > > > > > and region->ops=null. > > > > > > > > > > Vvendor driver should override this region's len, flags, > > > > > > > > > > rw, mmap in its > > > > > > > > > > vfio_pci_mediate_ops. > > > > > > > > > > > > > > > > > > TBH, I don't like this interface at all. Userspace doesn't > > > > > > > > > pass data > > > > > > > > > to the kernel via INFO ioctls. We have a SET_IRQS ioctl for > > > > > > > > > configuring user signaling with eventfds. I think we only > > > > > > > > > need to > > > > > > > > > define an IRQ type that tells the user to re-evaluate the > > > > > > > > > sparse mmap > > > > > > > > > information for a region. The user would enumerate the > > > > > > > > > device IRQs via > > > > > > > > > GET_IRQ_INFO, find one of this type where the IRQ info would > > > > > > > > > also > > > > > > > > > indicate which region(s) should be re-evaluated on signaling. > > > > > > > > > The user > > > > > > > > > would enable that signaling via SET_IRQS and simply > > > > > > > > > re-evaluate the > > > > > > > > ok. I'll try to switch to this way. Thanks for this suggestion. > > > > > > > > > > > > > > > > > sparse mmap capability for the associated regions when > > > > > > > > > signaled. > > > > > > > > > > > > > > > > Do you like the "disablable" flag of sparse mmap ? > > > > > > > > I think it's a lightweight way for user to switch mmap state of > > > > > > > > a whole region, > > > > > > > > otherwise going through a complete flow of GET_REGION_INFO and > > > >
Re: [libvirt] [PATCH v4 2/7] virConnectRegisterCloseCallback: Cleanup 'opaque' if there is no connectRegisterCloseCallback
On 11/14/19 12:44 PM, Marc Hartmayer wrote:
> The commit 'close callback: move it to driver' (88f09b75eb99) moved
> the responsibility for the close callback to the driver. But if the
> driver doesn't support the connectRegisterCloseCallback API this
> function does nothing, even no unsupported error report. This behavior
> may lead to problems, for example memory leaks, as the caller cannot
> differentiate whether the close callback was 'really' registered or
> not.
>
Full context:
v1 subtrhead with jferlan and danpb:
https://www.redhat.com/archives/libvir-list/2018-March/msg00906.html
https://www.redhat.com/archives/libvir-list/2018-April/msg00116.html
v2 subthread with john:
https://www.redhat.com/archives/libvir-list/2018-April/msg02534.html
My first thought is, why not just make this API start raising an error
if it isn't supported?
But you tried that here:
https://www.redhat.com/archives/libvir-list/2018-March/msg00405.html
I'm not really sure I buy the argument that we can't change the
semantics of the API here. This is the only callback API that seems to
not raise an explicit error. It's documented to raise an error. And
there's possible memory leak due the ambiguity.
Yeah I see that virsh needs to be updated to match. In practice virsh
shouldn't be a problem: this issue will only hit for local drivers, and
virsh and client library will be updated together for that case.
In theory if a python app is using this API and not expecting an
exception, it could cause a regression. But it's also informing them
that, hey, that connection callback you requested wasn't working in the
first place. So it's arguably a correctness issue.
So IMO we should be able to adjust this to return a proper error.
BUT, if we stick with this direction, then we need to extend the doc
text here to describe all of this:
* Returns -1 if the driver can support close callback, but registering
one failed. User must free opaque?
* Returns 0 if the driver does not support close callback. We will free
data for you
* Returns 0 if the driver successfully registered a close callback. When
that callback is triggered, opaque will be free'd
But that sounds pretty nutty IMO :/
> Therefore call directly @freecb if the connectRegisterCloseCallback
> API is not supported by the driver used by the connection and update
> the documentation.
>
> Signed-off-by: Marc Hartmayer
> ---
> src/libvirt-host.c | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/src/libvirt-host.c b/src/libvirt-host.c
> index 221a1b7a4353..94383ed449a9 100644
> --- a/src/libvirt-host.c
> +++ b/src/libvirt-host.c
> @@ -1398,7 +1398,7 @@ virConnectIsAlive(virConnectPtr conn)
> * @conn: pointer to connection object
> * @cb: callback to invoke upon close
> * @opaque: user data to pass to @cb
> - * @freecb: callback to free @opaque
> + * @freecb: callback to free @opaque when not used anymore
> *
> * Registers a callback to be invoked when the connection
> * is closed. This callback is invoked when there is any
> @@ -1412,7 +1412,9 @@ virConnectIsAlive(virConnectPtr conn)
> *
> * The @freecb must not invoke any other libvirt public
> * APIs, since it is not called from a re-entrant safe
> - * context.
> + * context. Only if virConnectRegisterCloseCallback is
> + * successful, @freecb will be called, otherwise the
> + * caller is responsible to free @opaque.
This reads wrong to me. If cb() is successfully registered, then
freecb() is invoked automatically after cb() is called, right? This
comment makes it sound like freecb() is invoked immediately when
virConnectRegisterCloseCallback returns 0
Hopefully I'm not confusing things more :)
- Cole
> *
> * Returns 0 on success, -1 on error
> */
> @@ -1428,9 +1430,13 @@ virConnectRegisterCloseCallback(virConnectPtr conn,
> virCheckConnectReturn(conn, -1);
> virCheckNonNullArgGoto(cb, error);
>
> -if (conn->driver->connectRegisterCloseCallback &&
> -conn->driver->connectRegisterCloseCallback(conn, cb, opaque, freecb)
> < 0)
> -goto error;
> +if (conn->driver->connectRegisterCloseCallback) {
> +if (conn->driver->connectRegisterCloseCallback(conn, cb, opaque,
> freecb) < 0)
> +goto error;
> +} else {
> +if (freecb)
> +freecb(opaque);
> +}
>
> return 0;
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 6/7] remote: shrink the critical sections
On 11/14/19 12:44 PM, Marc Hartmayer wrote: > To free the structs and save the error, it is not necessary to hold > @priv->lock, > therefore move these parts after the mutex unlock. > > Signed-off-by: Marc Hartmayer > --- > src/remote/remote_daemon_dispatch.c | 32 ++--- > 1 file changed, 16 insertions(+), 16 deletions(-) Reviewed-by: Cole Robinson Do I understand correctly that 1,3-5 are all independent and can be pushed separately? If so I will do that tomorrow. I'm doing some archaeology on patch #2 - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/6] PCI hostdev partial assignment support
On 11/26/19 8:33 AM, Daniel Henrique Barboza wrote: > changes from previous version [1]: > - do not overload address type='none'. A new address type called > 'unassigned' is introduced; > - there is no unassigned' flag being created in virDomainHostdevDef. > The logic added by new address type is enough; > - do not allow function zero of multifunction devices to be > unassigned. > > Nothing too special to discuss in this cover letter. More details > can be found at the discussions of the previous version [1]. Commit > messages of the patches have more background info as well. > > [1] https://www.redhat.com/archives/libvir-list/2019-November/msg01099.html > For 1-4: Reviewed-by: Cole Robinson You asked in the last posting whether to use ADDRESS_TYPE_NONE and add a new hostdev unassigned= attribute, or this approach. I think this is the correct approach. Address type='unassigned' captures the description well, and since address type='none' isn't printed in the XML, it would make XML output a bit more confusing IMO if no address was printed. One comment > Daniel Henrique Barboza (6): > Introducing new address type='unassigned' for PCI hostdevs > qemu: handle unassigned PCI hostdevs in command line and alias Is there a specific reason to skip alias assign, beside it not being needed for the command line? If it doesn't hurt, maybe we just keep it. I don't have a strong argument for it though. If no one says anything I'll leave it as is > virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices > formatdomain.html.in: document For the first 4 I'll give it a few days and push on Friday if no one complains. For the last two: > utils: PCI multifunction detection helpers > domain_conf.c: don't allow function zero to be unassigned The domain_conf.c additions should go into the virDomainHostdevDefValidate. But this validation check seems to actually read from host PCI space. I'm not sure if that's a good idea to do for every XML parse? What's the failure scenario without this error message? Does it fail in an obvious way? If so, maybe it's better to side step the issue and just let it fail if it's a valid configuration. Maybe laine knows better if there's precedent for similar checks at Validate time Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: ensure outputfile is deleted if rst2html/rst2man fail
On 12/11/19 12:59 PM, Daniel P. Berrangé wrote:
On Wed, Dec 11, 2019 at 12:52:20PM -0600, Eric Blake wrote:
On 12/11/19 12:45 PM, Daniel P. Berrangé wrote:
This avoids leaving a zero length or partially generated output
file on errors.
Signed-off-by: Daniel P. Berrangé
---
docs/Makefile.am | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index eb8de80b9c..9a1f7a6117 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -259,21 +259,21 @@ man8_MANS = $(manpages8_rst:%.rst=%.8)
grep -v '^\.\. contents::' < $< | \
sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
-e 's|RUNSTATEDIR|$(runstatedir)|g' | \
- $(RST2MAN) > $@
+ $(RST2MAN) > $@ || { rm $@ && exit 1; }
But still allows a truncated view of the file if another process accesses
the file while RST2MAN is still running. Even better is to generate output
to a temp file then atomically mv it into place, so that no concurrent
process can ever see an incomplete file.
Is that a problem that actually impacts negatively in real world for builds?
In various places in the make rules, we've either done the "|| rm" approach,
or the temp file and rename approach. Personally I find the recipes using
the "|| rm" approach are more maintainable/readable.
If you're trying to browse the docs and build at the same time, such as
checking if your tweak to a doc renders well, you could hit it; but
you're also right that it's not a show-stopper problem. Go with
whatever is easier to maintain if you don't care about the issue of
atomicity in the files (the rm to avoid a long-term corruption is more
important than the atomic mv to avoid a short-term corruption).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: ensure outputfile is deleted if rst2html/rst2man fail
On Wed, Dec 11, 2019 at 12:52:20PM -0600, Eric Blake wrote:
> On 12/11/19 12:45 PM, Daniel P. Berrangé wrote:
> > This avoids leaving a zero length or partially generated output
> > file on errors.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > docs/Makefile.am | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/docs/Makefile.am b/docs/Makefile.am
> > index eb8de80b9c..9a1f7a6117 100644
> > --- a/docs/Makefile.am
> > +++ b/docs/Makefile.am
> > @@ -259,21 +259,21 @@ man8_MANS = $(manpages8_rst:%.rst=%.8)
> >grep -v '^\.\. contents::' < $< | \
> >sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
> >-e 's|RUNSTATEDIR|$(runstatedir)|g' | \
> > - $(RST2MAN) > $@
> > + $(RST2MAN) > $@ || { rm $@ && exit 1; }
>
> But still allows a truncated view of the file if another process accesses
> the file while RST2MAN is still running. Even better is to generate output
> to a temp file then atomically mv it into place, so that no concurrent
> process can ever see an incomplete file.
Is that a problem that actually impacts negatively in real world for builds?
In various places in the make rules, we've either done the "|| rm" approach,
or the temp file and rename approach. Personally I find the recipes using
the "|| rm" approach are more maintainable/readable.
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] [RFC PATCH 4/9] vfio-pci: register default dynamic-trap-bar-info region
On Wed, 11 Dec 2019 01:25:55 -0500 Yan Zhao wrote: > On Wed, Dec 11, 2019 at 12:38:05AM +0800, Alex Williamson wrote: > > On Tue, 10 Dec 2019 02:44:44 -0500 > > Yan Zhao wrote: > > > > > On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: > > > > On Mon, 9 Dec 2019 01:22:12 -0500 > > > > Yan Zhao wrote: > > > > > > > > > On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote: > > > > > > On Fri, 6 Dec 2019 01:04:07 -0500 > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote: > > > > > > > > > > > > > > > On Wed, 4 Dec 2019 22:26:50 -0500 > > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > > > Dynamic trap bar info region is a channel for QEMU and vendor > > > > > > > > > driver to > > > > > > > > > communicate dynamic trap info. It is of type > > > > > > > > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > > > > > > > > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > > > > > > > > > > > > > > > > > This region has two fields: dt_fd and trap. > > > > > > > > > When QEMU detects a device regions of this type, it will > > > > > > > > > create an > > > > > > > > > eventfd and write its eventfd id to dt_fd field. > > > > > > > > > When vendor drivre signals this eventfd, QEMU reads trap > > > > > > > > > field of this > > > > > > > > > info region. > > > > > > > > > - If trap is true, QEMU would search the device's PCI BAR > > > > > > > > > regions and disable all the sparse mmaped subregions (if the > > > > > > > > > sparse > > > > > > > > > mmaped subregion is disablable). > > > > > > > > > - If trap is false, QEMU would re-enable those subregions. > > > > > > > > > > > > > > > > > > A typical usage is > > > > > > > > > 1. vendor driver first cuts its bar 0 into several sections, > > > > > > > > > all in a > > > > > > > > > sparse mmap array. So initally, all its bar 0 are > > > > > > > > > passthroughed. > > > > > > > > > 2. vendor driver specifys part of bar 0 sections to be > > > > > > > > > disablable. > > > > > > > > > 3. on migration starts, vendor driver signals dt_fd and set > > > > > > > > > trap to true > > > > > > > > > to notify QEMU disabling the bar 0 sections of disablable > > > > > > > > > flags on. > > > > > > > > > 4. QEMU disables those bar 0 section and hence let vendor > > > > > > > > > driver be able > > > > > > > > > to trap access of bar 0 registers and make dirty page > > > > > > > > > tracking possible. > > > > > > > > > 5. on migration failure, vendor driver signals dt_fd to QEMU > > > > > > > > > again. > > > > > > > > > QEMU reads trap field of this info region which is false and > > > > > > > > > QEMU > > > > > > > > > re-passthrough the whole bar 0 region. > > > > > > > > > > > > > > > > > > Vendor driver specifies whether it supports > > > > > > > > > dynamic-trap-bar-info region > > > > > > > > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > > > > > > > > > vfio_pci_mediate_ops->open(). > > > > > > > > > > > > > > > > > > If vfio-pci detects this cap, it will create a default > > > > > > > > > dynamic_trap_bar_info region on behalf of vendor driver with > > > > > > > > > region len=0 > > > > > > > > > and region->ops=null. > > > > > > > > > Vvendor driver should override this region's len, flags, rw, > > > > > > > > > mmap in its > > > > > > > > > vfio_pci_mediate_ops. > > > > > > > > > > > > > > > > TBH, I don't like this interface at all. Userspace doesn't > > > > > > > > pass data > > > > > > > > to the kernel via INFO ioctls. We have a SET_IRQS ioctl for > > > > > > > > configuring user signaling with eventfds. I think we only need > > > > > > > > to > > > > > > > > define an IRQ type that tells the user to re-evaluate the > > > > > > > > sparse mmap > > > > > > > > information for a region. The user would enumerate the device > > > > > > > > IRQs via > > > > > > > > GET_IRQ_INFO, find one of this type where the IRQ info would > > > > > > > > also > > > > > > > > indicate which region(s) should be re-evaluated on signaling. > > > > > > > > The user > > > > > > > > would enable that signaling via SET_IRQS and simply re-evaluate > > > > > > > > the > > > > > > > ok. I'll try to switch to this way. Thanks for this suggestion. > > > > > > > > > > > > > > > sparse mmap capability for the associated regions when > > > > > > > > signaled. > > > > > > > > > > > > > > Do you like the "disablable" flag of sparse mmap ? > > > > > > > I think it's a lightweight way for user to switch mmap state of a > > > > > > > whole region, > > > > > > > otherwise going through a complete flow of GET_REGION_INFO and > > > > > > > re-setup > > > > > > > region might be too heavy. > > > > > > > > > > > > No, I don't like the disable-able flag. At what frequency do we > > > > > > expect > > > > > > regions to change? It seems like we'd only change when switching > > > >
Re: [libvirt] [PATCH] docs: ensure outputfile is deleted if rst2html/rst2man fail
On 12/11/19 12:45 PM, Daniel P. Berrangé wrote:
This avoids leaving a zero length or partially generated output
file on errors.
Signed-off-by: Daniel P. Berrangé
---
docs/Makefile.am | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index eb8de80b9c..9a1f7a6117 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -259,21 +259,21 @@ man8_MANS = $(manpages8_rst:%.rst=%.8)
grep -v '^\.\. contents::' < $< | \
sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
-e 's|RUNSTATEDIR|$(runstatedir)|g' | \
- $(RST2MAN) > $@
+ $(RST2MAN) > $@ || { rm $@ && exit 1; }
But still allows a truncated view of the file if another process
accesses the file while RST2MAN is still running. Even better is to
generate output to a temp file then atomically mv it into place, so that
no concurrent process can ever see an incomplete file.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: stop using syntax highlighting for man page code blocks
Some versions of the rst2man convertor are buggy failing to
cope with syntax highlighting in code blocks.
This isn't something we really need for the man page code
blocks, so we can just delete the highlighting directive.
Signed-off-by: Daniel P. Berrangé
---
docs/manpages/libvirtd.rst | 4 +-
docs/manpages/virsh.rst| 576 ++---
docs/manpages/virt-admin.rst | 50 +--
docs/manpages/virt-login-shell.rst | 2 +-
docs/manpages/virtlockd.rst| 4 +-
docs/manpages/virtlogd.rst | 4 +-
6 files changed, 320 insertions(+), 320 deletions(-)
diff --git a/docs/manpages/libvirtd.rst b/docs/manpages/libvirtd.rst
index b7489a57fc..4125604f9a 100644
--- a/docs/manpages/libvirtd.rst
+++ b/docs/manpages/libvirtd.rst
@@ -199,7 +199,7 @@ EXAMPLES
To retrieve the version of libvirtd:
-.. code-block:: shell
+.. code-block::
# libvirtd --version
libvirtd (libvirt) 0.8.2
@@ -207,7 +207,7 @@ To retrieve the version of libvirtd:
To start libvirtd, instructing it to daemonize and create a PID file:
-.. code-block:: shell
+.. code-block::
# libvirtd -d
# ls -la RUNSTATEDIR/libvirtd.pid
diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index e9d6deaee1..e063b76641 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -38,7 +38,7 @@ KVM, LXC, OpenVZ, VirtualBox and VMware ESX.
The basic structure of most virsh usage is:
-.. code-block:: shell
+.. code-block::
virsh [OPTION]... [ARG]...
@@ -214,7 +214,7 @@ help
**Syntax:**
-.. code-block:: shell
+.. code-block::
help [command-or-group]
@@ -228,7 +228,7 @@ group as an option. For example:
**Example 1:**
-.. code-block:: shell
+.. code-block::
virsh # help host
@@ -250,7 +250,7 @@ option instead. For example:
**Example 2:**
-.. code-block:: shell
+.. code-block::
virsh # help list
NAME
@@ -272,7 +272,7 @@ quit, exit
**Syntax:**
-.. code-block:: shell
+.. code-block::
quit
exit
@@ -286,7 +286,7 @@ version
**Syntax:**
-.. code-block:: shell
+.. code-block::
version [--daemon]
@@ -296,7 +296,7 @@ is included in the output.
**Example:**
-.. code-block:: shell
+.. code-block::
$ virsh version
Compiled against library: libvirt 1.2.3
@@ -317,7 +317,7 @@ cd
**Syntax:**
-.. code-block:: shell
+.. code-block::
cd [directory]
@@ -334,7 +334,7 @@ pwd
**Syntax:**
-.. code-block:: shell
+.. code-block::
pwd
@@ -347,7 +347,7 @@ connect
**Syntax:**
-.. code-block:: shell
+.. code-block::
connect [URI] [--readonly]
@@ -388,7 +388,7 @@ uri
**Syntax:**
-.. code-block:: shell
+.. code-block::
uri
@@ -400,7 +400,7 @@ hostname
**Syntax:**
-.. code-block:: shell
+.. code-block::
hostname
@@ -412,7 +412,7 @@ sysinfo
**Syntax:**
-.. code-block:: shell
+.. code-block::
sysinfo
@@ -424,7 +424,7 @@ nodeinfo
**Syntax:**
-.. code-block:: shell
+.. code-block::
nodeinfo
@@ -440,7 +440,7 @@ nodecpumap
**Syntax:**
-.. code-block:: shell
+.. code-block::
nodecpumap [--pretty]
@@ -456,7 +456,7 @@ nodecpustats
**Syntax:**
-.. code-block:: shell
+.. code-block::
nodecpustats [cpu] [--percent]
@@ -471,7 +471,7 @@ nodememstats
**Syntax:**
-.. code-block:: shell
+.. code-block::
nodememstats [cell]
@@ -484,7 +484,7 @@ nodesuspend
**Syntax:**
-.. code-block:: shell
+.. code-block::
nodesuspend [target] [duration]
@@ -503,7 +503,7 @@ node
**Syntax:**
-.. code-block:: shell
+.. code-block::
node-memory-tune [shm-pages-to-scan] [shm-sleep-millisecs]
[shm-merge-across-nodes]
@@ -525,7 +525,7 @@ capabilities
**Syntax:**
-.. code-block:: shell
+.. code-block::
capabilities
@@ -545,7 +545,7 @@ domcapabilities
**Syntax:**
-.. code-block:: shell
+.. code-block::
domcapabilities [virttype] [emulatorbin] [arch] [machine]
@@ -588,7 +588,7 @@ pool-capabilities
**Syntax:**
-.. code-block:: shell
+.. code-block::
pool-capabilities
@@ -605,7 +605,7 @@ inject
**Syntax:**
-.. code-block:: shell
+.. code-block::
inject-nmi domain
@@ -617,7 +617,7 @@ list
**Syntax:**
-.. code-block:: shell
+.. code-block::
list [--inactive | --all]
[--managed-save] [--title]
@@ -637,7 +637,7 @@ specified it prints out information about running domains.
An example format for the list is as follows:
-.. code-block:: shell
+.. code-block::
``virsh`` list
IdName State
@@ -775,7 +775,7 @@ printed in an extra column. This flag is usable only with
the default
**Example 2:**
-.. code-block:: shell
+.. code-block::
$ virsh list --title
IdNameState Title
@@ -790,7 +790,7 @@ freecell
**Syntax:**
-.. code-block:: shell
+.. code-block::
freecell [{ [--cellno] cellno | --all }]
@@ -809,7 +809,7 @@ freepages
**Syntax:**
-..
[libvirt] [PATCH] docs: ensure outputfile is deleted if rst2html/rst2man fail
This avoids leaving a zero length or partially generated output
file on errors.
Signed-off-by: Daniel P. Berrangé
---
docs/Makefile.am | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index eb8de80b9c..9a1f7a6117 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -259,21 +259,21 @@ man8_MANS = $(manpages8_rst:%.rst=%.8)
grep -v '^\.\. contents::' < $< | \
sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
-e 's|RUNSTATEDIR|$(runstatedir)|g' | \
- $(RST2MAN) > $@
+ $(RST2MAN) > $@ || { rm $@ && exit 1; }
%.7: %.rst
$(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
grep -v '^\.\. contents::' < $< | \
sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
-e 's|RUNSTATEDIR|$(runstatedir)|g' | \
- $(RST2MAN) > $@
+ $(RST2MAN) > $@ || { rm $@ && exit 1; }
%.8: %.rst
$(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
grep -v '^\.\. contents::' < $< | \
sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
-e 's|RUNSTATEDIR|$(runstatedir)|g' | \
- $(RST2MAN) > $@
+ $(RST2MAN) > $@ || { rm $@ && exit 1; }
manpages/virkeycode-%.rst: $(top_srcdir)/src/keycodemapdb/data/keymaps.csv \
$(top_srcdir)/src/keycodemapdb/tools/keymap-gen Makefile.am
@@ -414,11 +414,11 @@ manpages/%.html.in: manpages/%.rst
grep -v '^:Manual ' < $< | \
sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
-e 's|RUNSTATEDIR|$(runstatedir)|g' | \
- $(RST2HTML) > $@
+ $(RST2HTML) > $@ || { rm $@ && exit 1; }
%.html.in: %.rst
$(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
- $(RST2HTML) $< > $@
+ $(RST2HTML) $< > $@ || { rm $@ && exit 1; }
%.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \
$(acl_generated)
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] ci: Fetch list of available container images dynamically
Any static list of images is destined to become outdated eventually,
so let's start generating it dynamically instead.
Unfortunately there doesn't seem to be a straightforward way to get
Podman/Docker to list all repositories under quay.io/libvirt, so we
have to resort to searching and filtering manually; and since the
two tools behave slightly differently in that regard, it's more
sane to have the logic in a separate shell script than it would be
to keep it inline in the Makefile with all the annoying escaping
that would entail.
Signed-off-by: Andrea Bolognani
---
ci/Makefile | 37 +
ci/list-images.sh | 26 ++
2 files changed, 39 insertions(+), 24 deletions(-)
create mode 100644 ci/list-images.sh
diff --git a/ci/Makefile b/ci/Makefile
index 27c1049b38..acb655941c 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -238,6 +238,17 @@ ci-build@%:
ci-check@%:
$(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check"
+ci-list-images:
+ @echo
+ @echo "Available x86 container images:"
+ @echo
+ @sh list-images.sh "$(CI_ENGINE)" "$(CI_IMAGE_PREFIX)" | grep -v cross
+ @echo
+ @echo "Available cross-compiler container images:"
+ @echo
+ @sh list-images.sh "$(CI_ENGINE)" "$(CI_IMAGE_PREFIX)" | grep cross
+ @echo
+
ci-help:
@echo "Build libvirt inside containers used for CI"
@echo
@@ -246,30 +257,8 @@ ci-help:
@echo "ci-build@\$$IMAGE - run a default 'make'"
@echo "ci-check@\$$IMAGE - run a 'make check'"
@echo "ci-shell@\$$IMAGE - run an interactive shell"
- @echo
- @echo "Available x86 container images:"
- @echo
- @echo "centos-7"
- @echo "debian-9"
- @echo "debian-10"
- @echo "debian-sid"
- @echo "fedora-29"
- @echo "fedora-30"
- @echo "fedora-rawhide"
- @echo "ubuntu-16"
- @echo "ubuntu-18"
- @echo
- @echo "Available cross-compiler container images:"
- @echo
- @echo "debian-{9,10,sid}-cross-aarch64"
- @echo "debian-{9,10,sid}-cross-armv6l"
- @echo "debian-{9,10,sid}-cross-armv7l"
- @echo "debian-{10,sid}-cross-i686"
- @echo "debian-{9,10,sid}-cross-mips64el"
- @echo "debian-{9,10,sid}-cross-mips"
- @echo "debian-{9,10,sid}-cross-mipsel"
- @echo "debian-{9,10,sid}-cross-ppc64le"
- @echo "debian-{9,10,sid}-cross-s390x"
+ @echo "ci-list-images - list available images"
+ @echo "ci-help - show this help message"
@echo
@echo "Available make variables:"
@echo
diff --git a/ci/list-images.sh b/ci/list-images.sh
new file mode 100644
index 00..35efdb6982
--- /dev/null
+++ b/ci/list-images.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+engine="$1"
+prefix="$2"
+
+do_podman() {
+# Podman freaks out if the search term ends with a dash, which ours
+# by default does, so let's strip it. The repository name is the
+# second field in the output, and it already starts with the registry
+podman search --limit 100 "${prefix%-}" | while read _ repo _; do
+echo "$repo"
+done
+}
+
+do_docker() {
+# Docker doesn't include the registry name in the output, so we have
+# to add it. The repository name is the first field in the output
+registry="${prefix%%/*}"
+docker search --limit 100 "$prefix" | while read repo _; do
+echo "$registry/$repo"
+done
+}
+
+"do_$engine" | grep "^$prefix" | sed "s,^$prefix,,g" | while read repo; do
+echo "$repo"
+done | sort -u
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] doc: vtpm only support secrets by UUID at this point
On 12/10/19 10:08 AM, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau > > Support by usage name can be considered separately (with a 'usage' > attribute?). > > Cc: Stefan Berger > Signed-off-by: Marc-André Lureau > --- > docs/formatsecret.html.in | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in > index 8d0630a7c3..8f5383cf64 100644 > --- a/docs/formatsecret.html.in > +++ b/docs/formatsecret.html.in > @@ -334,8 +334,8 @@ Secret value set >of the vTPM. >The usage type='vtpm' element must contain >a single name element that specifies a usage name > - for the secret. The vTPM secret can then be used by UUID or by > - this usage name via the encryption element of > + for the secret. The vTPM secret can then be used by UUID > + via the encryption element of >a tpm when using an >emulator. >Since 5.6.0. The following is an example > I've pushed this now - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] travis: Update name for Ubuntu 18.04 image
The corresponding libvirt-jenkins-ci commit is f289e64a5fd9. Signed-off-by: Andrea Bolognani --- Pushed as trivial. .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index b47f54553e..44ff5e9898 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,7 +21,7 @@ matrix: - services: - docker env: -- IMAGE="ubuntu-18" +- IMAGE="ubuntu-1804" - MAKE_ARGS="syntax-check distcheck" script: - make -C ci/ ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS" -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] qemu: honour parseOpaque instead of refetching caps
The use of the parseOpaque parameter was mistakenly removed in
commit 4a4132b4625778cf80acb9c92d06351b44468ac3
Author: Daniel P. Berrangé
Date: Tue Dec 3 10:49:49 2019 +
conf: don't use passed in caps in post parse method
causing the method to re-fetch qemuCaps that were already just
fetched and put into parseOpaque.
This is inefficient when parsing incoming XML, but for live
XML this is more serious as it means we use the capabilities
for the current QEMU binary on disk, rather than the running
QEMU.
That commit, however, did have a useful side effect of fixing
a crasher bug in the qemu post parse callback introduced by
commit 5e939cea896fb3373a6f68f86e325c657429ed3d
Author: Jiri Denemark
Date: Thu Sep 26 18:42:02 2019 +0200
qemu: Store default CPU in domain XML
The qemuDomainDefSetDefaultCPU() method in that patch did not
allow for the possibility that qemuCaps would be NULL and thus
resulted in a SEGV.
This shows a risk in letting each check in the post parse
callback look for qemuCaps == NULL. The safer option is to
check once upfront and immediately stop (postpone) further
validation.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_domain.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 3e1bd52d9f..cf2e9aebd2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4854,16 +4854,18 @@ static int
qemuDomainDefPostParse(virDomainDefPtr def,
unsigned int parseFlags,
void *opaque,
- void *parseOpaque G_GNUC_UNUSED)
+ void *parseOpaque)
{
virQEMUDriverPtr driver = opaque;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-g_autoptr(virQEMUCaps) qemuCaps = NULL;
+virQEMUCapsPtr qemuCaps = parseOpaque;
-if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
-def->emulator))) {
+/* Note that qemuCaps may be NULL when this function is called. This
+ * function shall not fail in that case. It will be re-run on VM startup
+ * with the capabilities populated.
+ */
+if (!qemuCaps)
return 1;
-}
if (def->os.bootloader || def->os.bootloaderArgs) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] qemu: check os type / virt type / arch in validate callback
Don't check os type / virt type / arch in the post-parse callback
because we can't assume qemuCaps is non-NULL at this point. It
also conceptually belongs to the validation callback.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_domain.c | 42
tests/qemuxml2argvtest.c | 9 ++---
2 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 27926c7670..3e1bd52d9f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4865,27 +4865,6 @@ qemuDomainDefPostParse(virDomainDefPtr def,
return 1;
}
-if (def->os.type != VIR_DOMAIN_OSTYPE_HVM) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Emulator '%s' does not support os type '%s'"),
- def->emulator, virDomainOSTypeToString(def->os.type));
-return -1;
-}
-
-if (!virQEMUCapsIsArchSupported(qemuCaps, def->os.arch)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Emulator '%s' does not support arch '%s'"),
- def->emulator, virArchToString(def->os.arch));
-return -1;
-}
-
-if (!virQEMUCapsIsVirtTypeSupported(qemuCaps, def->virtType)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Emulator '%s' does not support virt type '%s'"),
- def->emulator,
virDomainVirtTypeToString(def->virtType));
-return -1;
-}
-
if (def->os.bootloader || def->os.bootloaderArgs) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("bootloader is not supported by QEMU"));
@@ -5142,6 +5121,27 @@ qemuDomainDefValidate(const virDomainDef *def,
def->emulator)))
goto cleanup;
+if (def->os.type != VIR_DOMAIN_OSTYPE_HVM) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Emulator '%s' does not support os type '%s'"),
+ def->emulator, virDomainOSTypeToString(def->os.type));
+goto cleanup;
+}
+
+if (!virQEMUCapsIsArchSupported(qemuCaps, def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Emulator '%s' does not support arch '%s'"),
+ def->emulator, virArchToString(def->os.arch));
+goto cleanup;
+}
+
+if (!virQEMUCapsIsVirtTypeSupported(qemuCaps, def->virtType)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Emulator '%s' does not support virt type '%s'"),
+ def->emulator,
virDomainVirtTypeToString(def->virtType));
+goto cleanup;
+}
+
if (def->mem.min_guarantee) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Parameter 'min_guarantee' not supported by QEMU."));
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ba4a92ec0a..8655db609e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2852,10 +2852,13 @@ mymain(void)
QEMU_CAPS_NEC_USB_XHCI);
/* VM XML has invalid arch/ostype/virttype combo, but the SKIP flag
- * will avoid the error. Still, we expect qemu driver to complain about
- * missing machine error, and not crash */
+ * will avoid the error during parse. This will cause us to fill in
+ * the missing machine type using the i386 binary, despite it being
+ * the wrong binary for the arch. We expect to get a failure about
+ * bad arch later when creating the pretend command.
+ */
DO_TEST_FULL("missing-machine",
- ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE,
+ ARG_FLAGS, FLAG_EXPECT_FAILURE,
ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE,
ARG_QEMU_CAPS, NONE);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/3] misc fixes for qemu domain validation/post-parse
Daniel P. Berrangé (3): tests: add a domain ID to live status XML doc qemu: check os type / virt type / arch in validate callback qemu: honour parseOpaque instead of refetching caps src/qemu/qemu_domain.c| 54 ++- .../qemustatusxml2xmldata/vcpus-multi-in.xml | 2 +- tests/qemuxml2argvtest.c | 9 ++-- 3 files changed, 35 insertions(+), 30 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/3] tests: add a domain ID to live status XML doc
The status XML represents a running VM, so we should always have an ID present for the domain. Signed-off-by: Daniel P. Berrangé --- tests/qemustatusxml2xmldata/vcpus-multi-in.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemustatusxml2xmldata/vcpus-multi-in.xml b/tests/qemustatusxml2xmldata/vcpus-multi-in.xml index 11ec74ecf8..be48c55fe0 100644 --- a/tests/qemustatusxml2xmldata/vcpus-multi-in.xml +++ b/tests/qemustatusxml2xmldata/vcpus-multi-in.xml @@ -310,7 +310,7 @@ -2 - + QEMUGuest1 c7a5fdbd-edaf-9455-926a-d65c16db1809 219100 -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: keep capabilities when running QEMU as root
On Wed, Dec 11, 2019 at 10:25:32AM -0500, Cole Robinson wrote: > On 12/4/19 5:11 AM, Daniel P. Berrangé wrote: > > When QEMU uid/gid is set to non-root this is pointless as if we just > > used a regular setuid/setgid call, the process will have all its > > capabilities cleared anyway by the kernel. > > > > When QEMU uid/gid is set to root, this is almost (always?) never > > what people actually want. People make QEMU run as root in order > > to access some privileged resource that libvirt doesn't support > > yet and this often requires capabilities. As a result they have > > to go find the qemu.conf param to turn this off. This is not > > viable for libguestfs - they want to control everything via the > > XML security label to request running as root regardless of the > > qemu.conf settings for user/group. > > > > Clearing capabilities was implemented originally because there > > was a proposal in Fedora to change permissions such that root, > > with no capabilities would not be able to compromise the system. > > ie a locked down root account. This never went anywhere though, > > and as a result clearing capabilities when running as root does > > not really get us any security benefit AFAICT. The root user > > can easily do something like create a cronjob, which will then > > faithfully be run with full capabilities, trivially bypassing > > the restriction we place. > > > > IOW, our clearing of capabilities is both useless from a security > > POV, and breaks valid use cases when people need to run as root. > > > > This removes the clear_emulator_capabilities configuration > > option from qemu.conf, and always runs QEMU with capabilities > > when root. The behaviour when non-root is unchanged. > > > > Signed-off-by: Daniel P. Berrangé > > Reviewed-by: Cole Robinson > > I checked what happens if that option is leftover in qemu.conf, > surprisingly dothing, not even a VIR_WARN printed for bogus options. But > it looks non-trivial to add in a standardized way This is the same as what happens if you randomly typo any existing config parameter. You can put arbitrary garbage in the config file and its always ignored. 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] qemu: keep capabilities when running QEMU as root
On 12/4/19 5:11 AM, Daniel P. Berrangé wrote: > When QEMU uid/gid is set to non-root this is pointless as if we just > used a regular setuid/setgid call, the process will have all its > capabilities cleared anyway by the kernel. > > When QEMU uid/gid is set to root, this is almost (always?) never > what people actually want. People make QEMU run as root in order > to access some privileged resource that libvirt doesn't support > yet and this often requires capabilities. As a result they have > to go find the qemu.conf param to turn this off. This is not > viable for libguestfs - they want to control everything via the > XML security label to request running as root regardless of the > qemu.conf settings for user/group. > > Clearing capabilities was implemented originally because there > was a proposal in Fedora to change permissions such that root, > with no capabilities would not be able to compromise the system. > ie a locked down root account. This never went anywhere though, > and as a result clearing capabilities when running as root does > not really get us any security benefit AFAICT. The root user > can easily do something like create a cronjob, which will then > faithfully be run with full capabilities, trivially bypassing > the restriction we place. > > IOW, our clearing of capabilities is both useless from a security > POV, and breaks valid use cases when people need to run as root. > > This removes the clear_emulator_capabilities configuration > option from qemu.conf, and always runs QEMU with capabilities > when root. The behaviour when non-root is unchanged. > > Signed-off-by: Daniel P. Berrangé Reviewed-by: Cole Robinson I checked what happens if that option is leftover in qemu.conf, surprisingly dothing, not even a VIR_WARN printed for bogus options. But it looks non-trivial to add in a standardized way - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 3/3] guests: add libprlsdk package to libvirt project
On Fri, 2019-12-06 at 18:53 +, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > guests/vars/projects/libvirt.yml | 1 + > 1 file changed, 1 insertion(+) 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 v2 2/3] guests: define mapping for the libprlsdk package
On Fri, 2019-12-06 at 18:53 +, Daniel P. Berrangé wrote: > Add a mapping exclusively for CentOS 7 to pull in the libprlsdk package, > since other distros don't have it available at this time. > > Signed-off-by: Daniel P. Berrangé > --- > guests/vars/mappings.yml | 4 > 1 file changed, 4 insertions(+) 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 v2 1/3] guests: add openvz repository on CentOS 7
On Fri, 2019-12-06 at 18:53 +, Daniel P. Berrangé wrote:
> +++ b/guests/lcitool
> +def _get_openvz_repo(self):
> +basedir = os.path.dirname(sys.argv[0])
> +repofile = os.path.join(basedir, "playbooks", "update", "templates",
> "openvz.repo.j2")
This should be
base = Util.get_base()
repofile = os.path.join(base, ...)
> +def _get_openvz_key(self):
> +basedir = os.path.dirname(sys.argv[0])
> +repofile = os.path.join(basedir, "playbooks", "update", "templates",
> "openvz.key")
Same here, except you probably want to call it keyfile instead of
repofile.
> @@ -723,6 +735,16 @@ class Application:
> elif os_name == "CentOS" and os_version == "7":
> +repo = self._get_openvz_repo()
> +repocmd = "\\n\\\n".join(repo.split("\n"))
> +key = self._get_openvz_key()
> +keycmd = "\\n\\\n".join(key.split("\n"))
> +
> +sys.stdout.write(
> +"RUN echo -e '%s' > /etc/yum.repos.d/openvz.repo &&
> \\\n" % repocmd +
> +"echo -e '%s' > /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ
> && \\\n" % keycmd +
> +"rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ")
This is different from what's right above and below it for, as far
as I can tell, no good reason.
You can make it nicer and more consistent like
repo = self._get_openvz_repo()
key = self._get_openvz_key()
varmap["repo"] = "\\n\\\n".join(repo.split("\n"))
varmap["key"] = "\\n\\\n".join(key.split("\n"))
sys.stdout.write(textwrap.dedent("""
RUN echo -e '{repo}' > /etc/yum.repos.d/openvz.repo && \\
echo -e '{key}' > /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ && \\
rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ
""").format(**varmap))
sys.stdout.write(textwrap.dedent("""
RUN {package_manager} update -y && \\
{package_manager} install -y epel-release && \\
{package_manager} install -y {pkgs} && \\
{package_manager} autoremove -y && \\
{package_manager} clean all -y
""").format(**varmap))
or even merge the two RUN statements to reduce the number of layers
that will end up in the resulting image.
Everything else looks good, so with the above changed
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
[libvirt] libvirt API/design questions
There's some pre-existing libvirt design questions I would like some feedback on, and one new one * `virsh blockresize` doesn't prevent shrink by default, couple users have blown their foot off and there's attempts at patches. https://www.redhat.com/archives/libvir-list/2019-October/msg00258.html https://www.redhat.com/archives/libvir-list/2019-November/msg00843.html Do we implement this as a protection in virsh, or change the API behavior and require a new API flag to allow shrinking? Or some other idea? * vhost-user-scsi and vhost-user-blk XML schema https://www.redhat.com/archives/libvirt-users/2019-October/msg00018.html Last proposal was using for this, which revisiting it now makes more sense than , because it vhost-user-X doesn't seem to use qemu's block layer at all. So vhost-user-scsi would be: vhost-user-blk maybe requires a new type='block' ? Otherwise it's unclear how to differentiate between the two. Regardless it would be nice to give the original patch author guidance here, they seemed motivated to get this working * Splitting domain_conf up. There's 30k lines in domain_conf.c and 3500 in domain_conf.h, it would be nice to unwind it a bit. Getting some sign off on this ahead of time is critical IMO so pieces can be posted and committed quickly because they will obviously go out of date fast. My thoughts: - domain_def.[ch]: DomainDefXXX and enum definitions. - probably New and Free functions too - domain_parse.[ch]: XML parsing - domain_format.[ch]: XML formatting - domain_validate.[ch]: validate and postparse handling - domain_util.[ch]: everything else, or keep it in domain_conf? domain_def should be easy but no idea how intertwined the rest are. If the domain_validate naming is acceptable for both validate and postparse functions, we could use that naming for qemu_domain.c split too. Also maybe it's worth considering if there's some way to sensibly split the conf/ directory. We could add a top level domain/ directory, but that's kinda ambiguously named as we already have network/ + storage/ + secret/ etc that have different meanings. Maybe sub dirs like conf/domain/ ? I'm curious if anyone has strong feelings either way. There's not really a clear place to dump shared DomainDef code at the moment, like possible domain_cgroup for sharing cgroup handling across lxc + qemu Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dockerfiles PATCH] Rename Ubuntu Dockerfiles
The corresponding libvirt-jenkins-ci commit is f289e64a5fd9. Signed-off-by: Andrea Bolognani --- Pushed as trivial. ...ntu-16.zip => buildenv-libosinfo-ubuntu-1604.zip | Bin ...ntu-18.zip => buildenv-libosinfo-ubuntu-1804.zip | Bin ...buntu-16.zip => buildenv-libvirt-ubuntu-1604.zip | Bin ...buntu-18.zip => buildenv-libvirt-ubuntu-1804.zip | Bin 4 files changed, 0 insertions(+), 0 deletions(-) rename buildenv-libosinfo-ubuntu-16.zip => buildenv-libosinfo-ubuntu-1604.zip (100%) rename buildenv-libosinfo-ubuntu-18.zip => buildenv-libosinfo-ubuntu-1804.zip (100%) rename buildenv-libvirt-ubuntu-16.zip => buildenv-libvirt-ubuntu-1604.zip (100%) rename buildenv-libvirt-ubuntu-18.zip => buildenv-libvirt-ubuntu-1804.zip (100%) diff --git a/buildenv-libosinfo-ubuntu-16.zip b/buildenv-libosinfo-ubuntu-1604.zip similarity index 100% rename from buildenv-libosinfo-ubuntu-16.zip rename to buildenv-libosinfo-ubuntu-1604.zip diff --git a/buildenv-libosinfo-ubuntu-18.zip b/buildenv-libosinfo-ubuntu-1804.zip similarity index 100% rename from buildenv-libosinfo-ubuntu-18.zip rename to buildenv-libosinfo-ubuntu-1804.zip diff --git a/buildenv-libvirt-ubuntu-16.zip b/buildenv-libvirt-ubuntu-1604.zip similarity index 100% rename from buildenv-libvirt-ubuntu-16.zip rename to buildenv-libvirt-ubuntu-1604.zip diff --git a/buildenv-libvirt-ubuntu-18.zip b/buildenv-libvirt-ubuntu-1804.zip similarity index 100% rename from buildenv-libvirt-ubuntu-18.zip rename to buildenv-libvirt-ubuntu-1804.zip -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: fix indentation fix
The RPM tags must not be indented.
Fixes: 6b8ab20f9b9b1a9383bd2cb9a075f57beb196c1c
Signed-off-by: Ján Tomko
---
libvirt.spec.in | 8
1 file changed, 4 insertions(+), 4 deletions(-)
Pushed as a build breaker fix.
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 4d75fbecaf..bd08222527 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -336,11 +336,11 @@ BuildRequires: device-mapper-devel
BuildRequires: xfsprogs-devel
%if %{with_storage_rbd}
%if 0%{?fedora} || 0%{?rhel} > 7
-BuildRequires: librados-devel
-BuildRequires: librbd-devel
+BuildRequires: librados-devel
+BuildRequires: librbd-devel
%else
-BuildRequires: librados2-devel
-BuildRequires: librbd1-devel
+BuildRequires: librados2-devel
+BuildRequires: librbd1-devel
%endif
%endif
%if %{with_storage_gluster}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 3/8] Remove VIR_STRNDUP usage with checked pointers
Remove the usage where sanity of the length argument is verified
by other conditions not matching the previous patches.
Signed-off-by: Ján Tomko
---
src/libxl/xen_common.c | 18 --
tools/vsh.c| 5 ++---
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 8c07df80b6..912dd8834a 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -815,9 +815,8 @@ xenParseSxprChar(const char *value,
goto error;
}
-if (offset != value &&
-VIR_STRNDUP(def->source->data.tcp.host, value, offset - value) < 0)
-goto error;
+if (offset != value)
+def->source->data.tcp.host = g_strndup(value, offset - value);
offset2 = strchr(offset, ',');
offset++;
@@ -843,9 +842,9 @@ xenParseSxprChar(const char *value,
goto error;
}
-if (offset != value &&
-VIR_STRNDUP(def->source->data.udp.connectHost, value, offset -
value) < 0)
-goto error;
+if (offset != value)
+def->source->data.udp.connectHost = g_strndup(value,
+ offset - value);
offset2 = strchr(offset, '@');
if (offset2 != NULL) {
@@ -860,10 +859,9 @@ xenParseSxprChar(const char *value,
goto error;
}
-if (offset3 > (offset2 + 1) &&
-VIR_STRNDUP(def->source->data.udp.bindHost,
-offset2 + 1, offset3 - offset2 - 1) < 0)
-goto error;
+if (offset3 > (offset2 + 1))
+def->source->data.udp.bindHost = g_strndup(offset2 + 1,
+ offset3 - offset2 -
1);
def->source->data.udp.bindService = g_strdup(offset3 + 1);
} else {
diff --git a/tools/vsh.c b/tools/vsh.c
index dd2c039b47..5ccda5ab44 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -348,9 +348,8 @@ vshCmddefCheckInternals(vshControl *ctl,
cmd->name);
return -1; /* alias options are tracked by the original name */
}
-if ((p = strchr(name, '=')) &&
-VIR_STRNDUP(name, name, p - name) < 0)
-return -1;
+if ((p = strchr(name, '=')))
+name = g_strndup(name, p - name);
for (j = i + 1; cmd->opts[j].name; j++) {
if (STREQ(name, cmd->opts[j].name) &&
cmd->opts[j].type != VSH_OT_ALIAS)
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 4/8] Remove all the uses that use subtraction in their length argument
Signed-off-by: Ján Tomko
Reviewed-by: Daniel Henrique Barboza
---
src/bhyve/bhyve_parse_command.c | 6 ++
src/libxl/xen_common.c | 8 +++-
src/libxl/xen_xm.c | 3 +--
src/lxc/lxc_driver.c| 3 +--
src/qemu/qemu_driver.c | 3 +--
src/qemu/qemu_monitor_json.c| 8 ++--
src/util/vircgroupv1.c | 5 ++---
src/util/vircommand.c | 1 -
src/util/virconf.c | 24 +++-
src/util/viriscsi.c | 3 +--
src/util/virkeyfile.c | 6 ++
src/util/virstoragefile.c | 3 +--
src/util/virsysinfo.c | 3 +--
src/vmware/vmware_conf.c| 6 +-
14 files changed, 25 insertions(+), 57 deletions(-)
diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c
index 5eec05c7d4..e74c4c2d0c 100644
--- a/src/bhyve/bhyve_parse_command.c
+++ b/src/bhyve/bhyve_parse_command.c
@@ -282,8 +282,7 @@ bhyveParseBhyveLPCArg(virDomainDefPtr def,
if (!separator)
goto error;
-if (VIR_STRNDUP(type, arg, separator - arg) < 0)
-goto error;
+type = g_strndup(arg, separator - arg);
/* Only support com%d */
if (STRPREFIX(type, "com") && type[4] == 0) {
@@ -578,8 +577,7 @@ bhyveParseBhyvePCIArg(virDomainDefPtr def,
else
separator++; /* Skip comma */
-if (VIR_STRNDUP(slotdef, arg, separator - arg - 1) < 0)
-goto error;
+slotdef = g_strndup(arg, separator - arg - 1);
conf = strchr(separator+1, ',');
if (conf) {
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 912dd8834a..89ced6ae2b 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -848,9 +848,8 @@ xenParseSxprChar(const char *value,
offset2 = strchr(offset, '@');
if (offset2 != NULL) {
-if (VIR_STRNDUP(def->source->data.udp.connectService,
-offset + 1, offset2 - offset - 1) < 0)
-goto error;
+def->source->data.udp.connectService = g_strndup(offset + 1,
+ offset2 - offset
- 1);
offset3 = strchr(offset2, ':');
if (offset3 == NULL) {
@@ -992,8 +991,7 @@ xenParseVifBridge(virDomainNetDefPtr net, char *bridge)
if ((vlanstr = strchr(bridge, '.'))) {
/* 'bridge' string contains a bridge name and single vlan tag */
-if (VIR_STRNDUP(net->data.bridge.brname, bridge, vlanstr - bridge) < 0)
-return -1;
+net->data.bridge.brname = g_strndup(bridge, vlanstr - bridge);
vlanstr++;
if (virStrToLong_ui(vlanstr, NULL, 10, ) < 0)
diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c
index 1b462d0487..a196912194 100644
--- a/src/libxl/xen_xm.c
+++ b/src/libxl/xen_xm.c
@@ -131,8 +131,7 @@ xenParseXMDisk(char *entry, int hvm)
/* No source file given, eg CDROM with no media */
ignore_value(virDomainDiskSetSource(disk, NULL));
} else {
-if (VIR_STRNDUP(tmp, head, offset - head) < 0)
-goto error;
+tmp = g_strndup(head, offset - head);
if (virDomainDiskSetSource(disk, tmp) < 0) {
VIR_FREE(tmp);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b40a96b4ce..fbf49ee957 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2137,8 +2137,7 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const
char *type,
if (!p)
goto parse_error;
-if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0)
-goto cleanup;
+result[i].path = g_strndup(temp, p - temp);
/* value */
temp = p + 1;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4d77fd6f6a..47665f4ea6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9251,8 +9251,7 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const
char *type,
if (!p)
goto parse_error;
-if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0)
-goto cleanup;
+result[i].path = g_strndup(temp, p - temp);
/* value */
temp = p + 1;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8a8347924a..b2f695a70c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -643,15 +643,11 @@ qemuMonitorJSONParseKeywords(const char *str,
separator = endmark;
}
-if (VIR_STRNDUP(keyword, start, separator - start) < 0)
-goto error;
+keyword = g_strndup(start, separator - start);
if (separator < endmark) {
separator++;
-if (VIR_STRNDUP(value, separator, endmark - separator) < 0) {
-VIR_FREE(keyword);
-goto error;
-}
+value = g_strndup(separator, endmark - separator);
if (strchr(value, ',')) {
char *p =
[libvirt] [PATCHv2 5/8] Remove the rest of VIR_STRNDUP
Replace all the uses passing a single parameter as the length.
Signed-off-by: Ján Tomko
---
src/conf/nwfilter_conf.c | 4 ++--
src/conf/nwfilter_params.c | 6 ++
src/interface/interface_backend_udev.c | 8 ++--
src/libxl/xen_common.c | 3 +--
src/libxl/xen_xl.c | 6 ++
src/libxl/xen_xm.c | 6 ++
src/qemu/qemu_monitor_json.c | 3 +--
src/rpc/virnetlibsshsession.c | 7 ++-
src/util/virjson.c | 11 +++
src/util/virkeyfile.c | 3 +--
src/util/virsocketaddr.c | 5 +
src/util/virstoragefile.c | 3 +--
src/util/virstring.c | 3 +--
13 files changed, 21 insertions(+), 47 deletions(-)
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 5bcf52a84c..d071c398bc 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -400,8 +400,8 @@ virNWFilterRuleDefAddString(virNWFilterRuleDefPtr nwf,
{
char *tmp;
-if (VIR_STRNDUP(tmp, string, maxstrlen) < 0 ||
-VIR_APPEND_ELEMENT_COPY(nwf->strings, nwf->nstrings, tmp) < 0)
+tmp = g_strndup(string, maxstrlen);
+if (VIR_APPEND_ELEMENT_COPY(nwf->strings, nwf->nstrings, tmp) < 0)
VIR_FREE(tmp);
return tmp;
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index b1a2c50f27..4110169135 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -884,8 +884,7 @@ virNWFilterVarAccessParse(const char *varAccess)
if (input[idx] == '\0') {
/* in the form 'IP', which is equivalent to IP[@0] */
-if (VIR_STRNDUP(dest->varName, input, idx) < 0)
-goto err_exit;
+dest->varName = g_strndup(input, idx);
dest->accessType = VIR_NWFILTER_VAR_ACCESS_ITERATOR;
dest->u.iterId = 0;
return dest;
@@ -898,8 +897,7 @@ virNWFilterVarAccessParse(const char *varAccess)
varNameLen = idx;
-if (VIR_STRNDUP(dest->varName, input, varNameLen) < 0)
-goto err_exit;
+dest->varName = g_strndup(input, varNameLen);
input += idx + 1;
virSkipSpaces();
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 7df5cf2cc3..26b1045f8a 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -953,12 +953,8 @@ udevGetIfaceDefVlan(struct udev *udev G_GNUC_UNUSED,
goto cleanup;
}
-if (VIR_STRNDUP(ifacedef->data.vlan.tag, vid_pos, vid_len) < 0)
-goto cleanup;
-if (VIR_STRNDUP(ifacedef->data.vlan.dev_name, dev_pos, dev_len) < 0) {
-VIR_FREE(ifacedef->data.vlan.tag);
-goto cleanup;
-}
+ifacedef->data.vlan.tag = g_strndup(vid_pos, vid_len);
+ifacedef->data.vlan.dev_name = g_strndup(dev_pos, dev_len);
ret = 0;
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 89ced6ae2b..415549a42c 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -1153,8 +1153,7 @@ xenParseVif(char *entry, const char *vif_typename)
} else if (STRPREFIX(key, "script=")) {
int len = nextkey ? (nextkey - data) : strlen(data);
VIR_FREE(script);
-if (VIR_STRNDUP(script, data, len) < 0)
-return NULL;
+script = g_strndup(data, len);
} else if (STRPREFIX(key, "model=")) {
int len = nextkey ? (nextkey - data) : strlen(data);
if (virStrncpy(model, data, len, sizeof(model)) < 0) {
diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index cdff71f11b..8112214b5f 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -1104,13 +1104,11 @@ xenParseXLChannel(virConfPtr conf, virDomainDefPtr def)
} else if (STRPREFIX(key, "name=")) {
int len = nextkey ? (nextkey - data) : strlen(data);
VIR_FREE(name);
-if (VIR_STRNDUP(name, data, len) < 0)
-goto cleanup;
+name = g_strndup(data, len);
} else if (STRPREFIX(key, "path=")) {
int len = nextkey ? (nextkey - data) : strlen(data);
VIR_FREE(path);
-if (VIR_STRNDUP(path, data, len) < 0)
-goto cleanup;
+path = g_strndup(data, len);
}
while (nextkey && (nextkey[0] == ',' ||
diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c
index a196912194..5d5d521c18 100644
--- a/src/libxl/xen_xm.c
+++ b/src/libxl/xen_xm.c
@@ -167,8 +167,7 @@ xenParseXMDisk(char *entry, int hvm)
/* The main type phy:, file:, tap: ... */
if ((tmp = strchr(src, ':')) != NULL) {
len = tmp - src;
-if (VIR_STRNDUP(tmp, src, len) < 0)
-goto error;
+tmp = g_strndup(src,
[libvirt] [PATCHv2 8/8] docs: hacking: document removal of VIR_STR(N)DUP
Signed-off-by: Ján Tomko Reviewed-by: Daniel Henrique Barboza --- docs/hacking.html.in | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 06deab9e90..74aba5d46b 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1040,9 +1040,6 @@ BAD: a single method. Keep the style consistent, converting existing code to GLib style in a separate, prior commit. - VIR_STRDUP, VIR_STRNDUP - Prefer the GLib APIs g_strdup and g_strndup. - virStrerror The GLib g_strerror() function should be used instead, which has a simpler calling convention as an added benefit. @@ -1071,8 +1068,6 @@ BAD: String allocation macros and functions: deprecated versionGLib versionNotes - VIR_STRDUPg_strdup - VIR_STRNDUPg_strndup virAsprintfg_strdup_printf virVasprintfg_strdup_vprint use g_vasprintf if you really need to know the returned length @@ -1108,6 +1103,9 @@ BAD: The GLib macros g_autoptr and G_DEFINE_AUTOPTR_CLEANUP_FUNC should be used to manage autoclean of virObject classes. This matches usage with GObject classes. + + VIR_STRDUP, VIR_STRNDUP + Prefer the GLib APIs g_strdup and g_strndup. deleted versionGLib versionNotes @@ -1128,6 +1126,8 @@ BAD: ATTRIBUTE_RETURN_CHECKG_GNUC_WARN_UNUSED_RESULT ATTRIBUTE_SENTINELG_GNUC_NULL_TERMINATED ATTRIBUTE_UNUSEDG_GNUC_UNUSED + VIR_STRDUPg_strdup + VIR_STRNDUPg_strndup -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 6/8] tests: delete tests for VIR_STR(N)DUP
Signed-off-by: Ján Tomko
Reviewed-by: Daniel Henrique Barboza
---
tests/virstringtest.c | 136 --
1 file changed, 136 deletions(-)
diff --git a/tests/virstringtest.c b/tests/virstringtest.c
index fc5f9bf937..1f195ab4b9 100644
--- a/tests/virstringtest.c
+++ b/tests/virstringtest.c
@@ -233,136 +233,6 @@ static int testRemove(const void *args)
}
-static bool fail;
-
-static const char *
-testStrdupLookup1(size_t i)
-{
-switch (i) {
-case 0:
-return "hello";
-case 1:
-return NULL;
-default:
-fail = true;
-return "oops";
-}
-}
-
-static size_t
-testStrdupLookup2(size_t i)
-{
-if (i)
-fail = true;
-return 5;
-}
-
-static int
-testStrdup(const void *data G_GNUC_UNUSED)
-{
-char *array[] = { NULL, NULL };
-size_t i = 0;
-size_t j = 0;
-size_t k = 0;
-int ret = -1;
-int value;
-
-value = VIR_STRDUP(array[i++], testStrdupLookup1(j++));
-if (value != 1) {
-virFilePrintf(stderr, "unexpected strdup result %d, expected 1\n",
value);
-goto cleanup;
-}
-/* coverity[dead_error_begin] */
-if (i != 1) {
-virFilePrintf(stderr, "unexpected side effects i=%zu, expected 1\n",
i);
-goto cleanup;
-}
-/* coverity[dead_error_begin] */
-if (j != 1) {
-virFilePrintf(stderr, "unexpected side effects j=%zu, expected 1\n",
j);
-goto cleanup;
-}
-if (STRNEQ_NULLABLE(array[0], "hello") || array[1]) {
-virFilePrintf(stderr, "incorrect array contents '%s' '%s'\n",
- NULLSTR(array[0]), NULLSTR(array[1]));
-goto cleanup;
-}
-
-value = VIR_STRNDUP(array[i++], testStrdupLookup1(j++),
-testStrdupLookup2(k++));
-if (value != 0) {
-virFilePrintf(stderr, "unexpected strdup result %d, expected 0\n",
value);
-goto cleanup;
-}
-/* coverity[dead_error_begin] */
-if (i != 2) {
-virFilePrintf(stderr, "unexpected side effects i=%zu, expected 2\n",
i);
-goto cleanup;
-}
-/* coverity[dead_error_begin] */
-if (j != 2) {
-virFilePrintf(stderr, "unexpected side effects j=%zu, expected 2\n",
j);
-goto cleanup;
-}
-/* coverity[dead_error_begin] */
-if (k != 1) {
-virFilePrintf(stderr, "unexpected side effects k=%zu, expected 1\n",
k);
-goto cleanup;
-}
-if (STRNEQ_NULLABLE(array[0], "hello") || array[1]) {
-virFilePrintf(stderr, "incorrect array contents '%s' '%s'\n",
- NULLSTR(array[0]), NULLSTR(array[1]));
-goto cleanup;
-}
-
-if (fail) {
-virFilePrintf(stderr, "side effects failed\n");
-goto cleanup;
-}
-
-ret = 0;
- cleanup:
-for (i = 0; i < G_N_ELEMENTS(array); i++)
-VIR_FREE(array[i]);
-return ret;
-}
-
-static int
-testStrndupNegative(const void *opaque G_GNUC_UNUSED)
-{
-int ret = -1;
-char *dst;
-const char *src = "Hello world";
-int value;
-
-if ((value = VIR_STRNDUP(dst, src, 5)) != 1) {
-fprintf(stderr, "unexpected virStrndup result %d, expected 1\n",
value);
-goto cleanup;
-}
-
-if (STRNEQ_NULLABLE(dst, "Hello")) {
-fprintf(stderr, "unexpected content '%s'", dst);
-goto cleanup;
-}
-
-VIR_FREE(dst);
-if ((value = VIR_STRNDUP(dst, src, -1)) != 1) {
-fprintf(stderr, "unexpected virStrndup result %d, expected 1\n",
value);
-goto cleanup;
-}
-
-if (STRNEQ_NULLABLE(dst, src)) {
-fprintf(stderr, "unexpected content '%s'", dst);
-goto cleanup;
-}
-
-ret = 0;
- cleanup:
-VIR_FREE(dst);
-return ret;
-}
-
-
static int
testStringSortCompare(const void *opaque G_GNUC_UNUSED)
{
@@ -847,12 +717,6 @@ mymain(void)
const char *tokens8[] = { "gluster", "rdma", NULL };
TEST_SPLIT("gluster+rdma", "+", 2, tokens8);
-if (virTestRun("strdup", testStrdup, NULL) < 0)
-ret = -1;
-
-if (virTestRun("strdup", testStrndupNegative, NULL) < 0)
-ret = -1;
-
if (virTestRun("virStringSortCompare", testStringSortCompare, NULL) < 0)
ret = -1;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 7/8] util: remove VIR_STRDUP and VIR_STRNDUP
Signed-off-by: Ján Tomko
Reviewed-by: Daniel Henrique Barboza
---
src/libvirt_private.syms | 2 --
src/util/virstring.c | 49
src/util/virstring.h | 69
3 files changed, 120 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4a342239e5..38e2ab915b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3114,7 +3114,6 @@ virSkipSpaces;
virSkipSpacesAndBackslash;
virSkipSpacesBackwards;
virStrcpy;
-virStrdup;
virStringBufferIsPrintable;
virStringFilterChars;
virStringHasCaseSuffix;
@@ -3149,7 +3148,6 @@ virStringStripSuffix;
virStringToUpper;
virStringTrimOptionalNewline;
virStrncpy;
-virStrndup;
virStrToDouble;
virStrToLong_i;
virStrToLong_l;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 7cf32fda3e..fe5c026d2c 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -886,55 +886,6 @@ virStringIsEmpty(const char *str)
return str[0] == '\0';
}
-/**
- * virStrdup:
- * @dest: where to store duplicated string
- * @src: the source string to duplicate
- *
- * Wrapper over strdup, which aborts on OOM error.
- *
- * Returns: 0 for NULL src, 1 on successful copy, aborts on OOM
- */
-int
-virStrdup(char **dest,
- const char *src)
-{
-*dest = NULL;
-if (!src)
-return 0;
-*dest = g_strdup(src);
-
-return 1;
-}
-
-/**
- * virStrndup:
- * @dest: where to store duplicated string
- * @src: the source string to duplicate
- * @n: how many bytes to copy
- *
- * Wrapper over strndup, which aborts on OOM error.
- *
- * In case @n is smaller than zero, the whole @src string is
- * copied.
- *
- * Returns: 0 for NULL src, 1 on successful copy, aborts on OOM
- */
-int
-virStrndup(char **dest,
- const char *src,
- ssize_t n)
-{
-*dest = NULL;
-if (!src)
-return 0;
-if (n < 0)
-n = strlen(src);
-*dest = g_strndup(src, n);
-
-return 1;
-}
-
size_t virStringListLength(const char * const *strings)
{
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 081a5ff1aa..a2cd92cf83 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -134,75 +134,6 @@ int virStrdup(char **dest, const char *src)
int virStrndup(char **dest, const char *src, ssize_t n)
G_GNUC_WARN_UNUSED_RESULT ATTRIBUTE_NONNULL(1);
-/**
- * VIR_STRDUP:
- * @dst: variable to hold result (char*, not char**)
- * @src: string to duplicate
- *
- * DEPRECATED: use g_strdup instead
- *
- * Duplicate @src string and store it into @dst.
- *
- * This macro is safe to use on arguments with side effects.
- *
- * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM
- */
-#define VIR_STRDUP(dst, src) virStrdup(&(dst), src)
-
-/**
- * VIR_STRDUP_QUIET:
- * @dst: variable to hold result (char*, not char**)
- * @src: string to duplicate
- *
- * DEPRECATED: use g_strdup instead
- *
- * Duplicate @src string and store it into @dst.
- *
- * This macro is safe to use on arguments with side effects.
- *
- * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM
- */
-#define VIR_STRDUP_QUIET(dst, src) VIR_STRDUP(dst, src)
-
-/**
- * VIR_STRNDUP:
- * @dst: variable to hold result (char*, not char**)
- * @src: string to duplicate
- * @n: the maximum number of bytes to copy
- *
- * DEPRECATED: use g_strndup instead
- *
- * Duplicate @src string and store it into @dst. If @src is longer than @n,
- * only @n bytes are copied and terminating null byte '\0' is added. If @n
- * is a negative number, then the whole @src string is copied. That is,
- * VIR_STRDUP(dst, src) and VIR_STRNDUP(dst, src, -1) are equal.
- *
- * This macro is safe to use on arguments with side effects.
- *
- * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM
- */
-#define VIR_STRNDUP(dst, src, n) virStrndup(&(dst), src, n)
-
-/**
- * VIR_STRNDUP_QUIET:
- * @dst: variable to hold result (char*, not char**)
- * @src: string to duplicate
- * @n: the maximum number of bytes to copy
- *
- * DEPRECATED: use g_strndup instead
- *
- * Duplicate @src string and store it into @dst. If @src is longer than @n,
- * only @n bytes are copied and terminating null byte '\0' is added. If @n
- * is a negative number, then the whole @src string is copied. That is,
- * VIR_STRDUP_QUIET(dst, src) and VIR_STRNDUP_QUIET(dst, src, -1) are
- * equal.
- *
- * This macro is safe to use on arguments with side effects.
- *
- * Returns 0 if @src was NULL, 1 if @src was copied, aborts on OOM
- */
-#define VIR_STRNDUP_QUIET(dst, src, n) virStrndup(&(dst), src, n)
-
size_t virStringListLength(const char * const *strings);
int virStringSortCompare(const void *a, const void *b);
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/8] vsh: remove vshErrorOOM
We abort on allocation errors now so there is no need to
have a function for it.
Replace the only use by return -1, chosen by fair dice roll.
Signed-off-by: Ján Tomko
---
tools/vsh.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 6c78a7a373..dd2c039b47 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -64,17 +64,6 @@ const vshCmdGrp *cmdGroups;
const vshCmdDef *cmdSet;
-/* simple handler for oom conditions */
-static void
-vshErrorOOM(void)
-{
-fflush(stdout);
-fputs(_("error: Out of memory\n"), stderr);
-fflush(stderr);
-exit(EXIT_FAILURE);
-}
-
-
double
vshPrettyCapacity(unsigned long long val, const char **unit)
{
@@ -361,7 +350,7 @@ vshCmddefCheckInternals(vshControl *ctl,
}
if ((p = strchr(name, '=')) &&
VIR_STRNDUP(name, name, p - name) < 0)
-vshErrorOOM();
+return -1;
for (j = i + 1; cmd->opts[j].name; j++) {
if (STREQ(name, cmd->opts[j].name) &&
cmd->opts[j].type != VSH_OT_ALIAS)
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/8] Remove VIR_STRNDUP
Ján Tomko (8): Remove VIR_STRDUP usage that sneaked in in the meantime vsh: remove vshErrorOOM Remove VIR_STRNDUP usage with checked pointers Remove all the uses that use subtraction in their length argument Remove the rest of VIR_STRNDUP tests: delete tests for VIR_STR(N)DUP util: remove VIR_STRDUP and VIR_STRNDUP docs: hacking: document removal of VIR_STR(N)DUP docs/hacking.html.in | 10 +- src/bhyve/bhyve_parse_command.c| 6 +- src/conf/backup_conf.c | 4 +- src/conf/nwfilter_conf.c | 4 +- src/conf/nwfilter_params.c | 6 +- src/interface/interface_backend_udev.c | 8 +- src/libvirt_private.syms | 2 - src/libxl/xen_common.c | 29 +++--- src/libxl/xen_xl.c | 6 +- src/libxl/xen_xm.c | 9 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_monitor_json.c | 14 +-- src/rpc/virnetlibsshsession.c | 7 +- src/util/vircgroupv1.c | 5 +- src/util/vircommand.c | 1 - src/util/virconf.c | 24 ++--- src/util/viriscsi.c| 3 +- src/util/virjson.c | 11 +- src/util/virkeyfile.c | 9 +- src/util/virsocketaddr.c | 5 +- src/util/virstoragefile.c | 6 +- src/util/virstring.c | 52 +- src/util/virstring.h | 69 - src/util/virsysinfo.c | 3 +- src/vmware/vmware_conf.c | 6 +- tests/virstringtest.c | 136 - tools/vsh.c| 16 +-- 28 files changed, 63 insertions(+), 394 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/8] Remove VIR_STRDUP usage that sneaked in in the meantime
My hesitation to remove VIR_STRDUP without VIR_STRNDUP resulted in these being able to sneak in. Signed-off-by: Ján Tomko --- src/conf/backup_conf.c | 4 +--- src/qemu/qemu_monitor_json.c | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 6be2e0498e..aa11967d2a 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -483,9 +483,7 @@ virDomainBackupAlignDisks(virDomainBackupDefPtr def, continue; backupdisk = >disks[ndisks++]; - -if (VIR_STRDUP(backupdisk->name, domdisk->dst) < 0) -return -1; +backupdisk->name = g_strdup(domdisk->dst); if (backup_all && !virStorageSourceIsEmpty(domdisk->src) && diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 00e1d3ce15..8a8347924a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5564,8 +5564,7 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, goto cleanup; } -if (VIR_STRDUP(info->defaultCPU, tmp) < 0) -goto cleanup; +info->defaultCPU = g_strdup(tmp); } } -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] spec: fix indentation
On Wed, Dec 11, 2019 at 14:12:34 +0100, Ján Tomko wrote:
> The recent specfile addition broke syntax-check:
> cppi: ../libvirt.spec.in: line 338: not properly indented
> cppi: ../libvirt.spec.in: line 341: not properly indented
> cppi: ../libvirt.spec.in: line 344: not properly indented
>
> Fixes: ac063cb2e76d64a907f96bf0b6a29da4eb484ebc
> Signed-off-by: Ján Tomko
> ---
> Push under the 'are we still doing syntax-check?' rule
>
> libvirt.spec.in | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 01f28e62d6..4d75fbecaf 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -335,13 +335,13 @@ BuildRequires: device-mapper-devel
> # For XFS reflink clone support
> BuildRequires: xfsprogs-devel
> %if %{with_storage_rbd}
> -%if 0%{?fedora} || 0%{?rhel} > 7
> -BuildRequires: librados-devel
> -BuildRequires: librbd-devel
> -%else
> -BuildRequires: librados2-devel
> -BuildRequires: librbd1-devel
> -%endif
> +%if 0%{?fedora} || 0%{?rhel} > 7
> +BuildRequires: librados-devel
> +BuildRequires: librbd-devel
I don't think you are supposed to indent these. In fact none other are
indented at all.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] spec: fix indentation
The recent specfile addition broke syntax-check:
cppi: ../libvirt.spec.in: line 338: not properly indented
cppi: ../libvirt.spec.in: line 341: not properly indented
cppi: ../libvirt.spec.in: line 344: not properly indented
Fixes: ac063cb2e76d64a907f96bf0b6a29da4eb484ebc
Signed-off-by: Ján Tomko
---
Push under the 'are we still doing syntax-check?' rule
libvirt.spec.in | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 01f28e62d6..4d75fbecaf 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -335,13 +335,13 @@ BuildRequires: device-mapper-devel
# For XFS reflink clone support
BuildRequires: xfsprogs-devel
%if %{with_storage_rbd}
-%if 0%{?fedora} || 0%{?rhel} > 7
-BuildRequires: librados-devel
-BuildRequires: librbd-devel
-%else
-BuildRequires: librados2-devel
-BuildRequires: librbd1-devel
-%endif
+%if 0%{?fedora} || 0%{?rhel} > 7
+BuildRequires: librados-devel
+BuildRequires: librbd-devel
+%else
+BuildRequires: librados2-devel
+BuildRequires: librbd1-devel
+%endif
%endif
%if %{with_storage_gluster}
BuildRequires: glusterfs-api-devel >= 3.4.1
--
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/1] jenkins: Start building on Ubuntu
On Tue, Dec 10, 2019 at 02:54:23PM +0100, Andrea Bolognani wrote: > We've supported both Ubuntu 16.04 and Ubuntu 18.04 as build targets > for a long time now, and we are even building on the latter on > Travis CI, but we've never actually deployed the corresponding VMs > in the CentOS CI environment. Start doing that now. > > Signed-off-by: Andrea Bolognani > --- > jenkins/jobs/defaults.yaml| 2 ++ > jenkins/projects/libvirt-dbus.yaml| 1 + > jenkins/projects/libvirt-sandbox.yaml | 2 ++ > jenkins/projects/libvirt-tck.yaml | 2 ++ > jenkins/projects/libvirt.yaml | 2 ++ > jenkins/projects/virt-manager.yaml| 2 ++ > 6 files changed, 11 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] qemu: keep capabilities when running QEMU as root
ping On Wed, Dec 04, 2019 at 10:11:47AM +, Daniel P. Berrangé wrote: > When QEMU uid/gid is set to non-root this is pointless as if we just > used a regular setuid/setgid call, the process will have all its > capabilities cleared anyway by the kernel. > > When QEMU uid/gid is set to root, this is almost (always?) never > what people actually want. People make QEMU run as root in order > to access some privileged resource that libvirt doesn't support > yet and this often requires capabilities. As a result they have > to go find the qemu.conf param to turn this off. This is not > viable for libguestfs - they want to control everything via the > XML security label to request running as root regardless of the > qemu.conf settings for user/group. > > Clearing capabilities was implemented originally because there > was a proposal in Fedora to change permissions such that root, > with no capabilities would not be able to compromise the system. > ie a locked down root account. This never went anywhere though, > and as a result clearing capabilities when running as root does > not really get us any security benefit AFAICT. The root user > can easily do something like create a cronjob, which will then > faithfully be run with full capabilities, trivially bypassing > the restriction we place. > > IOW, our clearing of capabilities is both useless from a security > POV, and breaks valid use cases when people need to run as root. > > This removes the clear_emulator_capabilities configuration > option from qemu.conf, and always runs QEMU with capabilities > when root. The behaviour when non-root is unchanged. > > Signed-off-by: Daniel P. Berrangé > --- > docs/drvqemu.html.in | 46 +++--- > src/qemu/libvirtd_qemu.aug | 8 +- > src/qemu/qemu.conf | 11 --- > src/qemu/qemu_conf.c | 4 --- > src/qemu/qemu_conf.h | 1 - > src/qemu/qemu_domain.c | 3 +- > src/qemu/qemu_process.c| 5 > src/qemu/test_libvirtd_qemu.aug.in | 1 - > 8 files changed, 25 insertions(+), 54 deletions(-) > > diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in > index 294117ee1f..8beb28655c 100644 > --- a/docs/drvqemu.html.in > +++ b/docs/drvqemu.html.in > @@ -187,41 +187,29 @@ chmod o+x /path/to/directory > > > > -Linux process capabilities > - > > - The libvirt QEMU driver has a build time option allowing it to use > - the href="http://people.redhat.com/sgrubb/libcap-ng/index.html;>libcap-ng > - library to manage process capabilities. If this build option is > - enabled, then the QEMU driver will use this to ensure that all > - process capabilities are dropped before executing a QEMU virtual > - machine. Process capabilities are what gives the 'root' account > - its high power, in particular the CAP_DAC_OVERRIDE capability > - is what allows a process running as 'root' to access files owned > - by any user. > + The libvirt maintainers strongly recommend against > + running QEMU as the root user/group. This should not be required > + in most supported usage scenarios, as libvirt will generally do the > + right thing to grant QEMU access to files it is permitted to > + use when it is running non-root. > > > +Linux process capabilities > + > > - If the QEMU driver is configured to run virtual machines as non-root, > - then they will already lose all their process capabilities at time > - of startup. The Linux capability feature is thus aimed primarily at > - the scenario where the QEMU processes are running as root. In this > - case, before launching a QEMU virtual machine, libvirtd will use > - libcap-ng APIs to drop all process capabilities. It is important > - for administrators to note that this implies the QEMU process will > - only be able to access files owned by root, and > - not files owned by any other user. > + In versions of libvirt prior to 6.0.0, even if QEMU was configured > + to run as the root user / group, libvirt would strip all process > + capabilities. This meant that QEMU could only read/write files > + owned by root, or with open permissions. In reality, stripping > + capabilities did not have any security benefit, as it was trivial > + to get commands to run in another context with full capabilities, > + for example, by creating a cronjob. > > - > > - Thus, if a vendor / distributor has configured their libvirt package > - to run as 'qemu' by default, a number of changes will be required > - before an administrator can change a host to run guests as root. > - In particular it will be necessary to change ownership on the > - directories /var/run/libvirt/qemu/, > - /var/lib/libvirt/qemu/ and > - /var/cache/libvirt/qemu/ back to root, in addition
Re: [libvirt] [jenkins-ci PATCH v2 6/8] guests: Enable PowerTools repo on CentOS 8 guests
On Wed, Dec 11, 2019 at 1:33 PM Andrea Bolognani wrote:
>
> On Wed, 2019-12-11 at 13:26 +0100, Fabiano Fidêncio wrote:
> > And this is the fixup for this one:
> > ```
> > diff --git a/guests/lcitool b/guests/lcitool
> > index 059b789..c28c414 100755
> > --- a/guests/lcitool
> > +++ b/guests/lcitool
> > @@ -749,13 +749,23 @@ class Application:
> > {package_manager} clean all -y
> > """).format(**varmap))
> > elif os_name == "CentOS":
> > -sys.stdout.write(textwrap.dedent("""
> > -RUN {package_manager} update -y && \\
> > -{package_manager} install -y epel-release && \\
> > -{package_manager} install -y {pkgs} && \\
> > -{package_manager} autoremove -y && \\
> > -{package_manager} clean all -y
> > -""").format(**varmap))
> > +if os_version == "7":
> > +sys.stdout.write(textwrap.dedent("""
> > +RUN {package_manager} update -y && \\
> > +{package_manager} install -y epel-release && \\
> > +{package_manager} install -y {pkgs} && \\
> > +{package_manager} autoremove -y && \\
> > +{package_manager} clean all -y
> > +""").format(**varmap))
> > +else:
> > +sys.stdout.write(textwrap.dedent("""
> > +RUN {package_manager} update -y && \\
+{package_manager} install
'dnf-command(config-manager)' -y && \\
This is also needed: ^
> > +{package_manager} config-manager
> > --set-enabled PowerTools -y && \\
> > +{package_manager} install -y epel-release && \\
> > +{package_manager} install -y {pkgs} && \\
> > +{package_manager} autoremove -y && \\
> > +{package_manager} clean all -y
> > +""").format(**varmap))
> > else:
> > sys.stdout.write(textwrap.dedent("""
> > RUN {package_manager} update -y && \\
> >
> > ```
>
> Also looks good!
>
I'll push the patches when python3-flakes8 update reaches EPEL.
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] doc: vtpm only support secrets by UUID at this point
On 12/10/19 10:08 AM, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau Support by usage name can be considered separately (with a 'usage' attribute?). Cc: Stefan Berger Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Berger --- docs/formatsecret.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 8d0630a7c3..8f5383cf64 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -334,8 +334,8 @@ Secret value set of the vTPM. The usage type='vtpm' element must contain a single name element that specifies a usage name - for the secret. The vTPM secret can then be used by UUID or by - this usage name via the encryption element of + for the secret. The vTPM secret can then be used by UUID + via the encryption element of a tpm when using an emulator. Since 5.6.0. The following is an example -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 6/8] guests: Enable PowerTools repo on CentOS 8 guests
On Wed, 2019-12-11 at 13:26 +0100, Fabiano Fidêncio wrote:
> And this is the fixup for this one:
> ```
> diff --git a/guests/lcitool b/guests/lcitool
> index 059b789..c28c414 100755
> --- a/guests/lcitool
> +++ b/guests/lcitool
> @@ -749,13 +749,23 @@ class Application:
> {package_manager} clean all -y
> """).format(**varmap))
> elif os_name == "CentOS":
> -sys.stdout.write(textwrap.dedent("""
> -RUN {package_manager} update -y && \\
> -{package_manager} install -y epel-release && \\
> -{package_manager} install -y {pkgs} && \\
> -{package_manager} autoremove -y && \\
> -{package_manager} clean all -y
> -""").format(**varmap))
> +if os_version == "7":
> +sys.stdout.write(textwrap.dedent("""
> +RUN {package_manager} update -y && \\
> +{package_manager} install -y epel-release && \\
> +{package_manager} install -y {pkgs} && \\
> +{package_manager} autoremove -y && \\
> +{package_manager} clean all -y
> +""").format(**varmap))
> +else:
> +sys.stdout.write(textwrap.dedent("""
> +RUN {package_manager} update -y && \\
> +{package_manager} config-manager
> --set-enabled PowerTools -y && \\
> +{package_manager} install -y epel-release && \\
> +{package_manager} install -y {pkgs} && \\
> +{package_manager} autoremove -y && \\
> +{package_manager} clean all -y
> +""").format(**varmap))
> else:
> sys.stdout.write(textwrap.dedent("""
> RUN {package_manager} update -y && \\
>
> ```
Also looks good!
> We (Andrea and I) agree that this function has to be ungrossified
> sooner than later. But it'll be done in the future.
Absolutely fair.
--
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 v2 5/8] guests: Install EPEL repo on all CentOS guests
On Wed, 2019-12-11 at 13:25 +0100, Fabiano Fidêncio wrote:
> This is the fixup for this patch:
> ```
> diff --git a/guests/lcitool b/guests/lcitool
> index 803477e..059b789 100755
> --- a/guests/lcitool
> +++ b/guests/lcitool
> @@ -748,7 +748,7 @@ class Application:
> {package_manager} autoremove -y && \\
> {package_manager} clean all -y
> """).format(**varmap))
> -elif os_name == "CentOS" and os_version == "7":
> +elif os_name == "CentOS":
> sys.stdout.write(textwrap.dedent("""
> RUN {package_manager} update -y && \\
> {package_manager} install -y epel-release && \\
> ```
Looks good!
(Note that our mailing list doesn't support Markdown syntax ;)
--
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 v2 6/8] guests: Enable PowerTools repo on CentOS 8 guests
On Wed, Dec 11, 2019 at 10:32 AM Andrea Bolognani wrote:
>
> On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote:
> > +++ b/guests/playbooks/update/tasks/base.yml
> > +- name: Enable PowerTools repository
> > + command: '{{ package_manager }} config-manager --set-enabled PowerTools
> > -y'
> > + args:
> > +warn: no
> > + when:
> > +- os_name == 'CentOS'
> > +- os_version == '8'
>
> Same comment as the one I just made for EPEL: you need to make sure
> this line ends up in the generated Dockerfile too.
And this is the fixup for this one:
```
diff --git a/guests/lcitool b/guests/lcitool
index 059b789..c28c414 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -749,13 +749,23 @@ class Application:
{package_manager} clean all -y
""").format(**varmap))
elif os_name == "CentOS":
-sys.stdout.write(textwrap.dedent("""
-RUN {package_manager} update -y && \\
-{package_manager} install -y epel-release && \\
-{package_manager} install -y {pkgs} && \\
-{package_manager} autoremove -y && \\
-{package_manager} clean all -y
-""").format(**varmap))
+if os_version == "7":
+sys.stdout.write(textwrap.dedent("""
+RUN {package_manager} update -y && \\
+{package_manager} install -y epel-release && \\
+{package_manager} install -y {pkgs} && \\
+{package_manager} autoremove -y && \\
+{package_manager} clean all -y
+""").format(**varmap))
+else:
+sys.stdout.write(textwrap.dedent("""
+RUN {package_manager} update -y && \\
+{package_manager} config-manager
--set-enabled PowerTools -y && \\
+{package_manager} install -y epel-release && \\
+{package_manager} install -y {pkgs} && \\
+{package_manager} autoremove -y && \\
+{package_manager} clean all -y
+""").format(**varmap))
else:
sys.stdout.write(textwrap.dedent("""
RUN {package_manager} update -y && \\
```
We (Andrea and I) agree that this function has to be ungrossified
sooner than later. But it'll be done in the future.
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 5/8] guests: Install EPEL repo on all CentOS guests
On Wed, Dec 11, 2019 at 10:44 AM Fabiano Fidêncio wrote:
>
> On Wed, Dec 11, 2019 at 10:31 AM Andrea Bolognani wrote:
> >
> > On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote:
> > > +++ b/guests/playbooks/update/tasks/base.yml
> > > @@ -15,7 +15,6 @@
> > > state: latest
> > >when:
> > > - os_name == 'CentOS'
> > > -- os_version == '7'
> >
> > Welp, I just realized you need to apply the same change to the
> > _action_dockerfile() function in lcitool too... Up to you whether
> > to respin or fix Dockerfile generation in a follow-up patch.
>
> I will respin the series as it may need an extra patch between those
> this one and the PowerTools one.
Actually, I will not.
This is the fixup for this patch:
```
diff --git a/guests/lcitool b/guests/lcitool
index 803477e..059b789 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -748,7 +748,7 @@ class Application:
{package_manager} autoremove -y && \\
{package_manager} clean all -y
""").format(**varmap))
-elif os_name == "CentOS" and os_version == "7":
+elif os_name == "CentOS":
sys.stdout.write(textwrap.dedent("""
RUN {package_manager} update -y && \\
{package_manager} install -y epel-release && \\
```
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: prefer to use rst2html5 instead of rst2html
On Wed, 2019-12-11 at 11:07 +, Daniel P. Berrangé wrote:
> +++ b/m4/virt-external-programs.m4
> @@ -33,10 +33,13 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
>then
> AC_MSG_ERROR("xsltproc is required to build libvirt")
>fi
> - AC_PATH_PROGS([RST2HTML], [rst2html rst2html.py rst2html-3], [])
> +
> + dnl Drop the rst2html (aka HTML4) variants once we
Spurious double space here.
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] scripts: ignore remote protocol checks if pdwtags crashes
On Wed, 2019-12-11 at 11:00 +, Daniel P. Berrangé wrote: > On Wed, Dec 11, 2019 at 10:09:58AM +0100, Andrea Bolognani wrote: > > On Fri, 2019-12-06 at 17:18 +, Daniel P. Berrangé wrote: > > > On Debian 10, pdwtags reliably segfaults when parsing the libvirt remote > > > protocol files. > > > > Have you reported this bug? > > Nope, I asssumed it has already been reported since it has been affecting > libvirt for ages. Doesn't look like it: https://bugs.debian.org/dwarves -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: prefer to use rst2html5 instead of rst2html
Our website is written assuming HTML5 standard & doctype:
commit b1c81567c7172bc9dcd701cf46ea3f87725d62c7
Author: Daniel P. Berrangé
Date: Wed Jul 26 18:01:25 2017 +0100
docs: switch to using HTML5 doctype declaration
so we want the RST conversion to also use HTML5. Ubuntu 16.04 still
only has the HTML4 generating tools though, so we have that as a
fallback.
Signed-off-by: Daniel P. Berrangé
---
m4/virt-external-programs.m4 | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index ed634a4c73..aad21c81bf 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -33,10 +33,13 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
then
AC_MSG_ERROR("xsltproc is required to build libvirt")
fi
- AC_PATH_PROGS([RST2HTML], [rst2html rst2html.py rst2html-3], [])
+
+ dnl Drop the rst2html (aka HTML4) variants once we
+ dnl stop supporting Ubuntu 16.04 (Xenial)
+ AC_PATH_PROGS([RST2HTML], [rst2html5 rst2html5.py rst2html5-3 rst2html
rst2html.py rst2html-3], [])
if test -z "$RST2HTML"
then
-AC_MSG_ERROR("rst2html is required to build libvirt")
+AC_MSG_ERROR("rst2html5/rst2html is required to build libvirt")
fi
AC_PATH_PROG([AUGPARSE], [augparse], [/usr/bin/augparse])
AC_PROG_MKDIR_P
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] scripts: ignore remote protocol checks if pdwtags crashes
On Wed, Dec 11, 2019 at 10:09:58AM +0100, Andrea Bolognani wrote: > On Fri, 2019-12-06 at 17:18 +, Daniel P. Berrangé wrote: > > On Debian 10, pdwtags reliably segfaults when parsing the libvirt remote > > protocol files. > > Have you reported this bug? Nope, I asssumed it has already been reported since it has been affecting libvirt for ages. 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] docs: switch to use rst2html5 instead of rst2html
On Wed, Dec 11, 2019 at 10:16:28AM +0100, Andrea Bolognani wrote: > On Fri, 2019-12-06 at 17:50 +, Daniel P. Berrangé wrote: > > Now that we don't have to support CentOS 7's python2-docutils, we can > > use the rst2html5 tool instead, which generates HTML5 output instead > > of HTML4 output. > > Ubuntu 16.04 still has rst2html only, see > > https://packages.ubuntu.com/xenial/all/python3-docutils/filelist > > Can we just add the rst2html5 variants to the front of the list, so > that they will be picked up wherever available without breaking > Ubuntu 16.04? Yeah can do. 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/2] spec: Update Fedora minimum supported version
On Sat, Dec 07, 2019 at 04:43:36PM +0100, Fabiano Fidêncio wrote: > Fedora 29 has reached its end of life on November 26th 2019. > > For more info, please, refer to the following e-mail: > https://lists.fedoraproject.org/archives/list/devel-annou...@lists.fedoraproject.org/thread/ > > Signed-off-by: Fabiano Fidêncio > --- > libvirt.spec.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) 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 00/17] docs: remove all use of POD for man pages in favour of RST
On Tue, Dec 10, 2019 at 08:25:25PM -0500, Cole Robinson wrote: > On 12/6/19 9:50 AM, Daniel P. Berrangé wrote: > > As part of the goal to eliminate Perl from libvirt, this gets rid of the > > use of POD format for man pages. There's nothing especially bad about > > POD as a markup language compared to other lightweight markup languages > > like RST or Markdown. It hasn't found widespread usage outside of the > > Perl world though, and so switching from POD to RST brings a language > > which likely has more familiarity to contributors. > > > > This also nicely aligns with our use of RST of web pages, and indeed > > in this series things are setup so that all the man pages get published > > on the main libvirt.org website. Over time this will hopefuly draw > > search engines traffic to libvirt.org instead of random 3rd party > > websites hosting various out of date copies of the man pages. > > > > Reviewing the individual RST files is likely a very unpleasant task, > > especially the enourmous virsh man page. Most of the conversion work > > was automated with pod2rst, followed by lots of editting to cleanup > > its output. virsh had some further automated processing done to create > > headers for each command. > > > > It is probably more useful to review the rendered man page output > > and/or websites to see that it looks sane when read. > > > > Reviewed-by: Cole Robinson > > With some tweaks: > > * There's a leftover pod2man in the spec file. > * This conflicts with the virsh --tls-destination changes so don't > forget to re-merge those > * virt-host-validate patch needs this diff > > diff --git a/docs/Makefile.am b/docs/Makefile.am > index e1f8f7646d..4027c2e26c 100644 > --- a/docs/Makefile.am > +++ b/docs/Makefile.am > @@ -236,7 +236,7 @@ manpages_rst += \ >$(NULL) > endif ! WITH_LIBVIRTD > if WITH_HOST_VALIDATE > - manpages8_rst += manpages/virt-host-validate.rst > + manpages1_rst += manpages/virt-host-validate.rst > else ! WITH_HOST_VALIDATE >manpages_rst += manpages/virt-host-validate.rst > endif ! WITH_HOST_VALIDATE > > > Non-blocking stuff: > > The build process spits out noise like this now, but I didn't > investigate, maybe it was there before and I missed it: > > I/O error : Attempt to load network entity > http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd > manpages/virkeycode-osx.html.in:2: warning: failed to load external > entity "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd; > 1.0 Transitional//EN" > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd; That's fixed by this patch to use HTML5 instead of HTML4: https://www.redhat.com/archives/libvir-list/2019-December/msg00387.html > > > As for the format, there's some improvements and some worse things, most > are minor. The one place it's pretty ugly is virt-admin and virsh, with > the command format immediately after the command header. Old style looks > like: > > quit, exit > quit this interactive terminal > > Now it is: > >quit, exit > quit > exit > >quit this interactive terminal > > For larger commands it's better, so it's mostly noticeable at the start > of the document with the short commands. Maybe drop the shell section > entirely for short style commands, or maybe there's some other option, I > didn't look into it much It is a tradeoff between the man page formatting and the HTML. The first is a heading, linked from the ToC, the second gives the actual syntax & args. I don't think its worth changing just these few cases without args, but maybe make it clearer quit, exit syntax: quit syntax: exit quit this interactive terminal or something similar. > The other bit is nested Example: sections with ~ underline. It puts the > Example: at the same indent as the top level command, which is ugly and > tough to read. Just grep the rst for '' and see how it manifests in > the output manpage. I think those few sections can be replaced by > boldified text with *Example:* and it looks better, at least with the > man page, but I didn't review the HTML. It was done so that Example turns into a heading in the HTML, but I'l see if there's a way to make both look nice. 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/2] spec: Adjust librbd / librados dependency names
On Sat, Dec 07, 2019 at 04:43:37PM +0100, Fabiano Fidêncio wrote: > librbd1-devel and librados2-devel have their package name changed to > librbd-devel and librados-devel on all the supported Fedora versions and > CentOS / RHEL 8. > > For more info about this change, please, refer to the following page: > https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html-single/considerations_in_adopting_rhel_8/index > > Signed-off-by: Fabiano Fidêncio > --- > libvirt.spec.in | 5 + > 1 file changed, 5 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
[libvirt] [jenkins-ci PATCH] Update comment for virt-manager
We have Python 3 on CentOS 7 now, so the existing comment is no longer accurate. Signed-off-by: Andrea Bolognani --- Pushed as trivial. guests/playbooks/build/projects/virt-manager.yml | 4 +--- jenkins/projects/virt-manager.yaml | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/guests/playbooks/build/projects/virt-manager.yml b/guests/playbooks/build/projects/virt-manager.yml index c0d4294..9184a74 100644 --- a/guests/playbooks/build/projects/virt-manager.yml +++ b/guests/playbooks/build/projects/virt-manager.yml @@ -1,9 +1,7 @@ --- - set_fact: name: virt-manager -# virt-manager is Python 3 only, so it can't be built on CentOS 7; -# Ubuntu 16.04 has Python 3 but not the libxml2 bindings, so it can't -# build the project either +# CentOS 7 and Ubuntu 16.04 have Python 3 but not the libxml2 bindings machines: - libvirt-debian-9 - libvirt-debian-10 diff --git a/jenkins/projects/virt-manager.yaml b/jenkins/projects/virt-manager.yaml index 3dc8e2e..4524831 100644 --- a/jenkins/projects/virt-manager.yaml +++ b/jenkins/projects/virt-manager.yaml @@ -1,9 +1,7 @@ --- - project: name: virt-manager -# virt-manager is Python 3 only, so it can't be built on CentOS 7; -# Ubuntu 16.04 has Python 3 but not the libxml2 bindings, so it can't -# build the project either +# CentOS 7 and Ubuntu 16.04 have Python 3 but not the libxml2 bindings machines: - libvirt-debian-9 - libvirt-debian-10 -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 5/8] guests: Install EPEL repo on all CentOS guests
On Wed, Dec 11, 2019 at 10:31 AM Andrea Bolognani wrote: > > On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote: > > +++ b/guests/playbooks/update/tasks/base.yml > > @@ -15,7 +15,6 @@ > > state: latest > >when: > > - os_name == 'CentOS' > > -- os_version == '7' > > Welp, I just realized you need to apply the same change to the > _action_dockerfile() function in lcitool too... Up to you whether > to respin or fix Dockerfile generation in a follow-up patch. I will respin the series as it may need an extra patch between those this one and the PowerTools one. Nice catch! Best Regards, -- Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/1] qemu: fix concurrency crash bug in snapshot revert
On 12/10/19 11:36 AM, Pavel Mores wrote: This commit aims to fix https://bugzilla.redhat.com/show_bug.cgi?id=1610207 The cause was apparently incorrect handling of jobs in snapshot revert code which allowed a thread executing snapshot delete to begin job while snapshot revert was still running on another thread. The snapshot delete thread then waited on a condition variable in qemuMonitorSend() while the revert thread finished, changing (and effectively corrupting) the qemuMonitor structure under the delete thread which led to its crash. The incorrect handling of jobs in revert code was due to the fact that although qemuDomainRevertToSnapshot() correctly begins a job at the start, the job was implicitly ended when qemuProcessStop() was called because the job lives in the QEMU driver's private data (qemuDomainObjPrivate) that was purged during qemuProcessStop(). This fix prevents qemuProcessStop() from clearing jobs as the idea of qemuProcessStop() clearing jobs seems wrong in the first place. It was (inadvertently) introduced in commit 888aa4b6b9db65e3db273341e79846, which is effectively reverted by the second hunk of this commit. To preserve the desired effects of the faulty commit, the first hunk is included as suggested by Michal. Signed-off-by: Pavel Mores --- Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 5/8] guests: Install EPEL repo on all CentOS guests
On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote: > +++ b/guests/playbooks/update/tasks/base.yml > @@ -15,7 +15,6 @@ > state: latest >when: > - os_name == 'CentOS' > -- os_version == '7' Welp, I just realized you need to apply the same change to the _action_dockerfile() function in lcitool too... Up to you whether to respin or fix Dockerfile generation in a follow-up patch. -- 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 v2 6/8] guests: Enable PowerTools repo on CentOS 8 guests
On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote:
> +++ b/guests/playbooks/update/tasks/base.yml
> +- name: Enable PowerTools repository
> + command: '{{ package_manager }} config-manager --set-enabled PowerTools -y'
> + args:
> +warn: no
> + when:
> +- os_name == 'CentOS'
> +- os_version == '8'
Same comment as the one I just made for EPEL: you need to make sure
this line ends up in the generated Dockerfile too.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: switch to use rst2html5 instead of rst2html
On Fri, 2019-12-06 at 17:50 +, Daniel P. Berrangé wrote: > Now that we don't have to support CentOS 7's python2-docutils, we can > use the rst2html5 tool instead, which generates HTML5 output instead > of HTML4 output. Ubuntu 16.04 still has rst2html only, see https://packages.ubuntu.com/xenial/all/python3-docutils/filelist Can we just add the rst2html5 variants to the front of the list, so that they will be picked up wherever available without breaking Ubuntu 16.04? -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] scripts: ignore remote protocol checks if pdwtags crashes
On Fri, 2019-12-06 at 17:18 +, Daniel P. Berrangé wrote: > On Debian 10, pdwtags reliably segfaults when parsing the libvirt remote > protocol files. Have you reported this bug? -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] spec: Update Fedora minimum version & librbd / librados development package names
On Sat, Dec 7, 2019 at 4:43 PM Fabiano Fidêncio wrote: > > Fedora 29 has reached its EOL on November 26th 2019; > librbd1-devel has been replaced by librbd-devel; > librados2-devel has been replaced by librados; > > Fabiano Fidêncio (2): > spec: Update Fedora minimum supported version > spec: Adjust librbd / librados dependency names > > libvirt.spec.in | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > -- > 2.23.0 > ping? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 4/4] qemu: block: enable the snapshot image deletion feature
On Wed, Dec 11, 2019 at 08:34:38AM +0100, Peter Krempa wrote:
> On Tue, Dec 10, 2019 at 17:25:41 +0100, Pavel Mores wrote:
> > With all plumbing in place, we can now enable the new functionality.
> >
> > Signed-off-by: Pavel Mores
> > ---
> > src/qemu/qemu_driver.c | 12 ++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 9c07b6b393..0cbc746c22 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -18544,10 +18544,10 @@ qemuDomainBlockCommit(virDomainPtr dom,
> > bool persistjob = false;
> > bool blockdev = false;
> >
> > -/* XXX Add support for COMMIT_DELETE */
> > virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
> >VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
> >VIR_DOMAIN_BLOCK_COMMIT_RELATIVE |
> > + VIR_DOMAIN_BLOCK_COMMIT_DELETE |
> >VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, -1);
> >
> > if (!(vm = qemuDomainObjFromDomain(dom)))
> > @@ -18568,6 +18568,13 @@ qemuDomainBlockCommit(virDomainPtr dom,
> >
> > blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
> >
> > +if (!blockdev && (flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE)) {
> > +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> > + _("deleting committed images is only supported for
> > VMs "
> > + "with blockdev enabled"));
> > +goto endjob;
> > +}
>
> make[1]: Leaving directory '/home/pipo/build/libvirt/gcc/src'
> /home/pipo/libvirt/src/qemu/qemu_driver.c:18572:
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> /home/pipo/libvirt/src/qemu/qemu_driver.c-18573-
> _("deleting committed images is only supported for VMs "
> /home/pipo/libvirt/src/qemu/qemu_driver.c-18574-
> "with blockdev enabled"));
> build-aux/syntax-check.mk: found diagnostic without %
> make: *** [/home/pipo/libvirt/build-aux/syntax-check.mk:756:
> sc_prohibit_diagnostic_without_format] Error 1
>
> Please make sure you run 'make syntax-check' before submitting patches.
>
> I'll fix this before pushing with with:
>
> if (!blockdev && (flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE)) {
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>_("deleting committed images is not supported by this
> VM"));
> goto endjob;
> }
>
Okay, my apologies, and thanks for the speedy review!
pvl
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
