Re: [libvirt] [PATCH 3/3] tests: deterministichash: Make hash tables arch-independent

[Bjoern Walk]

Peter Krempa  [2017-08-02, 05:39PM +0200]:
> It turns out that our implementation of the hashing function is
> endian-dependent and thus if used on various architectures the testsuite
> may have different results. Work this around by mocking virHashCodeGen
> to something which does not use bit operations instead of just setting a
> deterministic seed.

This does fix the issue on S390. Anyways, I'd also like to see the tests
fixed that rely on the ordering of the hash table. And code that uses
the hash table should be tested that it does NOT rely on the ordering.

Tested-by: Bjoern Walk 


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

[libvirt] RFC: support for configuring all ports of a multiport SRIOV VF when assigning to guest

[Laine Stump]

("No matter how far you've gone down the wrong road, turn back." -
paraphrase of a Turkish proverb that is apropos to this discussion)

Several years ago, when I was apparently naive and narrow in my thinking
and someone wanted us support setting the MAC address and vlan tag for
SRIOV VFs when assigning them to a guest with PCI device assignment
(this was before VFIO existed), I had the idea to do this by creating a
new type of  device:

   


My thinking was that  already had elements for mac address,
802.11Qb[gh] virtualport config, and vlan tag (or maybe it was that we
were *going to add* support for vlan tag), so by just adding a 
that was a PCI address, we would have everything we needed. Basically,
there is some amount of config that needs to be applied to the device
before it's assigned to the guest, and since the device ends up being a
netdev in the guest, all that config is already present in an
. As a bonus, because it was an  we could easily
re-use the recently added "pool of devices" network type (with some
minor adjustment) to avoid needing to hardcode the host-side PCI address
of the VF.

At the time Dan Berrange countered (I think - correct me if I'm wrong!)
that we should instead do this with modifications to , but
somehow I managed to either convince him, or maybe he just finally tired
of my stubbornness and decided it was easier to deal with the after
effects of giving in rather than continuing to debate with me :-)

So right now if you want to assign an SRIOV VF network device to a guest
with VFIO, you need something like this (ignoring network device pools
for the moment):


  

  
  
  

  


(or in place of , you could have a  element for
802.11Qb[gh]).


The SRIOV cards that we had around when we were doing this work had
multiple physical ports on them (either 2 or 4), but each physical port
was associated with its own PCI Physical Function (PF), and each of the
PCI Virtual Functions associated with a PF was tied to a single netdev,
i.e. in all cases there was always a 1:1 correspondence between a netdev
and a PCI device. All of libvirt's code dealing with SRIOV VFs and PFs
assumes this 1:1 relationship.

And then came Mellanox "dual port" SRIOV cards

A Mellanox SRIOV NIC doesn't necessarily do that. Instead, it can
operate in "dual port" mode, where it has a single PCI PF device for
both physical ports; the single PF PCI device has 2 separate netdevs
associated with it (so when you look in the "net" subdirectory for the
PCI device, you'll see two netdevs listed, and when you look in the
"device" subdirectory of those two netdevs in sysfs, they both point
back to the same PCI device). VFs associated with that PF will also each
have two netdevs associated with them. This means that when you assign a
VF to a guest, the guest is getting a single PCI device, but it's
getting two netdevs. (I've been told that the advantage of doing both
ports with a single PCI device is that each Mellanox PCI device uses a
huge amount of MMIO space, two ports on each device cuts the MMIO usage
in half).

In order for this to be useful, libvirt needs to set the mac address and
vlan tag of *both* netdevs prior to starting the guest. But we have no
way to represent that in our configuration. In the past it's been
suggested that we just do something like this:

   
 
 
 ...
   

but I have two problems with that:

1)  is supposed to represent a single network device, but
this is trying to make it represent 2 network devices (and what if
someone else comes up with a card that puts *4* netdevs on the same PCI
device?)

2) We would need to do the same thing for  tag. It starts to get ugly.

Alternately we could add a new  subelement, like this:



  

  
  
  

  
  


  

  


(or some variation of that) just so that all the stuff for the 2nd port
is grouped together. But I don't like that the config for port 1 is at a
different level in the hierarchy than the config for port 2, and we
still have the problem that we're trying to describe *2* netdevs with a
single  element, which just feels wrong.

- OR -

what if we admit that  was a bad idea, and try
doing it all with , something like this:

  

  


  
  

  


  
  

  

  

The downsides are:

1) It's providing a 2nd way of describing single port VFs, which could
confuse people (my recommendation would be to deprecate usage of
 in the documentation, while still allowing
it; i.e. we'd still have to maintain that code while discouraging its use).

2) This wouldn't be able to take advantage of the pools of devices
maintained by libvirt networks. (This isn't a problem for Openstack,
since they don't use that anyway, but ovirt does use it).

3) It's an explicit admission that I made a bad decision in 2011 :-P

The upsides?

1) it models the 

Re: [libvirt] [PATCH v3 1/2] qemu: Allow qemuDomainSaveMemory saving VM state to a pipe

[John Ferlan]

[pardon the top post]
Doing some (personal) mail box cleaning and found this "older" patch...
Sorry that this has sat unattended (and probably now forgotten or given
up on) for so long. I don't even recall v1 or v2 reviews - been too
long. In any case, if you still would like to see this addressed, could
you rebase on top of tree and send again along with a few points below...

On 03/08/2017 11:22 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> Base upon patches from Roy Keene 
> 
> Currently qemuDomainSaveMemory can save vm's config
> and memory to fd.
> It write a magic QEMU_SAVE_PARTIAL firstly,
> then re-open it to change QEMU_SAVE_PARTIAL as QEMU_SAVE_MAGIC.
> 
> For pipes this is not possible, attempting to re-open the pipe
> will not connect you to the same consumer.
> Seeking is also not possible on a pipe.
> 
> This patch introduce VIR_DOMAIN_SAVE_DIRECT
> If set, write QEMU_SAVE_MAGIC directly.
> 
> This is useful to me for saving a VM state directly to
> Ceph RBD images without having an intermediate file.

The above really needs to be cleaned up to be less choppy. I see some of
this is copied later too

> 
> Signed-off-by: Roy Keene 
> Signed-off-by: Chen Hanxiao 
> ---
> v3:
>   add news.xml
> 
> v2-resend:
>   rebase on upstream
> 
> v2:
>   rename VIR_DOMAIN_SAVE_PIPE to VIR_DOMAIN_SAVE_DIRECT
>   remove S_ISFIFO check for dst path
> 
>  docs/news.xml|  9 +++
>  include/libvirt/libvirt-domain.h |  1 +
>  src/qemu/qemu_driver.c   | 54 
> ++--
>  3 files changed, 46 insertions(+), 18 deletions(-)
> 

You'll need to understand the changes in the code made more recently for
commit ids 'a2d2aae14' and 'ec986bc5' which altered the flow of the code
to make things more common...

I had to go all the way back to Dec archives to find any comment
regarding this series:

https://www.redhat.com/archives/libvir-list/2016-December/msg00318.html

and before that Roy's series:

https://www.redhat.com/archives/libvir-list/2016-November/msg00269.html

So while I know it's important to have some continuity for some reasons
vis-a-vis the subject line; however, at this point I think you just need
to make a fresh subject indicating the ability to save 'directly'. Using
a pipe just happens to be an implementation of that concept.  You can
and should include pointers to the archives of the previous series. It
helps set context for the reviewer when a v4 arrives in which they don't
recognize the subject.


> diff --git a/docs/news.xml b/docs/news.xml
> index 9515395..57088db 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -37,6 +37,15 @@
>applications running on the platform.
>  
>
> +  
> +
> +   qemu: Allow qemuDomainSaveMemory saving VM state to a pipe> + 
>
> +
> +Introduce flag VIR_DOMAIN_SAVE_DIRECT to enable command 'save'
> +to write to PIPE, for PIPE can't be reopened.
> +
> +  
>  
>  
>

This should be its own separate third patch.

Still you should reword to avoid directly mentioning pipe or fifo, but
using directly to some already opened target. Perhaps in the description
you can mention that by directly, this means you could use a pipe and
provide that example (but try to do so without ceph). IOW: How would
someone use this in a general sense. Would some sort of socket be
another way to use this?

The difference w/ direct is that the to be saved "save-image"
destination doesn't necessarily need to be to a file, it can be to
something else via a pipe or some other indirection, right? (IIUC at least)

> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index c490d71..f58fe2c 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1169,6 +1169,7 @@ typedef enum {
>  VIR_DOMAIN_SAVE_BYPASS_CACHE = 1 << 0, /* Avoid file system cache 
> pollution */
>  VIR_DOMAIN_SAVE_RUNNING  = 1 << 1, /* Favor running over paused */
>  VIR_DOMAIN_SAVE_PAUSED   = 1 << 2, /* Favor paused over running */
> +VIR_DOMAIN_SAVE_DIRECT   = 1 << 3, /* Write QEMU_SAVE_MAGIC directly 
> */

I'd probably avoid mentioning QEMU_SAVE_MAGIC...

>  } virDomainSaveRestoreFlags;
>  
>  int virDomainSave   (virDomainPtr domain,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2032fac..29b7677 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3059,6 +3059,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
>  virQEMUSaveHeader header;
>  bool bypassSecurityDriver = false;
>  bool needUnlink = false;
> +bool canReopen = true;

I think I would have gone with something like:

bool isDirect = (flags & VIR_DOMAIN_SAVE_DIRECT);

>  int ret = -1;
>  int fd = -1;
>   

Re: [libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages

[John Ferlan]

On 07/28/2017 04:33 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> Many vendor id's and product id's have leading zeros.
> Show them in error messages.
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  src/util/virhostdev.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Looking at some other examples...

if (usbsrc->vendor) {
virBufferAsprintf(buf, "\n", usbsrc->vendor);
virBufferAsprintf(buf, "\n", usbsrc->product);

and

if (usbdev->vendor >= 0)
virBufferAsprintf(buf, " vendor='0x%04X'", usbdev->vendor);

if (usbdev->product >= 0)
virBufferAsprintf(buf, " product='0x%04X'", usbdev->product);

Perhaps the best thing to do is be consistent with all of them...  Could
take a bit of searching, but cscope's egrep is pretty good w/
"vendor.*%.*x" (and X).

There's also a usage in libxl_conf, where "%x:%x" is used. So it may be
best to find all possible print's of vendor and make them all consistent.

John

> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 579563c..0e6b5a3 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -1390,7 +1390,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
>  } else if (!autoAddress) {
>  goto out;
>  } else {
> -VIR_INFO("USB device %x:%x could not be found at previous"
> +VIR_INFO("USB device %04x:%04x could not be found at previous"
>   " address (bus:%u device:%u)",
>   vendor, product, bus, device);
>  }
> @@ -1418,12 +1418,12 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr 
> hostdev,
>  } else if (rc > 1) {
>  if (autoAddress) {
>  virReportError(VIR_ERR_OPERATION_FAILED,
> -   _("Multiple USB devices for %x:%x were found,"
> +   _("Multiple USB devices for %04x:%04x were 
> found,"
>   " but none of them is at bus:%u device:%u"),
> vendor, product, bus, device);
>  } else {
>  virReportError(VIR_ERR_OPERATION_FAILED,
> -   _("Multiple USB devices for %x:%x, "
> +   _("Multiple USB devices for %04x:%04x, "
>   "use  to specify one"),
> vendor, product);
>  }
> @@ -1435,7 +1435,7 @@ virHostdevFindUSBDevice(virDomainHostdevDefPtr hostdev,
>  usbsrc->autoAddress = true;
>  
>  if (autoAddress) {
> -VIR_INFO("USB device %x:%x found at bus:%u device:%u (moved"
> +VIR_INFO("USB device %04x:%04x found at bus:%u device:%u (moved"
>   " from bus:%u device:%u)",
>   vendor, product,
>   usbsrc->bus, usbsrc->device,
> 

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

Re: [libvirt] [PATCH v3 2/2] virsh: introduce flage --direct for save command

[John Ferlan]

On 03/08/2017 11:22 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> Base upon patches from Roy Keene 
> 
> This patch introduces --direct flag for save command.
> 
> We could use this flag to save vm to a PIPE.
> 
> We could saving a VM state directly to Ceph RBD images
> without having an intermediate file.
> 
> How to test:
> fifo="$(mktemp -u)"; mkfifo "${fifo}" && virsh save --pipe cirros  "${fifo}" &
> cat "${fifo}" | rbd --id cinder import - hotsnapshot/test1234 & wait; rm -f 
> "${fifo}"

Is there a way to do this without rbd at the other end?  Is there
another example to provide...  Perhaps something that would be
documented else where was well rather than just in the bowels of virsh.
Perhaps somewhere where the other VIR_DOMAIN_SAVE_* flags are described.

> 
> Signed-off-by: Roy Keene 
> Signed-off-by: Chen Hanxiao 
> ---
> v3:
>   rebase on upstream
> 
> v2-resend:
>   rebase on upstream
> 
> v2:
>   rename VIR_DOMAIN_SAVE_PIPE to VIR_DOMAIN_SAVE_DIRECT
> 
>  tools/virsh-domain.c | 6 ++
>  tools/virsh.pod  | 5 -
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 09a9f82..d96e894 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -4192,6 +4192,10 @@ static const vshCmdOptDef opts_save[] = {
>   .type = VSH_OT_BOOL,
>   .help = N_("set domain to be paused on restore")
>  },
> +{.name = "direct",
> + .type = VSH_OT_BOOL,
> + .help = N_("write the file directly, needed by PIPE/FIFO")

remove ", needed by PIPE/FIFO"

> +},
>  {.name = "verbose",
>   .type = VSH_OT_BOOL,
>   .help = N_("display the progress of save")
> @@ -4228,6 +4232,8 @@ doSave(void *opaque)
>  flags |= VIR_DOMAIN_SAVE_RUNNING;
>  if (vshCommandOptBool(cmd, "paused"))
>  flags |= VIR_DOMAIN_SAVE_PAUSED;
> +if (vshCommandOptBool(cmd, "direct"))
> +flags |= VIR_DOMAIN_SAVE_DIRECT;
>  
>  if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
>  goto out;
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index ee79046..9dcb527 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1928,7 +1928,7 @@ have also reverted all storage volumes back to the same 
> contents as when
>  the state file was created.
>  
>  =item B I I [I<--bypass-cache>] [I<--xml> B]
> -[{I<--running> | I<--paused>}] [I<--verbose>]
> +[{I<--running> | I<--paused>}] [I<--direct>] [I<--verbose>]
>  
>  Saves a running domain (RAM, but not disk state) to a state file so that
>  it can be restored
> @@ -1943,6 +1943,9 @@ with B command (sent by another virsh 
> instance). Another option
>  is to send SIGINT (usually with C) to the virsh process running
>  B command. I<--verbose> displays the progress of save.
>  
> +Usually B command will save the domain's state as a regular file.
> +If you want to save it into a PIPE/FIFO, then flag I<--direct> must be set.
> +

As noted in review of .1 - how does --bypass-cache affect things usage
wise?  Does it matter, is it helpful.

I wish I could think of a better way to say this right now, but it's
late so I'm just winging it.  If there's a way to say generically what
direct does where "pipe" is a possible mechanism to use (as is I assume
a socket).

John

>  This is roughly equivalent to doing a hibernate on a running computer,
>  with all the same limitations.  Open network connections may be
>  severed upon restore, as TCP timeouts may have expired.
> 

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

Re: [libvirt] [PATCH 0/3] tests: Make hash table mocking arch independent

[Eric Blake]

On 08/02/2017 10:39 AM, Peter Krempa wrote:
> Plus one fix.
> 
> Peter Krempa (3):
>   util: hash: Include stdbool.h in the header file
>   util: hash: Make virHashCodeGen mockable
>   tests: deterministichash: Make  hash tables arch-independent

Series: Reviewed-by: Eric Blake 

> 
>  src/libvirt_private.syms   |  4 
>  src/util/virhash.h |  1 +
>  src/util/virhashcode.h |  3 ++-
>  .../qemumonitorjson-nodename-relative.result   | 24 
> +++---
>  .../qemumonitorjson-nodename-same-backing.result   | 24 
> +++---
>  tests/virdeterministichashmock.c   | 17 +++
>  tests/virmacmaptestdata/simple2.json   | 12 +--
>  7 files changed, 50 insertions(+), 35 deletions(-)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | 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 24/24] tests: qemumonitorjson: Test extraction of iSCSI device node names

[Eric Blake]

On 08/02/2017 05:13 PM, Jim Fehlig wrote:
> FYI, I've noticed that this patch causes build failures of 3.6.0 on
> s390x and ppc64
> 
> [  189s] 100) node-name-detect(iscsi)  ...
> [  189s] In
> '/home/abuild/rpmbuild/BUILD/libvirt-3.6.0/tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi.result':
> 
> [  189s] Offset 6
> [  189s] Expect [scsi0-0-1
> [  189s] filename: 'json:{"lun": "0", "portal": "example.com:3260",
> "driver": "iscsi", "transport": "tcp", "target":
> "iqn.2016-09.com.example:server"}'

> As you can see the device order is swapped in the "Actual" output. It is
> odd that I don't see the failure on other arches. I have little
> experience with the monitor test code. Any ideas on what might cause that?

Known issue with endianness affecting the mocked hashing; patch posted here:
https://www.redhat.com/archives/libvir-list/2017-August/msg00091.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | 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 v2] qemu: command: align disk serial check to schema

[John Ferlan]

On 03/28/2017 04:10 AM, Nikolay Shirokovskiy wrote:
> Disk serial schema has extra '.+' allowed characters in comparison
> with check in code. Looks like there is no reason for that as qemu
> allows any character AFAIK for serial. This discrepancy is originated
> in 85d15b51 where ability to add serial was added.
> ---
> 
> Diff from v1:
> 
> * fix xml2argv disk serial test to use all valid chars
> 
> Looks like there is no existing infrastructure to test every invalid 
> character.
> 
>  src/qemu/qemu_command.c  | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 

Doing some (personal) mail box cleaning and found this "older" patch...
Sorry that this has sat unattended (and probably now forgotten or given
up on) for so long.

In any case, I adjusted the test to add a second disk rather than
modifying the first one which was added as a result of commit id
'5aee81a0c'.

Reviewed-by: John Ferlan 

I'll push shortly.


John

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c76f923..c5369b0 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -432,7 +432,7 @@ qemuBuildIoEventFdStr(virBufferPtr buf,
>  }
>  
>  #define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
> -  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ "
> +  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ .+"
>  
>  static int
>  qemuSafeSerialParamValue(const char *value)
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args 
> b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> index 2cefdca..fa0fc93 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.args
> @@ -18,6 +18,6 @@ QEMU_AUDIO_DRV=none \
>  -boot c \
>  -usb \
>  -drive 'file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-1,\
> -serial=\ \ WD-WMAP9A966149' \
> +serial=abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_\ .+' 
> \
>  -device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
>  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml 
> b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> index 565462e..d54d73b 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-serial.xml
> @@ -17,7 +17,7 @@
>  
>
>
> -WD-WMAP9A966149
> +  
> abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ 
> .+
>
>  
>  
> 

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

Re: [libvirt] [PATCH 24/24] tests: qemumonitorjson: Test extraction of iSCSI device node names

[Jim Fehlig]

On 07/26/2017 04:00 AM, Peter Krempa wrote:

---
  .../qemumonitorjson-nodename-iscsi-blockstats.json | 113 
  ...qemumonitorjson-nodename-iscsi-named-nodes.json | 114 +
  .../qemumonitorjson-nodename-iscsi.result  |  13 +++
  tests/qemumonitorjsontest.c|   1 +
  4 files changed, 241 insertions(+)
  create mode 100644 
tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi-blockstats.json
  create mode 100644 
tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi-named-nodes.json
  create mode 100644 
tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi.result


FYI, I've noticed that this patch causes build failures of 3.6.0 on s390x and 
ppc64

[  189s] 100) node-name-detect(iscsi)  ...
[  189s] In 
'/home/abuild/rpmbuild/BUILD/libvirt-3.6.0/tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi.result':

[  189s] Offset 6
[  189s] Expect [scsi0-0-1
[  189s] filename: 'json:{"lun": "0", "portal": "example.com:3260", 
"driver": "iscsi", "transport": "tcp", "target": "iqn.2016-09.com.example:server"}'

[  189s] format node : '#block301'
[  189s] format drv  : 'raw'
[  189s] storage node: '#block250'
[  189s] storage drv : 'iscsi'
[  189s]
[  189s] drive-virtio-disk0
[  189s] filename: 'json:{"lun": "0", "portal": "example.com:3260", 
"driver": "iscsi", "transport": "tcp", "target": "iqn.2016-09.com.example:server"}'

[  189s] format node : '#block169'
[  189s] format drv  : 'raw'
[  189s] storage node: '#block038]
[  189s] Actual [virtio-disk0
[  189s] filename: 'json:{"lun": "0", "portal": "example.com:3260", 
"driver": "iscsi", "transport": "tcp", "target": "iqn.2016-09.com.example:server"}'

[  189s] format node : '#block169'
[  189s] format drv  : 'raw'
[  189s] storage node: '#block038'
[  189s] storage drv : 'iscsi'
[  189s]
[  189s] drive-scsi0-0-1
[  189s] filename: 'json:{"lun": "0", "portal": "example.com:3260", 
"driver": "iscsi", "transport": "tcp", "target": "iqn.2016-09.com.example:server"}'

[  189s] format node : '#block301'
[  189s] format drv  : 'raw'
[  189s] storage node: '#block250]
[  189s]   ... FAILED

As you can see the device order is swapped in the "Actual" output. It is odd 
that I don't see the failure on other arches. I have little experience with the 
monitor test code. Any ideas on what might cause that?


Regards,
Jim



diff --git 
a/tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi-blockstats.json 
b/tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi-blockstats.json
new file mode 100644
index 0..b13386ecb
--- /dev/null
+++ b/tests/qemumonitorjsondata/qemumonitorjson-nodename-iscsi-blockstats.json
@@ -0,0 +1,113 @@
+[
+{
+  "device": "drive-virtio-disk0",
+  "parent": {
+"stats": {
+  "flush_total_time_ns": 0,
+  "wr_highest_offset": 0,
+  "wr_total_time_ns": 0,
+  "failed_wr_operations": 0,
+  "failed_rd_operations": 0,
+  "wr_merged": 0,
+  "wr_bytes": 0,
+  "timed_stats": [
+
+  ],
+  "failed_flush_operations": 0,
+  "account_invalid": false,
+  "rd_total_time_ns": 0,
+  "flush_operations": 0,
+  "wr_operations": 0,
+  "rd_merged": 0,
+  "rd_bytes": 0,
+  "invalid_flush_operations": 0,
+  "account_failed": false,
+  "rd_operations": 0,
+  "invalid_wr_operations": 0,
+  "invalid_rd_operations": 0
+},
+"node-name": "#block038"
+  },
+  "stats": {
+"flush_total_time_ns": 0,
+"wr_highest_offset": 0,
+"wr_total_time_ns": 0,
+"failed_wr_operations": 0,
+"failed_rd_operations": 0,
+"wr_merged": 0,
+"wr_bytes": 0,
+"timed_stats": [
+
+],
+"failed_flush_operations": 0,
+"account_invalid": true,
+"rd_total_time_ns": 995504,
+"flush_operations": 0,
+"wr_operations": 0,
+"rd_merged": 0,
+"rd_bytes": 512,
+"invalid_flush_operations": 0,
+"account_failed": true,
+"idle_time_ns": 117550038551,
+"rd_operations": 1,
+"invalid_wr_operations": 0,
+"invalid_rd_operations": 0
+  },
+  "node-name": "#block169"
+},
+{
+  "device": "drive-scsi0-0-1",
+  "parent": {
+"stats": {
+  "flush_total_time_ns": 0,
+  "wr_highest_offset": 0,
+  "wr_total_time_ns": 0,
+  "failed_wr_operations": 0,
+  "failed_rd_operations": 0,
+  "wr_merged": 0,
+  "wr_bytes": 0,
+  "timed_stats": [
+
+  ],
+  "failed_flush_operations": 0,
+  "account_invalid": false,
+  "rd_total_time_ns": 0,
+  "flush_operations": 0,
+  "wr_operations": 0,
+  "rd_merged": 0,
+  "rd_bytes": 0,
+  "invalid_flush_operations": 0,
+  "account_failed": false,
+ 

Re: [libvirt] [PATCH RESEND] Interface: return appropriate status for bridge device

[John Ferlan]

[pardon the top-post comment]
Doing some (personal) mail box cleaning and found this "older" patch...
Sorry that this has sat unattended (and probably now forgotten or given
up on) for so long. Suppose it's an assumption that someone like Laine
who understand bonds/bridges better than others would pick it up...

Although, it's not my space, I'll provide some feedback in general and
if you want to rework, send again - then hopefully someone more in the
know about "expectations" can look at it...

On 02/13/2017 05:06 AM, Lin Ma wrote:
> In function udevInterfaceIsActive, The current design relies on the sys
> attribute 'operstate' to determine whether the interface is active.
> 
> For the device node of virtual network(say virbr0), There is already a
> dummy tap device to maintain a fixed mac on it, So it's available and
> its status should be considered as active. But if no anyelse tap device

anyelse needs to be adjusted.

> connects to it, The value of operstate of virbr0 in sysfs is 'down', So
> udevInterfaceIsActive returns 0, It causes 'virsh iface-list --all' to
> report the status of virbr0 as 'inactive'.

Personally, I'm not sure if this is a feature or a problem, hopefully
Laine can elaborate!

> 
> This patch fixes the issue by counting slave devices for bridge device.
> 
> Signed-off-by: Lin Ma 
> ---
>  src/interface/interface_backend_udev.c | 22 ++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/src/interface/interface_backend_udev.c 
> b/src/interface/interface_backend_udev.c
> index 5d0fc64..9ac4674 100644
> --- a/src/interface/interface_backend_udev.c
> +++ b/src/interface/interface_backend_udev.c
> @@ -1127,6 +1127,11 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
>  struct udev_device *dev;
>  virInterfaceDefPtr def = NULL;
>  int status = -1;
> +struct dirent **member_list = NULL;
> +const char *devtype;
> +int member_count = 0;
> +char *member_path;
> +size_t i;
>  
>  dev = udev_device_new_from_subsystem_sysname(udev, "net",
>   ifinfo->name);
> @@ -1146,6 +1151,23 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
>  /* Check if it's active or not */
>  status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up");
>  

The following hunk should be in a "helper" - that is create another
function...

if (!status) {
devtype = udev_device_get_devtype(dev);
if (STREQ_NULLABLE(devtype, "bridge"))
status = newProperlyNamedHelper(arg1, arg2);
}

Although it's almost too bad the bridge/bond code processing scandir
list output couldn't be combined in some way...  It's made tricky by
using bridge vs. bond, but I think the code has a lot of similarities
and allocation of specific fields and list traversal could probably be
handled by some callback type function. Not required for another patch,
but might be interesting to try.

> +if (!status) {
> +devtype = udev_device_get_devtype(dev);
> +if (STREQ_NULLABLE(devtype, "bridge")) {
> +if (virAsprintf(_path, "%s/%s",
> +udev_device_get_syspath(dev), "brif") < 0)

This should have been properly aligned (udev_* under _path).
Although I assume this is essentially a copy from udevGetIfaceDefBridge.

> +goto cleanup;
> +member_count = scandir(member_path, _list,
> +udevBridgeScanDirFilter, alphasort);

Similarly w/r/t alignment, but udevB* under member_path.

> +if (member_count > 0)
> +status = 1;
> +for (i = 0; i < member_count; i++)
> +VIR_FREE(member_list[i]);
> +VIR_FREE(member_list);

This I believe could be replaced by virStringListFree (and similar for
other consumers using scandir results)


John

> +VIR_FREE(member_path);
> +}
> +}
> +>  udev_device_unref(dev);
>  
>   cleanup:
> 

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

[libvirt] [PATCH] conf: Fix printing of 'type' and 'tty_compat' for Chr devices

[John Ferlan]

Commit id '0c1d8632' caused a regression in the virt-manager
test suite when formatting the .

Adust the code to print the type in it's own new helper called
virDomainChrTypeFormat and have the virDomainChrSourceDefFormat
manage just formatting the source and change to a void type since
only 0 could be returned. Adjust the callers to handle properly.

Signed-off-by: John Ferlan 
---

 Although technically a CI build breaker since virt-manager test is
 failing, I figured I'd let this one go through the formal review just
 in case someone has agita over new function name or would like to see
 things done in a different manner. 


 src/conf/domain_conf.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eb70523..878c15d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22804,10 +22804,9 @@ virDomainNetDefFormat(virBufferPtr buf,
 /* Assumes that "". */
 static int
-virDomainChrSourceDefFormat(virBufferPtr buf,
-virDomainChrSourceDefPtr def,
-bool tty_compat,
-unsigned int flags)
+virDomainChrTypeFormat(virBufferPtr buf,
+   virDomainChrSourceDefPtr def,
+   bool tty_compat)
 {
 const char *type = virDomainChrTypeToString(def->type);
 
@@ -22825,6 +22824,15 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
 }
 virBufferAddLit(buf, ">\n");
 
+return 0;
+}
+
+
+static void
+virDomainChrSourceDefFormat(virBufferPtr buf,
+virDomainChrSourceDefPtr def,
+unsigned int flags)
+{
 switch ((virDomainChrType)def->type) {
 case VIR_DOMAIN_CHR_TYPE_NULL:
 case VIR_DOMAIN_CHR_TYPE_VC:
@@ -22923,8 +22931,6 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
 }
 virBufferAddLit(buf, "/>\n");
 }
-
-return 0;
 }
 
 static int
@@ -22953,8 +22959,9 @@ virDomainChrDefFormat(virBufferPtr buf,
   def->source->type == VIR_DOMAIN_CHR_TYPE_PTY &&
   !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
   def->source->data.file.path);
-if (virDomainChrSourceDefFormat(buf, def->source, tty_compat, flags) < 0)
+if (virDomainChrTypeFormat(buf, def->source, tty_compat) < 0)
 return -1;
+virDomainChrSourceDefFormat(buf, def->source, flags);
 
 /* Format  block */
 switch (def->deviceType) {
@@ -23053,6 +23060,8 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
 return -1;
 }
 
+virBufferAsprintf(buf, "type) {
 case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
 break;
@@ -23067,9 +23076,9 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
 break;
 
 case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
-if (virDomainChrSourceDefFormat(, def->data.passthru, false,
-flags) < 0)
+if (virDomainChrTypeFormat(buf, def->data.passthru, false) < 0)
 return -1;
+virDomainChrSourceDefFormat(, def->data.passthru, flags);
 break;
 
 default:
@@ -23082,7 +23091,6 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
 if (virBufferCheckError() < 0)
 return -1;
 
-virBufferAsprintf(buf, "\n");
 virBufferAddBuffer(buf, );
@@ -23390,10 +23398,10 @@ virDomainRNGDefFormat(virBufferPtr buf,
 break;
 
 case VIR_DOMAIN_RNG_BACKEND_EGD:
-virBufferAdjustIndent(buf, 2);
-if (virDomainChrSourceDefFormat(buf, def->source.chardev,
-false, flags) < 0)
+if (virDomainChrTypeFormat(buf, def->source.chardev, false) < 0)
 return -1;
+virBufferAdjustIndent(buf, 2);
+virDomainChrSourceDefFormat(buf, def->source.chardev, flags);
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
 
@@ -24234,9 +24242,10 @@ virDomainRedirdevDefFormat(virBufferPtr buf,
 bus = virDomainRedirdevBusTypeToString(def->bus);
 
 virBufferAsprintf(buf, "source, false, flags) < 0)
+if (virDomainChrTypeFormat(buf, def->source, false) < 0)
 return -1;
+virBufferAdjustIndent(buf, 2);
+virDomainChrSourceDefFormat(buf, def->source, flags);
 virDomainDeviceInfoFormat(buf, >info,
   flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
 virBufferAdjustIndent(buf, -2);
-- 
2.9.4

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

[libvirt] [PATCH 1/2] docs: Fix syntax-check error

[John Ferlan]

commit id '40cb5581' caused syntax-check error:

prohibit_empty_lines_at_EOF
docs/manifest.json
maint.mk: empty line(s) or no newline at EOF
maint.mk:929: recipe for target 'sc_prohibit_empty_lines_at_EOF' failed
make: *** [sc_prohibit_empty_lines_at_EOF] Error 1

I just edited the file and replaced the closing } and it made things happy

Signed-off-by: John Ferlan 
---
 docs/manifest.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/manifest.json b/docs/manifest.json
index 0f8fcba..9466390 100644
--- a/docs/manifest.json
+++ b/docs/manifest.json
@@ -15,4 +15,4 @@
 "theme_color": "#ff",
 "background_color": "#ff",
 "display": "standalone"
-}
\ No newline at end of file
+}
-- 
2.9.4

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

[libvirt] [PATCH 2/2] docs: Fix syntax-check error

[John Ferlan]

Commit id '94d2d6429' caused a syntax-error check to fail:

docs/Makefile.am:276:   $(AM_V_GEN)sed -e '/<\/span>/r '"$(srcdir)/$@.code.in" \
maint.mk: Wrap long lines in Makefiles
cfg.mk:721: recipe for target 'sc_prohibit_long_lines' failed
make: *** [sc_prohibit_long_lines] Error 1
make: *** Waiting for unfinished jobs

Altered the line to put another line wrap between sed and -e

Signed-off-by: John Ferlan 
---
 docs/Makefile.am | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 633a0fa..d04b2d5 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -273,7 +273,8 @@ MAINTAINERCLEANFILES += \
|| { rm $@ && exit 1; }
 
 %.php: %.php.tmp %.php.code.in
-   $(AM_V_GEN)sed -e '/<\/span>/r 
'"$(srcdir)/$@.code.in" \
+   $(AM_V_GEN)sed \
+   -e '/<\/span>/r '"$(srcdir)/$@.code.in" \
-e /php_placeholder/d < $@.tmp > $(srcdir)/$@ \
|| { rm $(srcdir)/$@ && exit 1; }
 
-- 
2.9.4

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

[libvirt] [PATCH 0/2] Fix a couple of syntax check errors

[John Ferlan]

Pushed under build breaker rule

John Ferlan (2):
  docs: Fix syntax-check error
  docs: Fix syntax-check error

 docs/Makefile.am   | 3 ++-
 docs/manifest.json | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.9.4

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

Re: [libvirt] [PATCH 3/8] Use a separate buffer for subelements

[Cole Robinson]

On 07/26/2017 09:29 AM, Ján Tomko wrote:
> Convert virDomainSmartcardDefFormat to use a separate buffer
> for possible subelements, to avoid the need for duplicated
> formatting logic in virDomainDeviceInfoNeedsFormat.
> ---
>  src/conf/domain_conf.c | 39 +++
>  1 file changed, 23 insertions(+), 16 deletions(-)
> 

Looks like this patch causes a regression, currently breaking the virt-manager
test suite that danpb pointed out to me. Edit an existing VM and add



The returned XML is invalid:


   type='spicevmc'>
  


Unfortunately there aren't any xml2xml tests for smartcard bits that would
have caught this...

Thanks,
Cole

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6c958bcf6..ead94976d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -23047,37 +23047,32 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
>  unsigned int flags)
>  {
>  const char *mode = virDomainSmartcardTypeToString(def->type);
> +virBuffer childBuf = VIR_BUFFER_INITIALIZER;
>  size_t i;
>  
> +virBufferAdjustIndent(, virBufferGetIndent(buf, false) + 2);
> +
>  if (!mode) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("unexpected smartcard type %d"), def->type);
>  return -1;
>  }
>  
> -virBufferAsprintf(buf, " -virBufferAdjustIndent(buf, 2);
>  switch (def->type) {
>  case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
> -if (!virDomainDeviceInfoNeedsFormat(>info, flags)) {
> -virBufferAdjustIndent(buf, -2);
> -virBufferAddLit(buf, "/>\n");
> -return 0;
> -}
> -virBufferAddLit(buf, ">\n");
>  break;
>  
>  case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
> -virBufferAddLit(buf, ">\n");
> -for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++)
> -virBufferEscapeString(buf, "%s\n",
> +for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
> +virBufferEscapeString(, 
> "%s\n",
>def->data.cert.file[i]);
> -virBufferEscapeString(buf, "%s\n",
> +}
> +virBufferEscapeString(, "%s\n",
>def->data.cert.database);
>  break;
>  
>  case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
> -if (virDomainChrSourceDefFormat(buf, def->data.passthru, false,
> +if (virDomainChrSourceDefFormat(, def->data.passthru, false,
>  flags) < 0)
>  return -1;
>  break;
> @@ -23087,10 +23082,22 @@ virDomainSmartcardDefFormat(virBufferPtr buf,
> _("unexpected smartcard type %d"), def->type);
>  return -1;
>  }
> -if (virDomainDeviceInfoFormat(buf, >info, flags) < 0)
> +if (virDomainDeviceInfoFormat(, >info, flags) < 0) {
> +virBufferFreeAndReset();
>  return -1;
> -virBufferAdjustIndent(buf, -2);
> -virBufferAddLit(buf, "\n");
> +}
> +
> +if (virBufferCheckError() < 0)
> +return -1;
> +
> +virBufferAsprintf(buf, " +if (virBufferUse()) {
> +virBufferAddLit(buf, ">\n");
> +virBufferAddBuffer(buf, );
> +virBufferAddLit(buf, "\n");
> +} else {
> +virBufferAddLit(buf, "/>\n");
> +}
>  return 0;
>  }
>  
> 

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

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Alex Williamson]

On Wed, 2 Aug 2017 21:16:28 +0530
Kirti Wankhede  wrote:

> On 8/2/2017 6:29 PM, Gao, Ping A wrote:
> > 
> > On 2017/8/2 18:19, Kirti Wankhede wrote:  
> >>
> >> On 8/2/2017 3:56 AM, Alex Williamson wrote:  
> >>> On Tue, 1 Aug 2017 13:54:27 +0800
> >>> "Gao, Ping A"  wrote:
> >>>  
>  On 2017/7/28 0:00, Gao, Ping A wrote:  
> > On 2017/7/27 0:43, Alex Williamson wrote:
> >> [cc +libvir-list]
> >>
> >> On Wed, 26 Jul 2017 21:16:59 +0800
> >> "Gao, Ping A"  wrote:
> >>
> >>> The vfio-mdev provide the capability to let different guest share the
> >>> same physical device through mediate sharing, as result it bring a
> >>> requirement about how to control the device sharing, we need a QoS
> >>> related interface for mdev to management virtual device resource.
> >>>
> >>> E.g. In practical use, vGPUs assigned to different quests almost has
> >>> different performance requirements, some guests may need higher 
> >>> priority
> >>> for real time usage, some other may need more portion of the GPU
> >>> resource to get higher 3D performance, corresponding we can define 
> >>> some
> >>> interfaces like weight/cap for overall budget control, priority for
> >>> single submission control.
> >>>
> >>> So I suggest to add some common attributes which are vendor agnostic 
> >>> in
> >>> mdev core sysfs for QoS purpose.
> >> I think what you're asking for is just some standardization of a QoS
> >> attribute_group which a vendor can optionally include within the
> >> existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
> >> transparently enable this, but it really only provides the standard,
> >> all of the support code is left for the vendor.  I'm fine with that,
> >> but of course the trouble with and sort of standardization is arriving
> >> at an agreed upon standard.  Are there QoS knobs that are generic
> >> across any mdev device type?  Are there others that are more specific
> >> to vGPU?  Are there existing examples of this that we can steal their
> >> specification?
> > Yes, you are right, standardization QoS knobs are exactly what I wanted.
> > Only when it become a part of the mdev framework and libvirt, then QoS
> > such critical feature can be leveraged by cloud usage. HW vendor only
> > need to focus on the implementation of the corresponding QoS algorithm
> > in their back-end driver.
> >
> > Vfio-mdev framework provide the capability to share the device that lack
> > of HW virtualization support to guests, no matter the device type,
> > mediated sharing actually is a time sharing multiplex method, from this
> > point of view, QoS can be take as a generic way about how to control the
> > time assignment for virtual mdev device that occupy HW. As result we can
> > define QoS knob generic across any device type by this way. Even if HW
> > has build in with some kind of QoS support, I think it's not a problem
> > for back-end driver to convert mdev standard QoS definition to their
> > specification to reach the same performance expectation. Seems there are
> > no examples for us to follow, we need define it from scratch.
> >
> > I proposal universal QoS control interfaces like below:
> >
> > Cap: The cap limits the maximum percentage of time a mdev device can own
> > physical device. e.g. cap=60, means mdev device cannot take over 60% of
> > total physical resource.
> >
> > Weight: The weight define proportional control of the mdev device
> > resource between guests, it’s orthogonal with Cap, to target load
> > balancing. E.g. if guest 1 should take double mdev device resource
> > compare with guest 2, need set weight ratio to 2:1.
> >
> > Priority: The guest who has higher priority will get execution first,
> > target to some real time usage and speeding interactive response.
> >
> > Above QoS interfaces cover both overall budget control and single
> > submission control. I will sent out detail design later once get 
> > aligned.
>  Hi Alex,
>  Any comments about the interface mentioned above?  
> >>> Not really.
> >>>
> >>> Kirti, are there any QoS knobs that would be interesting
> >>> for NVIDIA devices?
> >>>  
> >> We have different types of vGPU for different QoS factors.
> >>
> >> When mdev devices are created, its resources are allocated irrespective
> >> of which VM/userspace app is going to use that mdev device. Any
> >> parameter we add here should be tied to particular mdev device and not
> >> to the guest/app that are going to use it. 'Cap' and 'Priority' are
> >> along that line. All mdev device might not need/use these parameters,
> >> these can be made optional interfaces.  
> > 
> > We also define some QoS parameters in 

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Kirti Wankhede]

On 8/2/2017 6:29 PM, Gao, Ping A wrote:
> 
> On 2017/8/2 18:19, Kirti Wankhede wrote:
>>
>> On 8/2/2017 3:56 AM, Alex Williamson wrote:
>>> On Tue, 1 Aug 2017 13:54:27 +0800
>>> "Gao, Ping A"  wrote:
>>>
 On 2017/7/28 0:00, Gao, Ping A wrote:
> On 2017/7/27 0:43, Alex Williamson wrote:  
>> [cc +libvir-list]
>>
>> On Wed, 26 Jul 2017 21:16:59 +0800
>> "Gao, Ping A"  wrote:
>>  
>>> The vfio-mdev provide the capability to let different guest share the
>>> same physical device through mediate sharing, as result it bring a
>>> requirement about how to control the device sharing, we need a QoS
>>> related interface for mdev to management virtual device resource.
>>>
>>> E.g. In practical use, vGPUs assigned to different quests almost has
>>> different performance requirements, some guests may need higher priority
>>> for real time usage, some other may need more portion of the GPU
>>> resource to get higher 3D performance, corresponding we can define some
>>> interfaces like weight/cap for overall budget control, priority for
>>> single submission control.
>>>
>>> So I suggest to add some common attributes which are vendor agnostic in
>>> mdev core sysfs for QoS purpose.  
>> I think what you're asking for is just some standardization of a QoS
>> attribute_group which a vendor can optionally include within the
>> existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
>> transparently enable this, but it really only provides the standard,
>> all of the support code is left for the vendor.  I'm fine with that,
>> but of course the trouble with and sort of standardization is arriving
>> at an agreed upon standard.  Are there QoS knobs that are generic
>> across any mdev device type?  Are there others that are more specific
>> to vGPU?  Are there existing examples of this that we can steal their
>> specification?  
> Yes, you are right, standardization QoS knobs are exactly what I wanted.
> Only when it become a part of the mdev framework and libvirt, then QoS
> such critical feature can be leveraged by cloud usage. HW vendor only
> need to focus on the implementation of the corresponding QoS algorithm
> in their back-end driver.
>
> Vfio-mdev framework provide the capability to share the device that lack
> of HW virtualization support to guests, no matter the device type,
> mediated sharing actually is a time sharing multiplex method, from this
> point of view, QoS can be take as a generic way about how to control the
> time assignment for virtual mdev device that occupy HW. As result we can
> define QoS knob generic across any device type by this way. Even if HW
> has build in with some kind of QoS support, I think it's not a problem
> for back-end driver to convert mdev standard QoS definition to their
> specification to reach the same performance expectation. Seems there are
> no examples for us to follow, we need define it from scratch.
>
> I proposal universal QoS control interfaces like below:
>
> Cap: The cap limits the maximum percentage of time a mdev device can own
> physical device. e.g. cap=60, means mdev device cannot take over 60% of
> total physical resource.
>
> Weight: The weight define proportional control of the mdev device
> resource between guests, it’s orthogonal with Cap, to target load
> balancing. E.g. if guest 1 should take double mdev device resource
> compare with guest 2, need set weight ratio to 2:1.
>
> Priority: The guest who has higher priority will get execution first,
> target to some real time usage and speeding interactive response.
>
> Above QoS interfaces cover both overall budget control and single
> submission control. I will sent out detail design later once get aligned. 
>  
 Hi Alex,
 Any comments about the interface mentioned above?
>>> Not really.
>>>
>>> Kirti, are there any QoS knobs that would be interesting
>>> for NVIDIA devices?
>>>
>> We have different types of vGPU for different QoS factors.
>>
>> When mdev devices are created, its resources are allocated irrespective
>> of which VM/userspace app is going to use that mdev device. Any
>> parameter we add here should be tied to particular mdev device and not
>> to the guest/app that are going to use it. 'Cap' and 'Priority' are
>> along that line. All mdev device might not need/use these parameters,
>> these can be made optional interfaces.
> 
> We also define some QoS parameters in Intel vGPU types, but it only
> provided a default fool-style way. We still need a flexible approach
> that give user the ability to change QoS parameters freely and
> dynamically according to their requirement , not restrict to the current
> limited and static vGPU types.
> 
>> In the above proposal, I'm not sure 

[libvirt] [PATCH 1/3] util: hash: Include stdbool.h in the header file

[Peter Krempa]

The functions declared in virhash.h return bool, but stdbool.h was not
included.
---
 src/util/virhash.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/util/virhash.h b/src/util/virhash.h
index 00b2550e7..5b24fc000 100644
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -14,6 +14,7 @@
 # define __VIR_HASH_H__

 # include 
+# include 

 /*
  * The hash table.
-- 
2.13.2

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

[libvirt] [PATCH 0/3] tests: Make hash table mocking arch independent

[Peter Krempa]

Plus one fix.

Peter Krempa (3):
  util: hash: Include stdbool.h in the header file
  util: hash: Make virHashCodeGen mockable
  tests: deterministichash: Make  hash tables arch-independent

 src/libvirt_private.syms   |  4 
 src/util/virhash.h |  1 +
 src/util/virhashcode.h |  3 ++-
 .../qemumonitorjson-nodename-relative.result   | 24 +++---
 .../qemumonitorjson-nodename-same-backing.result   | 24 +++---
 tests/virdeterministichashmock.c   | 17 +++
 tests/virmacmaptestdata/simple2.json   | 12 +--
 7 files changed, 50 insertions(+), 35 deletions(-)

-- 
2.13.2

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

[libvirt] [PATCH 2/3] util: hash: Make virHashCodeGen mockable

[Peter Krempa]

Export the function from the util module so that dynamic linking can
override it.
---
 src/libvirt_private.syms | 4 
 src/util/virhashcode.h   | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 054315fb7..32ac0835e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1774,6 +1774,10 @@ virHashUpdateEntry;
 virHashValueFree;


+# util/virhashcode.h
+virHashCodeGen;
+
+
 # util/virhook.h
 virHookCall;
 virHookInitialize;
diff --git a/src/util/virhashcode.h b/src/util/virhashcode.h
index 7732f816c..f8171df26 100644
--- a/src/util/virhashcode.h
+++ b/src/util/virhashcode.h
@@ -30,6 +30,7 @@

 # include "internal.h"

-uint32_t virHashCodeGen(const void *key, size_t len, uint32_t seed);
+uint32_t virHashCodeGen(const void *key, size_t len, uint32_t seed)
+ATTRIBUTE_NOINLINE;

 #endif /* __VIR_HASH_CODE_H__ */
-- 
2.13.2

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

[libvirt] [PATCH 3/3] tests: deterministichash: Make hash tables arch-independent

[Peter Krempa]

It turns out that our implementation of the hashing function is
endian-dependent and thus if used on various architectures the testsuite
may have different results. Work this around by mocking virHashCodeGen
to something which does not use bit operations instead of just setting a
deterministic seed.
---
 .../qemumonitorjson-nodename-relative.result   | 24 +++---
 .../qemumonitorjson-nodename-same-backing.result   | 24 +++---
 tests/virdeterministichashmock.c   | 17 +++
 tests/virmacmaptestdata/simple2.json   | 12 +--
 4 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/tests/qemumonitorjsondata/qemumonitorjson-nodename-relative.result 
b/tests/qemumonitorjsondata/qemumonitorjson-nodename-relative.result
index 6c0c77618..5288319d3 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-nodename-relative.result
+++ b/tests/qemumonitorjsondata/qemumonitorjson-nodename-relative.result
@@ -1,15 +1,3 @@
-drive-ide0-0-1
-filename: '/var/lib/libvirt/images/relsnap.qcow2'
-format node : '#block1290'
-format drv  : 'qcow2'
-storage node: '#block1107'
-storage drv : 'file'
-  filename: '/var/lib/libvirt/images/base.qcow2'
-  format node : '#block927'
-  format drv  : 'qcow2'
-  storage node: '#block800'
-  storage drv : 'file'
-
 drive-ide0-0-0
 filename: '/var/lib/libvirt/images/img3'
 format node : '#block118'
@@ -31,3 +19,15 @@ storage drv : 'file'
   format drv  : 'qcow2'
   storage node: '#block614'
   storage drv : 'file'
+
+drive-ide0-0-1
+filename: '/var/lib/libvirt/images/relsnap.qcow2'
+format node : '#block1290'
+format drv  : 'qcow2'
+storage node: '#block1107'
+storage drv : 'file'
+  filename: '/var/lib/libvirt/images/base.qcow2'
+  format node : '#block927'
+  format drv  : 'qcow2'
+  storage node: '#block800'
+  storage drv : 'file'
diff --git 
a/tests/qemumonitorjsondata/qemumonitorjson-nodename-same-backing.result 
b/tests/qemumonitorjsondata/qemumonitorjson-nodename-same-backing.result
index 87431f7ca..7b12a1746 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-nodename-same-backing.result
+++ b/tests/qemumonitorjsondata/qemumonitorjson-nodename-same-backing.result
@@ -1,15 +1,3 @@
-drive-sata0-0-1
-filename: '/var/lib/libvirt/images/b.qcow2'
-format node : '#block548'
-format drv  : 'qcow2'
-storage node: '#block487'
-storage drv : 'file'
-  filename: '/var/lib/libvirt/images/base.qcow2'
-  format node : '#block771'
-  format drv  : 'qcow2'
-  storage node: '#block692'
-  storage drv : 'file'
-
 drive-sata0-0-0
 filename: '/var/lib/libvirt/images/a.qcow2'
 format node : '#block132'
@@ -21,3 +9,15 @@ storage drv : 'file'
   format drv  : 'qcow2'
   storage node: '#block224'
   storage drv : 'file'
+
+drive-sata0-0-1
+filename: '/var/lib/libvirt/images/b.qcow2'
+format node : '#block548'
+format drv  : 'qcow2'
+storage node: '#block487'
+storage drv : 'file'
+  filename: '/var/lib/libvirt/images/base.qcow2'
+  format node : '#block771'
+  format drv  : 'qcow2'
+  storage node: '#block692'
+  storage drv : 'file'
diff --git a/tests/virdeterministichashmock.c b/tests/virdeterministichashmock.c
index d01f1c9e4..cd80cfcb5 100644
--- a/tests/virdeterministichashmock.c
+++ b/tests/virdeterministichashmock.c
@@ -20,10 +20,19 @@

 #include 

-#include "virrandom.h"
+#include "util/virhashcode.h"

-uint64_t virRandomBits(int nbits ATTRIBUTE_UNUSED)
+uint32_t
+virHashCodeGen(const void *key,
+   size_t len,
+   uint32_t seed ATTRIBUTE_UNUSED)
 {
-return 4; /* chosen by fair dice roll.
- guaranteed to be random. */
+const uint8_t *k = key;
+uint32_t h = 0;
+size_t i;
+
+for (i = 0; i < len; i++)
+h += k[i];
+
+return h;
 }
diff --git a/tests/virmacmaptestdata/simple2.json 
b/tests/virmacmaptestdata/simple2.json
index 91b2cde0c..52132c241 100644
--- a/tests/virmacmaptestdata/simple2.json
+++ b/tests/virmacmaptestdata/simple2.json
@@ -1,16 +1,16 @@
 [
   {
-"domain": "f25",
+"domain": "f24",
 "macs": [
-  "00:11:22:33:44:55",
-  "aa:bb:cc:00:11:22"
+  "aa:bb:cc:dd:ee:ff",
+  "a1:b2:c3:d4:e5:f6"
 ]
   },
   {
-"domain": "f24",
+"domain": "f25",
 "macs": [
-  "aa:bb:cc:dd:ee:ff",
-  "a1:b2:c3:d4:e5:f6"
+  "00:11:22:33:44:55",
+  "aa:bb:cc:00:11:22"
 ]
   }
 ]
-- 
2.13.2

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

Re: [libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse

[John Ferlan]

>>
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 7728321cb..4dc49fdb0 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>
>> [...]
>>
>>> @@ -23309,6 +23321,10 @@ static int
>>> virDomainPanicDefFormat(virBufferPtr buf,
>>>  virBufferAdjustIndent(, indent + 2);
>>>  if (virDomainDeviceInfoFormat(, >info, 0) < 0)
>>>  return -1;
>>> +
>>> +if (virBufferCheckError() < 0)
>>> +return -1;
>>> +
>>
>> Seeing a virBufferFreeAndReset below this if condition first had me
>> thinking, well that's unnecessary; however, in actuality I think we have
>> a leak any time virBufferUse doesn't return a non zero value call
>> virBufferAddBuffer to consume the buffer.
> 
> I do not see the leak. If we made no attempt at all to use the buffer,
> nothing should have been allocated. If we tried to add something to it,
> and failed on OOM, virBufferSetError should free the content and set use
> to zero. The only possible leak would be when we try to extend the
> buffer without actually writing any content to it - but we have no
> reason to do that in an XML file, since there's always going to be
> at least the element name.
> 
> Jan
> 

Hmm.. right - I guess it's seeing the FreeAndReset in some places and
not others that got me to thinking about this and of course being
somehow convinced that there could be a leak.  Perhaps those places
where FreeAndReset is called unnecessarily could be cleaned up (they're
not wrong, but they do nothing as long as the AddBuffer was used).

John

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

Re: [libvirt] [PATCH 7/8] conf: check for buffer errors before virBufferUse

[Ján Tomko]

On Tue, Aug 01, 2017 at 05:45:10PM -0400, John Ferlan wrote:
[...]


diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 0f99f3096..db7efffdf 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -930,6 +930,11 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
   bank->controls[j]->max_allocation);
 }



One could move the VIR_FREE(cpus_str) to right here and avoid the two



One has pushed that as a separate patch:
https://www.redhat.com/archives/libvir-list/2017-August/msg00088.html


It also seems this particular function uses virBufferAdjustIndent on buf
instead of on the child buf (or in this case controlBuf)...



While inconsistent, that does not concern me.


+if (virBufferCheckError() < 0) {
+VIR_FREE(cpus_str);
+return -1;
+}
+
 if (virBufferUse()) {
 virBufferAddLit(buf, ">\n");
 virBufferAddBuffer(buf, );


[...]


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7728321cb..4dc49fdb0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c


[...]


@@ -23309,6 +23321,10 @@ static int virDomainPanicDefFormat(virBufferPtr buf,
 virBufferAdjustIndent(, indent + 2);
 if (virDomainDeviceInfoFormat(, >info, 0) < 0)
 return -1;
+
+if (virBufferCheckError() < 0)
+return -1;
+


Seeing a virBufferFreeAndReset below this if condition first had me
thinking, well that's unnecessary; however, in actuality I think we have
a leak any time virBufferUse doesn't return a non zero value call
virBufferAddBuffer to consume the buffer.


I do not see the leak. If we made no attempt at all to use the buffer,
nothing should have been allocated. If we tried to add something to it,
and failed on OOM, virBufferSetError should free the content and set use
to zero. The only possible leak would be when we try to extend the
buffer without actually writing any content to it - but we have no
reason to do that in an XML file, since there's always going to be
at least the element name.

Jan



Not necessarily something to stop this patch, but perhaps a leak for:

virDomainDiskDefFormat
virDomainControllerDriverFormat
virDomainControllerDefFormat
virDomainFSDefFormat
virDomainMemballoonDefFormat
virDomainRNGDefFormat
virDomainVideoDefFormat
virDomainInputDefFormat
virDomainIOMMUDefFormat
virDomainDiskDefFormat



 if (virBufferUse()) {
 virBufferAddLit(buf, ">\n");
 virBufferAddBuffer(buf, );
@@ -23655,6 +23671,9 @@ virDomainInputDefFormat(virBufferPtr buf,


[...]


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

[libvirt] [PATCH] virCapabilitiesFormatCaches: free cpus_str right after use

[Ján Tomko]

This will simplify the cleanup when we start checking for
buffer errors.
---
Pushed as trivial.

 src/conf/capabilities.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 0f99f3096..561a6cf9e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -904,6 +904,7 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
   bank->size >> (kilos * 10),
   kilos ? "KiB" : "B",
   cpus_str);
+VIR_FREE(cpus_str);
 
 virBufferAdjustIndent(, indent + 4);
 for (j = 0; j < bank->ncontrols; j++) {
@@ -937,8 +938,6 @@ virCapabilitiesFormatCaches(virBufferPtr buf,
 } else {
 virBufferAddLit(buf, "/>\n");
 }
-
-VIR_FREE(cpus_str);
 }
 
 virBufferAdjustIndent(buf, -2);
-- 
2.13.0

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

Re: [libvirt] [PATCH 24/24] tests: qemumonitorjson: Test extraction of iSCSI device node names

[Peter Krempa]

On Wed, Aug 02, 2017 at 13:50:49 +0200, Bjoern Walk wrote:
> So, this test fails on S390 because the actual test in
> testBlockNodeNameDetect is dependent on the ordering of the entries in
> the hash table, which is different on big endian machines.
> 
> There are two other tests which have multiple results in the backing
> chain, 'same-backing' and 'relative', but their keys are reasonable
> similar that we got lucky and the hashes are ordered the same.
> 
> I don't suspect that code that uses the tested function,
> qemuBlockNodeNameGetBackingChain, is dependent on the ordering of keys
> so this is actually only a problem in the test suite.

Yes, the hash function is non-deterministic. I'm working on mocking it
so that it uses a simpler algorithm which will be endian independent.

> 
> Sorry for the late notice for this.

Well, since the issue is not in this series, but the other tests needing
specific ordering were lucky enough to be ordered the same way, it was
rather hard to catch.

> 
> Bjoern



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



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

[libvirt] Release of libvirt-php-0.5.4

[Michal Privoznik]

I just did the release of libvirt-php-0.5.4. You can download it here:

  https://libvirt.org/sources/php/libvirt-php-0.5.4.tar.gz

There were several bug fixes, style conversions, etc.
New features include:
- Added API bindings for getting/setting network autostart
- Implement NWFilter API bindings
- examples enhancement
and many others. Thanks anybody who contributed!

Michal

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

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Gao, Ping A]

On 2017/8/2 18:19, Kirti Wankhede wrote:
>
> On 8/2/2017 3:56 AM, Alex Williamson wrote:
>> On Tue, 1 Aug 2017 13:54:27 +0800
>> "Gao, Ping A"  wrote:
>>
>>> On 2017/7/28 0:00, Gao, Ping A wrote:
 On 2017/7/27 0:43, Alex Williamson wrote:  
> [cc +libvir-list]
>
> On Wed, 26 Jul 2017 21:16:59 +0800
> "Gao, Ping A"  wrote:
>  
>> The vfio-mdev provide the capability to let different guest share the
>> same physical device through mediate sharing, as result it bring a
>> requirement about how to control the device sharing, we need a QoS
>> related interface for mdev to management virtual device resource.
>>
>> E.g. In practical use, vGPUs assigned to different quests almost has
>> different performance requirements, some guests may need higher priority
>> for real time usage, some other may need more portion of the GPU
>> resource to get higher 3D performance, corresponding we can define some
>> interfaces like weight/cap for overall budget control, priority for
>> single submission control.
>>
>> So I suggest to add some common attributes which are vendor agnostic in
>> mdev core sysfs for QoS purpose.  
> I think what you're asking for is just some standardization of a QoS
> attribute_group which a vendor can optionally include within the
> existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
> transparently enable this, but it really only provides the standard,
> all of the support code is left for the vendor.  I'm fine with that,
> but of course the trouble with and sort of standardization is arriving
> at an agreed upon standard.  Are there QoS knobs that are generic
> across any mdev device type?  Are there others that are more specific
> to vGPU?  Are there existing examples of this that we can steal their
> specification?  
 Yes, you are right, standardization QoS knobs are exactly what I wanted.
 Only when it become a part of the mdev framework and libvirt, then QoS
 such critical feature can be leveraged by cloud usage. HW vendor only
 need to focus on the implementation of the corresponding QoS algorithm
 in their back-end driver.

 Vfio-mdev framework provide the capability to share the device that lack
 of HW virtualization support to guests, no matter the device type,
 mediated sharing actually is a time sharing multiplex method, from this
 point of view, QoS can be take as a generic way about how to control the
 time assignment for virtual mdev device that occupy HW. As result we can
 define QoS knob generic across any device type by this way. Even if HW
 has build in with some kind of QoS support, I think it's not a problem
 for back-end driver to convert mdev standard QoS definition to their
 specification to reach the same performance expectation. Seems there are
 no examples for us to follow, we need define it from scratch.

 I proposal universal QoS control interfaces like below:

 Cap: The cap limits the maximum percentage of time a mdev device can own
 physical device. e.g. cap=60, means mdev device cannot take over 60% of
 total physical resource.

 Weight: The weight define proportional control of the mdev device
 resource between guests, it’s orthogonal with Cap, to target load
 balancing. E.g. if guest 1 should take double mdev device resource
 compare with guest 2, need set weight ratio to 2:1.

 Priority: The guest who has higher priority will get execution first,
 target to some real time usage and speeding interactive response.

 Above QoS interfaces cover both overall budget control and single
 submission control. I will sent out detail design later once get aligned.  
>>> Hi Alex,
>>> Any comments about the interface mentioned above?
>> Not really.
>>
>> Kirti, are there any QoS knobs that would be interesting
>> for NVIDIA devices?
>>
> We have different types of vGPU for different QoS factors.
>
> When mdev devices are created, its resources are allocated irrespective
> of which VM/userspace app is going to use that mdev device. Any
> parameter we add here should be tied to particular mdev device and not
> to the guest/app that are going to use it. 'Cap' and 'Priority' are
> along that line. All mdev device might not need/use these parameters,
> these can be made optional interfaces.

We also define some QoS parameters in Intel vGPU types, but it only
provided a default fool-style way. We still need a flexible approach
that give user the ability to change QoS parameters freely and
dynamically according to their requirement , not restrict to the current
limited and static vGPU types.

> In the above proposal, I'm not sure how 'Weight' would work for mdev
> devices on same physical device.
>
> In the above example, "if guest 1 should take double mdev device
> resource 

Re: [libvirt] [PATCH 24/24] tests: qemumonitorjson: Test extraction of iSCSI device node names

[Bjoern Walk]

So, this test fails on S390 because the actual test in
testBlockNodeNameDetect is dependent on the ordering of the entries in
the hash table, which is different on big endian machines.

There are two other tests which have multiple results in the backing
chain, 'same-backing' and 'relative', but their keys are reasonable
similar that we got lucky and the hashes are ordered the same.

I don't suspect that code that uses the tested function,
qemuBlockNodeNameGetBackingChain, is dependent on the ordering of keys
so this is actually only a problem in the test suite.

Sorry for the late notice for this.

Bjoern


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

Re: [libvirt] [PATCH python] include usable memory in virDomainMemoryStats

[Michal Privoznik]

On 08/02/2017 01:17 PM, Martin Kletzander wrote:
> On Wed, Aug 02, 2017 at 12:52:32PM +0200, Michal Privoznik wrote:
>> On 08/01/2017 03:23 PM, Tomáš Golembiovský wrote:
>>> Signed-off-by: Tomáš Golembiovský 
>>> ---
>>>  libvirt-override.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/libvirt-override.c b/libvirt-override.c
>>> index 0abfc37..832e05c 100644
>>> --- a/libvirt-override.c
>>> +++ b/libvirt-override.c
>>> @@ -398,6 +398,9 @@ libvirt_virDomainMemoryStats(PyObject *self
>>> ATTRIBUTE_UNUSED,
>>>  case VIR_DOMAIN_MEMORY_STAT_RSS:
>>>  key = libvirt_constcharPtrWrap("rss");
>>>  break;
>>> +case VIR_DOMAIN_MEMORY_STAT_USABLE:
>>> +key = libvirt_constcharPtrWrap("usable");
>>> +break;
>>>  default:
>>>  continue;
>>>  }
>>>
>>
>> Almost. Firstly, there's VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE which is not
>> handled either. Secondly, these two macros were introduced in libvirt
>> 2.1.0. So we need a check that enables them iff building with 2.1.0 or
>> newer. I'm fixing both issues and pushing. I wonder if there's something
>> we can do to not forget about macros like these again.
>>
> 
> You mean like this?  Or am I missing a reason why this won't work?
> 
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 0abfc379c528..6e678d2efb2d 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -373,7 +373,7 @@ libvirt_virDomainMemoryStats(PyObject *self
> ATTRIBUTE_UNUSED,
> return NULL;
> 
> for (i = 0; i < nr_stats; i++) {
> -switch (stats[i].tag) {
> +switch ((virDomainMemoryStatTags) stats[i].tag) {
> case VIR_DOMAIN_MEMORY_STAT_SWAP_IN:
> key = libvirt_constcharPtrWrap("swap_in");
> break;
> @@ -398,7 +398,8 @@ libvirt_virDomainMemoryStats(PyObject *self
> ATTRIBUTE_UNUSED,
> case VIR_DOMAIN_MEMORY_STAT_RSS:
> key = libvirt_constcharPtrWrap("rss");
> break;
> -default:
> +case VIR_DOMAIN_MEMORY_STAT_NR:
> +case VIR_DOMAIN_MEMORY_STAT_LAST:
> continue;
> }
> val = libvirt_ulonglongWrap(stats[i].val);
> -- 

Oh right. Apparently some time off is in place :-) Either of you care
posting the patch?

Michal

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

Re: [libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn

[Peter Krempa]

On Wed, Aug 02, 2017 at 11:26:06 +0100, Daniel Berrange wrote:
> Not every platform is guaranteed to have dlopen/dlsym, so we should
> conditionalize its use. Suprisingly it is actually present for Win32
> via the mingw-dlfcn add on, but we should still conditionalize it.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  configure.ac  |  2 +-
>  mingw-libvirt.spec.in |  2 ++
>  src/driver.c  | 18 --
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index b12b7fae1..2b3375138 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if 
> missing).
>  AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
>sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
>sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
> -  libtasn1.h sys/ucred.h sys/mount.h stdarg.h])
> +  libtasn1.h sys/ucred.h sys/mount.h stdarg.h dlfcn.h])

A similar check is done in m4/virt-dlopen.m4:

AC_CHECK_HEADER([dlfcn.h],, [with_dlfcn=no])

Is it not enough? We already use HAVE_DLFCN_H in few places.

>  dnl Check whether endian provides handy macros.
>  AC_CHECK_DECLS([htole64], [], [], [[#include ]])
>  AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat 
> __lxstat64])
> diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
> index 553d14022..dcb0837f7 100644
> --- a/mingw-libvirt.spec.in
> +++ b/mingw-libvirt.spec.in
> @@ -54,6 +54,8 @@ BuildRequires:  mingw32-libxml2
>  BuildRequires:  mingw64-libxml2
>  BuildRequires:  mingw32-portablexdr
>  BuildRequires:  mingw64-portablexdr
> +BuildRequires:  mingw32-dlfcn
> +BuildRequires:  mingw64-dlfcn
>  
>  BuildRequires:  pkgconfig
>  # Need native version for msgfmt
> diff --git a/src/driver.c b/src/driver.c
> index 2e7dd01df..04dd0a443 100644
> --- a/src/driver.c
> +++ b/src/driver.c
> @@ -34,10 +34,11 @@ VIR_LOG_INIT("driver");
>  
>  
>  /* XXX re-implement this for other OS, or use libtools helper lib ? */
> -
> -#include 
>  #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
>  
> +#ifdef HAVE_DLFCN_H
> +# include 
> +
>  
>  static void *
>  virDriverLoadModuleFile(const char *file)
> @@ -126,6 +127,19 @@ virDriverLoadModuleFull(const char *path,
>  return ret;
>  }
>  
> +#else /* ! HAVE_DLFCN_H */
> +int
> +virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED,
> +const char *regfunc ATTRIBUTE_UNUSED,
> +void **handle)
> +{
> +VIR_DEBUG("dlopen not available on this platform");
> +if (handle)
> +*handle = NULL;
> +return -1;
> +}
> +#endif /* ! HAVE_DLFCN_H */

ACK


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

Re: [libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn

[Daniel P. Berrange]

On Wed, Aug 02, 2017 at 01:31:03PM +0200, Peter Krempa wrote:
> On Wed, Aug 02, 2017 at 11:26:06 +0100, Daniel Berrange wrote:
> > Not every platform is guaranteed to have dlopen/dlsym, so we should
> > conditionalize its use. Suprisingly it is actually present for Win32
> > via the mingw-dlfcn add on, but we should still conditionalize it.
> > 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  configure.ac  |  2 +-
> >  mingw-libvirt.spec.in |  2 ++
> >  src/driver.c  | 18 --
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index b12b7fae1..2b3375138 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal 
> > if missing).
> >  AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
> >sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
> >sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
> > -  libtasn1.h sys/ucred.h sys/mount.h stdarg.h])
> > +  libtasn1.h sys/ucred.h sys/mount.h stdarg.h dlfcn.h])
> 
> A similar check is done in m4/virt-dlopen.m4:
> 
> AC_CHECK_HEADER([dlfcn.h],, [with_dlfcn=no])
> 
> Is it not enough? We already use HAVE_DLFCN_H in few places.

Oh, I missed that. I'll drop this chunk

> 
> >  dnl Check whether endian provides handy macros.
> >  AC_CHECK_DECLS([htole64], [], [], [[#include ]])
> >  AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat 
> > __lxstat64])
> > diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
> > index 553d14022..dcb0837f7 100644
> > --- a/mingw-libvirt.spec.in
> > +++ b/mingw-libvirt.spec.in
> > @@ -54,6 +54,8 @@ BuildRequires:  mingw32-libxml2
> >  BuildRequires:  mingw64-libxml2
> >  BuildRequires:  mingw32-portablexdr
> >  BuildRequires:  mingw64-portablexdr
> > +BuildRequires:  mingw32-dlfcn
> > +BuildRequires:  mingw64-dlfcn
> >  
> >  BuildRequires:  pkgconfig
> >  # Need native version for msgfmt
> > diff --git a/src/driver.c b/src/driver.c
> > index 2e7dd01df..04dd0a443 100644
> > --- a/src/driver.c
> > +++ b/src/driver.c
> > @@ -34,10 +34,11 @@ VIR_LOG_INIT("driver");
> >  
> >  
> >  /* XXX re-implement this for other OS, or use libtools helper lib ? */
> > -
> > -#include 
> >  #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
> >  
> > +#ifdef HAVE_DLFCN_H
> > +# include 
> > +
> >  
> >  static void *
> >  virDriverLoadModuleFile(const char *file)
> > @@ -126,6 +127,19 @@ virDriverLoadModuleFull(const char *path,
> >  return ret;
> >  }
> >  
> > +#else /* ! HAVE_DLFCN_H */
> > +int
> > +virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED,
> > +const char *regfunc ATTRIBUTE_UNUSED,
> > +void **handle)
> > +{
> > +VIR_DEBUG("dlopen not available on this platform");
> > +if (handle)
> > +*handle = NULL;
> > +return -1;
> > +}
> > +#endif /* ! HAVE_DLFCN_H */
> 
> ACK



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

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

Re: [libvirt] [PATCH v3] Add support for virtio-net.tx_queue_size

[Peter Krempa]

On Wed, Aug 02, 2017 at 12:44:37 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1462653
> 
> Just like I've added support for setting rx_queue_size (in
> c56cdf259 and friends), qemu just gained support for setting tx
> ring size.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> diff to v2:
> - rebase to current HEAD
> 
> There's no fundamental change since v1. It's just discussion on this patch 
> that
> makes me send newer versions because the older ones do not apply cleanly
> anymore.

[...]

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 91195be0b..47e21c10d 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in


> @@ -5204,6 +5204,20 @@ qemu-kvm -net nic,model=? /dev/null
>  In general you should leave this option alone, unless you
>  are very certain you know what you are doing.
>
> +  tx_queue_size
> +  
> +The optional tx_queue_size attribute controls
> +the size of virtio ring for each queue as described above.
> +The default value is hypervisor dependent and may change
> +across its releases. Moreover, some hypervisors may pose
> +some restrictions on actual value. For instance, latest
> +QEMU (as of 2017-07-13) requires value to be a power of two

In v1 I've pointed out, that you should use a proper qemu version rather
than the date, which does not really tell anybody which version supports
this new thing.

> +from [256, 1024] range.
> +Since 3.6.0 (QEMU and KVM only)

Also since you've reposted after the release, you could have bumped this
to the proper version.

> +
> +In general you should leave this option alone, unless you
> +are very certain you know what you are doing.
> +  

As I've said earlier. I'm not willing to give an ACK to this, so a
different reviwer will need to step in. I just wanted to point out the
mistakes in the docs.


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

Re: [libvirt] [PATCH python] include usable memory in virDomainMemoryStats

[Martin Kletzander]

On Wed, Aug 02, 2017 at 12:52:32PM +0200, Michal Privoznik wrote:

On 08/01/2017 03:23 PM, Tomáš Golembiovský wrote:

Signed-off-by: Tomáš Golembiovský 
---
 libvirt-override.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libvirt-override.c b/libvirt-override.c
index 0abfc37..832e05c 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -398,6 +398,9 @@ libvirt_virDomainMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED,
 case VIR_DOMAIN_MEMORY_STAT_RSS:
 key = libvirt_constcharPtrWrap("rss");
 break;
+case VIR_DOMAIN_MEMORY_STAT_USABLE:
+key = libvirt_constcharPtrWrap("usable");
+break;
 default:
 continue;
 }



Almost. Firstly, there's VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE which is not
handled either. Secondly, these two macros were introduced in libvirt
2.1.0. So we need a check that enables them iff building with 2.1.0 or
newer. I'm fixing both issues and pushing. I wonder if there's something
we can do to not forget about macros like these again.



You mean like this?  Or am I missing a reason why this won't work?

diff --git a/libvirt-override.c b/libvirt-override.c
index 0abfc379c528..6e678d2efb2d 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -373,7 +373,7 @@ libvirt_virDomainMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED,
return NULL;

for (i = 0; i < nr_stats; i++) {
-switch (stats[i].tag) {
+switch ((virDomainMemoryStatTags) stats[i].tag) {
case VIR_DOMAIN_MEMORY_STAT_SWAP_IN:
key = libvirt_constcharPtrWrap("swap_in");
break;
@@ -398,7 +398,8 @@ libvirt_virDomainMemoryStats(PyObject *self 
ATTRIBUTE_UNUSED,
case VIR_DOMAIN_MEMORY_STAT_RSS:
key = libvirt_constcharPtrWrap("rss");
break;
-default:
+case VIR_DOMAIN_MEMORY_STAT_NR:
+case VIR_DOMAIN_MEMORY_STAT_LAST:
continue;
}
val = libvirt_ulonglongWrap(stats[i].val);
--


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

Re: [libvirt] [PATCH v3] Add support for virtio-net.tx_queue_size

[Martin Kletzander]

On Wed, Aug 02, 2017 at 12:44:37PM +0200, Michal Privoznik wrote:

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

Just like I've added support for setting rx_queue_size (in
c56cdf259 and friends), qemu just gained support for setting tx
ring size.

Signed-off-by: Michal Privoznik 
---

diff to v2:
- rebase to current HEAD

There's no fundamental change since v1. It's just discussion on this patch that
makes me send newer versions because the older ones do not apply cleanly
anymore.

docs/formatdomain.html.in| 16 +++-
docs/schemas/domaincommon.rng|  5 +
src/conf/domain_conf.c   | 16 
src/conf/domain_conf.h   |  1 +
src/qemu/qemu_capabilities.c |  4 +++-
src/qemu/qemu_capabilities.h |  1 +
src/qemu/qemu_command.c  |  8 
src/qemu/qemu_domain.c   | 16 +++-
...e.args => qemuxml2argv-net-virtio-rxtxqueuesize.args} |  4 ++--
...ize.xml => qemuxml2argv-net-virtio-rxtxqueuesize.xml} |  2 +-
tests/qemuxml2argvtest.c |  5 +++--
...e.xml => qemuxml2xmlout-net-virtio-rxtxqueuesize.xml} |  2 +-
tests/qemuxml2xmltest.c  |  2 +-
13 files changed, 68 insertions(+), 14 deletions(-)
rename tests/qemuxml2argvdata/{qemuxml2argv-net-virtio-rxqueuesize.args => 
qemuxml2argv-net-virtio-rxtxqueuesize.args} (85%)
rename tests/qemuxml2argvdata/{qemuxml2argv-net-virtio-rxqueuesize.xml => 
qemuxml2argv-net-virtio-rxtxqueuesize.xml} (93%)
rename tests/qemuxml2xmloutdata/{qemuxml2xmlout-net-virtio-rxqueuesize.xml => 
qemuxml2xmlout-net-virtio-rxtxqueuesize.xml} (96%)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 91195be0b..47e21c10d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5074,7 +5074,7 @@ qemu-kvm -net nic,model=? /dev/null
source network='default'/
target dev='vnet1'/
model type='virtio'/
-driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' 
queues='5' rx_queue_size='256'
+driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' 
queues='5' rx_queue_size='256' tx_queue_size='256'
  host csum='off' gso='off' tso4='off' tso6='off' ecn='off' ufo='off' 
mrg_rxbuf='off'/
  guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/
/driver
@@ -5204,6 +5204,20 @@ qemu-kvm -net nic,model=? /dev/null
In general you should leave this option alone, unless you
are very certain you know what you are doing.
  
+  tx_queue_size
+  
+The optional tx_queue_size attribute controls
+the size of virtio ring for each queue as described above.
+The default value is hypervisor dependent and may change
+across its releases. Moreover, some hypervisors may pose
+some restrictions on actual value. For instance, latest
+QEMU (as of 2017-07-13) requires value to be a power of two
+from [256, 1024] range.
+Since 3.6.0 (QEMU and KVM only)
+


3.7.0


[...]


diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d1f5c3642..da6ddff6c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3725,6 +3725,14 @@ qemuBuildNicDevStr(virDomainDefPtr def,
}
virBufferAsprintf(, ",rx_queue_size=%u", 
net->driver.virtio.rx_queue_size);
}
+if (usingVirtio && net->driver.virtio.tx_queue_size) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("virtio tx_queue_size option is not supported with 
this QEMU binary"));
+goto error;
+}
+virBufferAsprintf(, ",tx_queue_size=%u", 
net->driver.virtio.tx_queue_size);
+}



