Re: [libvirt] [PATCH 1/5] util, resctrl: using 64bit interface instead of 32bit for counters

2019-12-16 Thread Wang Huaqiang


On 2019/12/16 下午6:12, Daniel P. Berrangé wrote:

On Sun, Dec 15, 2019 at 02:39:20AM +, Wang, Huaqiang wrote:

Hi Daniel,

Thanks for your review.

About patch 1/5, I understand we should be very cautious when we changes  the 
determined
interface.

I'd like to reserved the old 32bit interface  and propose a new 64bit interface 
just as you
suggested if you still think so after you  got my intention.
Here actually I want to fix a bug, maybe I should title this patch as *bug 
fixing*. Reason is the
underlying hardware, the cache monitor and memory bandwidth counters, are 64bit 
width.
Using 32bit interface to access these counters are problematic. This bug is not 
found because
this interface is only used for tracking the amount of cache that used before 
this patch, normally
the occupied cache will not exceed 4GB range. (32bit counter can counter value 
up to 4GB).
But for memory, this counter records the data passing through the memory 
controller issued
by this CPU in bytes and accumulatively, this value can easily exceed the 4GB 
bound, so I
don’t want to reserve the old 32 bit interface and let user use it, because it 
will report incorrect value.

We simply have to document the limitati onof the old interface. We can
*NOT* change it, because it WILL break API compatibility for apps that
deserailize the current data.



Got. I'll resend new patches for review.


Thanks

Huaqiang





Regards,
Daniel



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Daniel Henrique Barboza




On 12/16/19 9:48 PM, Alex Williamson wrote:

On Mon, 16 Dec 2019 21:09:20 -0300

Yes, but libvirt should not assume that it can manipulate the bindings
of adjacent devices without being explicitly directed to do so.  The
error may be a hindrance to you, but it might also prevent, for
example, the only other NIC in the system being detached from the host
driver.  Is it worth making the VM run without explicitly listing all
devices to assign at the cost of disrupting host services or subverting
the additional isolation a user might be attempting to configure with
having unused devices bound to vfio-pci.  This seems like a bad idea,
the VM should be configured to explicitly list every device it needs to
have assigned or partially assigned.  Thanks,



Thanks Alex. I know what's going wrong with this patch series after your 
messages and
a close inspection of what Libvirt is already doing. Libvirt does a sanity
check for PCI endpoint devices before assigning them to vfio-pci, but the
new detection code I am adding isn't. The result is that Cole can't run his
VM because this new detection code is demanding that a PCI Bridge, that belongs
to the same IOMMU of the devices Cole wants to passthrough, be assigned
to vfio-pci. Which is wrong.


I'll re-send this series fixing that.


Thanks,


DHB



Alex



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Alex Williamson
On Mon, 16 Dec 2019 21:09:20 -0300
Daniel Henrique Barboza  wrote:

> On 12/16/19 8:43 PM, Alex Williamson wrote:
> > On Mon, 16 Dec 2019 20:24:56 -0300
> > Daniel Henrique Barboza  wrote:
> >   
> >>
> >> The code isn't forcing a device to be assigned to the guest. It is forcing
> >> all the IOMMU devices to be declared in the domain XML to be detached from
> >> the host.  
> > 
> > Detached from the host by unbinding from host drivers and binding to
> > vfio-pci and "partially" assigned to the guest?  That's wrong, we can't
> > do that.  Not only will vfio-pci not bind to anything but endpoints,
> > you'll break the host binding bridges that might be part of the group,
> > and there are valid use cases for sequestering a device with pci-stub
> > rather than vfio-pci to add another barrier to the user getting access
> > to the device.
> > 
> >> What I did was to extend a verification Libvirt already does, to check for
> >> PCI devices of the same IOMMU X being used by other domains, to check the
> >> the host as well. Guest start fails if there is any device left in IOMMU X
> >> that's not present in the domain.  
> > 
> > Yep, can't do that.  
> 
> 
> Thanks for the info.
> 
> To keep the discussion focused, this is the error I'm trying to dodge:
> 
> error: internal error: qemu unexpectedly closed the monitor: 
> 2019-10-04T12:39:41.091312Z qemu-system-ppc64: -device 
> vfio-pci,host=0001:09:00.3,id=hostdev0,bus=pci.2.0,addr=0x1.0x3:
> vfio 0001:09:00.3: group 1 is not viable
> Please ensure all devices within the iommu_group are bound to their vfio bus 
> driver.
> 
> This happens when not all PCI devices from IOMMU group 1 are bind to 
> vfio_pci, regardless
> of whether QEMU is going to use all of them in the guest. Binding all the 
> IOMMU
> devices to vfio-pci makes QEMU satisfied, in this particular case.
> 
> What is the minimal condition to avoid this error? What Libvirt is doing ATM 
> is not enough
> (it will fail to launch with this QEMU error above), and what I'm proposing 
> is wrong.
> Can we say that all PCI endpoints of the same IOMMU must be assigned to 
> vfio-pci?

Yes, but libvirt should not assume that it can manipulate the bindings
of adjacent devices without being explicitly directed to do so.  The
error may be a hindrance to you, but it might also prevent, for
example, the only other NIC in the system being detached from the host
driver.  Is it worth making the VM run without explicitly listing all
devices to assign at the cost of disrupting host services or subverting
the additional isolation a user might be attempting to configure with
having unused devices bound to vfio-pci.  This seems like a bad idea,
the VM should be configured to explicitly list every device it needs to
have assigned or partially assigned.  Thanks,

Alex

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Daniel Henrique Barboza




On 12/16/19 8:43 PM, Alex Williamson wrote:

On Mon, 16 Dec 2019 20:24:56 -0300
Daniel Henrique Barboza  wrote:



The code isn't forcing a device to be assigned to the guest. It is forcing
all the IOMMU devices to be declared in the domain XML to be detached from
the host.


Detached from the host by unbinding from host drivers and binding to
vfio-pci and "partially" assigned to the guest?  That's wrong, we can't
do that.  Not only will vfio-pci not bind to anything but endpoints,
you'll break the host binding bridges that might be part of the group,
and there are valid use cases for sequestering a device with pci-stub
rather than vfio-pci to add another barrier to the user getting access
to the device.
  

What I did was to extend a verification Libvirt already does, to check for
PCI devices of the same IOMMU X being used by other domains, to check the
the host as well. Guest start fails if there is any device left in IOMMU X
that's not present in the domain.


Yep, can't do that.



Thanks for the info.

To keep the discussion focused, this is the error I'm trying to dodge:

error: internal error: qemu unexpectedly closed the monitor: 
2019-10-04T12:39:41.091312Z qemu-system-ppc64: -device 
vfio-pci,host=0001:09:00.3,id=hostdev0,bus=pci.2.0,addr=0x1.0x3:
vfio 0001:09:00.3: group 1 is not viable
Please ensure all devices within the iommu_group are bound to their vfio bus 
driver.

This happens when not all PCI devices from IOMMU group 1 are bind to vfio_pci, 
regardless
of whether QEMU is going to use all of them in the guest. Binding all the IOMMU
devices to vfio-pci makes QEMU satisfied, in this particular case.

What is the minimal condition to avoid this error? What Libvirt is doing ATM is 
not enough
(it will fail to launch with this QEMU error above), and what I'm proposing is 
wrong.
Can we say that all PCI endpoints of the same IOMMU must be assigned to 
vfio-pci?


Thanks,

DHB

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Alex Williamson
On Mon, 16 Dec 2019 20:24:56 -0300
Daniel Henrique Barboza  wrote:

> On 12/16/19 8:06 PM, Alex Williamson wrote:
> > On Mon, 16 Dec 2019 17:28:28 -0500
> > Cole Robinson  wrote:
> >   
> >> On 12/16/19 8:36 AM, Daniel Henrique Barboza wrote:  
> >>> changes from version 3 [1]:  
> > 
> > Thanks for catching this!  PCIe root ports or bridges being part of an
> > IOMMU group is part of the nature of the grouping.  However, only
> > endpoint devices can be bound to vfio-pci and thus participate in this
> > "partial assignment".  If the code is trying to force all other devices
> > in the IOMMU group that aren't assigned into this partial assignment
> > mode, that's just fundamentally broken.  Thanks,  
> 
> The code isn't forcing a device to be assigned to the guest. It is forcing
> all the IOMMU devices to be declared in the domain XML to be detached from
> the host.

Detached from the host by unbinding from host drivers and binding to
vfio-pci and "partially" assigned to the guest?  That's wrong, we can't
do that.  Not only will vfio-pci not bind to anything but endpoints,
you'll break the host binding bridges that might be part of the group,
and there are valid use cases for sequestering a device with pci-stub
rather than vfio-pci to add another barrier to the user getting access
to the device.
 
> What I did was to extend a verification Libvirt already does, to check for
> PCI devices of the same IOMMU X being used by other domains, to check the
> the host as well. Guest start fails if there is any device left in IOMMU X
> that's not present in the domain.

Yep, can't do that.

> In short, the code is implying that all IOMMU devices must be detached from
> the host, regardless of whether they're going to be used in the guest,
> regardless of whether they are PCI root ports or bridges. Is this assumption
> correct, considering kernel/QEMU?

Nope, please don't do this.  Thanks,

Alex

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain

