Re: [libvirt] [PATCH 2/2] qemu: agent: fix unsafe agent access

[Michal Privoznik]

On 23.11.2016 08:08, Nikolay Shirokovskiy wrote:
> 
> 
> On 22.11.2016 17:49, Michal Privoznik wrote:
>> On 14.11.2016 15:24, Nikolay Shirokovskiy wrote:
>>> qemuDomainObjExitAgent is unsafe.
>>>
>>> First it accesses domain object without domain lock.
>>> Second it uses outdated logic that goes back to commit 79533da1 of
>>> year 2009 when code was quite different. (unref function
>>> instead of unreferencing only unlocked and disposed object
>>> in case of last reference and leaved unlocking to the caller otherwise).
>>> Nowadays this logic may lead to disposing locked object
>>> i guess.
>>
>> Well, I agree that the order of those two steps should be reversed, so
>> ACK to that.
>>
>>>
>>> Another problem is that the callers of qemuDomainObjEnterAgent
>>> use domain object again (namely priv->agent) without domain lock.
>>
>> But why is this a problem?
> 
> qemuProcessHandleAgentEOF for example can zero out priv->agent after 
> we call qemuDomainObjEnterAgent and before we call 
> qemuAgentGetTime(priv->agent, seconds, nseconds).
> Because we drop domain lock in qemuDomainObjEnterAgent and 
> qemuProcessHandleAgentEOF does not acquire job condition, only domain lock. 
> At the same time qemuAgentGetTime is not ready for NULL agent and will crash.
> It could get even worse as priv->agent is not atomic and instead of
> NULL we can get garbage there. Long story short we just should
> not access domain state without lock. Thats why I change all the places
> so we get copy of priv->agent before we drop domain lock.

Ah. Sounds reasonable. Mind adding it to the commit message?

ACK then.

Michal

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

[libvirt] [PATCH 1/2] docs: Add apps.html link to index.html

[Michal Privoznik]

I think when trying to introduce libvirt (we have a section for
that in our index page) it might be useful to promote success
stories - other applications that are based on libvirt.

Signed-off-by: Michal Privoznik 
---
 docs/index.html.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/index.html.in b/docs/index.html.in
index b9a8461..7665f8c 100644
--- a/docs/index.html.in
+++ b/docs/index.html.in
@@ -39,6 +39,7 @@
   BHyve and
   more
 targets Linux, FreeBSD, Windows and 
OS-X
+applications known to use libvirt
   
 
 
-- 
2.8.4

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

[libvirt] [PATCH 2/2] docs: List go-libvirt in bindings

[Michal Privoznik]

I've came across this site [1] which refers to yet another Go
libvirt bindings. Add them to the list.

1: https://www.digitalocean.com/company/blog/introducing-go-qemu-and-go-libvirt/

Signed-off-by: Michal Privoznik 
---
 docs/bindings.html.in | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/docs/bindings.html.in b/docs/bindings.html.in
index 7fe26df..6f18c07 100644
--- a/docs/bindings.html.in
+++ b/docs/bindings.html.in
@@ -15,8 +15,18 @@
 C# bindings.
   
  
-Go: Kyle Kelley et al. are developing
-https://github.com/rgbkrk/libvirt-go;>Go bindings.
+Go: There are two bindings available:
+https://github.com/rgbkrk/libvirt-go;>libvirt-go and
+https://github.com/digitalocean/go-libvirt;>go-libvirt.
+
+The libvirt-go bindings are developed by Kyle Kelley et
+al. and are built on the top of libvirt's client library.
+
+
+The go-libvirt bindings are then developed by Ben
+LeMasurier et al. and they hook straight into libvirt's RPC and thus do
+not rely on cgo.
+
   
   
 Java: Daniel Veillard develops
-- 
2.8.4

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

[libvirt] [PATCH 0/2] docs: Couple of improvements

[Michal Privoznik]

CMSIA

(Commit Messages Say It All)

Michal Privoznik (2):
  docs: Add apps.html link to index.html
  docs: List go-libvirt in bindings

 docs/bindings.html.in | 14 --
 docs/index.html.in|  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.8.4

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

Re: [libvirt] [PATCH 2/2] qemu: agent: fix unsafe agent access

[Nikolay Shirokovskiy]

On 22.11.2016 17:49, Michal Privoznik wrote:
> On 14.11.2016 15:24, Nikolay Shirokovskiy wrote:
>> qemuDomainObjExitAgent is unsafe.
>>
>> First it accesses domain object without domain lock.
>> Second it uses outdated logic that goes back to commit 79533da1 of
>> year 2009 when code was quite different. (unref function
>> instead of unreferencing only unlocked and disposed object
>> in case of last reference and leaved unlocking to the caller otherwise).
>> Nowadays this logic may lead to disposing locked object
>> i guess.
> 
> Well, I agree that the order of those two steps should be reversed, so
> ACK to that.
> 
>>
>> Another problem is that the callers of qemuDomainObjEnterAgent
>> use domain object again (namely priv->agent) without domain lock.
> 
> But why is this a problem?

qemuProcessHandleAgentEOF for example can zero out priv->agent after 
we call qemuDomainObjEnterAgent and before we call 
qemuAgentGetTime(priv->agent, seconds, nseconds).
Because we drop domain lock in qemuDomainObjEnterAgent and 
qemuProcessHandleAgentEOF does not acquire job condition, only domain lock. 
At the same time qemuAgentGetTime is not ready for NULL agent and will crash.
It could get even worse as priv->agent is not atomic and instead of
NULL we can get garbage there. Long story short we just should
not access domain state without lock. Thats why I change all the places
so we get copy of priv->agent before we drop domain lock.

> 
>>
>> This patch address these two problems.
>>
>> qemuDomainGetAgent is dropped as unused.
>> ---
>>
>> I wonder should we make qemuDomainObj(Enter|Exit)Agent be macros. So 
>> we don't need to declare a variable to hold agent reference and zero
>> it out at the end for safety.
>>
>> Also the qemu monitor code has probably same problems as the agent monitor 
>> code
>> seems to be copied from there.
>>
> 
> Well, qemu monitor can't come and go. I mean, if it does, the domain is
> killed. That's not the case with agent monitor.

Nevertheless in the same situation as described above we can get
undefined behaviour on domain destroy.

Nikolay

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

Re: [libvirt] [Qemu-ppc] [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default

[David Gibson]

On Tue, Nov 22, 2016 at 01:26:49PM +1100, Alexey Kardashevskiy wrote:
> On 22/11/16 00:08, Andrea Bolognani wrote:
> > On Mon, 2016-11-21 at 13:12 +1100, Alexey Kardashevskiy wrote:
> > 1) switch to PCI Express on newer machine types, and
> >expose some sort of capability through QMP so that
> >libvirt can know about the switch
> >  
> > [...]
> > Option 1) would break horribly with existing libvirt
> > versions, and so would Option 2) if we default to using
>   
>  How exactly 1) will break libvirt? Migrating from pseries-2.7 to
>  pseries-2.8 does not work anyway, and machines are allowed to behave
>  different from version to version, what distinct difference will using
>  "pseries-pcie-X.Y" make?
> >>>  
> >>> Existing libvirt versions assume that pseries guests have
> >>  
> >> If libvirt is using just "pseries" (without a version), then having a
> >> virtual PCIe-PCI bridge (and "pci.0" always available by default) will do 
> >> it.
> > 
> > Please don't. Any device that is included in the guest
> > by default and can't be turned off makes libvirt's life
> > significantly more difficult (see below).
> >
> >> If libvirt is using a specific version of pseries, then it already knows
> >> that <=2.7 has pci.0 as a root, pcie.0 otherwise. libvirt has a knowledge
> >> what QEMU version has what, right?
> > 
> > It doesn't yet, that's the point :)
> > 
> > We *could* add such knowledge to libvirt[1], but *existing*
> > libvirt versions would still not know about it, which means
> > that upgrading QEMU withough upgrading libvirt will result
> > in failure to create new guests.
> >
> > 
> >> In what scenario will an additional machine type help?
> > 
> > Because then libvirt could learn that
> > 
> >   pseries-x.y   <->  pci.0
> >   pseries-pcie-x.y  <->  pcie.0
> > 
> > the same way it already knows that
> > 
> >   pc-i440fx-x.y <->  pci.0
> >   pc-q35-x.y<->  pcie.0
> > 
> > and choosing between one or the other would be, I think,
> > much easier for the user as well.
> >
> >>> a legacy PCI root bus, and will base their PCI address
> >>> allocation / PCI topology decisions on that fact: they
> >>> will, for example, use legacy PCI bridges.
> >>>  
> >>> So if you used a new QEMU binary with a libvirt version
> >>> that doesn't know about the change, new guests would end up
> >>> using the wrong controllers. Existing guests would not be
> >>> affected as they would stick with the older machine types,
> >>> of course.
> >>>  
>  I believe after we introduced the very first
>  pseries-pcie-X.Y, we will just stop adding new pseries-X.Y.
> >>>  
> >>> Isn't i440fx still being updated despite the fact that q35
> >>> exists? Granted, there are a lot more differences between
> >>> those two machine types than just the root bus type.
> >>  
> >> I do not know about i440<->q35 but in pseries the difference is going to be
> >> very simple.
> >>  
> >> For example, we did not change the machine type when we switched from
> >> default OHCI to XHCI, switching from PCI to PCIe does not sound like we
> >> need a whole new machine type for this either.
> > 
> > The change from OHCI to XHCI only affected the *default* USB
> > controller, which libvirt tries its best not to use anyway:
> > instead, it will prefer to use '-M ...,usb=off' along with
> > '-device ...' and set both the controller model and its PCI
> > address explicitly, partially to shield its users from such
> > changes in QEMU.
> 
> 
> Ok. Always likes this approach really. We should start moving to this
> direction with PHB - stop adding the default PHB at all when -nodefaults is
> passed (or -machine pseries,pci=off ?) and let libvirt manage PHBs itself
> (and provide another spapr-phb type like spapr-pcie-host-bridge or add a
> "pcie_root_bus_type" property to the existing PHB type).
> 
> What will be wrong with this approach?

Hm, that's a good point.  If were removing the default PHB entirely,
that I would consider a possible case for a new machine type.  Putting
construction of the PHBs into libvirt's hand could make life simpler
there.  Although it would make it a bit of a pain for people starting
qemu by hand.

I think this option is worth some thought.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [Qemu-ppc] [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default

[David Gibson]

On Fri, Nov 18, 2016 at 09:17:22AM +0100, Andrea Bolognani wrote:
> On Thu, 2016-11-17 at 13:02 +1100, Alexey Kardashevskiy wrote:
> > > That said, considering that a big part of the PCI address
> > > allocation logic is based off whether the specific machine
> > > type exposes a legay PCI Root Bus or a PCI Express Root Bus,
> > > libvirt will need a way to be able to tell which one is which.
> > > 
> > > Version checks are pretty much out of the question, as they
> > > fail as soon as downstream releases enter the picture. A
> > > few ways we could deal with the situation:
> > > 
> > >   1) switch to PCI Express on newer machine types, and
> > >  expose some sort of capability through QMP so that
> > >  libvirt can know about the switch
> 
> [...]
> > > Option 1) would break horribly with existing libvirt
> > > versions, and so would Option 2) if we default to using
> > 
> > How exactly 1) will break libvirt? Migrating from pseries-2.7 to
> > pseries-2.8 does not work anyway, and machines are allowed to behave
> > different from version to version, what distinct difference will using
> > "pseries-pcie-X.Y" make?
> 
> Existing libvirt versions assume that pseries guests have
> a legacy PCI root bus, and will base their PCI address
> allocation / PCI topology decisions on that fact: they
> will, for example, use legacy PCI bridges.

Um.. yeah.. trouble is libvirt's PCI-E address allocation probably
won't work for spapr PCI-E either, because of the weird PCI-E without
root complex presentation we get in PAPR.

> So if you used a new QEMU binary with a libvirt version
> that doesn't know about the change, new guests would end up
> using the wrong controllers. Existing guests would not be
> affected as they would stick with the older machine types,
> of course.
> 
> > I believe after we introduced the very first
> > pseries-pcie-X.Y, we will just stop adding new pseries-X.Y.
> 
> Isn't i440fx still being updated despite the fact that q35
> exists? Granted, there are a lot more differences between
> those two machine types than just the root bus type.

Right, there are heaps of differences between i440fx and q35, and
reasons to keep both updated.  For pseries we have neither the impetus
nor the resources to maintain two different machine type variant,
where the only difference is between legacy PCI and weirdly presented
PCI-E.

> Even if no newer pseries-x.y were to be added after
> introducing pseries-pcie, you could still easily create
> guests that use either root bus, so no loss in functionality.
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC 00/15] qmp: Report supported device types on 'query-machines'

[Eduardo Habkost]

(CCing people from the spapr PCI-express thread)

On Mon, Nov 21, 2016 at 11:11:58PM -0200, Eduardo Habkost wrote:
> The Problem
> ===
> 
> Currently management software has no way to find out which device
> types can be plugged in a machine, unless the machine is already
> initialized.
> 
> Even after the machine is initialized, there's no way to map
> existing bus types to supported device types unless management
> software hardcodes the mapping between bus types and device
> types.
> 
> Example: floppy support on q35 vs i440fx
> 
> 
> There's no way for libvirt to find out that there's no floppy
> controller on pc-q35-* machine-types by default.
> 
> With this series, pc-i440fx-* will report "floppy" as a supported
> device type, but pc-q35-* will not.
> 
> Example: Legacy PCI vs vs PCIe devices
> --
> 
> Some devices require a PCIe bus to be available, others work on
> both legacy PCI and PCIe, while others work only on a legacy PCI
> bus.
> 
> Currently management software has no way to know which devices
> can be added to a given machine, unless it hardcodes machine-type
> names and device-types names.

Another example:

spapr and PCIe root bus
---

See the thread at:
  Subject: [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default

If we mak new spapr machine-type versions create a PCIe root bus,
management software will need a way to find out:

1) The type of the default bus for the machine type;
2) The ID of the default bus for the machine type.

Otherwise, management software will have to hardcode it based on
machine-type version. The proposed interface should solve this
problem.



There are other comment below, at "Limitations" section:

> 
> The Proposed Interface
> ==
> 
> This series adds a new field to the output of 'query-machines':
> 'supported-device-types'. It will contain a list of QOM type
> names, that can be used to find the list of device types that can
> be plugged in the machine by default. The type names reported on
> the new field can then be used as the 'implements' argument on
> the 'qom-list-types' command, to find out which device types can
> be plugged on the machine.
> 
> Example output
> --
> 
>   (QEMU) query-machines
>   {
> "return": [
> [...]
> {
> "supported-device-types": [
> "sys-bus-device"
> ],
> "cpu-max": 1,
> "hotpluggable-cpus": false,
> "name": "none"
> },
> [...]
> {
> "supported-device-types": [
> "sys-bus-device"
> ],
> "cpu-max": 1,
> "hotpluggable-cpus": false,
> "name": "xenpv"
> },
> [...]
> {
> "supported-device-types": [
> "sys-bus-device",
> "floppy",
> "i2c-slave",
> "pci-device",
> "isa-device",
> "ide-device"
> ],
> "name": "pc-i440fx-2.8",
> "alias": "pc",
> "is-default": true,
> "cpu-max": 255,
> "hotpluggable-cpus": true
> },
> [...]
> {
> "supported-device-types": [
> "sys-bus-device",
> "floppy",
> "isa-device",
> "ide-device"
> ],
> "cpu-max": 1,
> "hotpluggable-cpus": true,
> "name": "isapc"
> },
> [...]
> {
> "supported-device-types": [
> "sys-bus-device",
> "floppy",
> "i2c-slave",
> "pci-device",
> "isa-device",
> "ide-device"
> ],
> "cpu-max": 128,
> "hotpluggable-cpus": true,
> "name": "xenfv"
> },
> [...]
> {
> "alias": "q35",
> "supported-device-types": [
> "sys-bus-device",
> "i2c-slave",
> "PCIE-device",
> "isa-device",
> "ide-device"
> ],
> "cpu-max": 288,
> "hotpluggable-cpus": true,
> "name": "pc-q35-2.8"
> },
> [...]
> ]
>   }
> 
> Considered alternatives
> ===
> 
> Indirect mapping (machine => bus => device)
> ---
> 
> This RFC implements a mechanism to implement ax
>   machine-type => supported-device-types
> mapping. An alternative solution I considered was to expose an
> indirect mapping:
>   machine-type => default-bus-types
> followed by
>   bus-type => supported-device-types.
> 
> But exposing only the resulting supported device-types list
> imposes less restrictions on how the device and bus type
> 

Re: [libvirt] [PATCH v4 9/9] docs: Add vhost-scsi

[John Ferlan]

On 11/21/2016 10:58 PM, Eric Farman wrote:
> Signed-off-by: Eric Farman 
> ---
>  docs/formatdomain.html.in | 24 
>  1 file changed, 24 insertions(+)
> 

This will get squashed in with the conf patch. I'll also generate the
news.html.in entry:

  vhost-scsi: Add support scsi_host hostdev passthrough
  Add the capability to pass through a scsi_host HBA and the
  associated LUNs to the guest.
  


ACK -

I'll push these in a bit, just seeing what I get on my test system... Of
course I'm also curious what would happen if I try to pass through a
vHBA ;-)...


John
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 4e40aa1..6bd02cc 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3694,6 +3694,17 @@
>  /devices
>  ...
>  
> +or:
> +
> +
> +  ...
> +  devices
> +hostdev mode='subsystem' type='scsi_host'
> +  source protocol='vhost' wwpn='naa.50014057667280d8'/
> +/hostdev
> +  /devices
> +  ...
> +
>  
>hostdev
>The hostdev element is the main container for 
> describing
> @@ -3732,6 +3743,12 @@
>  If a disk lun in the domain already has the rawio capability,
>  then this setting not required.
>
> +  scsi_host
> +  since 2.5.0For SCSI devices, user
> +is responsible to make sure the device is not used by host. This
> +type passes all LUNs presented by a single HBA to
> +the guest.
> +  
>  
>  
>Note: The managed attribute is only used with PCI 
> devices
> @@ -3795,6 +3812,13 @@
>  credentials to the iSCSI server.
>  
>
> +  scsi_host
> +  Since 2.5.0, multiple LUNs behind a
> +single SCSI HBA are described by a protocol
> +attribute set to "vhost" and a wwpn attribute that
> +is the vhost_scsi wwpn (16 hexadecimal digits with a prefix of
> +"naa.") established in the host configfs.
> +  
>  
>
>vendor, product
> 

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

Re: [libvirt] [PATCH v4 8/9] tests: Introduce basic vhost-scsi tests

[John Ferlan]

On 11/21/2016 10:58 PM, Eric Farman wrote:
> These tests were cloned from hostdev-scsi-virtio-scsi in both
> xml2argv and xml2xml
> 
> We add ones for both vhost-scsi-ccw and vhost-scsi-pci since
> the syntaxes are slightly different between them.
> 
> Signed-off-by: Eric Farman 
> Reviewed-by: Boris Fiuczynski 
> ---
>  .../qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.args  | 23 
>  .../qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.xml   | 34 ++
>  .../qemuxml2argv-hostdev-scsi-vhost-scsi-pci.args  | 24 +
>  .../qemuxml2argv-hostdev-scsi-vhost-scsi-pci.xml   | 42 
> ++
>  tests/qemuxml2argvtest.c   |  6 
>  .../qemuxml2xmlout-hostdev-scsi-vhost-scsi-ccw.xml |  1 +
>  .../qemuxml2xmlout-hostdev-scsi-vhost-scsi-pci.xml |  1 +
>  tests/qemuxml2xmltest.c|  6 
>  8 files changed, 137 insertions(+)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.args
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.xml
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-pci.args
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-pci.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-vhost-scsi-ccw.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-vhost-scsi-pci.xml
> 