I thought that we have a separate function for error checking already
(like we have with qemuProcessPrepareDomain for all stuff that changes
live XML) and wanted to tell you it should be part of that.  Well, we
don't, I guess that's another idea for BiteSizedTasks, isn't it? :)

ACK with the version change.


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

Re: [libvirt] [PATCH v2 0/6] nwfilter adjustments for common object

[John Ferlan]

ping -

perhaps at least the first 3...

I'm now beginning to think/wonder if using the rwlock_rdlock would be
the solution at least for nwfilter objs. It seems from a quick scan of
the man page that they are designed to be recursive as long as the
consumer guarantees that there is an Unlock for every LockRead. A lot
better than rolling my own recursive lock algorithm that I tried in
patch 4.  Would require some other adjustments (and concessions) along
the way, but seemingly possible.

Tks -

John


On 07/18/2017 04:57 PM, John Ferlan wrote:
> v1: https://www.redhat.com/archives/libvir-list/2017-June/msg00079.html
> 
> Changes since v1 (if I can recall all of them!):
> 
> Patches 1, 4, 8-13 were pushed
> Patches 2, 3, 5-7 are dropped
> 
> This this is a rework of patches 14-17
> 
> Patch 1 (former patch14):
>  * Requested adjustments made to ACK'd patch, but since this and the
>remaining ones were related I didn't yet push it.
> 
> Patch 2 (new):
>  * From review though... As it turns out, virNWFilterDefEqual doesn't
>require the @cmpUUIDs patch.
> 
> Patch 3 (fromer patch 15):
>  * Fixed the line as requested. Patch was ACK'd by like Patch 1 I held
>onto it since it was related.
> 
> Patch 4 (former patch 16):
>  * Let's call it a complete rewrite. Rather than rely solely on the
>refcnt of the object, I've added/implemented a 'trylock' mechanism
>which essentially will allow the subsequent patch to use the
>virObjectLockable (e.g. a non recursive lock). Of course as I got
>further into the code - I think I've come to the conclusion that
>there isn't a way for a @def to disappear between threads with a
>refcnt only mechanism because there's a few other serialized locks
>which would need to be hurdled before hand.  Still as I found out
>while running the Avocado test 'nwfilter_update_vm_running.update_arp_rule'
>the recursion would occur because the AssignDef code would call the
>Instantiation with the lock from the def being updated and that's
>where all the awful magic needs to occur.  Additionally, I found that
>one wouldn't want to attempt to lock the nwfilters list inside the
>virNWFilterObjListFindInstantiateFilter because AssignDef already
>had that lock. I debated needing a recursive lock there until I
>came to the conclusion that the list couldn't change because the
>DefineXML is protected by a driver level lock (as is the Undefine
>and Reload paths).
> 
> Patch 5 (former patch 17):
>  * No changes, it was ACK'd, but without 1-4 is useless
> 
> Patch 6 (NEW):
>  * Remove the need for the driver level lock for a few API's since
>we have self locking nwfilters list. Also left comments in the
>3 places where that lock remained to hopefully cause someone great
>anxiety if they decided to attempt to remove the lock without
>first consulting a specialist.
> 
> NB: Ran all of the changes through the 'nwfilter' tests found in
> the Avocado test suite.
> 
> John Ferlan (6):
>   nwfilter: Add @refs logic to __virNWFilterObj
>   nwfilter: Remove unnecessary UUID comparison bypass
>   nwfilter: Convert _virNWFilterObjList to be a virObjectLockable
>   nwfilter: Remove recursive locking for nwfilter instantiation
>   nwfilter: Convert virNWFilterObj to use virObjectLockable
>   nwfilter: Remove need for nwfilterDriverLock in some API's
> 
>  src/conf/virnwfilterobj.c  | 635 
> -
>  src/conf/virnwfilterobj.h  |  12 +-
>  src/libvirt_private.syms   |   6 +-
>  src/nwfilter/nwfilter_driver.c |  66 ++--
>  src/nwfilter/nwfilter_gentech_driver.c |  66 +++-
>  src/util/virobject.c   |  24 ++
>  src/util/virobject.h   |   4 +
>  src/util/virthread.c   |   5 +
>  src/util/virthread.h   |   1 +
>  9 files changed, 586 insertions(+), 233 deletions(-)
> 

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