2019-12-16 Thread Cole Robinson
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
> (series based on master commit 97cafa610ecf5)
> 
> This work was proposed by Cole in [1]. This is Cole's reasoning for
> it, copy/pasted from [1]:
> 
> ---
> The benefits of moving to validate time is that XML is rejected by
> 'virsh define' rather than at 'virsh start' time. It also makes it easier
> to follow the cli building code, and makes it easier to verify qemu_command.c
> test suite code coverage for the important stuff like covering every CLI
> option. It's also a good intermediate step for sharing validation with
> domain capabilities building, like is done presently for rng models.
> ---
> 
> Cole also mentioned that the machine features validation was a good
> place to start. I took it a step further and did it across the
> board on all qemu_command.c.
> 
> This series didn't create any new validation, just moved them
> to domain define time. Any other outcome is unintended.
> 
> 
> Not all cases where covered in this work. For a first version
> I decided to do only the most trivial cases. These are the
> other cases I left out for another day:
> 
> - all verifications contained in functions that are also called
> by qemu_hotplug.c. This means that the hotplug process will also
> use the validation code, and we can't just move it to qemu_domain.c.
> Besides, not all validation done in domain define time applies
> to hotplug, so it's not simply a matter of calling the same function
> from qemu_domain in qemu_hotplug. I have patches that moved the
> verification of all virtio options to qemu_domain.c, while also
> considering qemu_hotplug validation. In the end I decided to leave
> it away from this work for now because (1) it took 8 patches just
> for virtio case alone and (2) there are a lot of these cases in
> qemu_command.c and it would be too much to do it all at in this
> same series.
> 
> - moving CPU Model validation is trickier than the rest because
> there are code in DefPostParse() that makes CPU Model changes that
> are then validated in qemu_command.c. Moving the validation to define
> time doesn't cut in this case - the validation is considering
> PostParse changes in the CPU Model and some of the will fail if
> done by qemuDomainDefValidate time. I didn't think too much about
> how to handle this case, but given that the approach would be
> different from the other cases handled here, I left it out too.
> 
> - SHMem: part of SHMem validation is being used by hotplug code.
> I could've moved the non-hotplug validation to define time,
> instead I decided it's better to to leave it to do it all at
> once in the future.
> 
> - DBus-VMState: validation of this fella must be done by first
> querying if the hash vm->priv->dbusVMState has elements.
> qemuDomainDefValidate does not have 'vm->priv' in it's API,
> meaning that I would need to either change the API to include
> it (which means changing domainValidateCallback), or do the
> validation by another branch where I can get access to vm->priv.
> 
> - vmcoreinfo: there are no challenges in moving vmcoreinfo
> validation to qemuDomainDefValidate. The problem were the
> unit tests. Moving this validation to domain define time
> breaks 925 tests on qemuxml2xmltest.c, including tests labelled
> as 'minimal' which should pass under any circurstances, as
> far as I understand. It is possible to just shoehorn
> QEMU_CAPS_DEVICE_VMCOREINFO in the qemuCaps of all tests, but
> this would tamper the idea of having to deal with NONE or
> LATEST or a specific set of capabilities for certain tests.
> Another case I would rather discuss separately.
> 
> 
> [1] https://www.redhat.com/archives/libvir-list/2019-October/msg00568.html
> 
> Daniel Henrique Barboza (26):
>   qemu_command.c: move PSeries features validation to qemu_domain.c
>   qemu_command.c: move mem.nosharepages validation to qemu_domain.c
>   qemu_command.c: move validation of vmport to qemu_domain.c
>   qemu_command.c: move nvdimm validation to qemu_domain.c
>   qemu_command.c: move I/O APIC validation to qemu_domain.c
>   numa_conf: add virDomainNumaNodesDistancesAreBeingSet() helper
>   qemu_command.c move NUMA validation to qemu_domain.c
>   qemu_command.c: move NVRAM validation to qemu_domain.c
>   qemu_command: move qemuBuildSoundDevStr caps validation to qemu_domain
>   qemu_command.c: move sound codec validation to qemu_domain.c
>   qemu_command: move qemuBuildHubDevStr caps validation to qemu_domain
>   qemu_command: move qemuBuildChrChardevStr caps validation to
> qemu_domain
>   qemu: move qemuBuildHostdevCommandLine caps validation to qemu_domain
>   qemu_command.c: move vmGenID validation to qemu_domain.c
>   qemu: move qemuBuildSgaCommandLine validation to qemu_domain.c
>   qemu: move virDomainClockDef validation to qemu_domain.c
>   qemu: move qemuBuildPMCommandLine validation to qemu_domain.c
>   qemu: move qemuBuildBootCommandLine validation to qemu_domain.c
>   qemu_command.c: move pcihole64 validation to 

Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Daniel Henrique Barboza




On 12/16/19 8:06 PM, Alex Williamson wrote:

On Mon, 16 Dec 2019 17:28:28 -0500
Cole Robinson  wrote:


On 12/16/19 8:36 AM, Daniel Henrique Barboza wrote:

changes from version 3 [1]:


Thanks for catching this!  PCIe root ports or bridges being part of an
IOMMU group is part of the nature of the grouping.  However, only
endpoint devices can be bound to vfio-pci and thus participate in this
"partial assignment".  If the code is trying to force all other devices
in the IOMMU group that aren't assigned into this partial assignment
mode, that's just fundamentally broken.  Thanks,


The code isn't forcing a device to be assigned to the guest. It is forcing
all the IOMMU devices to be declared in the domain XML to be detached from
the host.

What I did was to extend a verification Libvirt already does, to check for
PCI devices of the same IOMMU X being used by other domains, to check the
the host as well. Guest start fails if there is any device left in IOMMU X
that's not present in the domain.

In short, the code is implying that all IOMMU devices must be detached from
the host, regardless of whether they're going to be used in the guest,
regardless of whether they are PCI root ports or bridges. Is this assumption
correct, considering kernel/QEMU?


Thanks,


DHB





Alex



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Daniel Henrique Barboza




On 12/16/19 7:28 PM, Cole Robinson wrote:

On 12/16/19 8:36 AM, Daniel Henrique Barboza wrote:



I've attached the nodedev XML for the three devices with iommuGroup 1.
Only the two nvidia devices are assigned to my VM, but not the PCIe
controller device.


Just noticed the attached file. This might indicate that there's a bug
in patch 3 that is wrongly assuming that there are more IOMMU devices
in group 1 than the ones you declared.


I'll look into it. The error message could also be more helpful, declaring
which PCI hostdev it thinks it should be declared in the XML.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Alex Williamson
On Mon, 16 Dec 2019 17:28:28 -0500
Cole Robinson  wrote:

> On 12/16/19 8:36 AM, Daniel Henrique Barboza wrote:
> > changes from version 3 [1]:
> > - removed last 2 patches that made function 0 of 
> > PCI multifunction devices mandatory
> > - new patch: news.xml update
> > - changed 'since' version to 6.0.0 in patch 4
> > - unassigned hostdevs are now getting qemu aliases
> > 
> > [1] https://www.redhat.com/archives/libvir-list/2019-November/msg01263.html
> > 
> > Daniel Henrique Barboza (5):
> >   Introducing new address type='unassigned' for PCI hostdevs
> >   qemu: handle unassigned PCI hostdevs in command line
> >   virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices
> >   formatdomain.html.in: document 
> >   news.xml: add address type='unassigned' entry
> >   
> 
> Codewise it looks fine now. But I'm looking more closely at patch #3 and
> realizing that it can explicitly reject a previously accepted VM config.
> And indeed, now that I give it a test with my GPU passthrough setup, it
> is rejecting my previosly working config.
> 
> error: Requested operation is not valid: All devices of the same IOMMU
> group 1 of the PCI device :01:00.0 must belong to domain win10
> 
> I've attached the nodedev XML for the three devices with iommuGroup 1.
> Only the two nvidia devices are assigned to my VM, but not the PCIe
> controller device.
> 
> Is the libvirt heuristic missing something? Or is this acting as expected?
> 
> I didn't quite gather that this is a change to reject previously
> accepted configurations, so I will defer to Laine and Alex as to whether
> this should be committed.

Thanks for catching this!  PCIe root ports or bridges being part of an
IOMMU group is part of the nature of the grouping.  However, only
endpoint devices can be bound to vfio-pci and thus participate in this
"partial assignment".  If the code is trying to force all other devices
in the IOMMU group that aren't assigned into this partial assignment
mode, that's just fundamentally broken.  Thanks,

Alex

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Daniel Henrique Barboza




On 12/16/19 7:28 PM, Cole Robinson wrote:

On 12/16/19 8:36 AM, Daniel Henrique Barboza wrote:

changes from version 3 [1]:
- removed last 2 patches that made function 0 of
PCI multifunction devices mandatory
- new patch: news.xml update
- changed 'since' version to 6.0.0 in patch 4
- unassigned hostdevs are now getting qemu aliases

[1] https://www.redhat.com/archives/libvir-list/2019-November/msg01263.html

Daniel Henrique Barboza (5):
   Introducing new address type='unassigned' for PCI hostdevs
   qemu: handle unassigned PCI hostdevs in command line
   virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices
   formatdomain.html.in: document 
   news.xml: add address type='unassigned' entry



Codewise it looks fine now. But I'm looking more closely at patch #3 and
realizing that it can explicitly reject a previously accepted VM config.
And indeed, now that I give it a test with my GPU passthrough setup, it
is rejecting my previosly working config.

error: Requested operation is not valid: All devices of the same IOMMU
group 1 of the PCI device :01:00.0 must belong to domain win10

I've attached the nodedev XML for the three devices with iommuGroup 1.
Only the two nvidia devices are assigned to my VM, but not the PCIe
controller device.

Is the libvirt heuristic missing something? Or is this acting as expected?


You mentioned that you declared 3 devices of IOMMU group 1. Unless the code in
patch 3 has a bug, there are more PCI hostdevs in IOMMU group 1 that were left
out of the domain XML.




I didn't quite gather that this is a change to reject previously
accepted configurations, so I will defer to Laine and Alex as to whether
this should be committed.



I mentioned in the commit msg of patch 03 that this would break working
configurations that didn't comply with the new 'all devices of the IOMMU
group must be included in the domain XML' directive. Perhaps this is worth
mentioning in the 'news' page to warn users about it.