Merging w/ patch 6 (conf changes)...

> diff --git 
> a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.args 
> b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.args
> new file mode 100644
> index 000..25e7bd4
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.args
> @@ -0,0 +1,23 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name QEMUGuest2 \
> +-S \
> +-M s390-ccw \
> +-m 214 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \
> +-no-acpi \
> +-boot c \
> +-device virtio-scsi-ccw,id=scsi0,devno=fe.0.0001 \
> +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-virtio-disk0 \
> +-device 
> virtio-blk-ccw,devno=fe.0.,drive=drive-virtio-disk0,id=virtio-disk0 \
> +-device 
> vhost-scsi-ccw,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0,devno=fe.0.0002
>  \

syntax-check complains about above 2 lines being too long - fixed before
push.

> +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0003
> diff --git 
> a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.xml 
> b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.xml
> new file mode 100644
> index 000..e428518
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-ccw.xml
> @@ -0,0 +1,34 @@
> +
> +  QEMUGuest2
> +  c7a5fdbd-edaf-9466-926a-d65c16db1809
> +  219100
> +  219100
> +  1
> +  
> +hvm
> +
> +  
> +  
> +  destroy
> +  restart
> +  destroy
> +  
> +/usr/bin/qemu
> +
> +  
> +  
> +  
> +
> +
> +  
> +
> +
> +  
> +  
> +
> +
> +  
> +
> +
> +  
> +
> diff --git 
> a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-pci.args 
> b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-pci.args
> new file mode 100644
> index 000..7071e62
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-pci.args
> @@ -0,0 +1,24 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name QEMUGuest2 \
> +-S \
> +-M pc \
> +-m 214 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \
> +-nographic \
> +-nodefaults \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \
> +-no-acpi \
> +-boot c \
> +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-device 
> vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0,bus=pci.0,addr=0x5
>  \

Likewise, line too long - fixed before push.

ACK w/ adjustments (I'll make them)

John

> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> diff --git 
> a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-pci.xml 
> b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-pci.xml
> new file mode 100644
> index 000..aea6f7f
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi-pci.xml
> @@ -0,0 +1,42 @@
> +
> +  QEMUGuest2
> +  

Re: [libvirt] [PATCH v4 7/9] security: Include vhost-scsi in security labels

[John Ferlan]

On 11/21/2016 10:58 PM, Eric Farman wrote:
> Ensure that the vhost-scsi wwpn information is passed to the
> different security policies.
> 
> Signed-off-by: Eric Farman 
> ---
>  src/security/security_apparmor.c | 20 -
>  src/security/security_dac.c  | 46 
> ++--
>  src/security/security_selinux.c  | 43 +++--
>  3 files changed, 104 insertions(+), 5 deletions(-)
> 

Going to swap the order of this - put it after qemu and before conf,
making conf the "last" patch

ACK

John

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

Re: [libvirt] [PATCH v4 6/9] conf: Wire up the vhost-scsi connection from/to XML

[John Ferlan]

On 11/21/2016 10:58 PM, Eric Farman wrote:
> With the QEMU components in place, provide the XML parsing to
> invoke that code when given the following XML snippet:
> 
> 
>   
> 
> 
> An optional address element can be specified within the hostdev
> (pick CCW or PCI as necessary):
> 
>   
>function='0x0'/>
> 
> Signed-off-by: Eric Farman 
> ---
>  docs/schemas/domaincommon.rng | 23 +++
>  src/conf/domain_audit.c   |  7 
>  src/conf/domain_conf.c| 92 
> ++-
>  3 files changed, 121 insertions(+), 1 deletion(-)
> 

Typically when we add rng we add the xml2xml in the same patch - I'll
merge patch 8 into here to keep things "normal" and "familiar".  Also
also merge patch 9 into here since that too would be needed...  Will
move patch 7 to just before here, so that the conf will be the last patch.

> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 19d45fd..bb903ef 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3974,6 +3974,7 @@
>
>
>
> +  
>  
>
>  
> @@ -4102,6 +4103,28 @@
>  
>
>  
> +  
> +
> +  scsi_host
> +
> +
> +  
> +
> +  
> +
> +  vhost 
> +
> +  
> +  
> +
> +  (naa\.)[0-9a-fA-F]{16}
> +
> +  
> +
> +  
> +
> +  
> +
>
>  
>storage
> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
> index 2decf02..2d9ff5e 100644
> --- a/src/conf/domain_audit.c
> +++ b/src/conf/domain_audit.c
> @@ -392,6 +392,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
> virDomainHostdevDefPtr hostdev,
>  virDomainHostdevSubsysUSBPtr usbsrc = >source.subsys.u.usb;
>  virDomainHostdevSubsysPCIPtr pcisrc = >source.subsys.u.pci;
>  virDomainHostdevSubsysSCSIPtr scsisrc = >source.subsys.u.scsi;
> +virDomainHostdevSubsysSCSIVHostPtr hostsrc = 
> >source.subsys.u.scsi_host;
>  
>  virUUIDFormat(vm->def->uuid, uuidstr);
>  if (!(vmname = virAuditEncode("vm", vm->def->name))) {
> @@ -444,6 +445,12 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
> virDomainHostdevDefPtr hostdev,
>  }
>  break;
>  }
> +case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST:

Looks like a missed spot from the already pushed patch (no big deal,
just making a mental note).  The default: exists so so I see why.

> +if (VIR_STRDUP_QUIET(address, hostsrc->wwpn) < 0) {
> +VIR_WARN("OOM while encoding audit message");
> +goto cleanup;
> +}
> +break;
>  default:
>  VIR_WARN("Unexpected hostdev type while encoding audit message: 
> %d",
>   hostdev->source.subsys.type);
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 3a4123d..618a214 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2323,6 +2323,9 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr 
> def)
>  } else {
>  VIR_FREE(scsisrc->u.host.adapter);
>  }
> +} else if (def->source.subsys.type == 
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) {
> +virDomainHostdevSubsysSCSIVHostPtr hostsrc = 
> >source.subsys.u.scsi_host;
> +VIR_FREE(hostsrc->wwpn);
>  }
>  break;
>  }
> @@ -6094,6 +6097,58 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr 
> sourcenode,
>  return ret;
>  }
>  
> +static int
> +virDomainHostdevSubsysSCSIVHostDefParseXML(xmlNodePtr sourcenode,
> +   virDomainHostdevDefPtr def)
> +{
> +char *protocol = NULL;
> +virDomainHostdevSubsysSCSIVHostPtr hostsrc = 
> >source.subsys.u.scsi_host;
> +
> +if (!(protocol = virXMLPropString(sourcenode, "protocol"))) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("Missing scsi_host subsystem protocol"));
> +return -1;
> +}
> +
> +if ((hostsrc->protocol =
> + virDomainHostdevSubsysSCSIHostProtocolTypeFromString(protocol)) <= 
> 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("Unknown scsi_host subsystem protocol '%s'"),
> +   protocol);
> +goto cleanup;
> +}
> +
> +switch ((virDomainHostdevSubsysSCSIHostProtocolType) hostsrc->protocol) {
> +case VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_VHOST:
> +if (!(hostsrc->wwpn = virXMLPropString(sourcenode, "wwpn"))) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("missing vhost-scsi hostdev source wwpn"));
> +goto cleanup;
> +}
> +
> +if (!STRPREFIX(hostsrc->wwpn, "naa.") ||
> +!virValidateWWN(hostsrc->wwpn + 4)) {
> +

Re: [libvirt] [PATCH v4 5/9] qemu: Allow hotplug of vhost-scsi device

[John Ferlan]

On 11/21/2016 10:58 PM, Eric Farman wrote:
> Adjust the device string that is built for vhost-scsi devices so that it
> can be invoked from hotplug.
> 
> From the QEMU command line, the file descriptors are expect to be numeric 
> only.
> However, for hotplug, the file descriptors are expected to begin with at least
> one alphabetic character else this error occurs:
> 
>   # virsh attach-device guest_0001 ~/vhost.xml
>   error: Failed to attach device from /root/vhost.xml
>   error: internal error: unable to execute QEMU command 'getfd':
>   Parameter 'fdname' expects a name not starting with a digit
> 
> We also close the file descriptor in this case, so that shutting down the
> guest cleans up the host cgroup entries and allows future guests to use
> vhost-scsi devices.  (Otherwise the guest will silently end.)
> 
> Signed-off-by: Eric Farman 
> ---
>  src/qemu/qemu_hotplug.c | 164 
> 
>  1 file changed, 164 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 0508c67..02e248f 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2443,6 +2443,126 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
>  goto cleanup;
>  }
>  
> +static int
> +qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver,
> +virDomainObjPtr vm,
> +virDomainHostdevDefPtr hostdev)
> +{
> +int ret = -1;
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +virErrorPtr orig_err;
> +virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_HOSTDEV,
> +   { .hostdev = hostdev } };
> +virDomainCCWAddressSetPtr ccwaddrs = NULL;
> +char *vhostfdName = NULL;
> +int vhostfd = -1;
> +char *devstr = NULL;
> +bool teardowncgroup = false;
> +bool teardownlabel = false;
> +bool releaseaddr = false;
> +
> +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("SCSI passthrough is not supported by this version 
> of qemu"));
> +goto cleanup;
> +}
> +
> +if (qemuHostdevPrepareSCSIVHostDevices(driver, vm->def->name, , 
> 1) < 0) {
> +virDomainHostdevSubsysSCSIVHostPtr hostsrc = 
> >source.subsys.u.scsi_host;
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Unable to prepare scsi_host hostdev: %s"),
> +   hostsrc->wwpn);
> +goto cleanup;
> +}
> +
> +if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
> +goto cleanup;
> +teardowncgroup = true;
> +
> +if (virSecurityManagerSetHostdevLabel(driver->securityManager,
> +  vm->def, hostdev, NULL) < 0)
> +goto cleanup;
> +teardownlabel = true;
> +
> +if (virSCSIVHostOpenVhostSCSI() < 0)
> +goto cleanup;
> +
> +if (virAsprintf(, "vhostfd-%d", vhostfd) < 0)
> +goto cleanup;
> +
> +if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> +if (qemuDomainMachineIsS390CCW(vm->def) &&
> +virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW))
> +hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
> +}
> +
> +if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
> +hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +if (qemuDomainEnsurePCIAddress(vm, ) < 0)
> +goto cleanup;
> +} else if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
> +if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
> +goto cleanup;
> +if (virDomainCCWAddressAssign(hostdev->info, ccwaddrs,
> +  !hostdev->info->addr.ccw.assigned) < 0)
> +goto cleanup;
> +}
> +releaseaddr = true;
> +
> +if (qemuAssignDeviceHostdevAlias(vm->def, >info->alias, -1) < 0)
> +goto cleanup;
> +
> +if (!(devstr = qemuBuildSCSIVHostHostdevDevStr(vm->def,
> +   hostdev,
> +   priv->qemuCaps,
> +   vhostfdName)))
> +goto cleanup;
> +
> +if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
> +goto cleanup;
> +
> +qemuDomainObjEnterMonitor(driver, vm);
> +
> +if (qemuMonitorAddDeviceWithFd(priv->mon, devstr, vhostfd, vhostfdName) 
> < 0)
> +goto exit_monitor;
> +
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +goto audit;
> +
> +vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> +ret = 0;
> +
> + audit:
> +virDomainAuditHostdev(vm, hostdev, "attach", (ret == 0));
> +
> + cleanup:
> +virDomainCCWAddressSetFree(ccwaddrs);


Similar to AttachHost{PCI|USB|SCSI}Devices since

Re: [libvirt] [PATCH v4 4/9] qemu: Add vhost-scsi string for -device parameter

[John Ferlan]

[...]

>  
>  void
> +qemuHostdevReAttachSCSIVHostDevices(virQEMUDriverPtr driver,
> +const char *name,
> +virDomainHostdevDefPtr *hostdevs,
> +int nhostdevs)
> +{
> +size_t i;
> +virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +
> +for (i = 0; i < nhostdevs; i++) {
> +virDomainHostdevDefPtr hostdev = hostdevs[i];
> +virDomainDeviceDef dev;
> +
> +dev.type = VIR_DOMAIN_DEVICE_HOSTDEV;
> +dev.data.hostdev = hostdev;
> +
> +ignore_value(qemuRemoveSharedDevice(driver, , name));
> +}

This whole loop won't be necessary, true?  Since this isn't shared (it
does nothing for now).

I can remove before pushing - just confirm my suspicion...

ACK,

John

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

Re: [libvirt] [PATCH v4 3/9] util: Management routines for scsi_host devices

[John Ferlan]

[...]

> +
> +/* For virReportOOMError()  and virReportSystemError() */
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +

Made a minor adjustment to move this above the VIR_LOG_INIT - it's just
cosmetic...

[...]

> +virSCSIVHostDevicePtr
> +virSCSIVHostDeviceNew(const char *name)
> +{
> +virSCSIVHostDevicePtr dev;
> +
> +if (VIR_ALLOC(dev) < 0)
> +return NULL;
> +
> +if (VIR_STRDUP(dev->name, name) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("dev->name buffer overflow: %s"),
> +   name);
> +goto error;
> +}
> +
> +if (virAsprintf(>path, "%s/%s",
> +SYSFS_VHOST_SCSI_DEVICES, name) < 0)
> +goto cleanup;

I believe this is goto error;  (I can change before push)

ACK

John

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

Re: [libvirt] [PATCH v4 2/9] Introduce framework for a hostdev SCSI_host subsystem type

[John Ferlan]

On 11/21/2016 10:58 PM, Eric Farman wrote:
> We already have a "scsi" hostdev subsys type, which refers to a single
> LUN that is passed through to a guest.  But what of things where
> multiple LUNs are passed through via a single SCSI HBA, such as with
> the vhost-scsi target?  Create a new hostdev subsys type that will
> carry this.
> 
> Signed-off-by: Eric Farman 
> ---
>  src/conf/domain_conf.c  | 11 ++-
>  src/conf/domain_conf.h  | 18 ++
>  src/qemu/qemu_cgroup.c  |  7 +++
>  src/qemu/qemu_hotplug.c |  2 ++
>  src/security/security_apparmor.c|  4 
>  src/security/security_dac.c |  8 
>  src/security/security_selinux.c |  8 
>  tests/domaincapsschemadata/full.xml |  1 +
>  8 files changed, 58 insertions(+), 1 deletion(-)
> 

ACK

John


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

Re: [libvirt] [PATCH v4 1/9] qemu: Introduce vhost-scsi capability

[John Ferlan]

On 11/21/2016 10:58 PM, Eric Farman wrote:
> Do all the stuff for the vhost-scsi capability in QEMU,
> so it's in place for our checks later.
> 
> Signed-off-by: Eric Farman 
> Reviewed-by: Boris Fiuczynski 
> ---
>  src/qemu/qemu_capabilities.c| 2 ++
>  src/qemu/qemu_capabilities.h| 1 +
>  tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml| 1 +
>  tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml| 1 +
>  tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml| 1 +
>  tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml| 1 +
>  tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml| 1 +
>  tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml| 1 +
>  tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 +
>  tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 +
>  tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml   | 1 +
>  tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml| 1 +
>  tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml| 1 +
>  13 files changed, 14 insertions(+)
> 

ACK,

John

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

Re: [libvirt] [PATCH] virsh: Document --rdma-pin-all migrate option in man page

[Martin Kletzander]

On Tue, Nov 22, 2016 at 07:52:55PM +0100, Jiri Denemark wrote:

On Tue, Nov 22, 2016 at 19:47:21 +0100, Martin Kletzander wrote:

On Tue, Nov 22, 2016 at 07:29:00PM +0100, Jiri Denemark wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1368351
>
> Signed-off-by: Jiri Denemark 
> ---
> tools/virsh.pod | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 74d531122..894863848 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1618,7 +1618,7 @@ to the I namespace is displayed instead of being 
modified.
> =item B [I<--live>] [I<--offline>] [I<--direct>] [I<--p2p> 
[I<--tunnelled>]]
> [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>]
> [I<--copy-storage-inc>] [I<--change-protection>] [I<--unsafe>] [I<--verbose>]
> -[I<--abort-on-error>] [I<--postcopy>] [I<--postcopy-after-precopy>]
> +[I<--rdma-pin-all>] [I<--abort-on-error>] [I<--postcopy>] 
[I<--postcopy-after-precopy>]
> I I [I] [I] [I] 
[I]
> [I<--timeout> B [I<--timeout-suspend> | I<--timeout-postcopy>]]
> [I<--xml> B] [I<--migrate-disks> B] [I<--disks-port> B]
> @@ -1662,6 +1662,10 @@ guest CPU throttling rate can be set with 
I. If the
> initial throttling rate is not enough to ensure convergence, the rate is
> periodically increased by I.
>
> +I<--rdma-pin-all> can be used with RDMA migration (i.e., when I
> +starts with rdma://) to tell the hypervisor to pin all domain's memory at 
once
> +before migration starts rather then letting it pin memory pages as needed.
> +
> B: Individual hypervisors usually do not support all possible types of
> migration. For example, QEMU does not support direct migration.
>

You should also fix the help string when modifying this because "support
memory pinning" doesn't seem like it means what's meantioned in this
paragraph (which feel way more precise).

ACK with that adjusted.


Is "pin all memory before starting RDMA live migration" compatible with
what you had in mind? :-)



Yeah, way better than *support memory pinning* during RDMA live
migration ;)


Jirka


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: Add support for VIR_MIGRATE_PARAM_PERSIST_XML

[Martin Kletzander]

On Tue, Nov 22, 2016 at 02:46:19PM +0100, Jiri Denemark wrote:

Commit v1.3.3-181-gb028e9d7c implmented support for
VIR_MIGRATE_PARAM_PERSIST_XML migration parameter, but forgot to update
virsh.

https://bugzilla.redhat.com/show_bug.cgi?id=835300

Signed-off-by: Jiri Denemark 
---
docs/news.html.in|  2 ++
tools/virsh-domain.c | 23 +++
tools/virsh.pod  |  5 -
3 files changed, 29 insertions(+), 1 deletion(-)



ACK

Finally this will be fixed after almost 4½ years since the bug was
reported =)


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 8/8] virt-admin: Wire-up the logging APIs

[John Ferlan]

On 11/22/2016 11:29 AM, Erik Skultety wrote:
> On Wed, Nov 09, 2016 at 11:26:31AM -0500, John Ferlan wrote:
>>
>>
>> On 11/01/2016 06:27 AM, Erik Skultety wrote:
>>> Finally, now that all APIs have been introduced, wire them up to virt-admin
>>> and introduce dmn-log-info and dmn-log-define commands.
>>>
>>> Signed-off-by: Erik Skultety 
>>> ---
>>>  tools/virt-admin.c | 141 
>>> +
>>>  1 file changed, 141 insertions(+)
>>
>> virt-admin.pod?
>>
> 
> Shame on me. I'll fix this in the next version right after we reach a 
> consensus
> on all the patches.
> 
>> It seems you're defining a new set of configuration alteration comments,
>> so the virt-admin.pod would have a new "CONFIGURATION" section and all
>> the commands could be prefixed with "cfg-" - thoughts?
>>
> 
> We already have some cfg- stuff implemented under server- and client-, not
> mentioning that we already have some aliases too, so I think it's a bit late
> for this now that we follow the philosophy of placing server-related cfg stuff
> under server- and client-related stuff under client-, so I think having
> daemon-generic cfg stuff under daemon- might be reasonable.
> 