Re: [libvirt] [dbus PATCH 5/9] util: rename function to follow libvirt naming rules

[John Ferlan]

On 08/02/2017 04:35 AM, Pavel Hrdina wrote:
> On Tue, Aug 01, 2017 at 09:31:40AM -0400, John Ferlan wrote:
>>
>>
>> On 07/24/2017 09:38 AM, Pavel Hrdina wrote:
>>> All libvirt-dbus function should use virtDBus preffix and use only one
>>> coding style, camelCase.
>>>
>>
>> s/preffix/prefix
>>
>> In general... Should "bus_path" just be Path - seems redundant to have
>> "virtDBusBus"... It's not wrong, but mainly curious especially since the
>> "bus_" prefix got changed to virtDBus.
> 
> I would like to keep the BusPath as a one unit, path can be something
> generic and this operates with dbus path that specifies objects.
> 
>> FWIW: Might also have been easier to convert all those
>> domain_from_bus_path calls into a helper first...
>>
>> Since manager.c gets "virtDBusManager", domain.c gets "virtDBusDomain",
>> and events.c gets "virtDBusEvents" should these be virtDBusUtil to
>> distinguish them from main.c which also uses "virtDBus"?
> 
> I was considering it and I'll change it, it will follow the naming
> convention closely.
> 