About breaking existing configurations, there is the possibility of not
going forward with patch 03, which is enforcing this rule of declaring all the
IOMMU group. Existing domains will keep working as usual, the option to
unassign devices will still be present, but the user will have to deal with
the potential QEMU errors if not all PCI devices were detached from the host.

In this case, the 'unassigned' type will become more of a ON/OFF switch to
add/remove the PCI hostdev from the guest without removing it from the
domain XML. It is still useful, but we lose the idea of all the IOMMU
devices being described in the domain XML, which is something Laine
mentioned it would be desirable in one of the RFCs.


Thanks,


DHB




- Cole



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v1 24/26] qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c

2019-12-16 Thread Cole Robinson
On 12/16/19 5:46 PM, Cole Robinson wrote:
> On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
>> Move smartcard validation being done by qemuBuildSmartcardCommandLine()
>> to the existing qemuDomainSmartcardDefValidate() function. This
>> function is called by qemuDomainDeviceDefValidate(), allowing smartcard
>> validation in domain define time.
>>
>> Tests were adapted to consider the new caps being needed in
>> this earlier stage.
>>
>> Signed-off-by: Daniel Henrique Barboza 
>> ---
>>  src/qemu/qemu_command.c | 24 
>>  src/qemu/qemu_domain.c  | 37 +
>>  tests/qemuxml2xmltest.c | 16 +---
>>  3 files changed, 46 insertions(+), 31 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index efc70d6de9..116ed45a12 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -8271,24 +8271,10 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr 
>> logManager,
>>  
>>  switch (smartcard->type) {
>>  case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
>> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
>> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> -   _("this QEMU binary lacks smartcard host "
>> - "mode support"));
>> -return -1;
>> -}
>> -
>>  virBufferAddLit(, "ccid-card-emulated,backend=nss-emulated");
>>  break;
>>  
>>  case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
>> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
>> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> -   _("this QEMU binary lacks smartcard host "
>> - "mode support"));
>> -return -1;
>> -}
>> -
>>  virBufferAddLit(, "ccid-card-emulated,backend=certificates");
>>  for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
>>  virBufferAsprintf(, ",cert%zu=", i + 1);
>> @@ -8304,13 +8290,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr 
>> logManager,
>>  break;
>>  
>>  case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
>> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) {
>> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> -   _("this QEMU binary lacks smartcard "
>> - "passthrough mode support"));
>> -return -1;
>> -}
>> -
>>  if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
>>cmd, cfg, def,
>>smartcard->data.passthru,
>> @@ -8326,9 +8305,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr 
>> logManager,
>>  break;
>>  
>>  default:
>> -virReportError(VIR_ERR_INTERNAL_ERROR,
>> -   _("unexpected smartcard type %d"),
>> -   smartcard->type);
>>  return -1;
> 
> This still returns -1. If somehow we hit this condition, the startup
> will fail but no error will be reported. I think just 'return 0;' here
> instead

Actually virReportEnumRangeError seems like the common pattern in this case

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v1 24/26] qemu: move qemuBuildSmartcardCommandLine validation to qemu_domain.c

2019-12-16 Thread Cole Robinson
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
> Move smartcard validation being done by qemuBuildSmartcardCommandLine()
> to the existing qemuDomainSmartcardDefValidate() function. This
> function is called by qemuDomainDeviceDefValidate(), allowing smartcard
> validation in domain define time.
> 
> Tests were adapted to consider the new caps being needed in
> this earlier stage.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  src/qemu/qemu_command.c | 24 
>  src/qemu/qemu_domain.c  | 37 +
>  tests/qemuxml2xmltest.c | 16 +---
>  3 files changed, 46 insertions(+), 31 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index efc70d6de9..116ed45a12 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8271,24 +8271,10 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr 
> logManager,
>  
>  switch (smartcard->type) {
>  case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("this QEMU binary lacks smartcard host "
> - "mode support"));
> -return -1;
> -}
> -
>  virBufferAddLit(, "ccid-card-emulated,backend=nss-emulated");
>  break;
>  
>  case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("this QEMU binary lacks smartcard host "
> - "mode support"));
> -return -1;
> -}
> -
>  virBufferAddLit(, "ccid-card-emulated,backend=certificates");
>  for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
>  virBufferAsprintf(, ",cert%zu=", i + 1);
> @@ -8304,13 +8290,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr 
> logManager,
>  break;
>  
>  case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("this QEMU binary lacks smartcard "
> - "passthrough mode support"));
> -return -1;
> -}
> -
>  if (!(devstr = qemuBuildChrChardevStr(logManager, secManager,
>cmd, cfg, def,
>smartcard->data.passthru,
> @@ -8326,9 +8305,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr 
> logManager,
>  break;
>  
>  default:
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> -   _("unexpected smartcard type %d"),
> -   smartcard->type);
>  return -1;

This still returns -1. If somehow we hit this condition, the startup
will fail but no error will be reported. I think just 'return 0;' here
instead

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v1 23/26] qemu: move qemuBuildGraphicsEGLHeadlessCommandLine validation to qemu_domain.c

2019-12-16 Thread Cole Robinson
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
> Move EGL Headless validation from qemuBuildGraphicsEGLHeadlessCommandLine()
> to qemuDomainDeviceDefValidateGraphics(). This function is called by
> qemuDomainDefValidate(), validating the graphics parameters in domain
> define time.
> 
> Tests were adapted to consider validation in this earlier stage.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  src/qemu/qemu_command.c| 10 +-
>  src/qemu/qemu_domain.c |  7 +++
>  tests/qemuxml2argvdata/graphics-egl-headless.args  |  2 +-
>  .../qemuxml2argvdata/graphics-spice-egl-headless.args  |  2 +-
>  tests/qemuxml2argvdata/graphics-vnc-egl-headless.args  |  2 +-
>  tests/qemuxml2argvtest.c   |  9 ++---
>  tests/qemuxml2xmltest.c|  6 --
>  7 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index aceb42a289..efc70d6de9 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7729,7 +7729,6 @@ 
> qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>  static int
>  qemuBuildGraphicsEGLHeadlessCommandLine(virQEMUDriverConfigPtr cfg 
> G_GNUC_UNUSED,
>  virCommandPtr cmd,
> -virQEMUCapsPtr qemuCaps,
>  virDomainGraphicsDefPtr graphics)
>  {
>  g_auto(virBuffer) opt = VIR_BUFFER_INITIALIZER;
> @@ -7737,13 +7736,6 @@ 
> qemuBuildGraphicsEGLHeadlessCommandLine(virQEMUDriverConfigPtr cfg 
> G_GNUC_UNUSED
>  virBufferAddLit(, "egl-headless");
>  
>  if (graphics->data.egl_headless.rendernode) {
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_EGL_HEADLESS_RENDERNODE)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("This QEMU doesn't support OpenGL rendernode "
> - "with egl-headless graphics type"));
> -return -1;
> -}
> -
>  virBufferAddLit(, ",rendernode=");
>  virQEMUBuildBufferEscapeComma(,
>
> graphics->data.egl_headless.rendernode);
> @@ -7788,7 +7780,7 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
>  break;
>  case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS:
>  if (qemuBuildGraphicsEGLHeadlessCommandLine(cfg, cmd,
> -qemuCaps, graphics) 
> < 0)
> +graphics) < 0)
>  return -1;
>  
>  break;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9f29d2afbe..415f0916a2 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7717,6 +7717,13 @@ qemuDomainDeviceDefValidateGraphics(const 
> virDomainGraphicsDef *graphics,
>  break;
>  
>  case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS:
> +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_EGL_HEADLESS_RENDERNODE)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("This QEMU doesn't support OpenGL rendernode "
> + "with egl-headless graphics type"));
> +return -1;
> +}
> +

This looks like it is missing the associated check for
graphics->data.egl_headless.rendernode, like the original check had. I
wouldn't expect the test .args output later to change

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v1 04/26] qemu_command.c: move nvdimm validation to qemu_domain.c

2019-12-16 Thread Cole Robinson
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote:
> QEMU_CAPS_DEVICE_NVDIMM validation is now being done inside
> qemuDomainDefValidateMemory().
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  src/qemu/qemu_command.c |  5 -
>  src/qemu/qemu_domain.c  | 10 ++
>  tests/qemuxml2xmltest.c |  2 +-
>  3 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index e7365ba86a..25886bf49a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7180,11 +7180,6 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
>  
>  for (i = 0; i < def->nmems; i++) {
>  if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM) {
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("nvdimm isn't supported by this QEMU 
> binary"));
> -return -1;
> -}
>  virBufferAddLit(, ",nvdimm=on");
>  break;
>  }
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index d62e13f26c..8eb0905f22 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5021,6 +5021,7 @@ qemuDomainDefValidateMemory(const virDomainDef *def,
>  {
>  const long system_page_size = virGetSystemPageSizeKB();
>  const virDomainMemtune *mem = >mem;
> +size_t i;
>  
>  if (mem->nhugepages == 0)
>  return 0;
> @@ -5066,6 +5067,15 @@ qemuDomainDefValidateMemory(const virDomainDef *def,
>  return -1;
>  }
>  
> +for (i = 0; i < def->nmems; i++) {
> +if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_NVDIMM &&
> +!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("nvdimm isn't supported by this QEMU binary"));
> +return -1;
> +}
> +}
> +

This should be added to a function like
qemuDomainDeviceDefValidateMemory called via qemuDomainDeviceDefValidate

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Cole Robinson
On 12/16/19 8:36 AM, Daniel Henrique Barboza wrote:
> changes from version 3 [1]:
> - removed last 2 patches that made function 0 of 
> PCI multifunction devices mandatory
> - new patch: news.xml update
> - changed 'since' version to 6.0.0 in patch 4
> - unassigned hostdevs are now getting qemu aliases
> 
> [1] https://www.redhat.com/archives/libvir-list/2019-November/msg01263.html
> 
> Daniel Henrique Barboza (5):
>   Introducing new address type='unassigned' for PCI hostdevs
>   qemu: handle unassigned PCI hostdevs in command line
>   virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices
>   formatdomain.html.in: document 
>   news.xml: add address type='unassigned' entry
> 