Fair enough - no sense in making a new class of command abbreviations.

John

> Erik
> 
>> Then you'd have:
>>
>> cfg-log-outputs [outputs]
>>
>> where if [outputs] was provided, then you have a SET function while if
>> not you have a GET function
>>
>> Naturally then you'd have:
>>
>> cfg-log-filters [filters]
>>
>> and you could also have:
>>
>> cfg-log-level [level]
>>
>> which would allow adjustment of that too.
>>
>> OK - so I'm looking really far forward.
>>
>>
>>>
>>> diff --git a/tools/virt-admin.c b/tools/virt-admin.c
>>> index b1e0c49..0bada05 100644
>>> --- a/tools/virt-admin.c
>>> +++ b/tools/virt-admin.c
>>> @@ -971,6 +971,135 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd)
>>>  goto cleanup;
>>>  }
>>>  
>>> +/* ---
>>> + * Command dmn-log-info
>>
>> Is this dmn- or daemon-
>>
>> I know this was discussed at KVM Forum, but I've forgotten...
>>
>>> + * ---
>>> + */
>>> +static const vshCmdInfo info_dmn_log_info[] = {
>>> +{.name = "help",
>>> + .data = N_("view daemon's current logging settings")
>>> +},
>>> +{.name = "desc",
>>> + .data = N_("Returns all currently active logging settings on daemon. "
>>> +"These include global logging level, logging filters and "
>>> +"logging outputs.")
>>> +},
>>> +{.name = NULL}
>>> +};
>>> +
>>> +static const vshCmdOptDef opts_dmn_log_info[] = {
>>> +{.name = "outputs",
>>> + .type = VSH_OT_BOOL,
>>> + .help = N_("query logging outputs")
>>> +},
>>> +{.name = "filters",
>>> + .type = VSH_OT_BOOL,
>>> + .help = N_("query logging filters")
>>> +},
>>> +{.name = NULL}
>>> +};
>>> +
>>> +static bool
>>> +cmdDmnLogInfo(vshControl *ctl, const vshCmd *cmd)
>>> +{
>>> +bool optOutputs = vshCommandOptBool(cmd, "outputs");
>>> +bool optFilters = vshCommandOptBool(cmd, "filters");
>>> +bool all = optOutputs + optFilters == 0;
>>> +int nfilters, noutputs;
>>> +char *filters, *outputs;
>>> +vshAdmControlPtr priv = ctl->privData;
>>> +
>>> +if (all || optFilters) {
>>> +if ((nfilters = virAdmConnectGetLoggingFilters(priv->conn,
>>> +   , 0)) < 0) {
>>> +vshError(ctl, _("Unable to get daemon logging filters 
>>> information"));
>>> +return false;
>>> +}
>>> +}
>>> +
>>> +if (all || optOutputs) {
>>> +if ((noutputs = virAdmConnectGetLoggingOutputs(priv->conn,
>>> +   , 0)) < 0) {
>>> +vshError(ctl, _("Unable to get daemon logging outputs 
>>> information"));
>>> +return false;
>>> +}
>>> +}
>>> +
>>> +if (all || optFilters) {
>>> +vshPrintExtra(ctl, " %-15s", _("Logging filters: "));
>>> +vshPrint(ctl, "%s\n", filters);
>>> +}
>>> +
>>> +if (all || optOutputs) {
>>> +vshPrintExtra(ctl, " %-15s", _("Logging outputs: "));
>>> +vshPrint(ctl, "%s\n", outputs);
>>> +}
>>> +
>>> +return true;
>>> +}
>>> +
>>> +/* -
>>> + * Command daemon-log-define
>>
>> Is the dmn- or daemon-
>>
>>> + * -
>>> + */
>>> +static const vshCmdInfo info_dmn_log_define[] = {
>>> +{.name = "help",
>>> + .data = N_("change daemon's logging settings")
>>> +},
>>> +{.name = "desc",
>>> + .data = N_("Defines and installs a new set of logging settings on a 
>>> daemon. "
>>> +"These include global logging level, logging filters and "
>>> +"logging outputs.")
>>> +},
>>> +{.name = NULL}
>>> +};
>>> +
>>> +static const vshCmdOptDef opts_dmn_log_define[] = {
>>> +{.name = "outputs",

[libvirt] [PATCH] qemu: Report tunnelled post-copy migration as unsupported

[Jiri Denemark]

Post-copy migration needs bi-directional communication between the
source and the destination QEMU processes, which is not supported by
tunnelled migration.

https://bugzilla.redhat.com/show_bug.cgi?id=1371358

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d4a55d8f7..26b2e6d25 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3167,6 +3167,12 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
 goto cleanup;
 }
 
+if (flags & VIR_MIGRATE_POSTCOPY && flags & VIR_MIGRATE_TUNNELLED) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+   _("post-copy is not supported with tunnelled 
migration"));
+goto cleanup;
+}
+
 if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) {
 bool has_drive_mirror =  virQEMUCapsGet(priv->qemuCaps,
 QEMU_CAPS_DRIVE_MIRROR);
@@ -3645,6 +3651,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 goto cleanup;
 }
 
+if (flags & VIR_MIGRATE_POSTCOPY && flags & VIR_MIGRATE_TUNNELLED) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+   _("post-copy is not supported with tunnelled 
migration"));
+goto cleanup;
+}
+
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
 goto cleanup;
 
-- 
2.11.0.rc2

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

Re: [libvirt] [PATCH 7/8] admin: Introduce virAdmConnectSetLoggingFilters

[John Ferlan]

On 11/22/2016 11:29 AM, Erik Skultety wrote:
> On Wed, Nov 09, 2016 at 11:26:26AM -0500, John Ferlan wrote:
>>
>>
>> On 11/01/2016 06:27 AM, Erik Skultety wrote:
>>> Enable libvirt users to modify logging filters of a daemon from outside.
>>>
>>> Signed-off-by: Erik Skultety 
>>> ---
>>>  daemon/admin.c  | 10 ++
>>>  include/libvirt/libvirt-admin.h |  4 
>>>  src/admin/admin_protocol.x  | 12 +++-
>>>  src/admin_protocol-structs  |  5 +
>>>  src/libvirt-admin.c | 36 
>>>  src/libvirt_admin_private.syms  |  1 +
>>>  src/libvirt_admin_public.syms   |  1 +
>>>  7 files changed, 68 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/daemon/admin.c b/daemon/admin.c
>>> index 79961b2..b66ccd8 100644
>>> --- a/daemon/admin.c
>>> +++ b/daemon/admin.c
>>> @@ -434,6 +434,16 @@ adminConnectSetLoggingOutputs(virNetDaemonPtr dmn 
>>> ATTRIBUTE_UNUSED,
>>>  }
>>>  
>>>  static int
>>> +adminConnectSetLoggingFilters(virNetDaemonPtr dmn ATTRIBUTE_UNUSED,
>>> +  const char *filters,
>>> +  unsigned int flags)
>>> +{
>>> +virCheckFlags(0, -1);
>>> +
>>> +return virLogSetFilters(filters);
>>> +}
>>> +
>>> +static int
>>>  adminDispatchConnectGetLoggingOutputs(virNetServerPtr server 
>>> ATTRIBUTE_UNUSED,
>>>virNetServerClientPtr client 
>>> ATTRIBUTE_UNUSED,
>>>virNetMessagePtr msg 
>>> ATTRIBUTE_UNUSED,
>>> diff --git a/include/libvirt/libvirt-admin.h 
>>> b/include/libvirt/libvirt-admin.h
>>> index aa33fef..161727e 100644
>>> --- a/include/libvirt/libvirt-admin.h
>>> +++ b/include/libvirt/libvirt-admin.h
>>> @@ -416,6 +416,10 @@ int virAdmConnectSetLoggingOutputs(virAdmConnectPtr 
>>> conn,
>>> const char *outputs,
>>> unsigned int flags);
>>>  
>>> +int virAdmConnectSetLoggingFilters(virAdmConnectPtr conn,
>>> +   const char *filters,
>>> +   unsigned int flags);
>>> +
>>>  # ifdef __cplusplus
>>>  }
>>>  # endif
>>> diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
>>> index 4a05928..4eef088 100644
>>> --- a/src/admin/admin_protocol.x
>>> +++ b/src/admin/admin_protocol.x
>>> @@ -206,6 +206,11 @@ struct admin_connect_set_logging_outputs_args {
>>>  unsigned int flags;
>>>  };
>>>  
>>> +struct admin_connect_set_logging_filters_args {
>>> +admin_nonnull_string filters;
>>> +unsigned int flags;
>>> +};
>>> +
>>>  /* Define the program number, protocol version and procedure numbers here. 
>>> */
>>>  const ADMIN_PROGRAM = 0x06900690;
>>>  const ADMIN_PROTOCOL_VERSION = 1;
>>> @@ -306,5 +311,10 @@ enum admin_procedure {
>>>  /**
>>>   * @generate: both
>>>   */
>>> -ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16
>>> +ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16,
>>> +
>>> +/**
>>> + * @generate: both
>>> + */
>>> +ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17
>>>  };
>>> diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs
>>> index cbc99e3..1974c07 100644
>>> --- a/src/admin_protocol-structs
>>> +++ b/src/admin_protocol-structs
>>> @@ -145,6 +145,10 @@ struct admin_connect_set_logging_outputs_args {
>>>  admin_nonnull_string   outputs;
>>>  u_int  flags;
>>>  };
>>> +struct admin_connect_set_logging_filters_args {
>>> +admin_nonnull_string   filters;
>>> +u_int  flags;
>>> +};
>>>  enum admin_procedure {
>>>  ADMIN_PROC_CONNECT_OPEN = 1,
>>>  ADMIN_PROC_CONNECT_CLOSE = 2,
>>> @@ -162,4 +166,5 @@ enum admin_procedure {
>>>  ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14,
>>>  ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15,
>>>  ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16,
>>> +ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17,
>>>  };
>>> diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
>>> index 01ae26c..c74e6b9 100644
>>> --- a/src/libvirt-admin.c
>>> +++ b/src/libvirt-admin.c
>>> @@ -1221,3 +1221,39 @@ virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn,
>>>  virDispatchError(NULL);
>>>  return -1;
>>>  }
>>> +
>>> +/**
>>> + * virAdmConnectSetLoggingFilters:
>>> + * @conn: pointer to an active admin connection
>>> + * @filters: pointer to a string containing a list of filters to be defined
>>> + * @flags: extra flags; not used yet, so callers should always pass 0
>>> + *
>>> + * Redefine the existing (set of) filter(s) with a new one specified in
>>> + * @filters. If multiple filters are specified, they need to be delimited 
>>> by
>>> + * spaces. The format of each filter must conform to the format described 
>>> in
>>> + * daemon's configuration file (e.g. libvirtd.conf).
>>> + *
>>
>> So you didn't want to write the code that 

Re: [libvirt] [PATCH 6/8] admin: Introduce virAdmConnectSetLoggingOutputs

[John Ferlan]

On 11/22/2016 11:29 AM, Erik Skultety wrote:
>>>  };
>>> diff --git a/src/util/virlog.c b/src/util/virlog.c
>>> index 4ac72dc..d04a461 100644
>>> --- a/src/util/virlog.c
>>> +++ b/src/util/virlog.c
>>> @@ -1820,6 +1820,8 @@ virLogParseFilters(const char *src, virLogFilterPtr 
>>> **filters)
>>>   * @outputs: string defining a (set of) output(s)
>>>   *
>>>   * Replaces the current set of defined outputs with a new set of outputs.
>>> + * Should the set be empty, a default output is used according to the 
>>> daemon's
>>> + * runtime attributes.
>>>   *
>>>   * Returns 0 on success or -1 in case of an error.
>>>   */
>>> @@ -1828,12 +1830,13 @@ virLogSetOutputs(const char *src)
>>>  {
>>>  int ret = -1;
>>>  int noutputs = 0;
>>> +const char *outputstr = *src ? src : virLogGetDefaultOutput();
>>
>> Not sure this is right - virAdmConnectSetLoggingOutputs says 'outputs'
>> must be NonNull.
>>
>> I assume the generated remoteAdminConnectSetLoggingOutputs will end up
>> calling adminConnectSetLoggingOutputs with a NonNull 'outputs'.
>>
>> Thus this code gets called with NonNull outputs.
>>
> 
> So, what exactly are you proposing? Because what I understand from it now is
> that I should probably allow passing NULL outputs in
> virAdmConnectSetLoggingOutputs as a way of resetting the outputs to our 
> defaults
> (as an addition to "").
> 

I wonder if I was reading this as "src ? src : virLogGetDefaultOutput()"

Been too long for me to recall for sure, but I do see the *src now and
sure that does what you want and I think what I wrote (so what was a I
thinking?)...

>> Anyway, what you're really looking to do is check if the contents of
>> 'outputs' is empty, then use some default value.  In this case, since
>> this code "owns" virLogDefaultOutput is there really a reason to call
>> the API?
> 
> True, I certainly don't have to make the call.
> 

Usage of the API isn't necessary, but it can be done - some prefer one
way over another.

>>
>> NB: even though virlog.h says outputs cannot be passed as NULL that's
>> somewhat of a misnomer since all the compiler is checking is that the
>> argument in the call stack isn't NULL - it doesn't check if the argument
>> being passed in actually NULL.
>>
> 
> Yes, but this should be actual check for NULL should be the case when we're
> handling user-passed values in which case as it is right now, there's no need,
> user can't pass NULL and in all the other cases, our internal callers should
> respect the ATTRIBUTE_NONNULL constraint. Of course, if you were suggesting to
> actually allow users to pass NULL to the public API^^, then yes, this has to 
> be
> adjusted properly.

An empty string is fine and no need to check for NULL too, unless you
think setting up an empty outputs would be useful for some other
designation (but we can leave that to the future).

John
> 
> Thanks,
> Erik
> 
>> John
>>
>>>  virLogOutputPtr *outputs = NULL;
>>>  
>>>  if (virLogInitialize() < 0)
>>>  return -1;
>>>  
>>> -if ((noutputs = virLogParseOutputs(src, )) < 0)
>>> +if ((noutputs = virLogParseOutputs(outputstr, )) < 0)
>>>  goto cleanup;
>>>  
>>>  if (virLogDefineOutputs(outputs, noutputs) < 0)
>>>

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

Re: [libvirt] [PATCH 0/2] Forbid new-line char in name of domains and storagepools

[Sławek Kapłoński]

Hello,

Thanks a lot. I have no idea how I not checked it. I compiled it for
sure (but maybe not after this one change :/)

-- 
Best regards / Pozdrawiam
Sławek Kapłoński
sla...@kaplonski.pl

On Tue, 22 Nov 2016, Michal Privoznik wrote:

> On 11.11.2016 10:17, Sławek Kapłoński wrote:
> > As https://bugzilla.redhat.com/show_bug.cgi?id=818064 is closed and new-line
> > char is now forbiden in name of network, this patch forbids it also in 
> > names of
> > domains and storage pools.
> > 
> > Sławek Kapłoński (2):
> >   Forbid new-line char in name of new domain
> >   Forbid new-line char in name of new storagepool
> > 
> >  src/bhyve/bhyve_driver.c | 3 +++
> >  src/esx/esx_driver.c | 3 +++
> >  src/libxl/libxl_driver.c | 3 +++
> >  src/lxc/lxc_driver.c | 3 +++
> >  src/openvz/openvz_driver.c   | 3 +++
> >  src/qemu/qemu_driver.c   | 3 +++
> >  src/storage/storage_driver.c | 3 +++
> >  src/test/test_driver.c   | 3 +++
> >  src/uml/uml_driver.c | 3 +++
> >  src/vmware/vmware_driver.c   | 3 +++
> >  src/vz/vz_driver.c   | 3 +++
> >  src/xen/xen_driver.c | 3 +++
> >  src/xenapi/xenapi_driver.c   | 3 +++
> >  13 files changed, 39 insertions(+)
> > 
> 
> Fixed the problem in 1/2. ACKed and pushed.
> 
> Michal
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virsh: Document --rdma-pin-all migrate option in man page

[Jiri Denemark]

On Tue, Nov 22, 2016 at 19:47:21 +0100, Martin Kletzander wrote:
> On Tue, Nov 22, 2016 at 07:29:00PM +0100, Jiri Denemark wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1368351
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> > tools/virsh.pod | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/virsh.pod b/tools/virsh.pod
> > index 74d531122..894863848 100644
> > --- a/tools/virsh.pod
> > +++ b/tools/virsh.pod
> > @@ -1618,7 +1618,7 @@ to the I namespace is displayed instead of being 
> > modified.
> > =item B [I<--live>] [I<--offline>] [I<--direct>] [I<--p2p> 
> > [I<--tunnelled>]]
> > [I<--persistent>] [I<--undefinesource>] [I<--suspend>] 
> > [I<--copy-storage-all>]
> > [I<--copy-storage-inc>] [I<--change-protection>] [I<--unsafe>] 
> > [I<--verbose>]
> > -[I<--abort-on-error>] [I<--postcopy>] [I<--postcopy-after-precopy>]
> > +[I<--rdma-pin-all>] [I<--abort-on-error>] [I<--postcopy>] 
> > [I<--postcopy-after-precopy>]
> > I I [I] [I] [I] 
> > [I]
> > [I<--timeout> B [I<--timeout-suspend> | I<--timeout-postcopy>]]
> > [I<--xml> B] [I<--migrate-disks> B] [I<--disks-port> 
> > B]
> > @@ -1662,6 +1662,10 @@ guest CPU throttling rate can be set with 
> > I. If the
> > initial throttling rate is not enough to ensure convergence, the rate is
> > periodically increased by I.
> > 
> > +I<--rdma-pin-all> can be used with RDMA migration (i.e., when I
> > +starts with rdma://) to tell the hypervisor to pin all domain's memory at 
> > once
> > +before migration starts rather then letting it pin memory pages as needed.
> > +
> > B: Individual hypervisors usually do not support all possible types of
> > migration. For example, QEMU does not support direct migration.
> > 
> 
> You should also fix the help string when modifying this because "support
> memory pinning" doesn't seem like it means what's meantioned in this
> paragraph (which feel way more precise).
> 
> ACK with that adjusted.

Is "pin all memory before starting RDMA live migration" compatible with
what you had in mind? :-)

Jirka

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

Re: [libvirt] [PATCH] virsh: Document --rdma-pin-all migrate option in man page

[Martin Kletzander]

On Tue, Nov 22, 2016 at 07:29:00PM +0100, Jiri Denemark wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1368351

Signed-off-by: Jiri Denemark 
---
tools/virsh.pod | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 74d531122..894863848 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1618,7 +1618,7 @@ to the I namespace is displayed instead of being 
modified.
=item B [I<--live>] [I<--offline>] [I<--direct>] [I<--p2p> 
[I<--tunnelled>]]
[I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>]
[I<--copy-storage-inc>] [I<--change-protection>] [I<--unsafe>] [I<--verbose>]
-[I<--abort-on-error>] [I<--postcopy>] [I<--postcopy-after-precopy>]
+[I<--rdma-pin-all>] [I<--abort-on-error>] [I<--postcopy>] 
[I<--postcopy-after-precopy>]
I I [I] [I] [I] 
[I]
[I<--timeout> B [I<--timeout-suspend> | I<--timeout-postcopy>]]
[I<--xml> B] [I<--migrate-disks> B] [I<--disks-port> B]
@@ -1662,6 +1662,10 @@ guest CPU throttling rate can be set with 
I. If the
initial throttling rate is not enough to ensure convergence, the rate is
periodically increased by I.

+I<--rdma-pin-all> can be used with RDMA migration (i.e., when I
+starts with rdma://) to tell the hypervisor to pin all domain's memory at once
+before migration starts rather then letting it pin memory pages as needed.
+
B: Individual hypervisors usually do not support all possible types of
migration. For example, QEMU does not support direct migration.



You should also fix the help string when modifying this because "support
memory pinning" doesn't seem like it means what's meantioned in this
paragraph (which feel way more precise).

ACK with that adjusted.


--
2.11.0.rc2

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


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virsh: Document --rdma-pin-all migrate option in man page

[Jiri Denemark]

https://bugzilla.redhat.com/show_bug.cgi?id=1368351

Signed-off-by: Jiri Denemark 
---
 tools/virsh.pod | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 74d531122..894863848 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1618,7 +1618,7 @@ to the I namespace is displayed instead of being 
modified.
 =item B [I<--live>] [I<--offline>] [I<--direct>] [I<--p2p> 
[I<--tunnelled>]]
 [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>]
 [I<--copy-storage-inc>] [I<--change-protection>] [I<--unsafe>] [I<--verbose>]
-[I<--abort-on-error>] [I<--postcopy>] [I<--postcopy-after-precopy>]
+[I<--rdma-pin-all>] [I<--abort-on-error>] [I<--postcopy>] 
[I<--postcopy-after-precopy>]
 I I [I] [I] [I] 
[I]
 [I<--timeout> B [I<--timeout-suspend> | I<--timeout-postcopy>]]
 [I<--xml> B] [I<--migrate-disks> B] [I<--disks-port> B]
@@ -1662,6 +1662,10 @@ guest CPU throttling rate can be set with 
I. If the
 initial throttling rate is not enough to ensure convergence, the rate is
 periodically increased by I.
 
+I<--rdma-pin-all> can be used with RDMA migration (i.e., when I
+starts with rdma://) to tell the hypervisor to pin all domain's memory at once
+before migration starts rather then letting it pin memory pages as needed.
+
 B: Individual hypervisors usually do not support all possible types of
 migration. For example, QEMU does not support direct migration.
 
-- 
2.11.0.rc2

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

Re: [libvirt] [PATCH 6/8] admin: Introduce virAdmConnectSetLoggingOutputs

[Erik Skultety]

> >  };
> > diff --git a/src/util/virlog.c b/src/util/virlog.c
> > index 4ac72dc..d04a461 100644
> > --- a/src/util/virlog.c
> > +++ b/src/util/virlog.c
> > @@ -1820,6 +1820,8 @@ virLogParseFilters(const char *src, virLogFilterPtr 
> > **filters)
> >   * @outputs: string defining a (set of) output(s)
> >   *
> >   * Replaces the current set of defined outputs with a new set of outputs.
> > + * Should the set be empty, a default output is used according to the 
> > daemon's
> > + * runtime attributes.
> >   *
> >   * Returns 0 on success or -1 in case of an error.
> >   */
> > @@ -1828,12 +1830,13 @@ virLogSetOutputs(const char *src)
> >  {
> >  int ret = -1;
> >  int noutputs = 0;
> > +const char *outputstr = *src ? src : virLogGetDefaultOutput();
> 
> Not sure this is right - virAdmConnectSetLoggingOutputs says 'outputs'
> must be NonNull.
> 
> I assume the generated remoteAdminConnectSetLoggingOutputs will end up
> calling adminConnectSetLoggingOutputs with a NonNull 'outputs'.
> 
> Thus this code gets called with NonNull outputs.
> 

So, what exactly are you proposing? Because what I understand from it now is
that I should probably allow passing NULL outputs in
virAdmConnectSetLoggingOutputs as a way of resetting the outputs to our defaults
(as an addition to "").

> Anyway, what you're really looking to do is check if the contents of
> 'outputs' is empty, then use some default value.  In this case, since
> this code "owns" virLogDefaultOutput is there really a reason to call
> the API?

True, I certainly don't have to make the call.

> 
> NB: even though virlog.h says outputs cannot be passed as NULL that's
> somewhat of a misnomer since all the compiler is checking is that the
> argument in the call stack isn't NULL - it doesn't check if the argument
> being passed in actually NULL.
> 

Yes, but this should be actual check for NULL should be the case when we're
handling user-passed values in which case as it is right now, there's no need,
user can't pass NULL and in all the other cases, our internal callers should
respect the ATTRIBUTE_NONNULL constraint. Of course, if you were suggesting to
actually allow users to pass NULL to the public API^^, then yes, this has to be
adjusted properly.

Thanks,
Erik

> John
> 
> >  virLogOutputPtr *outputs = NULL;
> >  
> >  if (virLogInitialize() < 0)
> >  return -1;
> >  
> > -if ((noutputs = virLogParseOutputs(src, )) < 0)
> > +if ((noutputs = virLogParseOutputs(outputstr, )) < 0)
> >  goto cleanup;
> >  
> >  if (virLogDefineOutputs(outputs, noutputs) < 0)
> > 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 8/8] virt-admin: Wire-up the logging APIs

[Erik Skultety]

On Wed, Nov 09, 2016 at 11:26:31AM -0500, John Ferlan wrote:
> 
> 
> On 11/01/2016 06:27 AM, Erik Skultety wrote:
> > Finally, now that all APIs have been introduced, wire them up to virt-admin
> > and introduce dmn-log-info and dmn-log-define commands.
> > 
> > Signed-off-by: Erik Skultety 
> > ---
> >  tools/virt-admin.c | 141 
> > +
> >  1 file changed, 141 insertions(+)
> 
> virt-admin.pod?
>

Shame on me. I'll fix this in the next version right after we reach a consensus
on all the patches.

> It seems you're defining a new set of configuration alteration comments,
> so the virt-admin.pod would have a new "CONFIGURATION" section and all
> the commands could be prefixed with "cfg-" - thoughts?
> 

We already have some cfg- stuff implemented under server- and client-, not
mentioning that we already have some aliases too, so I think it's a bit late
for this now that we follow the philosophy of placing server-related cfg stuff
under server- and client-related stuff under client-, so I think having
daemon-generic cfg stuff under daemon- might be reasonable.

Erik

> Then you'd have:
> 
> cfg-log-outputs [outputs]
> 
> where if [outputs] was provided, then you have a SET function while if
> not you have a GET function
> 
> Naturally then you'd have:
> 
> cfg-log-filters [filters]
> 
> and you could also have:
> 
> cfg-log-level [level]
> 
> which would allow adjustment of that too.
> 
> OK - so I'm looking really far forward.
> 
> 
> > 
> > diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> > index b1e0c49..0bada05 100644
> > --- a/tools/virt-admin.c
> > +++ b/tools/virt-admin.c
> > @@ -971,6 +971,135 @@ cmdSrvClientsSet(vshControl *ctl, const vshCmd *cmd)
> >  goto cleanup;
> >  }
> >  
> > +/* ---
> > + * Command dmn-log-info
> 
> Is this dmn- or daemon-
> 
> I know this was discussed at KVM Forum, but I've forgotten...
> 
> > + * ---
> > + */
> > +static const vshCmdInfo info_dmn_log_info[] = {
> > +{.name = "help",
> > + .data = N_("view daemon's current logging settings")
> > +},
> > +{.name = "desc",
> > + .data = N_("Returns all currently active logging settings on daemon. "
> > +"These include global logging level, logging filters and "
> > +"logging outputs.")
> > +},
> > +{.name = NULL}
> > +};
> > +
> > +static const vshCmdOptDef opts_dmn_log_info[] = {
> > +{.name = "outputs",
> > + .type = VSH_OT_BOOL,
> > + .help = N_("query logging outputs")
> > +},
> > +{.name = "filters",
> > + .type = VSH_OT_BOOL,
> > + .help = N_("query logging filters")
> > +},
> > +{.name = NULL}
> > +};
> > +
> > +static bool
> > +cmdDmnLogInfo(vshControl *ctl, const vshCmd *cmd)
> > +{
> > +bool optOutputs = vshCommandOptBool(cmd, "outputs");
> > +bool optFilters = vshCommandOptBool(cmd, "filters");
> > +bool all = optOutputs + optFilters == 0;
> > +int nfilters, noutputs;
> > +char *filters, *outputs;
> > +vshAdmControlPtr priv = ctl->privData;
> > +
> > +if (all || optFilters) {
> > +if ((nfilters = virAdmConnectGetLoggingFilters(priv->conn,
> > +   , 0)) < 0) {
> > +vshError(ctl, _("Unable to get daemon logging filters 
> > information"));
> > +return false;
> > +}
> > +}
> > +
> > +if (all || optOutputs) {
> > +if ((noutputs = virAdmConnectGetLoggingOutputs(priv->conn,
> > +   , 0)) < 0) {
> > +vshError(ctl, _("Unable to get daemon logging outputs 
> > information"));
> > +return false;
> > +}
> > +}
> > +
> > +if (all || optFilters) {
> > +vshPrintExtra(ctl, " %-15s", _("Logging filters: "));
> > +vshPrint(ctl, "%s\n", filters);
> > +}
> > +
> > +if (all || optOutputs) {
> > +vshPrintExtra(ctl, " %-15s", _("Logging outputs: "));
> > +vshPrint(ctl, "%s\n", outputs);
> > +}
> > +
> > +return true;
> > +}
> > +
> > +/* -
> > + * Command daemon-log-define
> 
> Is the dmn- or daemon-
> 
> > + * -
> > + */
> > +static const vshCmdInfo info_dmn_log_define[] = {
> > +{.name = "help",
> > + .data = N_("change daemon's logging settings")
> > +},
> > +{.name = "desc",
> > + .data = N_("Defines and installs a new set of logging settings on a 
> > daemon. "
> > +"These include global logging level, logging filters and "
> > +"logging outputs.")
> > +},
> > +{.name = NULL}
> > +};
> > +
> > +static const vshCmdOptDef opts_dmn_log_define[] = {
> > +{.name = "outputs",
> > + .type = VSH_OT_STRING,
> > + .help = N_("comma separated list of logging outputs"),
> > + .flags = VSH_OFLAG_EMPTY_OK
> > +},
> > +{.name = "filters",

Re: [libvirt] [PATCH 7/8] admin: Introduce virAdmConnectSetLoggingFilters

[Erik Skultety]

On Wed, Nov 09, 2016 at 11:26:26AM -0500, John Ferlan wrote:
> 
> 
> On 11/01/2016 06:27 AM, Erik Skultety wrote:
> > Enable libvirt users to modify logging filters of a daemon from outside.
> > 
> > Signed-off-by: Erik Skultety 
> > ---
> >  daemon/admin.c  | 10 ++
> >  include/libvirt/libvirt-admin.h |  4 
> >  src/admin/admin_protocol.x  | 12 +++-
> >  src/admin_protocol-structs  |  5 +
> >  src/libvirt-admin.c | 36 
> >  src/libvirt_admin_private.syms  |  1 +
> >  src/libvirt_admin_public.syms   |  1 +
> >  7 files changed, 68 insertions(+), 1 deletion(-)
> > 
> > diff --git a/daemon/admin.c b/daemon/admin.c
> > index 79961b2..b66ccd8 100644
> > --- a/daemon/admin.c
> > +++ b/daemon/admin.c
> > @@ -434,6 +434,16 @@ adminConnectSetLoggingOutputs(virNetDaemonPtr dmn 
> > ATTRIBUTE_UNUSED,
> >  }
> >  
> >  static int
> > +adminConnectSetLoggingFilters(virNetDaemonPtr dmn ATTRIBUTE_UNUSED,
> > +  const char *filters,
> > +  unsigned int flags)
> > +{
> > +virCheckFlags(0, -1);
> > +
> > +return virLogSetFilters(filters);
> > +}
> > +
> > +static int
> >  adminDispatchConnectGetLoggingOutputs(virNetServerPtr server 
> > ATTRIBUTE_UNUSED,
> >virNetServerClientPtr client 
> > ATTRIBUTE_UNUSED,
> >virNetMessagePtr msg 
> > ATTRIBUTE_UNUSED,
> > diff --git a/include/libvirt/libvirt-admin.h 
> > b/include/libvirt/libvirt-admin.h
> > index aa33fef..161727e 100644
> > --- a/include/libvirt/libvirt-admin.h
> > +++ b/include/libvirt/libvirt-admin.h
> > @@ -416,6 +416,10 @@ int virAdmConnectSetLoggingOutputs(virAdmConnectPtr 
> > conn,
> > const char *outputs,
> > unsigned int flags);
> >  
> > +int virAdmConnectSetLoggingFilters(virAdmConnectPtr conn,
> > +   const char *filters,
> > +   unsigned int flags);
> > +
> >  # ifdef __cplusplus
> >  }
> >  # endif
> > diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
> > index 4a05928..4eef088 100644
> > --- a/src/admin/admin_protocol.x
> > +++ b/src/admin/admin_protocol.x
> > @@ -206,6 +206,11 @@ struct admin_connect_set_logging_outputs_args {
> >  unsigned int flags;
> >  };
> >  
> > +struct admin_connect_set_logging_filters_args {
> > +admin_nonnull_string filters;
> > +unsigned int flags;
> > +};
> > +
> >  /* Define the program number, protocol version and procedure numbers here. 
> > */
> >  const ADMIN_PROGRAM = 0x06900690;
> >  const ADMIN_PROTOCOL_VERSION = 1;
> > @@ -306,5 +311,10 @@ enum admin_procedure {
> >  /**
> >   * @generate: both
> >   */
> > -ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16
> > +ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16,
> > +
> > +/**
> > + * @generate: both
> > + */
> > +ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17
> >  };
> > diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs
> > index cbc99e3..1974c07 100644
> > --- a/src/admin_protocol-structs
> > +++ b/src/admin_protocol-structs
> > @@ -145,6 +145,10 @@ struct admin_connect_set_logging_outputs_args {
> >  admin_nonnull_string   outputs;
> >  u_int  flags;
> >  };
> > +struct admin_connect_set_logging_filters_args {
> > +admin_nonnull_string   filters;
> > +u_int  flags;
> > +};
> >  enum admin_procedure {
> >  ADMIN_PROC_CONNECT_OPEN = 1,
> >  ADMIN_PROC_CONNECT_CLOSE = 2,
> > @@ -162,4 +166,5 @@ enum admin_procedure {
> >  ADMIN_PROC_CONNECT_GET_LOGGING_OUTPUTS = 14,
> >  ADMIN_PROC_CONNECT_GET_LOGGING_FILTERS = 15,
> >  ADMIN_PROC_CONNECT_SET_LOGGING_OUTPUTS = 16,
> > +ADMIN_PROC_CONNECT_SET_LOGGING_FILTERS = 17,
> >  };
> > diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
> > index 01ae26c..c74e6b9 100644
> > --- a/src/libvirt-admin.c
> > +++ b/src/libvirt-admin.c
> > @@ -1221,3 +1221,39 @@ virAdmConnectSetLoggingOutputs(virAdmConnectPtr conn,
> >  virDispatchError(NULL);
> >  return -1;
> >  }
> > +
> > +/**
> > + * virAdmConnectSetLoggingFilters:
> > + * @conn: pointer to an active admin connection
> > + * @filters: pointer to a string containing a list of filters to be defined
> > + * @flags: extra flags; not used yet, so callers should always pass 0
> > + *
> > + * Redefine the existing (set of) filter(s) with a new one specified in
> > + * @filters. If multiple filters are specified, they need to be delimited 
> > by
> > + * spaces. The format of each filter must conform to the format described 
> > in
> > + * daemon's configuration file (e.g. libvirtd.conf).
> > + *
> 
> So you didn't want to write the code that would allow resetting filters
> back to their 

Re: [libvirt] [PATCH v2 0/3] qemu: agent: cleanup agent error flag correctly

[Michal Privoznik]

On 16.11.2016 14:43, Nikolay Shirokovskiy wrote:
> Patches 1 and 2 are refactorings to help review patch 3. 
> 
> diff from v1:
> =
> 1. error flag cleanup logic clarified
> 2. patches that touch io loop are dropped
> 
> Nikolay Shirokovskiy (3):
>   qemu: agent: handle agent connection errors in one place
>   qemu: agent: remove redundant check
>   qemu: agent: clenup agent error flag correctly
> 
>  src/qemu/qemu_driver.c| 12 +++---
>  src/qemu/qemu_migration.c | 13 ++
>  src/qemu/qemu_process.c   | 61 
> +--
>  3 files changed, 22 insertions(+), 64 deletions(-)
> 

ACK series.

Michal

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

Re: [libvirt] [PATCH 2/2] qemu: agent: fix unsafe agent access

[Michal Privoznik]

On 14.11.2016 15:24, Nikolay Shirokovskiy wrote:
> qemuDomainObjExitAgent is unsafe.
> 
> First it accesses domain object without domain lock.
> Second it uses outdated logic that goes back to commit 79533da1 of
> year 2009 when code was quite different. (unref function
> instead of unreferencing only unlocked and disposed object
> in case of last reference and leaved unlocking to the caller otherwise).
> Nowadays this logic may lead to disposing locked object
> i guess.

Well, I agree that the order of those two steps should be reversed, so
ACK to that.

> 
> Another problem is that the callers of qemuDomainObjEnterAgent
> use domain object again (namely priv->agent) without domain lock.

But why is this a problem?

> 
> This patch address these two problems.
> 
> qemuDomainGetAgent is dropped as unused.
> ---
> 
> I wonder should we make qemuDomainObj(Enter|Exit)Agent be macros. So 
> we don't need to declare a variable to hold agent reference and zero
> it out at the end for safety.
> 
> Also the qemu monitor code has probably same problems as the agent monitor 
> code
> seems to be copied from there.
> 

Well, qemu monitor can't come and go. I mean, if it does, the domain is
killed. That's not the case with agent monitor.

>  src/qemu/qemu_domain.c |  41 -
>  src/qemu/qemu_domain.h |   7 +--
>  src/qemu/qemu_driver.c | 153 
> -
>  3 files changed, 91 insertions(+), 110 deletions(-)

Michal

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

Re: [libvirt] [PATCH 1/2] qemu: drop write-only agentStart

[Michal Privoznik]

On 14.11.2016 15:24, Nikolay Shirokovskiy wrote:
> ---
>  src/qemu/qemu_domain.c  | 2 --
>  src/qemu/qemu_domain.h  | 1 -
>  src/qemu/qemu_process.c | 2 --
>  3 files changed, 5 deletions(-)
> 

ACK

Michal

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

Re: [libvirt] dpdk/vpp and cross-version migration for vhost

[Michael S. Tsirkin]

On Tue, Nov 22, 2016 at 09:02:23PM +0800, Yuanhan Liu wrote:
> On Thu, Nov 17, 2016 at 07:37:09PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Nov 17, 2016 at 05:49:36PM +0800, Yuanhan Liu wrote:
> > > On Thu, Nov 17, 2016 at 09:47:09AM +0100, Maxime Coquelin wrote:
> > > > 
> > > > 
> > > > On 11/17/2016 09:29 AM, Yuanhan Liu wrote:
> > > > >As usaual, sorry for late response :/
> > > > >
> > > > >On Thu, Oct 13, 2016 at 08:50:52PM +0300, Michael S. Tsirkin wrote:
> > > > >>Hi!
> > > > >>So it looks like we face a problem with cross-version
> > > > >>migration when using vhost. It's not new but became more
> > > > >>acute with the advent of vhost user.
> > > > >>
> > > > >>For users to be able to migrate between different versions
> > > > >>of the hypervisor the interface exposed to guests
> > > > >>by hypervisor must stay unchanged.
> > > > >>
> > > > >>The problem is that a qemu device is connected
> > > > >>to a backend in another process, so the interface
> > > > >>exposed to guests depends on the capabilities of that
> > > > >>process.
> > > > >>
> > > > >>Specifically, for vhost user interface based on virtio, this includes
> > > > >>the "host features" bitmap that defines the interface, as well as more
> > > > >>host values such as the max ring size.  Adding new features/changing
> > > > >>values to this interface is required to make progress, but on the 
> > > > >>other
> > > > >>hand we need ability to get the old host features to be compatible.
> > > > >
> > > > >It looks like to the same issue of vhost-user reconnect to me. For 
> > > > >example,
> > > > >
> > > > >- start dpdk 16.07 & qemu 2.5
> > > > >- kill dpdk
> > > > >- start dpdk 16.11
> > > > >
> > > > >Though DPDK 16.11 has more features comparing to dpdk 16.07 (say, 
> > > > >indirect),
> > > > >above should work. Because qemu saves the negotiated features before 
> > > > >the
> > > > >disconnect and stores it back after the reconnection.
> > > > >
> > > > >commit a463215b087c41d7ca94e51aa347cde523831873
> > > > >Author: Marc-André Lureau 
> > > > >Date:   Mon Jun 6 18:45:05 2016 +0200
> > > > >
> > > > >vhost-net: save & restore vhost-user acked features
> > > > >
> > > > >The initial vhost-user connection sets the features to be 
> > > > > negotiated
> > > > >with the driver. Renegotiation isn't possible without device 
> > > > > reset.
> > > > >
> > > > >To handle reconnection of vhost-user backend, ensure the same 
> > > > > set of
> > > > >features are provided, and reuse already acked features.
> > > > >
> > > > >Signed-off-by: Marc-André Lureau 
> > > > >
> > > > >
> > > > >So we could do similar to vhost-user? I mean, save the acked features
> > > > >before migration and store it back after it. This should be able to
> > > > >keep the compatibility. If user downgrades DPDK version, it also could
> > > > >be easily detected, and then exit with an error to user: migration
> > > > >failed due to un-compatible vhost features.
> > > > >
> > > > >Just some rough thoughts. Makes tiny sense?
> > > > 
> > > > My understanding is that the management tool has to know whether
> > > > versions are compatible before initiating the migration:
> > > 
> > > Makes sense. How about getting and restoring the acked features through
> > > qemu command lines then, say, through the monitor interface?
> > > 
> > > With that, it would be something like:
> > > 
> > > - start vhost-user backend (DPDK, VPP, or whatever) & qemu in the src host
> > > 
> > > - read the acked features (through monitor interface)
> > > 
> > > - start vhost-user backend in the dst host
> > > 
> > > - start qemu in the dst host with the just queried acked features
> > > 
> > >   QEMU then is expected to use this feature set for the later vhost-user
> > >   feature negotitation. Exit if features compatibility is broken.
> > > 
> > > Thoughts?
> > > 
> > >   --yliu
> > 
> > 
> > You keep assuming that you have the VM started first and
> > figure out things afterwards, but this does not work.
> > 
> > Think about a cluster of machines. You want to start a VM in
> > a way that will ensure compatibility with all hosts
> > in a cluster.
> 
> I see. I was more considering about the case when the dst
> host (including the qemu and dpdk combo) is given, and
> then determine whether it will be a successfull migration
> or not.
> 
> And you are asking that we need to know which host could
> be a good candidate before starting the migration. In such
> case, we indeed need some inputs from both the qemu and
> vhost-user backend.
> 
> For DPDK, I think it could be simple, just as you said, it
> could be either a tiny script, or even a macro defined in
> the source code file (we extend it every time we add a
> new feature) to let the libvirt to read it. Or something
> else.

There's the issue of APIs that tweak features as Maxime
suggested. Maybe the only thing to do is to 

[libvirt] [PATCH] NEWS: document the new libssh transport

[Pino Toscano]

---
 docs/news.html.in | 5 +
 1 file changed, 5 insertions(+)

diff --git a/docs/news.html.in b/docs/news.html.in
index 56c03b3..a330be8 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -22,6 +22,11 @@
   
   vbox: Add VirtualBox 5.1 support
   
+  libssh: New transport
+  The new libssh transport allows one to connect to a running
+  libvirtd via SSH, using the libssh library; for example:
+  qemu+libssh://server/system
+  
 
   
   Improvements
-- 
2.7.4

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

Re: [libvirt] [PATCH 0/3] Fix improper union member access on hostdevs and refactoring

[Michal Privoznik]

On 15.11.2016 19:25, Marc Hartmayer wrote:
> This patch series fixes improper union member accesses on
> hostdevs. Additionally, it adds a util function which checks whether a
> hostdev is a SCSI device or not. This function is used to simplify and
> clarify the code.
> 
> Marc Hartmayer (3):
>   qemu: Fix improper union member access on hostdevs
>   util: Add virHostdevIsSCSIDevice()
>   Refactoring: Use virHostdevIsSCSIDevice()
> 
>  src/Makefile.am  |  3 ++-
>  src/conf/domain_conf.c   |  8 +++-
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_command.c  |  3 +--
>  src/qemu/qemu_conf.c |  7 +++
>  src/qemu/qemu_domain.c   | 14 +-
>  src/qemu/qemu_process.c  |  3 +++
>  src/util/virhostdev.c| 23 +--
>  src/util/virhostdev.h|  3 +++
>  9 files changed, 38 insertions(+), 27 deletions(-)
> 

ACK series.

Michal

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

Re: [libvirt] [Qemu-ppc] [RFC PATCH qemu] spapr_pci: Create PCI-express root bus by default

[Eric Blake]

On 11/21/2016 07:08 AM, Andrea Bolognani wrote:

>> If libvirt is using a specific version of pseries, then it already knows
>> that <=2.7 has pci.0 as a root, pcie.0 otherwise. libvirt has a knowledge
>> what QEMU version has what, right?
> 
> It doesn't yet, that's the point :)
> 
> We *could* add such knowledge to libvirt[1], but *existing*
> libvirt versions would still not know about it, which means
> that upgrading QEMU withough upgrading libvirt will result
> in failure to create new guests.

But that's okay.  In general, we try to promise that:

old qemu + old libvirt works
new qemu + new libvirt works
old qemu + new libvirt works
new qemu + old libvirt is untested, and may require a libvirt upgrade

In other words, as long as you update your stack from top down (libvirt
before qemu), you should be fine.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/2] Allow hotplug of vhost-mq

[Andrea Bolognani]

On Fri, 2016-11-18 at 17:14 +0100, Michal Privoznik wrote:
> > > Basically this is trivial. Everything is prepared and the only
> > > thing that prevented us from doing this was missing exception in
> > > one check. Trivial.
> > >  
> > > Michal Privoznik (2):
> > >qemuDomainAttachNetDevice: Don't overwrite error on rollback
> > >qemuDomainAttachNetDevice: Enable multiqueue for vhost-user
> > 
> > Still a good candidate for a NEWS file entry, don't you
> > agree? :)
> 
> Sure:
> 
> Improvements:
> 
> With this release, hotplug of vhostuser typed interfaces with multiqueue
> feature was enabled. The packet stream from this kind of interfaces can
> thus be processed on multiple cores simultaneously enabling higher
> performance.

I've pushed a shortened version of the text you proposed.

But of course you already know that, because you're the one
who ACKed the relevant patch ;)

Thanks!

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] conf: Make scheduler formatting simpler

[Martin Kletzander]

Since the great rework of how we store vcpu- and iothread-related
data, we have overly complex part of code that is trying to format the
scheduler tuning data in as less lines as possible by grouping
settings for multiple threads.  That was designed as an input syntax
sugar for users, but we don't need to also use that when formatting
the XML.  Switching to simple enumeration makes the code nicer,
shorter and more welcoming to future changes.

Signed-off-by: Martin Kletzander 
---
 src/conf/domain_conf.c | 209 -
 ...l2xmlout-cputune-iothreadsched-zeropriority.xml |   7 +-
 .../qemuxml2xmlout-cputune-iothreadsched.xml   |   7 +-
 3 files changed, 43 insertions(+), 180 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6e008e22e3c7..51f1ee14498a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23066,183 +23066,35 @@ virDomainDefHasCapabilitiesFeatures(virDomainDefPtr 
def)
 }


-/**
- * virDomainFormatSchedDef:
- * @def: domain definiton
- * @buf: target XML buffer
- * @name: name of the target XML element
- * @func: function that returns the thread scheduler parameter struct for an 
object
- * @resourceMap: bitmap of indexes of objects that shall be formatted (used 
with @func)
- *
- * Formats one of the two scheduler tuning elements to the XML. This function
- * transforms the internal representation where the scheduler info is stored
- * per-object to the XML representation where the info is stored per group of
- * objects. This function autogroups all the relevant scheduler configs.
- *
- * Returns 0 on success -1 on error.
- */
-static int
-virDomainFormatSchedDef(virDomainDefPtr def,
-virBufferPtr buf,
+static void
+virDomainFormatSchedDef(virBufferPtr buf,
 const char *name,
-virDomainThreadSchedParamPtr (*func)(virDomainDefPtr, 
unsigned int),
-virBitmapPtr resourceMap)
-{
-virBitmapPtr schedMap = NULL;
-virBitmapPtr prioMap = NULL;
-virDomainThreadSchedParamPtr sched;
-char *tmp = NULL;
-ssize_t next;
-size_t i;
-int ret = -1;
-
-/* Okay, @func should never return NULL here because it does
- * so iff corresponding resource does not exists. But if it
- * doesn't we should not have been called in the first place.
- * But some compilers fails to see this complex reasoning and
- * deduct that this code is buggy. Shut them up by checking
- * for return value of sched. Even though we don't need to.
- */
-
-if (!(schedMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)) ||
-!(prioMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)))
-goto cleanup;
-
-for (i = VIR_PROC_POLICY_NONE + 1; i < VIR_PROC_POLICY_LAST; i++) {
-virBitmapClearAll(schedMap);
-
-/* find vcpus using a particular scheduler */
-next = -1;
-while ((next = virBitmapNextSetBit(resourceMap, next)) > -1) {
-sched = func(def, next);
-
-if (sched && sched->policy == i)
-ignore_value(virBitmapSetBit(schedMap, next));
-}
-
-/* it's necessary to discriminate priority levels for schedulers that
- * have them */
-while (!virBitmapIsAllClear(schedMap)) {
-virBitmapPtr currentMap = NULL;
-ssize_t nextprio;
-bool hasPriority = false;
-int priority = 0;
-
-switch ((virProcessSchedPolicy) i) {
-case VIR_PROC_POLICY_NONE:
-case VIR_PROC_POLICY_BATCH:
-case VIR_PROC_POLICY_IDLE:
-case VIR_PROC_POLICY_LAST:
-currentMap = schedMap;
-break;
-
-case VIR_PROC_POLICY_FIFO:
-case VIR_PROC_POLICY_RR:
-virBitmapClearAll(prioMap);
-hasPriority = true;
-
-/* we need to find a subset of vCPUs with the given scheduler
- * that share the priority */
-nextprio = virBitmapNextSetBit(schedMap, -1);
-if (!(sched = func(def, nextprio)))
-goto cleanup;
-
-priority = sched->priority;
-ignore_value(virBitmapSetBit(prioMap, nextprio));
-
-while ((nextprio = virBitmapNextSetBit(schedMap, nextprio)) > 
-1) {
-sched = func(def, nextprio);
-if (sched && sched->priority == priority)
-ignore_value(virBitmapSetBit(prioMap, nextprio));
-}
-
-currentMap = prioMap;
-break;
-}
-
-/* now we have the complete group */
-if (!(tmp = virBitmapFormat(currentMap)))
-goto cleanup;
-
-virBufferAsprintf(buf,
-  "<%sched %s='%s' scheduler='%s'",
-  name, name, tmp,
-

Re: [libvirt] [PATCH 0/2] vbox: add support for 5.1.x

[Andrea Bolognani]

On Fri, 2016-11-18 at 12:57 -0500, Dawid Zamirski wrote:
> On Fri, 2016-11-18 at 11:16 -0500, Dawid Zamirski wrote:
> > 
> > Hi Andrea,
> > 
> > Sure, in this case those patches simply enable support of VirtualBox
> > 5.1.x series in libvirt (prior to that libvirt supported vbox 2.x.x
> > to
> > 5.0.x)- and as such it does not affect any previous version that
> > libvirt supported.  When upstream VirtualBox releases new version
> > bumping major/minor version tag it is usually coupled with new COM
> > IID
> > strings that are bundled in the SDK which essentially bumps the VBOX
> > API version as well. Adding those SDK headers to livirt, enables it
> > to
> > communicate with the updated VBOX COM interface and avoids
> > "unsupported
> > API version" error message when one does virsh -c vbox:///session
> > with
> > VirtualBox 5.1.x installed on the machine.
> 
> Another try after realizing it's for official NEWS entry :-)
> 
> "Added support for API compatibility with VitualBox 5.1.x"

Thanks, I've reworked it a bit and pushed it :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] vz: fixed migration in p2p mode

[Andrea Bolognani]

On Mon, 2016-11-21 at 16:16 +0300, Pavel Glushchak wrote:
> On 18.11.2016 19:28, Andrea Bolognani wrote:
> > 
> > On Mon, 2016-11-14 at 18:20 +0300, Pavel Glushchak wrote:
> > > 
> > > dom xml generated on begin step should be passed
> > > to perform step in VIR_MIGRATE_PARAM_DEST_XML parameter.
> > > Otherwise 'XML error: failed to parse xml document' is
> > > raised on destination host as dom xml is NULL.
> > > 
> > > Signed-off-by: Pavel Glushchak 
> > > ---
> > >   src/vz/vz_driver.c | 5 +
> > >   1 file changed, 5 insertions(+)
> > 
> > Do you feel like this should be mentioned in the NEWS file?
> > 
> > If so, would you mind proposing a suitable (short, high-level)
> > description of the change?
> 
> Ok, let's add:
> vz: fixed problem when migration cannot be performed in P2P mode
> 
> Are you going to add this in the NEWS file yourself?

Thanks for responding. A slight variation of that has now
been pushed :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] virsh: Add support for VIR_MIGRATE_PARAM_PERSIST_XML

[Jiri Denemark]

Commit v1.3.3-181-gb028e9d7c implmented support for
VIR_MIGRATE_PARAM_PERSIST_XML migration parameter, but forgot to update
virsh.

https://bugzilla.redhat.com/show_bug.cgi?id=835300

Signed-off-by: Jiri Denemark 
---
 docs/news.html.in|  2 ++
 tools/virsh-domain.c | 23 +++
 tools/virsh.pod  |  5 -
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/docs/news.html.in b/docs/news.html.in
index 56c03b35c..a6f86e518 100644
--- a/docs/news.html.in
+++ b/docs/news.html.in
@@ -26,6 +26,8 @@
   
   Improvements
 
+  virsh: Add support for passing an alternative persistent XML
+  to migrate command
   vhostuser: Allow hotplug of multiqueue devices
   
   NEWS: Switch to an improved format
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a133c81ac..dedddeb96 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10105,6 +10105,10 @@ static const vshCmdOptDef opts_migrate[] = {
  .type = VSH_OT_INT,
  .help = N_("CPU throttling rate increment for auto-convergence")
 },
+{.name = "persistent-xml",
+ .type = VSH_OT_STRING,
+ .help = N_("filename containing updated persistent XML for the target")
+},
 {.name = NULL}
 };
 
@@ -10265,6 +10269,24 @@ doMigrate(void *opaque)
 VIR_FREE(xml);
 }
 
+if (vshCommandOptStringReq(ctl, cmd, "persistent-xml", ) < 0)
+goto out;
+if (opt) {
+char *xml;
+
+if (virFileReadAll(opt, VSH_MAX_XML_FILE, ) < 0) {
+vshError(ctl, _("cannot read file '%s'"), opt);
+goto save_error;
+}
+
+if (virTypedParamsAddString(, , ,
+VIR_MIGRATE_PARAM_PERSIST_XML, xml) < 0) {
+VIR_FREE(xml);
+goto save_error;
+}
+VIR_FREE(xml);
+}
+
 if ((rv = vshCommandOptInt(ctl, cmd, "auto-converge-initial", )) < 
0) {
 goto out;
 } else if (rv > 0) {
@@ -10421,6 +10443,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
 VSH_EXCLUSIVE_OPTIONS("live", "offline");
 VSH_EXCLUSIVE_OPTIONS("timeout-suspend", "timeout-postcopy");
 VSH_REQUIRE_OPTION("postcopy-after-precopy", "postcopy");
+VSH_REQUIRE_OPTION("persistent-xml", "persistent");
 
 if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
 return false;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index f278fecaf..74d531122 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1625,7 +1625,7 @@ I I [I] [I] 
[I] [I] [I<--comp-methods> B]
 [I<--comp-mt-level>] [I<--comp-mt-threads>] [I<--comp-mt-dthreads>]
 [I<--comp-xbzrle-cache>] [I<--auto-converge>] [I]
-[I]
+[I] [I<--persistent-xml> B]
 
 Migrate domain to another host.  Add I<--live> for live migration; <--p2p>
 for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled>
@@ -1679,6 +1679,9 @@ omitted, but can be used to supply an alternative XML 
file for use on
 the destination to supply a larger set of changes to any host-specific
 portions of the domain XML, such as accounting for naming differences
 between source and destination in accessing underlying storage.
+If I<--persistent> is enabled, I<--persistent-xml> B can be used to
+supply an alternative XML file which will be used as the persistent domain
+definition on the destination host.
 
 I<--timeout> B tells virsh to run a specified action when live
 migration exceeds that many seconds.  It can only be used with I<--live>.
-- 
2.11.0.rc2

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

Re: [libvirt] [PATCH] news: Add modern ivshmem support

[Andrea Bolognani]

On Mon, 2016-11-21 at 22:20 +0100, Martin Kletzander wrote:
> > > + The shmem device can now utilize qemu's ivshmem-plain and
> > > + ivshmem-doorbell, more modern versions of ivshmem
> > > + 
> > 
> > I was thinking more of along the lines of
> > 
> >   One-line summary
> >   Couple of sentences that actually explain the change
> > 
> > How about this instead?
> > 
> >   Add support for additional shmem models
> >   The shmem device can now utilize QEMU's ivshmem-plain and
> >   ivshmem-doorbell, more modern versions of ivshmem
> >   
> > 
> > I made QEMU uppercase while at it ;)
> 
> Sure, but in this case you want to separate the summary from the
> explanation.

That's what the  is supposed to do. Not the most
elegant solution, I agree.

> Either using  instead of  or at least making the
> one line summary bold or something.

We don't want the HTML version to be a complete bold-fest,
but the  idea is definitely worth investigating.

Do you feel like looking into that? Otherwise I'll do it,
but be warned that it might be a while before I can bring
myself to touch XSLT again :S

> I'm afraid, though, that the
> multiline requirement will drive even more people away from writing
> anything in the news file.

Mh, I didn't consider that. You might be right... On the
other hand, it's something that can easily be addressed
during review, and if someone has already provided a 90%
good NEWS file entry they probably won't back out just
because of the need for some minor tweaking.

> But as you said, we can do style changes
> later on.

We certainly can, and quite probably will ;)

> Does that mean ACK on your version? =D

Well, it's been pushed already ;)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2 5/5] conf: add deadline scheduler

[Martin Kletzander]

On Mon, Nov 21, 2016 at 01:56:08PM +0100, Martin Polednik wrote:

As the code for changing task scheduler is now able to choose deadline
scheduler, we can update domain configuration to parse the scheduler.
---
docs/formatdomain.html.in |  15 +++---
docs/schemas/domaincommon.rng |  16 +++
src/conf/domain_conf.c| 108 --
3 files changed, 130 insertions(+), 9 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4e40aa1..2f654f9 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -799,12 +799,12 @@
  
The optional vcpusched elements specifies the scheduler
type (values batch, idle, fifo,
-rr) for particular vCPU/IOThread threads (based on
-vcpus and iothreads, leaving out
-vcpus/iothreads sets the default). Valid
-vcpus values start at 0 through one less than the
-number of vCPU's defined for the domain. Valid iothreads
-values are described in the iothreadids
+rr, deadline) for particular vCPU/IOThread
+threads (based on vcpus and iothreads,
+leaving out vcpus/iothreads sets the
+default). Valid vcpus values start at 0 through one less
+than the number of vCPU's defined for the domain. Valid
+iothreads values are described in the 
iothreadids
description.
If no iothreadids are defined, then libvirt numbers
IOThreads from 1 to the number of iothreads available
@@ -812,6 +812,9 @@
rr), priority must be specified as
well (and is ignored for non-real-time ones). The value range
for the priority depends on the host kernel (usually 1-99).
+For deadline real-time scheduler, additional parameters runtime,
+deadline and period must be specified. The value of these parameters is
+specified in nanoseconds, where minimum is 1024 (1 usec).
Since 1.2.13
  

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 19d45fd..1461d34 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -851,6 +851,22 @@
  

  
+  
+
+  
+deadline
+  
+
+
+  
+
+
+  
+
+
+  
+
+  

  

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9ec23be..342745e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4518,11 +4518,13 @@ static int
virDomainVcpuDefPostParse(virDomainDefPtr def)
{
virDomainVcpuDefPtr vcpu;
+virDomainThreadSchedParamPtr sched;
size_t maxvcpus = virDomainDefGetVcpusMax(def);
size_t i;

for (i = 0; i < maxvcpus; i++) {
vcpu = virDomainDefGetVcpu(def, i);
+sched = >sched;

/* impossible but some compilers don't like it */
if (!vcpu)
@@ -4549,6 +4551,35 @@ virDomainVcpuDefPostParse(virDomainDefPtr def)
case VIR_TRISTATE_BOOL_LAST:
break;
}
+
+switch (sched->policy) {
+case VIR_PROC_POLICY_NONE:
+case VIR_PROC_POLICY_BATCH:
+case VIR_PROC_POLICY_IDLE:
+case VIR_PROC_POLICY_FIFO:
+case VIR_PROC_POLICY_RR:
+break;
+case VIR_PROC_POLICY_DEADLINE:
+if (sched->runtime < 1024 || sched->deadline < 1024 || sched->period 
< 1024) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("Scheduler runtime, deadline and period must be "
+"higher or equal to 1024 (1 us) (runtime(%llu), "
+"deadline(%llu), period(%llu))"),
+sched->runtime, sched->deadline, sched->period);
+return -1;
+}
+
+if (!(sched->runtime <= sched->deadline && sched->deadline <= 
sched->period)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_("Scheduler configuration does not satisfy "
+"(runtime(%llu) <= deadline(%llu) <= 
period(%llu))"),
+sched->runtime, sched->deadline, sched->period);
+return -1;
+}
+break;
+case VIR_PROC_POLICY_LAST:
+break;
+}
}



The indentation is totally off in this hunk ^^

@@ -23072,6 +23149,15 @@ 
virDomainSchedPriorityComparator(virDomainThreadSchedParamPtr baseSched,
return (baseSched->priority == sched->priority);
}

+static bool
+virDomainSchedDeadlineComparator(virDomainThreadSchedParamPtr baseSched,
+ virDomainThreadSchedParamPtr sched)
+{
+return (baseSched->runtime == sched->priority &&
+baseSched->deadline == sched->deadline &&
+baseSched->period == sched->period);
+}
+
static virDomainThreadSchedParamPtr

Re: [libvirt] [PATCH 0/3] NEWS: Improve generated file

[Andrea Bolognani]

On Tue, 2016-11-22 at 13:51 +0100, Michal Privoznik wrote:
> On 21.11.2016 21:47, Andrea Bolognani wrote:
> > 
> > Way too tired, can't even blurb.
> > 
> > 
> > Andrea Bolognani (3):
> >   NEWS: Archive 2016 releases
> >   NEWS: Update XSLT stylesheet
> >   NEWS: Update entries
> > 
> >  docs/{news.html.in => news-2016.html.in} |9 -
> >  docs/news.html.in| 3777 
> >+-
> >  docs/news.xsl|   34 +-
> >  3 files changed, 56 insertions(+), 3764 deletions(-)
> >  copy docs/{news.html.in => news-2016.html.in} (99%)
> 
> Looking good to me. ACK.

Pushed, thanks :)

-- 
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] Forbid new-line char in name of domains and storagepools

[Michal Privoznik]

On 11.11.2016 10:17, Sławek Kapłoński wrote:
> As https://bugzilla.redhat.com/show_bug.cgi?id=818064 is closed and new-line
> char is now forbiden in name of network, this patch forbids it also in names 
> of
> domains and storage pools.
> 
> Sławek Kapłoński (2):
>   Forbid new-line char in name of new domain
>   Forbid new-line char in name of new storagepool
> 
>  src/bhyve/bhyve_driver.c | 3 +++
>  src/esx/esx_driver.c | 3 +++
>  src/libxl/libxl_driver.c | 3 +++
>  src/lxc/lxc_driver.c | 3 +++
>  src/openvz/openvz_driver.c   | 3 +++
>  src/qemu/qemu_driver.c   | 3 +++
>  src/storage/storage_driver.c | 3 +++
>  src/test/test_driver.c   | 3 +++
>  src/uml/uml_driver.c | 3 +++
>  src/vmware/vmware_driver.c   | 3 +++
>  src/vz/vz_driver.c   | 3 +++
>  src/xen/xen_driver.c | 3 +++
>  src/xenapi/xenapi_driver.c   | 3 +++
>  13 files changed, 39 insertions(+)
> 

Fixed the problem in 1/2. ACKed and pushed.

Michal

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

Re: [libvirt] [PATCH 1/2] Forbid new-line char in name of new domain

[Michal Privoznik]

On 11.11.2016 10:17, Sławek Kapłoński wrote:
> New line character in name of domain is now forbidden because it
> mess virsh output and can be confusing for users.
> Validation of name is done in drivers, after parsing XML to avoid
> problems with dissappeared domains which was already created with
> new-line char in name.
> ---
>  src/bhyve/bhyve_driver.c   | 3 +++
>  src/esx/esx_driver.c   | 3 +++
>  src/libxl/libxl_driver.c   | 3 +++
>  src/lxc/lxc_driver.c   | 3 +++
>  src/openvz/openvz_driver.c | 3 +++
>  src/qemu/qemu_driver.c | 3 +++
>  src/test/test_driver.c | 3 +++
>  src/uml/uml_driver.c   | 3 +++
>  src/vmware/vmware_driver.c | 3 +++
>  src/vz/vz_driver.c | 3 +++
>  src/xen/xen_driver.c   | 3 +++
>  src/xenapi/xenapi_driver.c | 3 +++
>  12 files changed, 36 insertions(+)
> 
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index 38fb9f0..17f8524 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -541,6 +541,9 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char 
> *xml, unsigned int flag
> NULL, parse_flags)) == NULL)
>  goto cleanup;
>  
> +if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
> +goto cleanup;
> +
>  if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
>  goto cleanup;
>  
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index 17ef00f..166d4bc 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -3051,6 +3051,9 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char 
> *xml, unsigned int flags)
>  if (!def)
>  return NULL;
>  
> +if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
> +goto cleanup;
> +
>  /* Check if an existing domain should be edited */
>  if (esxVI_LookupVirtualMachineByUuid(priv->primary, def->uuid, NULL,
>   ,
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index b2f3b16..3efa91e 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -2791,6 +2791,9 @@ libxlDomainDefineXMLFlags(virConnectPtr conn, const 
> char *xml, unsigned int flag
>  NULL, parse_flags)))
>  goto cleanup;
>  
> +if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
> +goto cleanup;
> +
>  if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
>  goto cleanup;
>  
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 4a0165a..a6776a1 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -475,6 +475,9 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char 
> *xml, unsigned int flags)
>  NULL, parse_flags)))
>  goto cleanup;
>  
> +if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
> +goto cleanup;
> +
>  if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
>  goto cleanup;
>  
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 38a562e..1dfb1cc 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
> @@ -1000,6 +1000,9 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const 
> char *xml, unsigned int fla
>   NULL, parse_flags)) == NULL)
>  goto cleanup;
>  
> +if (virXMLCheckIllegalChars("name", vmdef->name, "\n") < 0)
> +goto cleanup;
> +
>  if (!(vm = virDomainObjListAdd(driver->domains, vmdef,
> driver->xmlopt,
> 0, NULL)))
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a82e58b..8d337eb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7339,6 +7339,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
>  NULL, parse_flags)))
>  goto cleanup;
>  
> +if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
> +goto cleanup;
> +
>  if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0)
>  goto cleanup;
>  
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 236874f..1b54839 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -2641,6 +2641,9 @@ static virDomainPtr 
> testDomainDefineXMLFlags(virConnectPtr conn,
> NULL, parse_flags)) == NULL)
>  goto cleanup;
>  
> +if (virXMLCheckIllegalChars("name", def->name, "\n") < 0)
> +goto cleanup;
> +
>  if (testDomainGenerateIfnames(def) < 0)
>  goto cleanup;
>  if (!(dom = virDomainObjListAdd(privconn->domains,
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index 4f4a69b..ad89e3e 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -2074,6 +2074,9 @@ umlDomainDefineXMLFlags(virConnectPtr conn, const char 
> *xml, 

Re: [libvirt] [Qemu-devel] [RFC 00/15] qmp: Report supported device types on 'query-machines'

[Eduardo Habkost]

On Tue, Nov 22, 2016 at 09:18:21AM +0100, David Hildenbrand wrote:
> 
> > 
> > Considered alternatives
> > ===
> > 
> > Indirect mapping (machine => bus => device)
> > ---
> > 
> > This RFC implements a mechanism to implement ax
> >   machine-type => supported-device-types
> > mapping. An alternative solution I considered was to expose an
> > indirect mapping:
> >   machine-type => default-bus-types
> > followed by
> >   bus-type => supported-device-types.
> > 
> > But exposing only the resulting supported device-types list
> > imposes less restrictions on how the device and bus type
> > hierarchy is implemented inside QEMU. There's still a
> >   machine-type => bus-type => device-type
> > mapping implemented internally, but it is an implementation
> > detail on the current version, and not part of the
> > externally-visible interface.
> > 
> > The Implementation
> > ==
> > 
> > This add a new field to MachineClass: default_bus_types, and a
> > new field to BusClass: supported_device_type.
> 
> Is it possible to modify a machine (setting some properties e.g. on the
> command line), that suddenly more devices are supported? Something like
> enabling an additional bus? (I assume so, because it is called "default
> bus types" :) )

Yes. See the "Out of scope: Configurable buses" section on the
cover letter.

> 
> If so, the indirect mapping could be of more benefit in the long run.

Yes, a new mapping may be necessary to find out which options
(e.g. "usb=on") or device-types (e.g. "-device isa-fdc", "-device
piix3-usb-uhci") will add new buses and make new device-types
supported by the machine. But this will probably require new QMP
commands, and I still want to keep the default case simple.

> 
> Thinking about my machine at home:
> 
> I just care about the available buses. If my machine doesn't have USB,
> but PCI, I can buy a USB PCI card and make it support USB. Then I can
> happily plug in USB devices. So the "default" state might at least
> no longer be sufficient when wanting to plug in a USB fan on a hot
> summer day ;) .
> 
> But, with the indirect mapping, I guess we would need yet another qmp
> command ...
> 
> -- 
> 
> David

-- 
Eduardo

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

Re: [libvirt] [PATCH 1/2] qemu: Create hugepage path on per domain basis

[Michal Privoznik]

On 22.11.2016 13:53, Daniel P. Berrange wrote:
> On Tue, Nov 22, 2016 at 01:45:42PM +0100, Michal Privoznik wrote:
>> If you've ever tried running a huge page backed guest under
>> different user than root, you probably failed. Problem is even
> 
> It works fine - this functionality has existed for years and apps
> like OpenStack use it and certainly never run QEMU as root.
> 
> In qemuStateInitialize we create $MOUNT/libvirt/qemu and
> chown it to the qemu:qemu user/group pair.

Well, this works as long as all the huge page enabled guests are run
under the the same user. For instance, if your user/group from qemu.conf
is root:root and you have one domain with qemu:qemu (configured via
domain XML) it won't start.

> 
> That all said
> 
>> though we have corresponding APIs in the security drivers,
>> there's no implementation and thus we don't relabel the huge page
>> path. But even if we did, so far all of the domains share the
>> same path:
>>
>>/hugepageMount/libvirt/qemu
>>
>> Our only option there would be to set 0777 mode on the qemu dir
>> which is totally unsafe. Therefore, we can create dir on
>> per-domain basis, i.e.:
>>
>>/hugepageMount/libvirt/qemu/domainName
>>
>> and chown domainName dir to the user that domain is configured to
>> run under.
> 
> ...I agree it is better to create a dir per QEMU, since that
> lets us run each QEMU as a distinct user or group ID.

Exactly.

Michal

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

Re: [libvirt] [Qemu-devel] [RFC 00/15] qmp: Report supported device types on 'query-machines'

[David Hildenbrand]

Considered alternatives
===

Indirect mapping (machine => bus => device)
---

This RFC implements a mechanism to implement ax
  machine-type => supported-device-types
mapping. An alternative solution I considered was to expose an
indirect mapping:
  machine-type => default-bus-types
followed by
  bus-type => supported-device-types.

But exposing only the resulting supported device-types list
imposes less restrictions on how the device and bus type
hierarchy is implemented inside QEMU. There's still a
  machine-type => bus-type => device-type
mapping implemented internally, but it is an implementation
detail on the current version, and not part of the
externally-visible interface.

The Implementation
==

This add a new field to MachineClass: default_bus_types, and a
new field to BusClass: supported_device_type.


Is it possible to modify a machine (setting some properties e.g. on the
command line), that suddenly more devices are supported? Something like
enabling an additional bus? (I assume so, because it is called "default
bus types" :) )

If so, the indirect mapping could be of more benefit in the long run.

Thinking about my machine at home:

I just care about the available buses. If my machine doesn't have USB,
but PCI, I can buy a USB PCI card and make it support USB. Then I can
happily plug in USB devices. So the "default" state might at least
no longer be sufficient when wanting to plug in a USB fan on a hot
summer day ;) .

But, with the indirect mapping, I guess we would need yet another qmp
command ...

--

David

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

Re: [libvirt] dpdk/vpp and cross-version migration for vhost

[Yuanhan Liu]

On Thu, Nov 17, 2016 at 07:37:09PM +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 17, 2016 at 05:49:36PM +0800, Yuanhan Liu wrote:
> > On Thu, Nov 17, 2016 at 09:47:09AM +0100, Maxime Coquelin wrote:
> > > 
> > > 
> > > On 11/17/2016 09:29 AM, Yuanhan Liu wrote:
> > > >As usaual, sorry for late response :/
> > > >
> > > >On Thu, Oct 13, 2016 at 08:50:52PM +0300, Michael S. Tsirkin wrote:
> > > >>Hi!
> > > >>So it looks like we face a problem with cross-version
> > > >>migration when using vhost. It's not new but became more
> > > >>acute with the advent of vhost user.
> > > >>
> > > >>For users to be able to migrate between different versions
> > > >>of the hypervisor the interface exposed to guests
> > > >>by hypervisor must stay unchanged.
> > > >>
> > > >>The problem is that a qemu device is connected
> > > >>to a backend in another process, so the interface
> > > >>exposed to guests depends on the capabilities of that
> > > >>process.
> > > >>
> > > >>Specifically, for vhost user interface based on virtio, this includes
> > > >>the "host features" bitmap that defines the interface, as well as more
> > > >>host values such as the max ring size.  Adding new features/changing
> > > >>values to this interface is required to make progress, but on the other
> > > >>hand we need ability to get the old host features to be compatible.
> > > >
> > > >It looks like to the same issue of vhost-user reconnect to me. For 
> > > >example,
> > > >
> > > >- start dpdk 16.07 & qemu 2.5
> > > >- kill dpdk
> > > >- start dpdk 16.11
> > > >
> > > >Though DPDK 16.11 has more features comparing to dpdk 16.07 (say, 
> > > >indirect),
> > > >above should work. Because qemu saves the negotiated features before the
> > > >disconnect and stores it back after the reconnection.
> > > >
> > > >commit a463215b087c41d7ca94e51aa347cde523831873
> > > >Author: Marc-André Lureau 
> > > >Date:   Mon Jun 6 18:45:05 2016 +0200
> > > >
> > > >vhost-net: save & restore vhost-user acked features
> > > >
> > > >The initial vhost-user connection sets the features to be 
> > > > negotiated
> > > >with the driver. Renegotiation isn't possible without device 
> > > > reset.
> > > >
> > > >To handle reconnection of vhost-user backend, ensure the same 
> > > > set of
> > > >features are provided, and reuse already acked features.
> > > >
> > > >Signed-off-by: Marc-André Lureau 
> > > >
> > > >
> > > >So we could do similar to vhost-user? I mean, save the acked features
> > > >before migration and store it back after it. This should be able to
> > > >keep the compatibility. If user downgrades DPDK version, it also could
> > > >be easily detected, and then exit with an error to user: migration
> > > >failed due to un-compatible vhost features.
> > > >
> > > >Just some rough thoughts. Makes tiny sense?
> > > 
> > > My understanding is that the management tool has to know whether
> > > versions are compatible before initiating the migration:
> > 
> > Makes sense. How about getting and restoring the acked features through
> > qemu command lines then, say, through the monitor interface?
> > 
> > With that, it would be something like:
> > 
> > - start vhost-user backend (DPDK, VPP, or whatever) & qemu in the src host
> > 
> > - read the acked features (through monitor interface)
> > 
> > - start vhost-user backend in the dst host
> > 
> > - start qemu in the dst host with the just queried acked features
> > 
> >   QEMU then is expected to use this feature set for the later vhost-user
> >   feature negotitation. Exit if features compatibility is broken.
> > 
> > Thoughts?
> > 
> > --yliu
> 
> 
> You keep assuming that you have the VM started first and
> figure out things afterwards, but this does not work.
> 
> Think about a cluster of machines. You want to start a VM in
> a way that will ensure compatibility with all hosts
> in a cluster.

I see. I was more considering about the case when the dst
host (including the qemu and dpdk combo) is given, and
then determine whether it will be a successfull migration
or not.

And you are asking that we need to know which host could
be a good candidate before starting the migration. In such
case, we indeed need some inputs from both the qemu and
vhost-user backend.

For DPDK, I think it could be simple, just as you said, it
could be either a tiny script, or even a macro defined in
the source code file (we extend it every time we add a
new feature) to let the libvirt to read it. Or something
else.

> If you don't, guest visible interface will change
> and you won't be able to migrate.
> 
> It does not make sense to discuss feature bits specifically
> since that is not the only part of interface.
> For example, max ring size supported might change.

I don't quite understand why we have to consider the max ring
size here? Isn't it a virtio device attribute, that QEMU could
provide such compatibility 

Re: [libvirt] [PATCH 1/2] qemu: Create hugepage path on per domain basis

[Daniel P. Berrange]

On Tue, Nov 22, 2016 at 01:45:42PM +0100, Michal Privoznik wrote:
> If you've ever tried running a huge page backed guest under
> different user than root, you probably failed. Problem is even

It works fine - this functionality has existed for years and apps
like OpenStack use it and certainly never run QEMU as root.

In qemuStateInitialize we create $MOUNT/libvirt/qemu and
chown it to the qemu:qemu user/group pair.

That all said

> though we have corresponding APIs in the security drivers,
> there's no implementation and thus we don't relabel the huge page
> path. But even if we did, so far all of the domains share the
> same path:
> 
>/hugepageMount/libvirt/qemu
> 
> Our only option there would be to set 0777 mode on the qemu dir
> which is totally unsafe. Therefore, we can create dir on
> per-domain basis, i.e.:
> 
>/hugepageMount/libvirt/qemu/domainName
> 
> and chown domainName dir to the user that domain is configured to
> run under.

...I agree it is better to create a dir per QEMU, since that
lets us run each QEMU as a distinct user or group ID.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

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

Re: [libvirt] [PATCH 0/3] NEWS: Improve generated file

[Michal Privoznik]

On 21.11.2016 21:47, Andrea Bolognani wrote:
> Way too tired, can't even blurb.
> 
> 
> Andrea Bolognani (3):
>   NEWS: Archive 2016 releases
>   NEWS: Update XSLT stylesheet
>   NEWS: Update entries
> 
>  docs/{news.html.in => news-2016.html.in} |9 -
>  docs/news.html.in| 3777 
> +-
>  docs/news.xsl|   34 +-
>  3 files changed, 56 insertions(+), 3764 deletions(-)
>  copy docs/{news.html.in => news-2016.html.in} (99%)
> 

Looking good to me. ACK.

Michal

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

Re: [libvirt] [PATCH] network: increase openvswitch call timeout

[Michal Privoznik]

On 21.11.2016 16:19, Stefan Zimmermann wrote:
> Since a successful completion of the calls to openvswitch is expected
> a long timeout should be chosen to account for heavily loaded systems.
> Therefore this patch increases the timeout value from 5 to 120 seconds.
> 
> Signed-off-by: Stefan Zimmermann 
> Reviewed-by: Boris Fiuczynski 
> ---
>  src/util/virnetdevopenvswitch.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

While this patch works for now, should we bother and make this compile
time configurable?

Michal

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

[libvirt] [PATCH 0/2] Fix case with non-root domain and hugepages

[Michal Privoznik]

Yet another bug found due to my work on containerizing qemu.

Michal Privoznik (2):
  qemu: Create hugepage path on per domain basis
  security: Implement virSecurityManagerSetHugepages

 src/qemu/qemu_command.c|  4 +-
 src/qemu/qemu_conf.c   | 45 --
 src/qemu/qemu_conf.h   | 16 +---
 src/qemu/qemu_driver.c | 19 +++--
 src/qemu/qemu_process.c| 25 +++-
 src/security/security_dac.c| 11 ++
 src/security/security_selinux.c| 10 +
 .../qemuxml2argv-hugepages-numa.args   |  4 +-
 .../qemuxml2argv-hugepages-pages.args  | 14 +++
 .../qemuxml2argv-hugepages-pages2.args |  2 +-
 .../qemuxml2argv-hugepages-pages3.args |  2 +-
 .../qemuxml2argv-hugepages-pages5.args |  2 +-
 .../qemuxml2argv-hugepages-shared.args | 12 +++---
 tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |  2 +-
 .../qemuxml2argv-memory-hotplug-dimm-addr.args |  4 +-
 .../qemuxml2argv-memory-hotplug-dimm.args  |  4 +-
 16 files changed, 118 insertions(+), 58 deletions(-)

-- 
2.8.4

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

[libvirt] [PATCH 1/2] qemu: Create hugepage path on per domain basis

[Michal Privoznik]

If you've ever tried running a huge page backed guest under
different user than root, you probably failed. Problem is even
though we have corresponding APIs in the security drivers,
there's no implementation and thus we don't relabel the huge page
path. But even if we did, so far all of the domains share the
same path:

   /hugepageMount/libvirt/qemu

Our only option there would be to set 0777 mode on the qemu dir
which is totally unsafe. Therefore, we can create dir on
per-domain basis, i.e.:

   /hugepageMount/libvirt/qemu/domainName

and chown domainName dir to the user that domain is configured to
run under.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_command.c|  4 +-
 src/qemu/qemu_conf.c   | 45 --
 src/qemu/qemu_conf.h   | 16 +---
 src/qemu/qemu_driver.c | 19 +++--
 src/qemu/qemu_process.c| 25 +++-
 .../qemuxml2argv-hugepages-numa.args   |  4 +-
 .../qemuxml2argv-hugepages-pages.args  | 14 +++
 .../qemuxml2argv-hugepages-pages2.args |  2 +-
 .../qemuxml2argv-hugepages-pages3.args |  2 +-
 .../qemuxml2argv-hugepages-pages5.args |  2 +-
 .../qemuxml2argv-hugepages-shared.args | 12 +++---
 tests/qemuxml2argvdata/qemuxml2argv-hugepages.args |  2 +-
 .../qemuxml2argv-memory-hotplug-dimm-addr.args |  4 +-
 .../qemuxml2argv-memory-hotplug-dimm.args  |  4 +-
 14 files changed, 97 insertions(+), 58 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4a5fce3..e59aef0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3303,7 +3303,7 @@ qemuBuildMemoryBackendStr(unsigned long long size,
 return -1;
 
 if (pagesize) {
-if (qemuGetHupageMemPath(cfg, pagesize, _path) < 0)
+if (qemuGetDomainHupageMemPath(def, cfg, pagesize, _path) < 0)
 goto cleanup;
 
 *backendType = "memory-backend-file";
@@ -7178,7 +7178,7 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
 return -1;
 }
 
-if (qemuGetHupageMemPath(cfg, def->mem.hugepages[0].size, _path) < 0)
+if (qemuGetDomainHupageMemPath(def, cfg, def->mem.hugepages[0].size, 
_path) < 0)
 return -1;
 
 virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path", mem_path, NULL);
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 0ed88f5..942ad86 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1457,7 +1457,7 @@ qemuTranslateSnapshotDiskSourcePool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 }
 
 char *
-qemuGetHugepagePath(virHugeTLBFSPtr hugepage)
+qemuGetBaseHugepagePath(virHugeTLBFSPtr hugepage)
 {
 char *ret;
 
@@ -1468,8 +1468,26 @@ qemuGetHugepagePath(virHugeTLBFSPtr hugepage)
 }
 
 
+char *
+qemuGetDomainHugepagePath(const virDomainDef *def,
+  virHugeTLBFSPtr hugepage)
+{
+char *base = qemuGetBaseHugepagePath(hugepage);
+char *ret;
+
+if (!base ||
+virAsprintf(, "%s/%s", base, def->name) < 0) {
+VIR_FREE(base);
+return NULL;
+}
+
+return ret;
+}
+
+
 /**
- * qemuGetDefaultHugepath:
+ * qemuGetDomainDefaultHugepath:
+ * @def: domain definition
  * @hugetlbfs: array of configured hugepages
  * @nhugetlbfs: number of item in the array
  *
@@ -1478,8 +1496,9 @@ qemuGetHugepagePath(virHugeTLBFSPtr hugepage)
  * Returns 0 on success, -1 otherwise.
  * */
 char *
-qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
-   size_t nhugetlbfs)
+qemuGetDomainDefaultHugepath(const virDomainDef *def,
+ virHugeTLBFSPtr hugetlbfs,
+ size_t nhugetlbfs)
 {
 size_t i;
 
@@ -1490,12 +1509,12 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
 if (i == nhugetlbfs)
 i = 0;
 
-return qemuGetHugepagePath([i]);
+return qemuGetDomainHugepagePath(def, [i]);
 }
 
 
 /**
- * qemuGetHupageMemPath: Construct HP enabled memory backend path
+ * qemuGetDomainHupageMemPath: Construct HP enabled memory backend path
  *
  * If no specific hugepage size is requested (@pagesize is zero)
  * the default hugepage size is used).
@@ -1505,9 +1524,10 @@ qemuGetDefaultHugepath(virHugeTLBFSPtr hugetlbfs,
  *-1 otherwise.
  */
 int
-qemuGetHupageMemPath(virQEMUDriverConfigPtr cfg,
- unsigned long long pagesize,
- char **memPath)
+qemuGetDomainHupageMemPath(const virDomainDef *def,
+   virQEMUDriverConfigPtr cfg,
+   unsigned long long pagesize,
+   char **memPath)
 {
 size_t i = 0;
 
@@ -1519,8 +1539,9 @@ qemuGetHupageMemPath(virQEMUDriverConfigPtr cfg,
 }
 
 if (!pagesize) {
-if (!(*memPath = qemuGetDefaultHugepath(cfg->hugetlbfs,
- 

[libvirt] [PATCH 2/2] security: Implement virSecurityManagerSetHugepages

[Michal Privoznik]

Since its introduction in 2012 this internal API did nothing. Now
it's finally the right time to put it into a good use. It's
implementation is fairly simple and exactly the same as
virSecurityDomainSetPathLabel.

Signed-off-by: Michal Privoznik 
---
 src/security/security_dac.c | 11 +++
 src/security/security_selinux.c | 10 ++
 2 files changed, 21 insertions(+)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index fd74e8b..3fa9df9 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1564,6 +1564,15 @@ virSecurityDACDomainSetPathLabel(virSecurityManagerPtr 
mgr,
 return virSecurityDACSetOwnership(priv, NULL, path, user, group);
 }
 
+static int
+virSecurityDACDomainSetHugepages(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ const char *path)
+{
+return virSecurityDACDomainSetPathLabel(mgr, def, path);
+}
+
+
 virSecurityDriver virSecurityDriverDAC = {
 .privateDataLen = sizeof(virSecurityDACData),
 .name   = SECURITY_DAC_NAME,
@@ -1613,4 +1622,6 @@ virSecurityDriver virSecurityDriverDAC = {
 .getBaseLabel   = virSecurityDACGetBaseLabel,
 
 .domainSetPathLabel = virSecurityDACDomainSetPathLabel,
+
+.domainSetSecurityHugepages = virSecurityDACDomainSetHugepages,
 };
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 89c93dc..0497acc 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2610,6 +2610,14 @@ 
virSecuritySELinuxDomainSetPathLabel(virSecurityManagerPtr mgr,
 return virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel);
 }
 
+static int
+virSecuritySELinuxDomainSetHugepages(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ const char *path)
+{
+return virSecuritySELinuxDomainSetPathLabel(mgr, def, path);
+}
+
 virSecurityDriver virSecurityDriverSELinux = {
 .privateDataLen = sizeof(virSecuritySELinuxData),
 .name   = SECURITY_SELINUX_NAME,
@@ -2656,4 +2664,6 @@ virSecurityDriver virSecurityDriverSELinux = {
 .getBaseLabel   = virSecuritySELinuxGetBaseLabel,
 
 .domainSetPathLabel = virSecuritySELinuxDomainSetPathLabel,
+
+.domainSetSecurityHugepages = virSecuritySELinuxDomainSetHugepages,
 };
-- 
2.8.4

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

Re: [libvirt] [PATCH 0/2] qemu: Fix propagation of 'qemu_id' parameter in the vcpus

[Ján Tomko]

On Tue, Nov 22, 2016 at 10:51:09AM +0100, Peter Krempa wrote:

The propagation code got misplaced in the code that optimizes the extraction of
the halted state of the vcpus.

Peter Krempa (2):
 tests: qemumonitorjson: Rename 'qemu-id' to 'enable-id' in cpu info
   test
 qemu: monitor: Properly propagate the 'qemu_id' field through the
   matcher

src/qemu/qemu_monitor.c|  3 ++
.../qemumonitorjson-cpuinfo-ppc64-basic.data   | 10 ++-
.../qemumonitorjson-cpuinfo-ppc64-hotplug-1.data   | 20 +++--
.../qemumonitorjson-cpuinfo-ppc64-hotplug-2.data   | 30 ++--
.../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data   | 30 ++--
.../qemumonitorjson-cpuinfo-ppc64-no-threads.data  | 24 ++--
...emumonitorjson-cpuinfo-x86-basic-pluggable.data | 15 ++
.../qemumonitorjson-cpuinfo-x86-full.data  | 33 ++
tests/qemumonitorjsontest.c|  5 +++-
9 files changed, 136 insertions(+), 34 deletions(-)



ACK series,

Jan


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/3] qemu: Don't call qemuMonitorGetCPUInfo to update vCPU halted state

[Peter Krempa]

On Tue, Nov 22, 2016 at 12:52:26 +0100, Viktor Mihajlovski wrote:
> On 22.11.2016 09:22, Peter Krempa wrote:
> > On Mon, Nov 21, 2016 at 17:14:42 +0100, Viktor Mihajlovski wrote:
> >> On 21.11.2016 16:30, Peter Krempa wrote:
> >>> The original implementation reused qemuMonitorGetCPUInfo to update the 
> >>> halted
> >>> state. The function is very complex and should not be called all the time 
> >>> just
> >>> to update a trivial parameter.
> >>>
> >>> Add infrastructure to properly update the state without the need to match 
> >>> in
> >>> hotplug parameters.
> >>>
> >>> Peter Krempa (3):
> >>>   qemu: monitor: Extract qemu cpu id along with other data
> >>>   qemu: monitor: Extract halted state to a bitmap indexed by cpu id
> >>>   qemu: domain:  Refresh vcpu halted state using qemuMonitorGetCpuHalted
> >>>
> > I've already pushed it. The hiccup might be in the fallback code that
> > does not remember correctly the cpu numbers as reported by qemu.
> > 
> > I'll post patches if it's so.
> > 
> Yep ... with the following squashed in, it worked for me, (legacy) hotplug
> and all.
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 3bace53..2d4ccbe 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -1729,6 +1729,7 @@ qemuMonitorGetCPUInfoLegacy(struct 
> qemuMonitorQueryCpusEntry *cpuentrie
>  if (i < ncpuentries) {
>  vcpus[i].tid = cpuentries[i].tid;
>  vcpus[i].halted = cpuentries[i].halted;
> +vcpus[i].qemu_id = cpuentries[i].qemu_id;
>  }
>  
>  /* for legacy hotplug to work we need to fake the vcpu count added by

I noticed that both the Hotplug and legacy code miss that line. It must
have vanished when I was cutting up the changes into patches.

See the series I've posted.


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] qemu: monitor: Properly propagate the 'qemu_id' field through the matcher

[Viktor Mihajlovski]

On 22.11.2016 10:51, Peter Krempa wrote:
> Commit 3f71c797689a4a70 added 'qemu_id' field to track the id of the cpu
> as reported by query-cpus. The patch did not include changes necessary
> to propagate the id through the functions matching the data to the
> libvirt cpu structures and thus all vcpus had id 0.
> ---
>  src/qemu/qemu_monitor.c|  3 +++
>  .../qemumonitorjson-cpuinfo-ppc64-basic.data   |  8 
>  .../qemumonitorjson-cpuinfo-ppc64-hotplug-1.data   | 16 +++
>  .../qemumonitorjson-cpuinfo-ppc64-hotplug-2.data   | 24 
> ++
>  .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data   | 24 
> ++
>  .../qemumonitorjson-cpuinfo-ppc64-no-threads.data  |  8 
>  ...emumonitorjson-cpuinfo-x86-basic-pluggable.data |  5 +
>  .../qemumonitorjson-cpuinfo-x86-full.data  | 11 ++
>  tests/qemumonitorjsontest.c|  3 +++
>  9 files changed, 102 insertions(+)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 3ff31e4..3f86887 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -1672,6 +1672,7 @@ qemuMonitorCPUInfoClear(qemuMonitorCPUInfoPtr cpus,
> 
>  for (i = 0; i < ncpus; i++) {
>  cpus[i].id = 0;
> +cpus[i].qemu_id = -1;
>  cpus[i].socket_id = -1;
>  cpus[i].core_id = -1;
>  cpus[i].thread_id = -1;
> @@ -1729,6 +1730,7 @@ qemuMonitorGetCPUInfoLegacy(struct 
> qemuMonitorQueryCpusEntry *cpuentries,
>  if (i < ncpuentries) {
>  vcpus[i].tid = cpuentries[i].tid;
>  vcpus[i].halted = cpuentries[i].halted;
> +vcpus[i].qemu_id = cpuentries[i].qemu_id;
>  }
> 
>  /* for legacy hotplug to work we need to fake the vcpu count added by
> @@ -1866,6 +1868,7 @@ qemuMonitorGetCPUInfoHotplug(struct 
> qemuMonitorQueryHotpluggableCpusEntry *hotpl
>  }
>  }
> 
> +vcpus[anyvcpu].qemu_id = cpuentries[j].qemu_id;
>  vcpus[anyvcpu].tid = cpuentries[j].tid;
>  vcpus[anyvcpu].halted = cpuentries[j].halted;
>  }
Looks good to me. Since presently s390 does only support legacy hotplug
w/o unplug, the second hunk will essentially do the trick.
[...]

Thanks!
-- 

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

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

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

Re: [libvirt] [PATCH] Revert "vz: fixed race in vzDomainAttach/DettachDevice"

[Nikolay Shirokovskiy]

ACK

On 18.11.2016 19:16, Maxim Nestratov wrote:
> This reverts commit 3a6cf6fc16.
> 
> Mistakenly this commit was pushed because I thought I missed the
> corret one b880ff42ddb while in fact I didn't.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/vz/vz_sdk.c | 12 
>  1 file changed, 12 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index d61bccf..f63b9a3 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -3552,12 +3552,6 @@ prlsdkAttachDevice(vzDriverPtr driver,
>  return -1;
>  }
>  
> -if (prlsdkUpdateDomain(driver, dom) < 0) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -_("Failed to save new config"));
> -return -1;
> -}
> -
>  job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE);
>  if (PRL_FAILED(waitDomainJob(job, dom)))
>  return -1;
> @@ -3623,12 +3617,6 @@ prlsdkDetachDevice(vzDriverPtr driver ATTRIBUTE_UNUSED,
>  goto cleanup;
>  }
>  
> -if (prlsdkUpdateDomain(driver, dom) < 0) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -_("Failed to save new config"));
> -goto cleanup;
> -}
> -
>  job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE);
>  if (PRL_FAILED(waitDomainJob(job, dom)))
>  goto cleanup;
> 

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

Re: [libvirt] [PATCH 0/3] qemu: Don't call qemuMonitorGetCPUInfo to update vCPU halted state

[Viktor Mihajlovski]

On 22.11.2016 09:22, Peter Krempa wrote:
> On Mon, Nov 21, 2016 at 17:14:42 +0100, Viktor Mihajlovski wrote:
>> On 21.11.2016 16:30, Peter Krempa wrote:
>>> The original implementation reused qemuMonitorGetCPUInfo to update the 
>>> halted
>>> state. The function is very complex and should not be called all the time 
>>> just
>>> to update a trivial parameter.
>>>
>>> Add infrastructure to properly update the state without the need to match in
>>> hotplug parameters.
>>>
>>> Peter Krempa (3):
>>>   qemu: monitor: Extract qemu cpu id along with other data
>>>   qemu: monitor: Extract halted state to a bitmap indexed by cpu id
>>>   qemu: domain:  Refresh vcpu halted state using qemuMonitorGetCpuHalted
>>>
>>>  src/qemu/qemu_domain.c   | 20 
>>>  src/qemu/qemu_domain.h   |  1 +
>>>  src/qemu/qemu_monitor.c  | 40 
>>>  src/qemu/qemu_monitor.h  |  3 +++
>>>  src/qemu/qemu_monitor_json.c |  3 +++
>>>  src/qemu/qemu_monitor_text.c | 11 +++
>>>  tests/qemumonitorjsontest.c  |  8 
>>>  7 files changed, 70 insertions(+), 16 deletions(-)
>>>
>> Could you please hold off pushing? I've just run a sniff test on s390x
>> and see erratic values for halted. I'll try to investigate and get back
>> to you.
> 
> I've already pushed it. The hiccup might be in the fallback code that
> does not remember correctly the cpu numbers as reported by qemu.
> 
> I'll post patches if it's so.
> 
Yep ... with the following squashed in, it worked for me, (legacy) hotplug
and all.

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 3bace53..2d4ccbe 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1729,6 +1729,7 @@ qemuMonitorGetCPUInfoLegacy(struct 
qemuMonitorQueryCpusEntry *cpuentrie
 if (i < ncpuentries) {
 vcpus[i].tid = cpuentries[i].tid;
 vcpus[i].halted = cpuentries[i].halted;
+vcpus[i].qemu_id = cpuentries[i].qemu_id;
 }
 
 /* for legacy hotplug to work we need to fake the vcpu count added by

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

[libvirt] [PATCH 1/3] qemu: refactor: use switch for enum in qemuProcessGraphicsReservePorts

[Nikolay Shirokovskiy]

---
 src/qemu/qemu_process.c | 30 +-
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3552a31..610 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4056,16 +4056,21 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
 glisten->type != VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)
 return 0;
 
-if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
-!graphics->data.vnc.autoport) {
-if (virPortAllocatorSetUsed(driver->remotePorts,
-graphics->data.vnc.port,
-true) < 0)
-return -1;
-graphics->data.vnc.portReserved = true;
+switch (graphics->type) {
+case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
+if (!graphics->data.vnc.autoport) {
+if (virPortAllocatorSetUsed(driver->remotePorts,
+graphics->data.vnc.port,
+true) < 0)
+return -1;
+graphics->data.vnc.portReserved = true;
+}
+break;
+
+case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+if (graphics->data.spice.autoport)
+return 0;
 
-} else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE &&
-   !graphics->data.spice.autoport) {
 if (graphics->data.spice.port > 0) {
 if (virPortAllocatorSetUsed(driver->remotePorts,
 graphics->data.spice.port,
@@ -4081,6 +4086,13 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
 return -1;
 graphics->data.spice.tlsPortReserved = true;
 }
+break;
+
+case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
+case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
+case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
+case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
+break;
 }
 
 return 0;
-- 
1.8.3.1

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

[libvirt] [PATCH 0/3] qemu: bugfixes for websocket port in graphics

[Nikolay Shirokovskiy]

Patch 1 is a preparation for patch 2.
Patch 3 particularly fixes next 2 cases:

== A. Can not restore domain with autoconfigured websocket.

domain 1 and 2 have autoconfigured websocket. 

1. domain 1 is started then, saved
2. domain 2 is started
3. domain 1 restoration is failed:

error: internal error: qemu unexpectedly closed the monitor: 
2016-11-21T10:23:11.356687Z 
qemu-kvm: -vnc 0.0.0.0:2,websocket=5700: Failed to start VNC server on 
`(null)': 
Failed to bind socket: Address already in use

== B. Can not migrate domain with autoconfigured websocket.

domain 1 on host A, domain 2 on host B, both have autoconfigured websocket

1. domain 1 started, domain 2 started
2. domain 1 migration to host B is failed with the above error.

Nikolay Shirokovskiy (3):
  qemu: refactor: use switch for enum in qemuProcessGraphicsReservePorts
  qemu: mark user defined websocket as used
  qemu: fix xml dump of autogenerated websocket

 src/conf/domain_conf.c  |  5 -
 src/conf/domain_conf.h  |  1 +
 src/qemu/qemu_process.c | 45 -
 3 files changed, 41 insertions(+), 10 deletions(-)

-- 
1.8.3.1

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

[libvirt] [PATCH 3/3] qemu: fix xml dump of autogenerated websocket

[Nikolay Shirokovskiy]

---
 src/conf/domain_conf.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6e008e2..fb6ff0b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22534,7 +22534,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, " autoport='%s'",
   def->data.vnc.autoport ? "yes" : "no");
 
-if (def->data.vnc.websocket)
+if (def->data.vnc.websocketGenerated &&
+(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
+virBufferAddLit(buf, " websocket='-1'");
+else if (def->data.vnc.websocket)
 virBufferAsprintf(buf, " websocket='%d'", 
def->data.vnc.websocket);
 
 virDomainGraphicsListenDefFormatAddr(buf, glisten, flags);
-- 
1.8.3.1

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

[libvirt] [PATCH 2/3] qemu: mark user defined websocket as used

[Nikolay Shirokovskiy]

We need extra state variable to distinguish between autogenerated
and user defined cases after auto generation is done.
---
 src/conf/domain_conf.h  |  1 +
 src/qemu/qemu_process.c | 19 +--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 541b600..9bc4522 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1468,6 +1468,7 @@ struct _virDomainGraphicsDef {
 int port;
 bool portReserved;
 int websocket;
+bool websocketGenerated;
 bool autoport;
 char *keymap;
 virDomainGraphicsAuthDef auth;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 610..1799f33 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3574,6 +3574,7 @@ qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
 if (virPortAllocatorAcquire(driver->webSocketPorts, ) < 0)
 return -1;
 graphics->data.vnc.websocket = port;
+graphics->data.vnc.websocketGenerated = true;
 }
 
 return 0;
@@ -4065,6 +4066,12 @@ qemuProcessGraphicsReservePorts(virQEMUDriverPtr driver,
 return -1;
 graphics->data.vnc.portReserved = true;
 }
+if (graphics->data.vnc.websocket != -1 &&   /* auto websocket */
+graphics->data.vnc.websocket && /* no websocket */
+virPortAllocatorSetUsed(driver->remotePorts,
+graphics->data.vnc.websocket,
+true) < 0)
+return -1;
 break;
 
 case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
@@ -6189,8 +6196,16 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 false);
 graphics->data.vnc.portReserved = false;
 }
-virPortAllocatorRelease(driver->webSocketPorts,
-graphics->data.vnc.websocket);
+if (graphics->data.vnc.websocketGenerated) {
+virPortAllocatorRelease(driver->webSocketPorts,
+graphics->data.vnc.websocket);
+graphics->data.vnc.websocketGenerated = false;
+graphics->data.vnc.websocket = -1;
+} else if (graphics->data.vnc.websocket) {
+virPortAllocatorSetUsed(driver->remotePorts,
+graphics->data.vnc.websocket,
+false);
+}
 }
 if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
 if (graphics->data.spice.autoport) {
-- 
1.8.3.1

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

[libvirt] [PATCH 2/2] qemu: monitor: Properly propagate the 'qemu_id' field through the matcher

[Peter Krempa]

Commit 3f71c797689a4a70 added 'qemu_id' field to track the id of the cpu
as reported by query-cpus. The patch did not include changes necessary
to propagate the id through the functions matching the data to the
libvirt cpu structures and thus all vcpus had id 0.
---
 src/qemu/qemu_monitor.c|  3 +++
 .../qemumonitorjson-cpuinfo-ppc64-basic.data   |  8 
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-1.data   | 16 +++
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-2.data   | 24 ++
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data   | 24 ++
 .../qemumonitorjson-cpuinfo-ppc64-no-threads.data  |  8 
 ...emumonitorjson-cpuinfo-x86-basic-pluggable.data |  5 +
 .../qemumonitorjson-cpuinfo-x86-full.data  | 11 ++
 tests/qemumonitorjsontest.c|  3 +++
 9 files changed, 102 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 3ff31e4..3f86887 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1672,6 +1672,7 @@ qemuMonitorCPUInfoClear(qemuMonitorCPUInfoPtr cpus,

 for (i = 0; i < ncpus; i++) {
 cpus[i].id = 0;
+cpus[i].qemu_id = -1;
 cpus[i].socket_id = -1;
 cpus[i].core_id = -1;
 cpus[i].thread_id = -1;
@@ -1729,6 +1730,7 @@ qemuMonitorGetCPUInfoLegacy(struct 
qemuMonitorQueryCpusEntry *cpuentries,
 if (i < ncpuentries) {
 vcpus[i].tid = cpuentries[i].tid;
 vcpus[i].halted = cpuentries[i].halted;
+vcpus[i].qemu_id = cpuentries[i].qemu_id;
 }

 /* for legacy hotplug to work we need to fake the vcpu count added by
@@ -1866,6 +1868,7 @@ qemuMonitorGetCPUInfoHotplug(struct 
qemuMonitorQueryHotpluggableCpusEntry *hotpl
 }
 }

+vcpus[anyvcpu].qemu_id = cpuentries[j].qemu_id;
 vcpus[anyvcpu].tid = cpuentries[j].tid;
 vcpus[anyvcpu].halted = cpuentries[j].halted;
 }
diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data 
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data
index 9969648..eaa797c 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data
@@ -3,6 +3,7 @@
 hotpluggable=no
 thread-id='21925'
 enable-id='1'
+query-cpus-id='0'
 type='host-spapr-cpu-core'
 qom_path='/machine/unattached/device[1]'
 topology: core='0' vcpus='8'
@@ -10,30 +11,37 @@
 online=yes
 hotpluggable=no
 thread-id='21926'
+query-cpus-id='1'
 [vcpu libvirt-id='2']
 online=yes
 hotpluggable=no
 thread-id='21927'
+query-cpus-id='2'
 [vcpu libvirt-id='3']
 online=yes
 hotpluggable=no
 thread-id='21928'
+query-cpus-id='3'
 [vcpu libvirt-id='4']
 online=yes
 hotpluggable=no
 thread-id='21930'
+query-cpus-id='4'
 [vcpu libvirt-id='5']
 online=yes
 hotpluggable=no
 thread-id='21931'
+query-cpus-id='5'
 [vcpu libvirt-id='6']
 online=yes
 hotpluggable=no
 thread-id='21932'
+query-cpus-id='6'
 [vcpu libvirt-id='7']
 online=yes
 hotpluggable=no
 thread-id='21933'
+query-cpus-id='7'
 [vcpu libvirt-id='8']
 online=no
 hotpluggable=yes
diff --git 
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data 
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data
index 643f6ec..a0aca86 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data
@@ -3,6 +3,7 @@
 hotpluggable=no
 thread-id='21925'
 enable-id='1'
+query-cpus-id='0'
 type='host-spapr-cpu-core'
 qom_path='/machine/unattached/device[1]'
 topology: core='0' vcpus='8'
@@ -10,35 +11,43 @@
 online=yes
 hotpluggable=no
 thread-id='21926'
+query-cpus-id='1'
 [vcpu libvirt-id='2']
 online=yes
 hotpluggable=no
 thread-id='21927'
+query-cpus-id='2'
 [vcpu libvirt-id='3']
 online=yes
 hotpluggable=no
 thread-id='21928'
+query-cpus-id='3'
 [vcpu libvirt-id='4']
 online=yes
 hotpluggable=no
 thread-id='21930'
+query-cpus-id='4'
 [vcpu libvirt-id='5']
 online=yes
 hotpluggable=no
 thread-id='21931'
+query-cpus-id='5'
 [vcpu libvirt-id='6']
 online=yes
 hotpluggable=no
 thread-id='21932'
+query-cpus-id='6'
 [vcpu libvirt-id='7']
 online=yes
 hotpluggable=no
 thread-id='21933'
+query-cpus-id='7'
 [vcpu libvirt-id='8']
 online=yes
 hotpluggable=yes
 thread-id='22131'
 enable-id='2'
+query-cpus-id='8'
 type='host-spapr-cpu-core'
 alias='vcpu0'
 qom_path='/machine/peripheral/vcpu0'
@@ -47,30 +56,37 @@
 online=yes
 hotpluggable=yes
 thread-id='22132'
+query-cpus-id='9'
 [vcpu libvirt-id='10']
 

[libvirt] [PATCH 1/2] tests: qemumonitorjson: Rename 'qemu-id' to 'enable-id' in cpu info test

[Peter Krempa]

The field is named 'enable_id' in other structures and a patch recently
added 'qemu_id' which has different semantics. To avoid confusion in the
tests rename the field.
---
 .../qemumonitorjson-cpuinfo-ppc64-basic.data   |  2 +-
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-1.data   |  4 ++--
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-2.data   |  6 +++---
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data   |  6 +++---
 .../qemumonitorjson-cpuinfo-ppc64-no-threads.data  | 16 
 ...emumonitorjson-cpuinfo-x86-basic-pluggable.data | 10 +-
 .../qemumonitorjson-cpuinfo-x86-full.data  | 22 +++---
 tests/qemumonitorjsontest.c|  2 +-
 8 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data 
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data
index ae7c2f4..9969648 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data
@@ -2,7 +2,7 @@
 online=yes
 hotpluggable=no
 thread-id='21925'
-qemu-id='1'
+enable-id='1'
 type='host-spapr-cpu-core'
 qom_path='/machine/unattached/device[1]'
 topology: core='0' vcpus='8'
diff --git 
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data 
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data
index 5c0a6af..643f6ec 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data
@@ -2,7 +2,7 @@
 online=yes
 hotpluggable=no
 thread-id='21925'
-qemu-id='1'
+enable-id='1'
 type='host-spapr-cpu-core'
 qom_path='/machine/unattached/device[1]'
 topology: core='0' vcpus='8'
@@ -38,7 +38,7 @@
 online=yes
 hotpluggable=yes
 thread-id='22131'
-qemu-id='2'
+enable-id='2'
 type='host-spapr-cpu-core'
 alias='vcpu0'
 qom_path='/machine/peripheral/vcpu0'
diff --git 
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data 
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data
index ba4044e..43780ee 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data
@@ -2,7 +2,7 @@
 online=yes
 hotpluggable=no
 thread-id='21925'
-qemu-id='1'
+enable-id='1'
 type='host-spapr-cpu-core'
 qom_path='/machine/unattached/device[1]'
 topology: core='0' vcpus='8'
@@ -38,7 +38,7 @@
 online=yes
 hotpluggable=yes
 thread-id='22131'
-qemu-id='2'
+enable-id='2'
 type='host-spapr-cpu-core'
 alias='vcpu0'
 qom_path='/machine/peripheral/vcpu0'
@@ -75,7 +75,7 @@
 online=yes
 hotpluggable=yes
 thread-id='3'
-qemu-id='3'
+enable-id='3'
 type='host-spapr-cpu-core'
 alias='vcpu1'
 qom_path='/machine/peripheral/vcpu1'
diff --git 
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data 
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data
index d2c56ef..dfa2d3f 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data
@@ -2,7 +2,7 @@
 online=yes
 hotpluggable=no
 thread-id='21925'
-qemu-id='1'
+enable-id='1'
 type='host-spapr-cpu-core'
 qom_path='/machine/unattached/device[1]'
 topology: core='0' vcpus='8'
@@ -38,7 +38,7 @@
 online=yes
 hotpluggable=yes
 thread-id='23170'
-qemu-id='3'
+enable-id='3'
 type='host-spapr-cpu-core'
 alias='vcpu0'
 qom_path='/machine/peripheral/vcpu0'
@@ -75,7 +75,7 @@
 online=yes
 hotpluggable=yes
 thread-id='22741'
-qemu-id='2'
+enable-id='2'
 type='host-spapr-cpu-core'
 alias='vcpu1'
 qom_path='/machine/peripheral/vcpu1'
diff --git 
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data 
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data
index c2f541b..2519ad3 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data
@@ -2,7 +2,7 @@
 online=yes
 hotpluggable=no
 thread-id='35232'
-qemu-id='1'
+enable-id='1'
 type='host-spapr-cpu-core'
 qom_path='/machine/unattached/device[1]'
 topology: core='0' vcpus='1'
@@ -10,7 +10,7 @@
 online=yes
 hotpluggable=no
 thread-id='35233'
-qemu-id='2'
+enable-id='2'
 type='host-spapr-cpu-core'
 qom_path='/machine/unattached/device[2]'
 topology: core='8' vcpus='1'
@@ -18,7 +18,7 @@
 online=yes
 hotpluggable=no
 thread-id='35234'
-qemu-id='3'
+enable-id='3'
 type='host-spapr-cpu-core'
 

[libvirt] [PATCH 0/2] qemu: Fix propagation of 'qemu_id' parameter in the vcpus

[Peter Krempa]

The propagation code got misplaced in the code that optimizes the extraction of
the halted state of the vcpus.

Peter Krempa (2):
  tests: qemumonitorjson: Rename 'qemu-id' to 'enable-id' in cpu info
test
  qemu: monitor: Properly propagate the 'qemu_id' field through the
matcher

 src/qemu/qemu_monitor.c|  3 ++
 .../qemumonitorjson-cpuinfo-ppc64-basic.data   | 10 ++-
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-1.data   | 20 +++--
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-2.data   | 30 ++--
 .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data   | 30 ++--
 .../qemumonitorjson-cpuinfo-ppc64-no-threads.data  | 24 ++--
 ...emumonitorjson-cpuinfo-x86-basic-pluggable.data | 15 ++
 .../qemumonitorjson-cpuinfo-x86-full.data  | 33 ++
 tests/qemumonitorjsontest.c|  5 +++-
 9 files changed, 136 insertions(+), 34 deletions(-)

-- 
2.10.2

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

Re: [libvirt] [PATCH 0/3] qemu: Don't call qemuMonitorGetCPUInfo to update vCPU halted state

[Peter Krempa]

On Mon, Nov 21, 2016 at 17:14:42 +0100, Viktor Mihajlovski wrote:
> On 21.11.2016 16:30, Peter Krempa wrote:
> > The original implementation reused qemuMonitorGetCPUInfo to update the 
> > halted
> > state. The function is very complex and should not be called all the time 
> > just
> > to update a trivial parameter.
> > 
> > Add infrastructure to properly update the state without the need to match in
> > hotplug parameters.
> > 
> > Peter Krempa (3):
> >   qemu: monitor: Extract qemu cpu id along with other data
> >   qemu: monitor: Extract halted state to a bitmap indexed by cpu id
> >   qemu: domain:  Refresh vcpu halted state using qemuMonitorGetCpuHalted
> > 
> >  src/qemu/qemu_domain.c   | 20 
> >  src/qemu/qemu_domain.h   |  1 +
> >  src/qemu/qemu_monitor.c  | 40 
> >  src/qemu/qemu_monitor.h  |  3 +++
> >  src/qemu/qemu_monitor_json.c |  3 +++
> >  src/qemu/qemu_monitor_text.c | 11 +++
> >  tests/qemumonitorjsontest.c  |  8 
> >  7 files changed, 70 insertions(+), 16 deletions(-)
> > 
> Could you please hold off pushing? I've just run a sniff test on s390x
> and see erratic values for halted. I'll try to investigate and get back
> to you.

I've already pushed it. The hiccup might be in the fallback code that
does not remember correctly the cpu numbers as reported by qemu.

I'll post patches if it's so.


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list