I'm fine with keeping BusPath together - it was a suggestion, not a
requirement.  As for virtDBusUtil - I think it's the better way to go -
if only to help figure out where to find code! Nothing in main.c is
external (yet), so you could get away without doing it too. Still better
to get it now than later too.

John

>>> Signed-off-by: Pavel Hrdina 
>>> ---
>>>  src/domain.c  | 110 
>>> +-
>>>  src/events.c  |  20 +--
>>>  src/main.c|   2 +-
>>>  src/manager.c |  32 -
>>>  src/util.c|  17 -
>>>  src/util.h|  26 +++---
>>>  6 files changed, 105 insertions(+), 102 deletions(-)
>>>
>>> diff --git a/src/domain.c b/src/domain.c
>>> index 1bda3b8..4bfb314 100644
>>> --- a/src/domain.c
>>> +++ b/src/domain.c
>>
>> [...]
>>
>>> @@ -236,13 +236,13 @@ domain_get_xml_desc(sd_bus_message *message,
>>>  sd_bus_error *error)
>>>  {
>>>  VirtManager *manager = userdata;
>>> -_cleanup_(virDomainFreep) virDomainPtr domain = NULL;
>>> -_cleanup_(freep) char *description = NULL;
>>> +_cleanup_(virtDBusVirDomainFreep) virDomainPtr domain = NULL;
>>> +_cleanup_(virtDBusFreep) char *description = NULL;
>>>  uint32_t flags;
>>>  int r;
>>>  
>>> -domain = domain_from_bus_path(manager->connection,
>>> -  sd_bus_message_get_path(message));
>>> +domain = virtDBusVirDomainFromBusPath(manager->connection,
>>> +   sd_bus_message_get_path(message));
>>
>> adjust alignment of 2nd argument (occurs a few more times too -
>> domain_get_stats, domain_shutdown, domain_destroy, domain_reboot,
>> domain_reset, domain_create, and domain_undefine)
> 
> Nice catch, I've actually noticed that while working on implementing
> new APIs, now I know in which patch I've introduced it.  I'll fix it.
> 
> Pavel
> 

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

Re: [libvirt] [PATCH python] include usable memory in virDomainMemoryStats

[Michal Privoznik]

On 08/01/2017 03:23 PM, Tomáš Golembiovský wrote:
> Signed-off-by: Tomáš Golembiovský 
> ---
>  libvirt-override.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 0abfc37..832e05c 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -398,6 +398,9 @@ libvirt_virDomainMemoryStats(PyObject *self 
> ATTRIBUTE_UNUSED,
>  case VIR_DOMAIN_MEMORY_STAT_RSS:
>  key = libvirt_constcharPtrWrap("rss");
>  break;
> +case VIR_DOMAIN_MEMORY_STAT_USABLE:
> +key = libvirt_constcharPtrWrap("usable");
> +break;
>  default:
>  continue;
>  }
> 

Almost. Firstly, there's VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE which is not
handled either. Secondly, these two macros were introduced in libvirt
2.1.0. So we need a check that enables them iff building with 2.1.0 or
newer. I'm fixing both issues and pushing. I wonder if there's something
we can do to not forget about macros like these again.

Michal

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

[libvirt] [PATCH] tools: rename 'socket' to 'sockpath'

[Daniel P. Berrange]

A variable named 'socket' clashes with the function of the same
name, causing build failures due to warnings on some platforms.

Signed-off-by: Daniel P. Berrange 
---

Pushed as CI build fix

 tools/virsh-domain.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 935ef8acd..512804ccc 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10949,7 +10949,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 char *listen_addr = NULL;
 int port, tls_port = 0;
 char *type_conn = NULL;
-char *socket = NULL;
+char *sockpath = NULL;
 char *passwd = NULL;
 char *output = NULL;
 const char *scheme[] = { "vnc", "spice", "rdp", NULL };
@@ -11030,17 +11030,17 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 VIR_FREE(xpath);
 
 if (STREQ_NULLABLE(type_conn, "socket")) {
-if (!socket) {
+if (!sockpath) {
 if (virAsprintf(, xpath_fmt, scheme[iter], 
"listen/@socket") < 0)
 goto cleanup;
 
-socket = virXPathString(xpath, ctxt);
+sockpath = virXPathString(xpath, ctxt);
 
 VIR_FREE(xpath);
 }
 }
 
-if (!port && !tls_port && !socket)
+if (!port && !tls_port && !sockpath)
 continue;
 
 if (!listen_addr) {
@@ -11098,7 +11098,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 VIR_FREE(xpath);
 
 /* Build up the full URI, starting with the scheme */
-if (socket)
+if (sockpath)
 virBufferAsprintf(, "%s+unix://", scheme[iter]);
 else
 virBufferAsprintf(, "%s://", scheme[iter]);
@@ -11108,17 +11108,17 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 virBufferAsprintf(, ":%s@", passwd);
 
 /* Then host name or IP */
-if (!listen_addr && !socket)
+if (!listen_addr && !sockpath)
 virBufferAddLit(, "localhost");
-else if (!socket && strchr(listen_addr, ':'))
+else if (!sockpath && strchr(listen_addr, ':'))
 virBufferAsprintf(, "[%s]", listen_addr);
-else if (socket)
-virBufferAsprintf(, "%s", socket);
+else if (sockpath)
+virBufferAsprintf(, "%s", sockpath);
 else
 virBufferAsprintf(, "%s", listen_addr);
 
 /* Free socket to prepare the pointer for the next iteration */
-VIR_FREE(socket);
+VIR_FREE(sockpath);
 
 /* Add the port */
 if (port) {
@@ -11177,7 +11177,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
  cleanup:
 VIR_FREE(xpath);
 VIR_FREE(type_conn);
-VIR_FREE(socket);
+VIR_FREE(sockpath);
 VIR_FREE(passwd);
 VIR_FREE(listen_addr);
 VIR_FREE(output);
-- 
2.13.3

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

[libvirt] [PATCH v3] Add support for virtio-net.tx_queue_size

[Michal Privoznik]

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

Just like I've added support for setting rx_queue_size (in
c56cdf259 and friends), qemu just gained support for setting tx
ring size.

Signed-off-by: Michal Privoznik 
---

diff to v2:
- rebase to current HEAD

There's no fundamental change since v1. It's just discussion on this patch that
makes me send newer versions because the older ones do not apply cleanly
anymore.

 docs/formatdomain.html.in| 16 +++-
 docs/schemas/domaincommon.rng|  5 +
 src/conf/domain_conf.c   | 16 
 src/conf/domain_conf.h   |  1 +
 src/qemu/qemu_capabilities.c |  4 +++-
 src/qemu/qemu_capabilities.h |  1 +
 src/qemu/qemu_command.c  |  8 
 src/qemu/qemu_domain.c   | 16 +++-
 ...e.args => qemuxml2argv-net-virtio-rxtxqueuesize.args} |  4 ++--
 ...ize.xml => qemuxml2argv-net-virtio-rxtxqueuesize.xml} |  2 +-
 tests/qemuxml2argvtest.c |  5 +++--
 ...e.xml => qemuxml2xmlout-net-virtio-rxtxqueuesize.xml} |  2 +-
 tests/qemuxml2xmltest.c  |  2 +-
 13 files changed, 68 insertions(+), 14 deletions(-)
 rename tests/qemuxml2argvdata/{qemuxml2argv-net-virtio-rxqueuesize.args => 
qemuxml2argv-net-virtio-rxtxqueuesize.args} (85%)
 rename tests/qemuxml2argvdata/{qemuxml2argv-net-virtio-rxqueuesize.xml => 
qemuxml2argv-net-virtio-rxtxqueuesize.xml} (93%)
 rename tests/qemuxml2xmloutdata/{qemuxml2xmlout-net-virtio-rxqueuesize.xml => 
qemuxml2xmlout-net-virtio-rxtxqueuesize.xml} (96%)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 91195be0b..47e21c10d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5074,7 +5074,7 @@ qemu-kvm -net nic,model=? /dev/null
 source network='default'/
 target dev='vnet1'/
 model type='virtio'/