Codewise it looks fine now. But I'm looking more closely at patch #3 and
realizing that it can explicitly reject a previously accepted VM config.
And indeed, now that I give it a test with my GPU passthrough setup, it
is rejecting my previosly working config.

error: Requested operation is not valid: All devices of the same IOMMU
group 1 of the PCI device :01:00.0 must belong to domain win10

I've attached the nodedev XML for the three devices with iommuGroup 1.
Only the two nvidia devices are assigned to my VM, but not the PCIe
controller device.

Is the libvirt heuristic missing something? Or is this acting as expected?

I didn't quite gather that this is a change to reject previously
accepted configurations, so I will defer to Laine and Alex as to whether
this should be committed.

- Cole

  pci__00_01_0
  /sys/devices/pci:00/:00:01.0
  computer
  
pcieport
  
  
0x060400
0
0
1
0
Xeon E3-1200 v5/E3-1500 v5/6th Gen Core Processor PCIe 
Controller (x16)
Intel Corporation


  
  
  


  
  

  



  pci__01_00_0
  /sys/devices/pci:00/:00:01.0/:01:00.0
  pci__00_01_0
  
vfio-pci
  
  
0x03
0
1
0
0
GP107 [GeForce GTX 1050 Ti]
NVIDIA Corporation

  
  
  


  
  

  



  pci__01_00_1
  /sys/devices/pci:00/:00:01.0/:01:00.1
  pci__00_01_0
  
vfio-pci
  
  
0x040300
0
1
0
1
GP107GL High Definition Audio Controller
NVIDIA Corporation

  
  
  


  
  

  

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v6 0/4] scripts: convert most perl scripts to python

2019-12-16 Thread Cole Robinson
On 12/9/19 12:14 PM, Daniel P. Berrangé wrote:
> This series is an effort to reduce the number of different
> languages we use by eliminating most use of perl in favour
> of python.
> 

Reviewed-by: Cole Robinson 

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [tck PATCH] Update x86_64 image & virt-builder image to F31

2019-12-16 Thread Laine Stump

On 12/13/19 11:51 AM, Daniel P. Berrangé wrote:

i386 was dropped even as a secondary arch in F31 so must remain
on F30

Signed-off-by: Daniel P. Berrangé 


I did that locally a couple weeks ago, then forgot to send it. (except 
that I missed the part to update the kernel images. I guess when F30 
goes EOL we'll just drop i686 testing completely?)



Reviewed-by: Laine Stump 




---
  conf/default.cfg | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/conf/default.cfg b/conf/default.cfg
index 39d9b04..3493ad7 100644
--- a/conf/default.cfg
+++ b/conf/default.cfg
@@ -49,7 +49,7 @@ images = (
hvm
xen
  )
-osname = fedora-30
+osname = fedora-31
}
  )
  
@@ -76,6 +76,7 @@ images = (

  #
  kernels = (
 # Fedora 30 i686 PAE has pv_ops, so one kernel can do both Xen and KVM 
guests here
+   # No i386 releases since F30
 {
   arch = i686
   ostype = (
@@ -85,15 +86,15 @@ kernels = (
   kernel = 
http://dl.fedoraproject.org/pub/fedora-secondary/releases/30/Everything/i386/os/images/pxeboot/vmlinuz
   initrd = 
http://dl.fedoraproject.org/pub/fedora-secondary/releases/30/Everything/i386/os/images/pxeboot/initrd.img
 }
-   # Fedora 30 x86_64 has pv_ops, so one kernel can do both Xen and KVM guests 
here
+   # Fedora 31 x86_64 has pv_ops, so one kernel can do both Xen and KVM guests 
here
 {
   arch = x86_64
   ostype = (
 hvm
 xen
   )
- kernel = 
http://dl.fedoraproject.org/pub/fedora/linux/releases/30/Everything/x86_64/os/images/pxeboot/vmlinuz
- initrd = 
http://dl.fedoraproject.org/pub/fedora/linux/releases/30/Everything/x86_64/os/images/pxeboot/initrd.img
+ kernel = 
http://dl.fedoraproject.org/pub/fedora/linux/releases/31/Everything/x86_64/os/images/pxeboot/vmlinuz
+ initrd = 
http://dl.fedoraproject.org/pub/fedora/linux/releases/31/Everything/x86_64/os/images/pxeboot/initrd.img
 }
 # LXC containers need a virtual container filesystem somewhere
  #   {



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] ongoing code transitions section on the wiki

2019-12-16 Thread Cole Robinson
Hi all, I added a 'Ongoing code transitions' section to the
BiteSizedTasks page on the wiki:

https://wiki.libvirt.org/page/BiteSizedTasks#Ongoing_code_transitions

BiteSizedTasks might not be the best location for it, but I wanted to
start a list and that page already exists.

I added 3 sections:

* g_autofree/g_autoptr, with some example conversion commits

* GLib in general, listing APIs that can replace custom libvirt code.
Currently the list is only g_key_file for virkeyfile, g_user_*_dir for
virGetUser*Dir, some virFile APIs for g_path_* APIs.

* gnulib removal. I filled in a few examples of possible gnulib module
replacements, and listed all the others as 'Unsorted/replacments unclear'.

If you know any GLib replacements for existing code or gnulib modules,
please fill them in there. Or any other incomplete code conversions
internally

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v4 3/5] virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices

2019-12-16 Thread Daniel Henrique Barboza
virHostdevPreparePCIDevices verifies if a PCI hostdev "A" is
in use by another domain by checking the 'activePCIHostdevs'
list. It also verifies if other domains are using any other
hostdev that belongs to the same IOMMU of "A".

This is not enough to cover all the cases. Suppose a PCI hostdev
"B" that shares the IOMMU with "A". We are currently able to abort
guest launch if "B" is being used by another domain, but if "B" is
not being used by any domain, we'll proceed guest execution. What
happens then is: if "A" is being used in managed mode, the guest
will fail to launch because Libvirt didn't detach "B" from the
host. If "A" is being used in un-managed mode, the guest might fail
or succeed to launch depending on whether both "A" and "B" were
detached manually prior to guest start.

Libvirt is now able to deal with these scenarios in a homogeneous
fashion, providing the same behavior for both managed and
un-managed mode while allowing parcial assignment for both cases as
well. This patch changes virHostdevPreparePCIDevices to check all
the PCI hostdevs of the domain against all the PCI hostdevs of the
IOMMU, failing to launch if the domain does not contain all the
PCI hostdevs declared, in both managed and un-managed cases.

After this patch, any existing domains that were using PCI hostdevs
with un-managed mode, and were getting away with partial assignment
without declaring all the IOMMU PCI hostdevs in the domain XML, will
be forced to do so. The missing PCI hostdevs can be declared with
"" to keep them invisible to the guest,
making the guest oblivious of this change. This is an annoyance for
such domains, but the payoff is more consistency of behavior between
managed and un-managed mode and more clarity on the domain XML,
forcing the admin to declare all PCI hostdevs of the IOMMU and
knowing exactly what will be detached from the host.

This is an example of this newly added error condition when the domain
does not declare all the PCI hostdevs of the same IOMMU:

$ sudo ./run tools/virsh start multipci-coldplug-partial-fail
error: Failed to start domain multipci-coldplug-partial-fail
error: Requested operation is not valid: All devices of the same IOMMU group 1 
of
the PCI device 0001:09:00.0 must belong to domain multipci-coldplug-partial-fail
$

Signed-off-by: Daniel Henrique Barboza 
---
 src/util/virhostdev.c | 64 ++-
 1 file changed, 57 insertions(+), 7 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 39e6b8f49f..2e7885112f 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -56,6 +56,13 @@ struct virHostdevIsPCINodeDeviceUsedData {
 bool usesVFIO;
 };
 
+struct virHostdevIsAllIOMMUGroupUsedData {
+virPCIDeviceListPtr pcidevs;
+const char *domainName;
+const char *deviceName;
+int iommuGroup;
+};
+
 /* This module makes heavy use of bookkeeping lists contained inside a
  * virHostdevManager instance to keep track of the devices' status. To make
  * it easy to spot potential ownership errors when moving devices from one
@@ -111,6 +118,27 @@ static int 
virHostdevIsPCINodeDeviceUsed(virPCIDeviceAddressPtr devAddr, void *o
 return 0;
 }
 
+static int virHostdevIsAllIOMMUGroupUsed(virPCIDeviceAddressPtr devAddr, void 
*opaque)
+{
+struct virHostdevIsAllIOMMUGroupUsedData *helperData = opaque;
+virPCIDevicePtr actual;
+
+actual = virPCIDeviceListFindByIDs(helperData->pcidevs,
+   devAddr->domain, devAddr->bus,
+   devAddr->slot, devAddr->function);
+if (actual) {
+return 0;
+} else {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _("All devices of the same IOMMU group %d of "
+ "the PCI device %s must belong to domain %s"),
+ helperData->iommuGroup,
+ helperData->deviceName,
+ helperData->domainName);
+return -1;
+}
+}
+
 static int virHostdevManagerOnceInit(void)
 {
 if (!VIR_CLASS_NEW(virHostdevManager, virClassForObject()))
@@ -724,9 +752,10 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 unsigned int flags)
 {
 g_autoptr(virPCIDeviceList) pcidevs = NULL;
+g_autofree unsigned int *searchedIOMMUs = NULL;
 int last_processed_hostdev_vf = -1;
-size_t i;
-int ret = -1;
+size_t i, j;
+int ret = -1, nSearchedIOMMUs = 0;
 virPCIDeviceAddressPtr devAddr = NULL;
 
 if (!nhostdevs)
@@ -735,6 +764,8 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 if (!(pcidevs = virHostdevGetPCIHostDeviceList(hostdevs, nhostdevs)))
 return -1;
 
+searchedIOMMUs = g_new0(unsigned int, virPCIDeviceListCount(pcidevs));
+
 virObjectLock(mgr->activePCIHostdevs);
 virObjectLock(mgr->inactivePCIHostdevs);
 
@@ -778,13 +809,32 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr,
 

[libvirt] [PATCH v4 4/5] formatdomain.html.in: document

2019-12-16 Thread Daniel Henrique Barboza
Signed-off-by: Daniel Henrique Barboza 
---
 docs/formatdomain.html.in | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index bfcdc026e6..281de89b10 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4152,6 +4152,20 @@
 attributes: iobase and irq.
 Since 1.2.1
   
+  unassigned
+  For PCI hostdevs, address type='unassigned'/
+has a special meaning. A PCI hostdev must be assigned to the domain
+with all the devices of its IOMMU group. If the admin so chooses,
+it is possible to filter which devices the guest OS will actually
+see, while the Libvirt domain is retaining ownership of all the
+devices of the IOMMU group. Using this address type in such cases
+will tell Libvirt that the device must not be assigned to the guest.
+This avoids scenarios in which the admin might be compelled to use
+a ACS patch to remove the device from the guest.
+address type='unassigned'/ is an invalid address
+type for all other device types.
+Since 6.0.0
+  
 
 
 Virtio-related options
-- 
2.23.0


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v4 5/5] news.xml: add address type='unassigned' entry

2019-12-16 Thread Daniel Henrique Barboza
Signed-off-by: Daniel Henrique Barboza 
---
 docs/news.xml | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 2a25b6ca49..e77cd743c8 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -54,6 +54,25 @@
   written in the RST as an alternative to HTML.
 
   
+  
+
+  new PCI hostdev address type: unassigned
+
+
+  A new PCI hostdev address type 'unassigned' is introduced,
+  giving users the option to choose which PCI hostdevs
+  within the same IOMMU group will not be assigned to the
+  guest. After this change, all PCI hostdevs of the same IOMMU
+  must be declared in the domain XML. Hostdevs that shouldn't
+  be used by the guest can be classified as
+  address type='unassigned'. Forcing all IOMMU
+  hostdevs to be declared makes the user aware of what will
+  be detached from the host, and this new address type gives
+  the user an extra layer of access control of hardware
+  resources, denying guest access to PCI hostdevs that
+  it shouldn't have access to.
+
+  
 
 
   
-- 
2.23.0


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v4 2/5] qemu: handle unassigned PCI hostdevs in command line

2019-12-16 Thread Daniel Henrique Barboza
Previous patch made it possible for the QEMU driver to check if
a given PCI hostdev is unassigned, by checking if dev->info->type is
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED, meaning that this device
shouldn't be part of the actual guest launch.

Signed-off-by: Daniel Henrique Barboza 
---
 src/qemu/qemu_command.c   |  4 +++
 .../hostdev-pci-address-unassigned.args   | 31 +++
 tests/qemuxml2argvtest.c  |  4 +++
 3 files changed, 39 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5d12ef3d63..7f7331744f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5419,6 +5419,10 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
 }
 }
 
+   /* Ignore unassigned devices  */
+   if (hostdev->info->type == 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED)
+   continue;
+
 unsigned int bootIndex = hostdev->info->bootIndex;
 
 /* bootNet will be non-0 if boot order was set and no other
diff --git a/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args 
b/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
new file mode 100644
index 00..42fae17444
--- /dev/null
+++ b/tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
@@ -0,0 +1,31 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-delete \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-delete/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-delete/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-delete/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name delete \
+-S \
+-machine pc,accel=kvm,usb=off,dump-guest-core=off \
+-m 256 \
+-realtime mlock=off \
+-smp 4,sockets=4,cores=1,threads=1 \
+-uuid 583a8e8e-f0ce-4f53-89ab-092862148b25 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-delete/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-device vfio-pci,host=0005:90:01.0,id=hostdev0,bus=pci.0,addr=0x3 \
+-device vfio-pci,host=0005:90:01.2,id=hostdev2,bus=pci.0,addr=0x4 \
+-device vfio-pci,host=0005:90:01.3,id=hostdev3,bus=pci.0,addr=0x5 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 8655db609e..92fbf3764e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1331,6 +1331,10 @@ mymain(void)
 QEMU_CAPS_KVM,
 QEMU_CAPS_DEVICE_VFIO_PCI);
 
+DO_TEST("hostdev-pci-address-unassigned",
+QEMU_CAPS_KVM,
+QEMU_CAPS_DEVICE_VFIO_PCI);
+
 DO_TEST("serial-file-log",
 QEMU_CAPS_CHARDEV_FILE_APPEND,
 QEMU_CAPS_DEVICE_ISA_SERIAL,
-- 
2.23.0


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH v4 1/5] Introducing new address type='unassigned' for PCI hostdevs

2019-12-16 Thread Daniel Henrique Barboza
Today, to use a PCI hostdev "A" in a domain, all PCI devices
that belongs to the same IOMMU group must also be declared in
the domain XML, meaning that all IOMMU devices are detached
from the host and all of them are visible to the guest.

The result is that the guest will have access to all devices,
but this is not necessarily what the administrator wanted. If only
the hostdev "A" was intended for guest usage, but hostdevs "B" and
"C" happened to be in the same IOMMU group of "A", the guest will
gain access to all 3 devices. This makes the administrator rely
on alternative solutions, such as use all hostdevs with un-managed
mode and detached all the IOMMU before the guest starts. If
use un-managed mode is not an option, the only alternative left is
an ACS patch to deny guest access to "B" and "C".

This patch introduces a new address type called "unassigned" to
handle this situation where a hostdev will be owned by a domain, but
not visible to the guest OS. This allows the administrator to
declare all the IOMMU while also choosing which hostdevs will be
usable by the guest. This new mechanic applies to all PCI hostdevs,
regardless of whether they are a PCI multifunction hostdev or not.
Using  in any case other than a PCI
hostdev will result in error.

Next patch will use this new address type in the QEMU driver to
avoid adding unassigned devices to the QEMU launch command line.

Signed-off-by: Daniel Henrique Barboza 
---
 docs/schemas/domaincommon.rng |  5 ++
 src/conf/device_conf.c|  2 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c|  7 ++-
 src/qemu/qemu_command.c   |  1 +
 src/qemu/qemu_domain.c|  1 +
 src/qemu/qemu_domain_address.c|  5 ++
 .../hostdev-pci-address-unassigned.xml| 42 ++
 .../hostdev-pci-address-unassigned.xml| 58 +++
 tests/qemuxml2xmltest.c   |  1 +
 10 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f5b51d20ad..372d248f25 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5470,6 +5470,11 @@
   
   
 
+
+  
+unassigned
+  
+
   
 
   
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 4c57f0995f..4dbd5c1ac9 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -45,6 +45,7 @@ VIR_ENUM_IMPL(virDomainDeviceAddress,
   "virtio-mmio",
   "isa",
   "dimm",
+  "unassigned",
 );
 
 static int
@@ -120,6 +121,7 @@ virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo 
*a,
 /* address types below don't have any specific data */
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
 break;
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index d98fae775c..e091d7cfe2 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -45,6 +45,7 @@ typedef enum {
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO,
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA,
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM,
+VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED,
 
 VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
 } virDomainDeviceAddressType;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cfec86a24d..454eb63be6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6338,9 +6338,11 @@ virDomainHostdevDefValidate(const virDomainHostdevDef 
*hostdev)
 switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) {
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
 if (hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
+hostdev->info->type != 
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED &&
 hostdev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("PCI host devices must use 'pci' address 
type"));
+   _("PCI host devices must use 'pci' or "
+ "'unassigned' address type"));
 return -1;
 }
 break;
@@ -7357,6 +7359,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_UNASSIGNED:
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
 break;
 }
@@ -7557,6 +7560,7 @@ 

[libvirt] [PATCH v4 0/5] PCI hostdev partial assignment support

2019-12-16 Thread Daniel Henrique Barboza
changes from version 3 [1]:
- removed last 2 patches that made function 0 of 
PCI multifunction devices mandatory
- new patch: news.xml update
- changed 'since' version to 6.0.0 in patch 4
- unassigned hostdevs are now getting qemu aliases

[1] https://www.redhat.com/archives/libvir-list/2019-November/msg01263.html

Daniel Henrique Barboza (5):
  Introducing new address type='unassigned' for PCI hostdevs
  qemu: handle unassigned PCI hostdevs in command line
  virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices
  formatdomain.html.in: document 
  news.xml: add address type='unassigned' entry

 docs/formatdomain.html.in | 14 
 docs/news.xml | 19 ++
 docs/schemas/domaincommon.rng |  5 ++
 src/conf/device_conf.c|  2 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c|  7 +-
 src/qemu/qemu_command.c   |  5 ++
 src/qemu/qemu_domain.c|  1 +
 src/qemu/qemu_domain_address.c|  5 ++
 src/util/virhostdev.c | 64 +--
 .../hostdev-pci-address-unassigned.args   | 31 +
 .../hostdev-pci-address-unassigned.xml| 42 
 tests/qemuxml2argvtest.c  |  4 ++
 .../hostdev-pci-address-unassigned.xml| 58 +
 tests/qemuxml2xmltest.c   |  1 +
 15 files changed, 251 insertions(+), 8 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.args
 create mode 100644 tests/qemuxml2argvdata/hostdev-pci-address-unassigned.xml
 create mode 100644 tests/qemuxml2xmloutdata/hostdev-pci-address-unassigned.xml

-- 
2.23.0


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [rust PATCH] Domain: Implement scheduler information related APIs

2019-12-16 Thread Vineeth Remanan Pillai
Thanks Sahid.

Vineeth

On Mon, Dec 16, 2019 at 5:59 AM Sahid Orentino Ferdjaoui
 wrote:
>
> On Sat, Dec 14, 2019 at 01:38:56PM +, Vineeth Remanan Pillai wrote:
> > Implement the following:
> > - virDomainGetSchedulerType
> > - virDomainSetSchedulerParameters
> > - virDomainSetSchedulerParametersFlags
> > - virDomainGetSchedulerParameters
> > - virDomainGetSchedulerParametersFlags
> >
> > Signed-off-by: Vineeth Remanan Pillai 
> > ---
> >  examples/hello.rs |  38 +++
> >  src/domain.rs | 260 +++---
> >  2 files changed, 283 insertions(+), 15 deletions(-)
>
> Reviewed-by: Sahid Orentino Ferdjaoui 
>
> I will merge it and push a new release as soon as I can. Thank you for
> your contribution.
>
> s.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [glib PATCH 0/2] po: change over to use Weblate instead of Zanata

2019-12-16 Thread Daniel P . Berrangé
Zanata is dead upstream, and Fedora is replacing it with Weblate

  https://fedoraproject.org/wiki/L10N_Move_to_Weblate

Libvirt has outsourced its translations to the Fedora translation
team, so we must follow this move.

Daniel P. Berrangé (2):
  po: delete empty translation files
  po: change update rules to use weblate instead of zanata

 po/Makefile.am  | 32 +++-
 po/af.mini.po   | 20 
 po/am.mini.po   | 20 
 po/anp.mini.po  | 19 ---
 po/ar.mini.po   | 21 -
 po/as.mini.po   | 20 
 po/ast.mini.po  | 20 
 po/bal.mini.po  | 20 
 po/be.mini.po   | 21 -
 po/bg.mini.po   | 20 
 po/bn.mini.po   | 20 
 po/bn_IN.mini.po| 20 
 po/bo.mini.po   | 20 
 po/br.mini.po   | 20 
 po/brx.mini.po  | 20 
 po/bs.mini.po   | 21 -
 po/cy.mini.po   | 21 -
 po/da.mini.po   | 20 
 po/de.mini.po   | 20 
 po/de_CH.mini.po| 20 
 po/el.mini.po   | 20 
 po/eo.mini.po   | 20 
 po/et.mini.po   | 20 
 po/eu.mini.po   | 20 
 po/fa.mini.po   | 20 
 po/gl.mini.po   | 20 
 po/gu.mini.po   | 20 
 po/he.mini.po   | 20 
 po/hr.mini.po   | 21 -
 po/hu.mini.po   | 20 
 po/ia.mini.po   | 20 
 po/id.mini.po   | 20 
 po/ilo.mini.po  | 20 
 po/is.mini.po   | 20 
 po/it.mini.po   | 20 
 po/ka.mini.po   | 20 
 po/kk.mini.po   | 20 
 po/km.mini.po   | 20 
 po/kn.mini.po   | 20 
 po/ko.mini.po   | 20 
 po/kw.mini.po   | 20 
 po/k...@kkcor.mini.po | 20 
 po/k...@uccor.mini.po | 20 
 po/kw_GB.mini.po| 20 
 po/ky.mini.po   | 20 
 po/lt.mini.po   | 21 -
 po/lv.mini.po   | 21 -
 po/mai.mini.po  | 20 
 po/mk.mini.po   | 20 
 po/ml.mini.po   | 20 
 po/mn.mini.po   | 20 
 po/mr.mini.po   | 20 
 po/ms.mini.po   | 20 
 po/nb.mini.po   | 20 
 po/nds.mini.po  | 20 
 po/ne.mini.po   | 20 
 po/nl.mini.po   | 20 
 po/nn.mini.po   | 20 
 po/nso.mini.po  | 20 
 po/or.mini.po   | 20 
 po/pa.mini.po   | 20 
 po/pt.mini.po   | 20 
 po/ro.mini.po   | 21 -
 po/si.mini.po   | 20 
 po/sk.mini.po   | 20 
 po/sl.mini.po   | 21 -
 po/sq.mini.po   | 20 
 po/sr.mini.po   | 21 -
 po/s...@latin.mini.po | 21 -
 po/sv.mini.po   | 20 
 po/ta.mini.po   | 20 
 po/te.mini.po   | 20 
 po/tg.mini.po   | 20 
 po/th.mini.po   | 20 
 po/tr.mini.po   | 20 
 po/tw.mini.po   | 19 ---
 po/ur.mini.po   | 20 
 po/vi.mini.po   | 20 
 po/wba.mini.po  | 19 ---
 po/yo.mini.po   | 19 ---
 po/zanata.xml   |  7 ---
 po/zh_CN.mini.po| 20 
 po/zh_HK.mini.po| 20 
 po/zh_TW.mini.po| 20 
 po/zu.mini.po   | 20 
 85 files changed, 19 insertions(+), 1687 deletions(-)
 delete mode 100644 po/af.mini.po
 delete mode 100644 po/am.mini.po
 delete mode 100644 po/anp.mini.po
 delete mode 100644 po/ar.mini.po
 delete mode 100644 po/as.mini.po
 delete mode 100644 po/ast.mini.po
 delete mode 100644 po/bal.mini.po
 delete mode 100644 po/be.mini.po
 delete mode 100644 po/bg.mini.po
 delete mode 100644 po/bn.mini.po
 delete mode 100644 po/bn_IN.mini.po
 delete mode 100644 po/bo.mini.po
 delete mode 100644 po/br.mini.po
 delete mode 100644 po/brx.mini.po
 delete mode 100644 po/bs.mini.po
 delete mode 100644 po/cy.mini.po
 delete mode 100644 po/da.mini.po
 delete 

[libvirt] [glib PATCH 1/2] po: delete empty translation files

2019-12-16 Thread Daniel P . Berrangé
Historically we stored all translation languages known to Zanata, even
if there were no translated strings present, because every language was
created in Zanata. With the move to Weblate we are only going to store
non-empty translations because languages won't be created in Weblate
until someone wants to actually translate them.

Signed-off-by: Daniel P. Berrangé 
---
 po/Makefile.am  | 12 ++--
 po/af.mini.po   | 20 
 po/am.mini.po   | 20 
 po/anp.mini.po  | 19 ---
 po/ar.mini.po   | 21 -
 po/as.mini.po   | 20 
 po/ast.mini.po  | 20 
 po/bal.mini.po  | 20 
 po/be.mini.po   | 21 -
 po/bg.mini.po   | 20 
 po/bn.mini.po   | 20 
 po/bn_IN.mini.po| 20 
 po/bo.mini.po   | 20 
 po/br.mini.po   | 20 
 po/brx.mini.po  | 20 
 po/bs.mini.po   | 21 -
 po/cy.mini.po   | 21 -
 po/da.mini.po   | 20 
 po/de.mini.po   | 20 
 po/de_CH.mini.po| 20 
 po/el.mini.po   | 20 
 po/eo.mini.po   | 20 
 po/et.mini.po   | 20 
 po/eu.mini.po   | 20 
 po/fa.mini.po   | 20 
 po/gl.mini.po   | 20 
 po/gu.mini.po   | 20 
 po/he.mini.po   | 20 
 po/hr.mini.po   | 21 -
 po/hu.mini.po   | 20 
 po/ia.mini.po   | 20 
 po/id.mini.po   | 20 
 po/ilo.mini.po  | 20 
 po/is.mini.po   | 20 
 po/it.mini.po   | 20 
 po/ka.mini.po   | 20 
 po/kk.mini.po   | 20 
 po/km.mini.po   | 20 
 po/kn.mini.po   | 20 
 po/ko.mini.po   | 20 
 po/kw.mini.po   | 20 
 po/k...@kkcor.mini.po | 20 
 po/k...@uccor.mini.po | 20 
 po/kw_GB.mini.po| 20 
 po/ky.mini.po   | 20 
 po/lt.mini.po   | 21 -
 po/lv.mini.po   | 21 -
 po/mai.mini.po  | 20 
 po/mk.mini.po   | 20 
 po/ml.mini.po   | 20 
 po/mn.mini.po   | 20 
 po/mr.mini.po   | 20 
 po/ms.mini.po   | 20 
 po/nb.mini.po   | 20 
 po/nds.mini.po  | 20 
 po/ne.mini.po   | 20 
 po/nl.mini.po   | 20 
 po/nn.mini.po   | 20 
 po/nso.mini.po  | 20 
 po/or.mini.po   | 20 
 po/pa.mini.po   | 20 
 po/pt.mini.po   | 20 
 po/ro.mini.po   | 21 -
 po/si.mini.po   | 20 
 po/sk.mini.po   | 20 
 po/sl.mini.po   | 21 -
 po/sq.mini.po   | 20 
 po/sr.mini.po   | 21 -
 po/s...@latin.mini.po | 21 -
 po/sv.mini.po   | 20 
 po/ta.mini.po   | 20 
 po/te.mini.po   | 20 
 po/tg.mini.po   | 20 
 po/th.mini.po   | 20 
 po/tr.mini.po   | 20 
 po/tw.mini.po   | 19 ---
 po/ur.mini.po   | 20 
 po/vi.mini.po   | 20 
 po/wba.mini.po  | 19 ---
 po/yo.mini.po   | 19 ---
 po/zh_CN.mini.po| 20 
 po/zh_HK.mini.po| 20 
 po/zh_TW.mini.po| 20 
 po/zu.mini.po   | 20 
 84 files changed, 2 insertions(+), 1677 deletions(-)
 delete mode 100644 po/af.mini.po
 delete mode 100644 po/am.mini.po
 delete mode 100644 po/anp.mini.po
 delete mode 100644 po/ar.mini.po
 delete mode 100644 po/as.mini.po
 delete mode 100644 po/ast.mini.po
 delete mode 100644 po/bal.mini.po
 delete mode 100644 po/be.mini.po
 delete mode 100644 po/bg.mini.po
 delete mode 100644 po/bn.mini.po
 delete mode 100644 po/bn_IN.mini.po
 delete mode 100644 po/bo.mini.po
 delete mode 100644 po/br.mini.po
 delete mode 100644 po/brx.mini.po
 delete mode 100644 po/bs.mini.po
 delete mode 100644 po/cy.mini.po
 delete mode 100644 po/da.mini.po
 delete mode 100644 po/de.mini.po
 

[libvirt] [glib PATCH 2/2] po: change update rules to use weblate instead of zanata

2019-12-16 Thread Daniel P . Berrangé
The Zanata project is dead and so Fedora is transitioning over to using
Weblate for managing translations. With Weblate, languages are only
created on the server when the first translation is started, so we must
check to see if any new languages exist during download, so that they
can be added to the local languages list.

Signed-off-by: Daniel P. Berrangé 
---
 po/Makefile.am | 20 +---
 po/zanata.xml  |  7 ---
 2 files changed, 17 insertions(+), 10 deletions(-)
 delete mode 100644 po/zanata.xml

diff --git a/po/Makefile.am b/po/Makefile.am
index 65827ac..cb72e0c 100644
--- a/po/Makefile.am
+++ b/po/Makefile.am
@@ -55,13 +55,27 @@ update-mini-po: $(POTFILE)
done
 
 push-pot: $(POTFILE)
-   zanata push --push-type=source
+   wlc upload --method replace -i libvirt-glib.pot 
libvirt-glib/application/en
 
-pull-po: $(POTFILE)
-   zanata pull --create-skeletons
+pull-po: $(POTFILE) check-new-langs
+   @for lang in $(LANGS); do \
+ echo "Downloading $$lang" && \
+ wlc download -o  $$lang.po libvirt-glib/application/$$lang ; \
+   done
$(MAKE) update-mini-po
$(MAKE) update-gmo
 
+check-new-langs:
+   @for po in `wlc ls libvirt-glib/application | \
+   grep filename | grep -v 'en.po' | \
+   sed -e 's/filename: //' -e 's/.po/.mini.po/'` ; \
+   do \
+ if ! test -f "$$po" ; \
+ then \
+   echo "New language $$po in weblate" && exit 1; \
+ fi ; \
+   done
+
 $(POTFILE): POTFILES $(POTFILE_DEPS)
$(XGETTEXT) -o $@-t $(XGETTEXT_ARGS) \
  --files-from=$(abs_srcdir)/POTFILES
diff --git a/po/zanata.xml b/po/zanata.xml
deleted file mode 100644
index 042c695..000
--- a/po/zanata.xml
+++ /dev/null
@@ -1,7 +0,0 @@
-
-http://zanata.org/namespace/config/;>
-  https://fedora.zanata.org/
-  libvirt-glib
-  master
-  gettext
-
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] cpu: add CLZERO CPUID support for AMD platforms (libvirt 4.5)