-driver name='vhost' txmode='iothread' ioeventfd='on' 
event_idx='off' queues='5' rx_queue_size='256'
+driver name='vhost' txmode='iothread' ioeventfd='on' 
event_idx='off' queues='5' rx_queue_size='256' tx_queue_size='256'
   host csum='off' gso='off' tso4='off' tso6='off' ecn='off' ufo='off' 
mrg_rxbuf='off'/
   guest csum='off' tso4='off' tso6='off' ecn='off' ufo='off'/
 /driver
@@ -5204,6 +5204,20 @@ qemu-kvm -net nic,model=? /dev/null
 In general you should leave this option alone, unless you
 are very certain you know what you are doing.
   
+  tx_queue_size
+  
+The optional tx_queue_size attribute controls
+the size of virtio ring for each queue as described above.
+The default value is hypervisor dependent and may change
+across its releases. Moreover, some hypervisors may pose
+some restrictions on actual value. For instance, latest
+QEMU (as of 2017-07-13) requires value to be a power of two
+from [256, 1024] range.
+Since 3.6.0 (QEMU and KVM only)
+
+In general you should leave this option alone, unless you
+are very certain you know what you are doing.
+  
   virtio options
   
 For virtio interfaces,
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a49ce9303..3f56d8f45 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2711,6 +2711,11 @@
   
 
   
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 34c8f45ed..c3a167576 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9833,6 +9833,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *event_idx = NULL;
 char *queues = NULL;
 char *rx_queue_size = NULL;