2019-12-16 Thread Jiri Denemark
On Tue, Dec 03, 2019 at 03:19:39 -0800, Ani Sinha wrote:
> Qemu commit e900135dcfb67 ("i386: Add CPUID bit for CLZERO and XSAVEERPTR")
> adds support for CLZERO CPUID bit. This commit extends support for this
> bit in libvirt.
> 
> This patch is based off of libvirt-4.5 tree.

Thanks, but we are really only interested in patches against current
master. Good thing is you sent it too so I reviewed that version.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH] cpu: add CLZERO CPUID support for AMD platforms

2019-12-16 Thread Jiri Denemark
On Tue, Dec 03, 2019 at 03:09:12 -0800, Ani Sinha wrote:
> Qemu commit e900135dcfb67 ("i386: Add CPUID bit for CLZERO and XSAVEERPTR")
> adds support for CLZERO CPUID bit.
> This commit extends support for this CPUID bit into libvirt.
> 
> Signed-off-by: Ani Sinha 
> ---
>  src/cpu_map/x86_EPYC-IBPB.xml | 1 +
>  src/cpu_map/x86_EPYC.xml  | 1 +
>  src/cpu_map/x86_features.xml  | 3 +++
>  3 files changed, 5 insertions(+)
> 
> diff --git a/src/cpu_map/x86_EPYC-IBPB.xml b/src/cpu_map/x86_EPYC-IBPB.xml
> index 283697e..a70fbd8 100644
> --- a/src/cpu_map/x86_EPYC-IBPB.xml
> +++ b/src/cpu_map/x86_EPYC-IBPB.xml
> @@ -14,6 +14,7 @@
>  
>  
>  
> +
>  
>  
>  
> diff --git a/src/cpu_map/x86_EPYC.xml b/src/cpu_map/x86_EPYC.xml
> index f060139..6c11d82 100644
> --- a/src/cpu_map/x86_EPYC.xml
> +++ b/src/cpu_map/x86_EPYC.xml
> @@ -14,6 +14,7 @@
>  
>  
>  
> +
>  
>  
>  