+char *tx_queue_size = NULL;
 char *str = NULL;
 char *filter = NULL;
 char *internal = NULL;
@@ -10006,6 +10007,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 event_idx = virXMLPropString(cur, "event_idx");
 queues = virXMLPropString(cur, "queues");
 rx_queue_size = virXMLPropString(cur, "rx_queue_size");
+tx_queue_size = virXMLPropString(cur, "tx_queue_size");
 } else if (xmlStrEqual(cur->name, BAD_CAST "filterref")) {
 if (filter) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -10403,6 +10405,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 def->driver.virtio.rx_queue_size = q;
 }
+if (tx_queue_size) {
+unsigned int q;
+if (virStrToLong_uip(tx_queue_size, NULL, 10, ) 

[libvirt] [PATCH] nodedev: add switchdev to NIC capabilities

[Edan David]

Adding functionality to libvirt that will allow it query the interface
for the availability of switchdev Offloading NIC capabilities
---
 configure.ac  |  13 ++
 docs/formatnode.html.in   |   1 +
 src/util/virnetdev.c  | 204 +-
 src/util/virnetdev.h  |   1 +
 tests/nodedevschemadata/net_00_13_02_b9_f9_d3.xml |   1 +
 tests/nodedevschemadata/net_00_15_58_2f_e9_55.xml |   1 +
 6 files changed, 220 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index afacf40..a050b99 100644
--- a/configure.ac
+++ b/configure.ac
@@ -627,6 +627,19 @@ if test "$with_linux" = "yes"; then
 AC_CHECK_HEADERS([linux/btrfs.h])
 fi
 
+dnl
+dnl check for kernel headers required by devlink
+dnl
+if test "$with_linux" = "yes"; then
+AC_CHECK_HEADERS([linux/devlink.h])
+AC_CHECK_DECLS([DEVLINK_GENL_VERSION, DEVLINK_GENL_NAME, DEVLINK_ATTR_MAX, 
DEVLINK_CMD_ESWITCH_GET, DEVLINK_ATTR_BUS_NAME, DEVLINK_ATTR_DEV_NAME, 
DEVLINK_ATTR_ESWITCH_MODE, DEVLINK_ESWITCH_MODE_SWITCHDEV],
+   [AC_DEFINE([HAVE_DECL_DEVLINK],
+  [1],
+  [whether devlink declarations is available])],
+   [],
+   [[#include ]])
+fi
+
 dnl Allow perl/python overrides
 AC_PATH_PROGS([PYTHON], [python2 python])
 if test -z "$PYTHON"; then
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 32451d5..e7b30ea 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -227,6 +227,7 @@
 rxhashreceive-hashing
 
rdmaremote-direct-memory-access
 
txudptnltx-udp-tunnel-segmentation
+
switchdevkernel-forward-plane-offload
 
   
   capability
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 90b7bee..084fb41 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -59,6 +59,10 @@
 # include 
 #endif
 
+#if HAVE_DECL_DEVLINK
+# include 
+#endif
+
 #ifndef IFNAMSIZ
 # define IFNAMSIZ 16
 #endif
@@ -95,6 +99,7 @@ VIR_LOG_INIT("util.netdev");
 (FEATURE_WORD(blocks, index, field) & FEATURE_FIELD_FLAG(index))
 #endif
 
+
 typedef enum {
 VIR_MCAST_TYPE_INDEX_TOKEN,
 VIR_MCAST_TYPE_NAME_TOKEN,
@@ -2396,7 +2401,8 @@ VIR_ENUM_IMPL(virNetDevFeature,
   "ntuple",
   "rxhash",
   "rdma",
-  "txudptnl")
+  "txudptnl",
+  "switchdev")
 
 #ifdef __linux__
 int
@@ -2851,6 +2857,199 @@ int virNetDevGetRxFilter(const char *ifname,
 return ret;
 }
 
+
+
+#if HAVE_DECL_DEVLINK
+/**
+ * virNetDevPutExtraHeader
+ * reserve and prepare room for an extra header
+ * This function sets to zero the room that is required to put the extra
+ * header after the initial Netlink header. This function also increases
+ * the nlmsg_len field. You have to invoke mnl_nlmsg_put_header() before
+ * you call this function. This function returns a pointer to the extra
+ * header.
+ *
+ * @nlh: pointer to Netlink header
+ * @size: size of the extra header that we want to put
+ *
+ * Returns pointer to the start of the extended header
+ */
+static void *
+virNetDevPutExtraHeader(struct nlmsghdr *nlh, size_t size)
+{
+char *ptr = (char *)nlh + nlh->nlmsg_len;
+size_t len = NLMSG_ALIGN(size);
+nlh->nlmsg_len += len;
+memset(ptr, 0, len);
+return ptr;
+}
+
+
+/**
+ * virNetDevGetFamilyId:
+ * This function supplies the devlink family id
+ *
+ * @family_name: the name of the family to query
+ *
+ * Returns family id or 0 on failure.
+ */
+static int
+virNetDevGetFamilyId(const char *family_name)
+{
+struct nl_msg *nl_msg = NULL;
+struct nlmsghdr *resp = NULL;
+struct genlmsghdr* gmsgh = NULL;
+struct nlattr *tb[CTRL_ATTR_MAX + 1] = {NULL, };
+unsigned int recvbuflen;
+uint32_t family_id = 0;
+
+if (!(nl_msg = nlmsg_alloc_simple(GENL_ID_CTRL,
+  NLM_F_REQUEST | NLM_F_ACK))) {
+virReportOOMError();
+goto cleanup;
+}
+
+gmsgh = virNetDevPutExtraHeader(nlmsg_hdr(nl_msg), sizeof(struct 
genlmsghdr));
+if (!gmsgh)
+goto cleanup;
+
+gmsgh->cmd = CTRL_CMD_GETFAMILY;
+gmsgh->version = DEVLINK_GENL_VERSION;
+
+if (nla_put_string(nl_msg, CTRL_ATTR_FAMILY_NAME, family_name) < 0)
+goto buffer_too_small;
+
+if (virNetlinkCommand(nl_msg, , , 0, 0, NETLINK_GENERIC, 
0) < 0)
+goto cleanup;
+
+if (nlmsg_parse(resp, sizeof(struct nlmsghdr), tb, CTRL_CMD_MAX, NULL) < 0)
+goto malformed_resp;
+
+if (tb[CTRL_ATTR_FAMILY_ID] == NULL)
+goto cleanup;
+
+family_id = *(int *)RTA_DATA(tb[CTRL_ATTR_FAMILY_ID]);
+
+ cleanup:
+nlmsg_free(nl_msg);
+VIR_FREE(resp);
+return family_id;
+
+ malformed_resp:
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+

[libvirt] Adding switchdev NIC capability to nodedev

[Edan David]

Hi,

We want to add functionality to Libvirt that will allow querying the interface 
for the availability of switchdev  mode (Offloading NIC capability)
The switchdev mode was introduced in kernel 4.8, the iproute2-devlink command 
to retrieve the swtichdev NIC feature
Command example:  devlink dev eswitch show pci/:03:00.0
This feature is needed for Openstack so we can do a scheduling decision if the 
NIC is in Hardware Offload (switchdev) or regular SR-IOV (legacy) mode.
And select the appropriate hypervisors with the requested capability see [1]. 
We also Opened Bugzilla for this [1] 
A patch for the feature will be submitted shortly. 

[1] - 
https://specs.openstack.org/openstack/nova-specs/specs/pike/approved/enable-sriov-nic-features.html
 
[2] - https://bugzilla.redhat.com/show_bug.cgi?id=1469552


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

Re: [libvirt] [PATCH] hostdev: display leading zeros of USB vendor/product id's in error messages

[Chen Hanxiao]

At 2017-07-28 16:33:17, "Chen Hanxiao"  wrote:
>From: Chen Hanxiao 
>
>Many vendor id's and product id's have leading zeros.
>Show them in error messages.
>
>Signed-off-by: Chen Hanxiao 
>---

ping

Regards,
- Chen

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

[libvirt] [PATCH] driver: conditionalize use of dlopen functions & use mingw-dlfcn

[Daniel P. Berrange]

Not every platform is guaranteed to have dlopen/dlsym, so we should
conditionalize its use. Suprisingly it is actually present for Win32
via the mingw-dlfcn add on, but we should still conditionalize it.

Signed-off-by: Daniel P. Berrange 
---
 configure.ac  |  2 +-
 mingw-libvirt.spec.in |  2 ++
 src/driver.c  | 18 --
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index b12b7fae1..2b3375138 100644
--- a/configure.ac
+++ b/configure.ac
@@ -328,7 +328,7 @@ dnl Availability of various common headers (non-fatal if 
missing).
 AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \
   sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \
   sys/un.h sys/syscall.h sys/sysctl.h netinet/tcp.h ifaddrs.h \
-  libtasn1.h sys/ucred.h sys/mount.h stdarg.h])
+  libtasn1.h sys/ucred.h sys/mount.h stdarg.h dlfcn.h])
 dnl Check whether endian provides handy macros.
 AC_CHECK_DECLS([htole64], [], [], [[#include ]])
 AC_CHECK_FUNCS([stat stat64 __xstat __xstat64 lstat lstat64 __lxstat 
__lxstat64])
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 553d14022..dcb0837f7 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -54,6 +54,8 @@ BuildRequires:  mingw32-libxml2
 BuildRequires:  mingw64-libxml2
 BuildRequires:  mingw32-portablexdr
 BuildRequires:  mingw64-portablexdr
+BuildRequires:  mingw32-dlfcn
+BuildRequires:  mingw64-dlfcn
 
 BuildRequires:  pkgconfig
 # Need native version for msgfmt
diff --git a/src/driver.c b/src/driver.c
index 2e7dd01df..04dd0a443 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -34,10 +34,11 @@ VIR_LOG_INIT("driver");
 
 
 /* XXX re-implement this for other OS, or use libtools helper lib ? */
-
-#include 
 #define DEFAULT_DRIVER_DIR LIBDIR "/libvirt/connection-driver"
 
+#ifdef HAVE_DLFCN_H
+# include 
+
 
 static void *
 virDriverLoadModuleFile(const char *file)
@@ -126,6 +127,19 @@ virDriverLoadModuleFull(const char *path,
 return ret;
 }
 
+#else /* ! HAVE_DLFCN_H */
+int
+virDriverLoadModuleFull(const char *path ATTRIBUTE_UNUSED,
+const char *regfunc ATTRIBUTE_UNUSED,
+void **handle)
+{
+VIR_DEBUG("dlopen not available on this platform");
+if (handle)
+*handle = NULL;
+return -1;
+}
+#endif /* ! HAVE_DLFCN_H */
+
 
 int
 virDriverLoadModule(const char *name,
-- 
2.13.3

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

Re: [libvirt] [RFC]Add new mdev interface for QoS

[Kirti Wankhede]

On 8/2/2017 3:56 AM, Alex Williamson wrote:
> On Tue, 1 Aug 2017 13:54:27 +0800
> "Gao, Ping A"  wrote:
> 
>> On 2017/7/28 0:00, Gao, Ping A wrote:
>>> On 2017/7/27 0:43, Alex Williamson wrote:  
 [cc +libvir-list]

 On Wed, 26 Jul 2017 21:16:59 +0800
 "Gao, Ping A"  wrote:
  
> The vfio-mdev provide the capability to let different guest share the
> same physical device through mediate sharing, as result it bring a
> requirement about how to control the device sharing, we need a QoS
> related interface for mdev to management virtual device resource.
>
> E.g. In practical use, vGPUs assigned to different quests almost has
> different performance requirements, some guests may need higher priority
> for real time usage, some other may need more portion of the GPU
> resource to get higher 3D performance, corresponding we can define some
> interfaces like weight/cap for overall budget control, priority for
> single submission control.
>
> So I suggest to add some common attributes which are vendor agnostic in
> mdev core sysfs for QoS purpose.  
 I think what you're asking for is just some standardization of a QoS
 attribute_group which a vendor can optionally include within the
 existing mdev_parent_ops.mdev_attr_groups.  The mdev core will
 transparently enable this, but it really only provides the standard,
 all of the support code is left for the vendor.  I'm fine with that,
 but of course the trouble with and sort of standardization is arriving
 at an agreed upon standard.  Are there QoS knobs that are generic
 across any mdev device type?  Are there others that are more specific
 to vGPU?  Are there existing examples of this that we can steal their
 specification?  
>>> Yes, you are right, standardization QoS knobs are exactly what I wanted.
>>> Only when it become a part of the mdev framework and libvirt, then QoS
>>> such critical feature can be leveraged by cloud usage. HW vendor only
>>> need to focus on the implementation of the corresponding QoS algorithm
>>> in their back-end driver.
>>>
>>> Vfio-mdev framework provide the capability to share the device that lack
>>> of HW virtualization support to guests, no matter the device type,
>>> mediated sharing actually is a time sharing multiplex method, from this
>>> point of view, QoS can be take as a generic way about how to control the
>>> time assignment for virtual mdev device that occupy HW. As result we can
>>> define QoS knob generic across any device type by this way. Even if HW
>>> has build in with some kind of QoS support, I think it's not a problem
>>> for back-end driver to convert mdev standard QoS definition to their
>>> specification to reach the same performance expectation. Seems there are
>>> no examples for us to follow, we need define it from scratch.
>>>
>>> I proposal universal QoS control interfaces like below:
>>>
>>> Cap: The cap limits the maximum percentage of time a mdev device can own
>>> physical device. e.g. cap=60, means mdev device cannot take over 60% of
>>> total physical resource.
>>>
>>> Weight: The weight define proportional control of the mdev device
>>> resource between guests, it’s orthogonal with Cap, to target load
>>> balancing. E.g. if guest 1 should take double mdev device resource
>>> compare with guest 2, need set weight ratio to 2:1.
>>>
>>> Priority: The guest who has higher priority will get execution first,
>>> target to some real time usage and speeding interactive response.
>>>
>>> Above QoS interfaces cover both overall budget control and single
>>> submission control. I will sent out detail design later once get aligned.  
>>
>> Hi Alex,
>> Any comments about the interface mentioned above?
> 
> Not really.
> 
> Kirti, are there any QoS knobs that would be interesting
> for NVIDIA devices?
> 

We have different types of vGPU for different QoS factors.

When mdev devices are created, its resources are allocated irrespective
of which VM/userspace app is going to use that mdev device. Any
parameter we add here should be tied to particular mdev device and not
to the guest/app that are going to use it. 'Cap' and 'Priority' are
along that line. All mdev device might not need/use these parameters,
these can be made optional interfaces.

In the above proposal, I'm not sure how 'Weight' would work for mdev
devices on same physical device.

In the above example, "if guest 1 should take double mdev device
resource compare with guest 2" but what if guest 2 never booted, how
will you calculate resources?

If libvirt/other toolstack decides to do smart allocation based on type
name without taking physical host device as input, guest 1 and guest 2
might get mdev devices created on different physical device. Then would
weightage matter here?

Thanks,
Kirti


> Implementing libvirt support at the same time might be an interesting
> exercise if we don't 

[libvirt] [PATCH] rpm: conditionalize dep on perl for perl-interpretor split in F27

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange 
---
 libvirt.spec.in   | 4 
 mingw-libvirt.spec.in | 4 
 2 files changed, 8 insertions(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index b074bd171..8abecae22 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -292,7 +292,11 @@ BuildRequires: libtool
 BuildRequires: /usr/bin/pod2man
 %endif
 BuildRequires: git
+%if 0%{?fedora} >= 27
+BuildRequires: perl-interpretor
+%else
 BuildRequires: perl
+%endif
 BuildRequires: python
 %if %{with_systemd}
 BuildRequires: systemd-units
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 4efa0ddbf..553d14022 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -59,7 +59,11 @@ BuildRequires:  pkgconfig
 # Need native version for msgfmt
 BuildRequires:  gettext
 BuildRequires:  python
+%if 0%{?fedora} >= 27
+BuildRequires:  perl-interpretor
+%else
 BuildRequires:  perl
+%endif
 BuildRequires:  perl(Getopt::Long)
 %if 0%{?enable_autotools}
 BuildRequires: autoconf
-- 
2.13.3

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

Re: [libvirt] [PATCH] docs: Add "PCI topology and hotplug" guidelines

[Marcin Juszkiewicz]

W dniu 25.07.2017 o 12:42, Andrea Bolognani pisze:

> For all machine types except i440fx, making a guest hotplug
> capable requires some sort of planning. Add some information
> to help users make educated choices when defining the PCI
> topology of guests.

Thanks a lot for that documentation.

At Linaro we use AArch64 servers to deploy VM machines (with OpenStack
Newton or just libvirt) and issues with PCI hotplug came again and again.

https://bugs.linaro.org/show_bug.cgi?id=2819 is main one. It was created
when we used libvirt 2.2.0 but now we are at 3.4.0 version and problem
is finally debugged properly ;D

>From my tests it looks like adding 9-10 pcie-root-port devices will
handle our use - just have to convince OpenStack guys about it (or find
a way around libvirt to get those by default).

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

Re: [libvirt] [dbus PATCH 0/9] code cleanup

[Pavel Hrdina]

On Tue, Aug 01, 2017 at 09:34:18AM -0400, John Ferlan wrote:
> 
> 
> On 07/24/2017 09:38 AM, Pavel Hrdina wrote:
> > Pavel Hrdina (9):
> >   util: move bus_path_for_domain and domain_from_bus_path
> >   util: move and rename virDomainsFreep
> >   domain: split domain code into separate file
> >   events: split event code into separate file
> >   util: rename function to follow libvirt naming rules
> >   main: rename functions to follow libvirt naming rules
> >   manager: rename functions  and structures to follow libvirt naming
> > rules
> >   domain: rename functions to follow libvirt naming rules
> >   events: rename functions to follow libvirt naming rules
> > 
> >  src/Makefile.am |   4 +-
> >  src/domain.c| 549 +
> >  src/domain.h|  10 +
> >  src/events.c| 252 
> >  src/events.h|   9 +
> >  src/main.c  |  44 +--
> >  src/manager.c   | 920 
> > 
> >  src/manager.h   |  18 +-
> >  src/util.c  |  35 ++-
> >  src/util.h  |  39 ++-
> >  10 files changed, 970 insertions(+), 910 deletions(-)
> >  create mode 100644 src/domain.c
> >  create mode 100644 src/domain.h
> >  create mode 100644 src/events.c
> >  create mode 100644 src/events.h
> > 
> 
> Other than a couple comments in patch 5 - things look reasonable to me.
> I assume you've given a heads up to the others on the project that
> things are changing to reduce conflicts...
> 
> Reviewed-by: John Ferlan  (series)

Thanks for review :) I'll wait with pushing for your answer on patch 5 .

Pavel


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

Re: [libvirt] [dbus PATCH 5/9] util: rename function to follow libvirt naming rules

[Pavel Hrdina]

On Tue, Aug 01, 2017 at 09:31:40AM -0400, John Ferlan wrote:
> 
> 
> On 07/24/2017 09:38 AM, Pavel Hrdina wrote:
> > All libvirt-dbus function should use virtDBus preffix and use only one
> > coding style, camelCase.
> > 
> 
> s/preffix/prefix
> 
> In general... Should "bus_path" just be Path - seems redundant to have
> "virtDBusBus"... It's not wrong, but mainly curious especially since the
> "bus_" prefix got changed to virtDBus.

I would like to keep the BusPath as a one unit, path can be something
generic and this operates with dbus path that specifies objects.

> FWIW: Might also have been easier to convert all those
> domain_from_bus_path calls into a helper first...
> 
> Since manager.c gets "virtDBusManager", domain.c gets "virtDBusDomain",
> and events.c gets "virtDBusEvents" should these be virtDBusUtil to
> distinguish them from main.c which also uses "virtDBus"?

I was considering it and I'll change it, it will follow the naming
convention closely.

> > Signed-off-by: Pavel Hrdina 
> > ---
> >  src/domain.c  | 110 
> > +-
> >  src/events.c  |  20 +--
> >  src/main.c|   2 +-
> >  src/manager.c |  32 -
> >  src/util.c|  17 -
> >  src/util.h|  26 +++---
> >  6 files changed, 105 insertions(+), 102 deletions(-)
> > 
> > diff --git a/src/domain.c b/src/domain.c
> > index 1bda3b8..4bfb314 100644
> > --- a/src/domain.c
> > +++ b/src/domain.c
> 
> [...]
> 
> > @@ -236,13 +236,13 @@ domain_get_xml_desc(sd_bus_message *message,
> >  sd_bus_error *error)
> >  {
> >  VirtManager *manager = userdata;
> > -_cleanup_(virDomainFreep) virDomainPtr domain = NULL;
> > -_cleanup_(freep) char *description = NULL;
> > +_cleanup_(virtDBusVirDomainFreep) virDomainPtr domain = NULL;
> > +_cleanup_(virtDBusFreep) char *description = NULL;
> >  uint32_t flags;
> >  int r;
> >  
> > -domain = domain_from_bus_path(manager->connection,
> > -  sd_bus_message_get_path(message));
> > +domain = virtDBusVirDomainFromBusPath(manager->connection,
> > +   sd_bus_message_get_path(message));
> 
> adjust alignment of 2nd argument (occurs a few more times too -
> domain_get_stats, domain_shutdown, domain_destroy, domain_reboot,
> domain_reset, domain_create, and domain_undefine)

Nice catch, I've actually noticed that while working on implementing
new APIs, now I know in which patch I've introduced it.  I'll fix it.

Pavel


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