We don't normally change existing CPU models as it can cause issues with
migration between non-matching versions of libvirt. If QEMU enables a
new feature for a given model (which doesn't seem to be the case of
clzero and EPYC, however), you'd automatically get it just by asking for
EPYC in libvirt. Otherwise, you need to ask for clzero explicitly
(adding it to libvirt's EPYC would not cause the feature to be enabled
anyway). Or you can use host-model CPU in which case, clzero will be
automatically added as long as it is supported by QEMU.

> diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml
> index 2bed1e0..dd62755 100644
> --- a/src/cpu_map/x86_features.xml
> +++ b/src/cpu_map/x86_features.xml
> @@ -473,6 +473,9 @@
>
>  
>
> +  
> +
> +  
>
>  
>

We want to keep the bits sorted by their position, I moved it up a bit,
just above wbnoinvd.

Also several cputest results have to be updated since our CPU data
describe some CPUs which already support clzero.

That said, I dropped the changes to EPYC* CPU models, moved the feature
definition in x86_features a bit, updated the test results, and pushed
this as commit 1d17f881a278c408ede19c7e6a5330e3f19fe0f0

Reviewed-by: Jiri Denemark 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [jenkins-ci PATCH 2/2] guests: Only install OpenVZ on CentOS 7

2019-12-16 Thread Fabiano Fidêncio
OpenVZ has been added as part of commit 0a7993d3ed30ae. However, it's
wrongly set to be installed on any supported CentOS.

Signed-off-by: Fabiano Fidêncio 
---
 guests/playbooks/update/tasks/base.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/guests/playbooks/update/tasks/base.yml 
b/guests/playbooks/update/tasks/base.yml
index f721e39..f085f19 100644
--- a/guests/playbooks/update/tasks/base.yml
+++ b/guests/playbooks/update/tasks/base.yml
@@ -50,6 +50,7 @@
 group: root
   when:
 - os_name == 'CentOS'
+- os_version == '7'
 
 - name: Update installed packages
   package:
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 0/2] CentOS fixes

2019-12-16 Thread Fabiano Fidêncio
This series consists in two simple patches:
- Mention CentOS8 as a valid OS version;
- Only install OpenVZ on CentOS 7;

Fabiano Fidêncio (2):
  guests: Mention CentOS8 as a valid OS version
  guests: Only install OpenVZ on CentOS 7

 guests/playbooks/update/tasks/base.yml | 1 +
 guests/vars/mappings.yml   | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [jenkins-ci PATCH 1/2] guests: Mention CentOS8 as a valid OS version

2019-12-16 Thread Fabiano Fidêncio
CentOS8 has been added as part of 03fd3a8ef16a3c4. However, it was not
mentioned as a valid OS version.

Signed-off-by: Fabiano Fidêncio 
---
 guests/vars/mappings.yml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index ef4e17a..c1a5d44 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -22,7 +22,8 @@
 #   - CentOS, Debian, Fedora, FreeBSD, OpenSUSE, Ubuntu
 #
 # Valid OS versions are:
-#   - CentOS7, Debian9, FedoraRawhide, OpenSUSE151, Ubuntu1804 and so on
+#   - CentOS7, CentOS8, Debian9, FedoraRawhide, OpenSUSE151, Ubuntu1804 and so
+# on
 #
 # The arch specific rules use a prefix "$ARCH-" where  $ARCH
 # is a libvirt arch name.
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] src: warn against virNodeGetInfo() API call due to inaccurate info

2019-12-16 Thread Jiri Denemark
On Mon, Dec 16, 2019 at 10:36:40 +, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  include/libvirt/libvirt-host.h |  4 
>  src/libvirt-host.c | 16 
>  2 files changed, 20 insertions(+)

Reviewed-by: Jiri Denemark 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [rust PATCH] Domain: Implement scheduler information related APIs

2019-12-16 Thread Sahid Orentino Ferdjaoui
On Sat, Dec 14, 2019 at 01:38:56PM +, Vineeth Remanan Pillai wrote:
> Implement the following:
> - virDomainGetSchedulerType
> - virDomainSetSchedulerParameters
> - virDomainSetSchedulerParametersFlags
> - virDomainGetSchedulerParameters
> - virDomainGetSchedulerParametersFlags
> 
> Signed-off-by: Vineeth Remanan Pillai 
> ---
>  examples/hello.rs |  38 +++
>  src/domain.rs | 260 +++---
>  2 files changed, 283 insertions(+), 15 deletions(-)

Reviewed-by: Sahid Orentino Ferdjaoui 

I will merge it and push a new release as soon as I can. Thank you for
your contribution.

s.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



[libvirt] [PATCH] src: warn against virNodeGetInfo() API call due to inaccurate info

2019-12-16 Thread Daniel P . Berrangé
Signed-off-by: Daniel P. Berrangé 
---
 include/libvirt/libvirt-host.h |  4 
 src/libvirt-host.c | 16 
 2 files changed, 20 insertions(+)

diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
index be65b4686b..b87d634813 100644
--- a/include/libvirt/libvirt-host.h
+++ b/include/libvirt/libvirt-host.h
@@ -147,6 +147,10 @@ typedef virSecurityModel *virSecurityModelPtr;
  *
  * a virNodeInfo is a structure filled by virNodeGetInfo() and providing
  * the information for the Node.
+ *
+ * Note that the information in this struct is not guaranteed to be an
+ * accurate relection of the system hardware. See the virNodeGetInfo()
+ * API documentation for further guidance.
  */
 
 typedef struct _virNodeInfo virNodeInfo;
diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index 94ba5a8e80..bc3d1d2803 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -399,6 +399,22 @@ virConnectGetMaxVcpus(virConnectPtr conn,
  *
  * Extract hardware information about the node.
  *
+ * Use of this API is strongly discouraged as the information provided
+ * is not guaranteed to be accurate on all hardware platforms.
+ *
+ * The mHZ value merely reflects the speed that the first CPU in the
+ * machine is currently running at. This speed may vary across CPUs
+ * and changes continually as the host OS throttles.
+ *
+ * The nodes/sockets/cores/threads data is potentially inaccurate as
+ * it assumes a symmetric installation. If one NUMA node has more
+ * sockets populated that another NUMA node this information will be
+ * wrong. It is also not able to report about CPU dies.
+ *
+ * Applications are recommended to use the virConnectGetCapabilities()
+ * call instead, which provides all the information except CPU mHZ,
+ * in a more accurate representation.
+ *
  * Returns 0 in case of success and -1 in case of failure.
  */
 int
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/5] util, resctrl: using 64bit interface instead of 32bit for counters

2019-12-16 Thread Daniel P . Berrangé
On Sun, Dec 15, 2019 at 02:39:20AM +, Wang, Huaqiang wrote:
> Hi Daniel,
> 
> Thanks for your review. 
> 
> About patch 1/5, I understand we should be very cautious when we changes  the 
> determined
> interface. 
> 
> I'd like to reserved the old 32bit interface  and propose a new 64bit 
> interface just as you
> suggested if you still think so after you  got my intention. 
> Here actually I want to fix a bug, maybe I should title this patch as *bug 
> fixing*. Reason is the
> underlying hardware, the cache monitor and memory bandwidth counters, are 
> 64bit width.
> Using 32bit interface to access these counters are problematic. This bug is 
> not found because
> this interface is only used for tracking the amount of cache that used before 
> this patch, normally
> the occupied cache will not exceed 4GB range. (32bit counter can counter 
> value up to 4GB).
> But for memory, this counter records the data passing through the memory 
> controller issued
> by this CPU in bytes and accumulatively, this value can easily exceed the 4GB 
> bound, so I
> don’t want to reserve the old 32 bit interface and let user use it, because 
> it will report incorrect value.

We simply have to document the limitati onof the old interface. We can
*NOT* change it, because it WILL break API compatibility for apps that
deserailize the current data.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 1/1] tools: do not loop in libvirt-guests test_connect

2019-12-16 Thread Daniel P . Berrangé
On Mon, Dec 16, 2019 at 08:20:59AM +0100, Christian Ehrhardt wrote:
> These days libvirt is pretty reliable and even remote connections
> (not the default for libvirt-guests anyway) either work or fail but are
> uncommon to be flaky.
> 
> On the other hand users might have disabled the service and while we are
> After=libvirtd for ordering we are not Requiring it. Adding that or any
> harder dependency might break our ordering. But if people have disabled
> libvirt they will do a full retry loop until timeout.
> 
> Lets drop the loop to be much faster if a remote is not reachable.
> 
> Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1854653

I extended this to add:

This reverts

  commit 4e7fc8305a53676ba2362bfaa8ca05c4851b7e12
  Author: Michal Prívozník 
  Date:   Fri Feb 21 12:46:08 2014 +0100

libvirt-guests: Wait for libvirtd to initialize

The race described in that commit no longer exists using systemd as
we now have socket activation. If not using systemd, then it is also
safe if using the libvirtd --daemon flag, since the parent process
won't return to the caller until the child is accepting connections.

and have now pushed this fix.

> 
> Reported-by: Doug Smythies 
> Signed-off-by: Christian Ehrhardt 
> ---
>  tools/libvirt-guests.sh.in | 21 ++---
>  1 file changed, 6 insertions(+), 15 deletions(-)



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] [rust PATCH] Domain: Implement scheduler information related APIs

2019-12-16 Thread Vineeth Remanan Pillai
Implement the following:
- virDomainGetSchedulerType
- virDomainSetSchedulerParameters
- virDomainSetSchedulerParametersFlags
- virDomainGetSchedulerParameters
- virDomainGetSchedulerParametersFlags

Signed-off-by: Vineeth Remanan Pillai 
---
 examples/hello.rs |  38 +++
 src/domain.rs | 260 +++---
 2 files changed, 283 insertions(+), 15 deletions(-)

diff --git a/examples/hello.rs b/examples/hello.rs
index e5ec6e4..1f336a9 100644
--- a/examples/hello.rs
+++ b/examples/hello.rs
@@ -89,6 +89,44 @@ fn show_domains(conn: ) -> Result<(), Error> {
  numa.node_set.unwrap_or(String::from("")));
 println!("Mode: {}", numa.mode.unwrap_or(0));
 }
+
+if let Ok((sched_type, nparams)) = 
dom.get_scheduler_type() {
+println!("SchedType: {}, nparams: {}",
+ sched_type, nparams);
+}
+
+if let Ok(sched_info) = dom.get_scheduler_parameters() {
+println!("Schedule Information:");
+println!("\tScheduler\t: {}", 
sched_info.scheduler_type);
+if let Some(shares) =  sched_info.cpu_shares {
+println!("\tcpu_shares\t: {}", shares);
+}
+if let Some(period) = sched_info.vcpu_bw.period {
+println!("\tvcpu_period\t: {}", period);
+}
+if let Some(quota) = sched_info.vcpu_bw.quota {
+println!("\tvcpu_quota\t: {}", quota);
+}
+if let Some(period) = sched_info.emulator_bw.period {
+println!("\temulator_period\t: {}", period);
+}
+if let Some(quota) = sched_info.emulator_bw.quota {
+println!("\temulator_quota\t: {}", quota);
+}
+if let Some(period) = sched_info.global_bw.period {
+println!("\tglobal_period\t: {}", period);
+}
+if let Some(quota) = sched_info.global_bw.quota {
+println!("\tglobal_quota\t: {}", quota);
+}
+if let Some(period) = sched_info.global_bw.period {
+println!("\tiothread_period\t: {}", period);
+}
+if let Some(quota) = sched_info.global_bw.quota {
+println!("\tiothread_quota\t: {}", quota);
+}
+}
+
 }
 }
 return Ok(());
diff --git a/src/domain.rs b/src/domain.rs
index acb9e6e..40a18d8 100644
--- a/src/domain.rs
+++ b/src/domain.rs
@@ -363,6 +363,29 @@ extern "C" {
  snaps: *mut *mut virDomainSnapshotPtr,
  flags: libc::c_uint)
  -> libc::c_int;
+
+fn virDomainGetSchedulerType(ptr: sys::virDomainPtr,
+ nparams: *mut libc::c_int)
+ -> *mut libc::c_char;
+fn virDomainGetSchedulerParameters(ptr: sys::virDomainPtr,
+   params: virTypedParameterPtr,
+   nparams: *mut libc::c_int)
+   -> libc::c_int;
+fn virDomainSetSchedulerParameters(ptr: sys::virDomainPtr,
+   params: virTypedParameterPtr,
+   nparams: libc::c_int)
+   -> libc::c_int;
+fn virDomainGetSchedulerParametersFlags(ptr: sys::virDomainPtr,
+params: virTypedParameterPtr,
+nparams: *mut libc::c_int,
+flags: libc::c_uint)
+-> libc::c_int;
+fn virDomainSetSchedulerParametersFlags(ptr: sys::virDomainPtr,
+params: virTypedParameterPtr,
+nparams: libc::c_int,
+flags: libc::c_uint)
+-> libc::c_int;
+
 }
 
 pub type DomainXMLFlags = self::libc::c_uint;
@@ -607,6 +630,124 @@ impl MemoryStats {
 }
 }
 
+/// Structure representing the CFS scheduler cpu bandwidth parameters
+/// see https://www.kernel.org/doc/html/latest/scheduler/sched-bwc.html
+#[derive(Clone, Debug, Default)]
+pub struct SchedBandwidth {
+pub period: Option,
+pub quota: Option,
+}
+
+#[derive(Clone, Debug, Default)]
+pub struct SchedulerInfo {

Re: [libvirt] [PATCH v4 6/7] remote: shrink the critical sections

2019-12-16 Thread Marc Hartmayer
On Fri, Dec 13, 2019 at 03:00 PM -0500, Cole Robinson  
wrote:
> On 12/12/19 9:13 AM, Marc Hartmayer wrote:
>> On Wed, Dec 11, 2019 at 07:48 PM -0500, Cole Robinson  
>> wrote:
>>> 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
>> 
>> 1, 3, and 5 are all independent.
>> 
>> Patch 4 depends on the second patch as
>> remoteDispatchConnectRegisterCloseCallback uses
>> virConnectRegisterCloseCallback. Otherwise we would never do the unref
>> for @client and @program when conn->driver->connectRegisterCloseCallback
>> was NULL.
>
> Thanks, I pushed 1, 3, 5, 6. I'll reply to your other message
>
> - Cole
>

Thanks.

-- 
Kind regards / Beste Grüße
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list