Re: [libvirt] [PATCH 0/7] Pass the path of compress program (redo)

[John Ferlan]

On 09/21/2016 10:23 PM, Chen Hanxiao wrote:
> 
> At 2016-09-22 05:30:48, "John Ferlan"  wrote:
>> Rather than try to describe what I was thinking about for the passing
>> the compress program directly series from Chen Hanxiao:
>>
>> http://www.redhat.com/archives/libvir-list/2016-September/msg00677.html
>>
>> and
>>
>> http://www.redhat.com/archives/libvir-list/2016-August/msg01237.html
>>
>> I took the liberty of creating my own set of patches which should end
>> in the more or less same result.
>>
>> The primary benefit is not calling virFileInPath twice since we already
>> get it at least once for the various compressed program callers, let's
>> pass what we originally found.
>>
>> John Ferlan (7):
>>  qemu: Move getCompressionType
>>  qemu: Adjust doCoreDump to call getCompressionType
>>  qemu: Introduce helper qemuGetCompressionProgram
>>  qemu: Remove getCompressionType
>>  qemu: Use qemuGetCompressionProgram for error paths
>>  qemu: Remove qemuCompressProgramAvailable
>>  qemu: Get/return compressedpath program
>>
>> src/qemu/qemu_driver.c | 235 
>> +++--
>> 1 file changed, 111 insertions(+), 124 deletions(-)
>>
>> -- 
> 
> Hi, John
> 
> Looks that Patch 1, 3 and 4 should be scrash into one :).

I disagree - then in becomes one mish-mash of a patch which is harder to
review.  The way I split things up (for me) was a logical progression of
this series.

1. Move code because we're about to use it from doCoreDump.
2. Use getCompressionType from within doCoreDump instead of passing it
3. Split out the "cfg->dumpImageFormat" into a separate helper and call
4. Remove need for getCompressionType for doCoreDump

As for the rest

5. Create new parameter to change how messaging is done. From doCoreDump
we can VIR_WARN since the code can continue; whereas, from
qemuDomainSaveFlags, qemuDomainManagedSave, and
qemuDomainSnapshotCreateActiveExternal we want to have the error and fail.

Using "ret < 0" in the error path allows us to know which of the two
options failed.

Yes, the error message for snapshot will now be different, but that's
far more easily fixed by a new parameter than the extra code created in
your patch 2 to handle that error message.

6. Remove need for qemuCompressProgramAvailable by calling
virFindFileInPath directly. No need to worry about the
QEMU_SAVE_FORMAT_RAW case since it's already handled.

7. Return that compressed program name instead of calling
qemuCompressProgramName again.

Again, since QEMU_SAVE_FORMAT_RAW was already handled we're good.

> Some other inline comments, Please check.

I'll post an adjustment for patch 5 error message tomorrow - it won't be
complicated.

Tks for the comments/review -

John

> 
> Regards,
> - Chen
> 

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

Re: [libvirt] [PATCH 9/9] qemu_hotplug: Support interface type of vhost-user hotplug

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1366108
> 
> So far, the hotplug of vhost-user "worked" by pure chance. Well,
> qemu would error on our commands, but nevertheless. Now that we
> have everything prepare, We should support hotplugging og

prepared, we ... of

> vhost-user. Firstly, we need to plug in chardev (through which
> qemu talks to OpenVSwitch), then netdev and only after that we
> can plug in the virtio-net-pci device.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_hotplug.c | 49 
> +
>  1 file changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index feb1f44..0bcb411 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -933,6 +933,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>  virDomainCCWAddressSetPtr ccwaddrs = NULL;
>  size_t i;
> +char *charDevAlias = NULL;
>  
>  /* preallocate new slot for device */
>  if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0)
> @@ -954,11 +955,11 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  case VIR_DOMAIN_NET_TYPE_DIRECT:
>  case VIR_DOMAIN_NET_TYPE_BRIDGE:
>  case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
>  /* These types are supported. */
>  break;
>  
>  case VIR_DOMAIN_NET_TYPE_USER:
> -case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
>  case VIR_DOMAIN_NET_TYPE_SERVER:
>  case VIR_DOMAIN_NET_TYPE_CLIENT:
>  case VIR_DOMAIN_NET_TYPE_MCAST:
> @@ -1148,6 +1149,26 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  goto cleanup;
>  }
>  
> +if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
> +if (!qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   "%s", _("Netdev support unavailable"));

"%s", on previous line

"vhost-user hot-plug not support by this version of qemu"

> +goto cleanup;
> +}
> +
> +if (virAsprintf(, "char%s", net->info.alias) < 0)
> +goto cleanup;
> +
> +qemuDomainObjEnterMonitor(driver, vm);
> +if (qemuMonitorAttachCharDev(priv->mon, charDevAlias, 
> net->data.vhostuser) < 0) {
> +ignore_value(qemuDomainObjExitMonitor(driver, vm));
> +virDomainAuditNet(vm, NULL, net, "attach", false);
> +goto cleanup;
> +}
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +goto cleanup;
> +}
> +
>  qemuDomainObjEnterMonitor(driver, vm);
>  if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
>  if (qemuMonitorAddNetdev(priv->mon, netstr,
> @@ -1268,6 +1289,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  }
>  VIR_FREE(vhostfd);
>  VIR_FREE(vhostfdName);
> +VIR_FREE(charDevAlias);
>  virObjectUnref(cfg);
>  virDomainCCWAddressSetFree(ccwaddrs);
>  
> @@ -1283,6 +1305,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  if (virAsprintf(_name, "host%s", net->info.alias) < 0)
>  goto cleanup;
>  qemuDomainObjEnterMonitor(driver, vm);
> +if (charDevAlias &&
> +qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0)
> +VIR_WARN("Failed to remove associated chardev %s", 
> charDevAlias);
>  if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0)
>  VIR_WARN("Failed to remove network backend for netdev %s",
>   netdev_name);
> @@ -3309,10 +3334,12 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
>  virNetDevVPortProfilePtr vport;
>  virObjectEventPtr event;
>  char *hostnet_name = NULL;
> +char *charDevAlias = NULL;
>  size_t i;
>  int ret = -1;
> +int actualType = virDomainNetGetActualType(net);
>  
> -if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> +if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>  /* this function handles all hostdev and netdev cleanup */
>  ret = qemuDomainRemoveHostDevice(driver, vm,
>   virDomainNetGetActualHostdev(net));
> @@ -3322,9 +3349,11 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
>  VIR_DEBUG("Removing network interface %s from domain %p %s",
>net->info.alias, vm, vm->def->name);
>  
> -if (virAsprintf(_name, "host%s", net->info.alias) < 0)
> +if (virAsprintf(_name, "host%s", net->info.alias) < 0 ||
> +virAsprintf(, "char%s", net->info.alias) < 0)


Part of me wonders if this should be

  ||
  (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
   virAsprintf(_name, "host%s", net->info.alias) < 0)

Since it's only necessary for 

Re: [libvirt] [PATCH 8/9] qemuBuildHostNetStr: Support VIR_DOMAIN_NET_TYPE_VHOSTUSER

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1366505
> 
> So far, this function lacked support for
> VIR_DOMAIN_NET_TYPE_VHOSTUSER leaving callers to hack around the
> problem by constructing the command line on their own. This is
> not ideal as it blocks hot plug support.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_command.c| 43 
> +-
>  .../qemuxml2argv-net-vhostuser-multiq.args |  6 +--
>  .../qemuxml2argv-net-vhostuser.args|  4 +-
>  3 files changed, 31 insertions(+), 22 deletions(-)
> 

Seems reasonable and at least the NULL parameters being sent are checked
in qemuBuildHostNetStr before being used.

ACK

John

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

Re: [libvirt] [PATCH 7/9] qemuBuildVhostuserCommandLine: Unify -netdev creation

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> Currently, what we do for vhost-user network is generate the
> following part of command line:
> 
> -netdev type=vhost-user,id=hostnet0,chardev=charnet0
> 
> There's no need for 'type=' it is the default. Drop it.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_command.c   | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args | 6 +++---
>  tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args| 4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 

Seems reasonable... and if you say so ;-)

ACK,

John

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

Re: [libvirt] [PATCH 6/9] qemuBuildHostNetStr: Explicitly enumerate net types

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> We tend to prevent using 'default' in switches. And it is for a
> good reason - control may end up in paths we wouldn't want for
> new values. In this specific case, if qemuBuildHostNetStr is
> called over VIR_DOMAIN_NET_TYPE_VHOSTUSER it would produce
> meaningless output. Fortunately, there no such call yet.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_command.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 

I agree in principal, although this function is called by both
qemu_command and qemu_hotplug...

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index e6b3d9d..12f3a6b 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3660,9 +3660,21 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
>  break;
>  
>  case VIR_DOMAIN_NET_TYPE_USER:
> -default:
> +case VIR_DOMAIN_NET_TYPE_INTERNAL:
>  virBufferAddLit(, "user");
>  break;
> +
> +case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +/* Should have been handled earlier via PCI/USB hotplug code. */

True for the hotplug code - it diverts the HOSTDEV call to
qemuDomainAttachHostDevice... The qemu_command code would not call here
and perhaps even could be considered an error.  The problem with
returning NULL here is if it *ever* happened, you'd get that failed for
some reason error.

> +virObjectUnref(cfg);
> +return NULL;
> +
> +case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
> +/* Unsupported yet. */

So, could this too produce meaningless code? Although not called, so I
suppose no big deal.  (thinking while typing)...


> +break;
> +
> +case VIR_DOMAIN_NET_TYPE_LAST:

Similarly meaningless code, but would also be an error ostensibly
wouldn't it?  Why not move the HOSTDEV down here (with that same
comment)... Generate a real error (invalid type=%d), do the Unref and
return NULL, then call it a good.

John

> +break;
>  }
>  
>  if (vlan >= 0) {
> 

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

Re: [libvirt] [PATCH 4/9] qemuBuildVhostuserCommandLine: Reuse qemuBuildChrChardevStr

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> There's no need to reinvent the wheel here. We already have a
> function to format virDomainChrSourceDefPtr. It's called
> qemuBuildChrChardevStr(). Use that instead of some dummy
> virBufferAsprintf().
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_command.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c419ecf..59fcb4c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7848,7 +7848,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
>virQEMUCapsPtr qemuCaps,
>unsigned int bootindex)
>  {
> -virBuffer chardev_buf = VIR_BUFFER_INITIALIZER;
> +char *chardev = NULL;
>  virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
>  unsigned int queues = net->driver.virtio.queues;
>  char *nic = NULL;
> @@ -7861,9 +7861,10 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
>  
>  switch ((virDomainChrType) net->data.vhostuser->type) {
>  case VIR_DOMAIN_CHR_TYPE_UNIX:
> -virBufferAsprintf(_buf, "socket,id=char%s,path=%s%s",
> -  net->info.alias, 
> net->data.vhostuser->data.nix.path,
> -  net->data.vhostuser->data.nix.listen ? ",server" : 
> "");
> +if (!(chardev = qemuBuildChrChardevStr(NULL, NULL, NULL, def,

logManager == NULL?
cmd == NULL?
cfg == NULL?

if (dev->logfile) code will surely have problems (as will the TLS code)

While true your narrow usage could avoid the TLS code that's going to
dereference cfg without doing the obligatory "cfg &&", I'm still not
clear how it would avoid the dev->logfile code.

Shouldn't this support that logManager anyway? If not, then perhaps it
needs to be more explicit.  IOW: Extra checks.

John

> +   net->data.vhostuser,
> +   net->info.alias, qemuCaps, 
> false)))
> +goto error;
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_NULL:
> @@ -7881,7 +7882,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
>  case VIR_DOMAIN_CHR_TYPE_LAST:
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("vhost-user type '%s' not supported"),
> -virDomainChrTypeToString(net->data.vhostuser->type));
> +   virDomainChrTypeToString(net->data.vhostuser->type));
>  goto error;
>  }
>  
> @@ -7899,7 +7900,8 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
>  }
>  
>  virCommandAddArg(cmd, "-chardev");
> -virCommandAddArgBuffer(cmd, _buf);
> +virCommandAddArg(cmd, chardev);
> +VIR_FREE(chardev);
>  
>  virCommandAddArg(cmd, "-netdev");
>  virCommandAddArgBuffer(cmd, _buf);
> @@ -7917,7 +7919,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
>  return 0;
>  
>   error:
> -virBufferFreeAndReset(_buf);
> +VIR_FREE(chardev);
>  virBufferFreeAndReset(_buf);
>  VIR_FREE(nic);
>  
> 

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

Re: [libvirt] [PATCH 5/9] qemuBuildHostNetStr: Realign

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_command.c | 64 
> -
>  1 file changed, 32 insertions(+), 32 deletions(-)
> 

ACK

John

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

Re: [libvirt] [PATCH 3/9] qemuBuildChrChardevStr: Introduce @nowait argument

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> This alone makes not much sense. But the aim is to reuse this
> function in qemuBuildVhostuserCommandLine() where 'nowait' is not
> supported for vhost-user devices.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_command.c | 34 ++
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 

This one had some merge conflicts with some changes I made - not
difficult to handle thankfully... Of course it caused me to notice some
bad indention in my code too 

These changes look fine to me though, with one question...

Considering my changes were to add the ability to use TLS for a TCP
chardev via 'tls-creds' - how does that play with your next change to
add a 'vhost-user' chardev option? OK beyond what will be some very
obvious issues with usage of NULL parameters.

IOW: Does one of these vhost-user chardevs "support" that "tls-creds"
option?

John

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

Re: [libvirt] [PATCH 2/9] qemuDomain{Attach, Remove}NetDevice: Prefer qemuDomainSupportsNetdev

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> Depending on domain OS type, and interface address type we might
> not want to use -netdev even though qemu has the capability. We
> should use more advanced check implemented in
> qemuDomainSupportsNetdev() function.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_hotplug.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

Seems to me part of the goal here is to follow the decision points that
qemu_command.c would make in qemuBuildInterfaceCommandLine, but that
logic isn't all that clear either.

If it helps, I'm in favor of the change - although I have a couple of
concerns (and per usual I left some thoughts along the way)...

it's a weak ACK, but let's see what you/Laine think too

John

> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index d1acdd9..feb1f44 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1108,7 +1108,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  
>  releaseaddr = true;
>  
> -if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {

same check as qemuBuildNetCommandLine, so this looks right.

>  vlan = -1;
>  } else {
>  vlan = qemuDomainNetVLAN(net);
> @@ -1134,7 +1134,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  goto cleanup;
>  }
>  
> -if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
>  if (!(netstr = qemuBuildHostNetStr(net, driver,
> ',', -1,

FWIW: That -1 is also 'vlan' value *if* qemuDomainSupportsNetdev is used
and true. If you follow the qemu_command logic, it too would pass -1 in
the vlan to qemuBuildInterfaceCommandLine.  So "theoretically" we're
still on par with qemu_command processing...

The else portion of the hotplug code doesn't seem to match the
qemu_command code's !qemuDomainSupportsNetdev and qemuBuildHostNetStr
call though (' ' vs. ',').  I know - separate issue, but I figured I'd
point it out at least.

> tapfdName, tapfdSize,
> @@ -1149,7 +1149,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  }
>  
>  qemuDomainObjEnterMonitor(driver, vm);
> -if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {

This particular check/usage isn't as clear to me at least with respect
to comparing to qemu_command processing of "-device" and hotplug usage
of netdev_add.  As for the else condition, it's qemu_command usage of
"-net" and (text only) hotplug usage of host_net_add.

As long as those are "equals", then this check seems to pass muster
too... At least w/r/t how qemu_command does things.

>  if (qemuMonitorAddNetdev(priv->mon, netstr,
>   tapfd, tapfdName, tapfdSize,
>   vhostfd, vhostfdName, vhostfdSize) < 0) {
> @@ -1195,7 +1195,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  } else {
>  qemuDomainObjEnterMonitor(driver, vm);
>  
> -if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {

This would seemingly be OK

>  if (qemuMonitorSetLink(priv->mon, net->info.alias, 
> VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) < 0) {
>  ignore_value(qemuDomainObjExitMonitor(driver, vm));
>  virDomainAuditNet(vm, NULL, net, "attach", false);
> @@ -1278,7 +1278,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  goto cleanup;
>  
>  if (vlan < 0) {
> -if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {

This would match, error code...

>  char *netdev_name;
>  if (virAsprintf(_name, "host%s", net->info.alias) < 0)
>  goto cleanup;
> @@ -3326,7 +3326,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
>  goto cleanup;
>  
>  qemuDomainObjEnterMonitor(driver, vm);
> -if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
> +if (qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {

and this would be the corollary to our Attach which seems OK.

>  if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
>  if (qemuDomainObjExitMonitor(driver, vm) < 0)
>  goto cleanup;
> 

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

Re: [libvirt] [PATCH 1/9] qemuDomainAttachNetDevice: Explicitly list allowed types for hotplug

[John Ferlan]

On 08/16/2016 11:41 AM, Michal Privoznik wrote:
> Instead of blindly claim support for hot-plugging of every

claiming

> interface type out there we should copy approach we have for
> device types: white listing supported types and explicitly error
> out on unsupported ones.
> For instance, trying to hotplug vhostuser interface results in
> nothing usable from guest currently. vhostuser typed interfaces
> require additional work on our side.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_hotplug.c | 24 
>  1 file changed, 24 insertions(+)
> 

Part of me thinks - sure ACK this, but the other part says why not
convert the "if - elseif - elseif" a few lines below to be essentially
this switch?

Then I wonder if Coverity would pick up on the HOSTDEV specific if and
"flag" the switch case HOSTDEV: and complain?

What would be the drawback to the switch option?

John
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 00e4a75..d1acdd9 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -947,6 +947,30 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
>  
>  actualType = virDomainNetGetActualType(net);
>  
> +
> +switch ((virDomainNetType) actualType) {
> +case VIR_DOMAIN_NET_TYPE_NETWORK:
> +case VIR_DOMAIN_NET_TYPE_ETHERNET:
> +case VIR_DOMAIN_NET_TYPE_DIRECT:
> +case VIR_DOMAIN_NET_TYPE_BRIDGE:
> +case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +/* These types are supported. */
> +break;
> +
> +case VIR_DOMAIN_NET_TYPE_USER:
> +case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
> +case VIR_DOMAIN_NET_TYPE_SERVER:
> +case VIR_DOMAIN_NET_TYPE_CLIENT:
> +case VIR_DOMAIN_NET_TYPE_MCAST:
> +case VIR_DOMAIN_NET_TYPE_INTERNAL:
> +case VIR_DOMAIN_NET_TYPE_UDP:
> +case VIR_DOMAIN_NET_TYPE_LAST:
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +   _("hotplug of interface type of %s is not implemented 
> yet"),
> +   virDomainNetTypeToString(actualType));
> +goto cleanup;
> +}
> +
>  if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
>  /* This is really a "smart hostdev", so it should be attached
>   * as a hostdev (the hostdev code will reach over into the
> 

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

[libvirt] ANNOUNCE: ruby-libvirt 0.7.0

[Chris Lalancette]

I'm pleased to release ruby-libvirt 0.7.0.  ruby-libvirt is a ruby wrapper
around the libvirt API.  The changelog between 0.6.0 and 0.7.0 is:

  * Fix network lease API to allow arguments that libvirt allows
  * Implement VIRT_STORAGE_POOL_CREATE flags
  * Implement more VIR_STORAGE_VOL flags
  * Implement VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN
  * Implement virDomainDefineXMLFlags
  * Implement virDomainRename
  * Implement virDomainSetUserPassword
  * Implement VIR_DOMAIN_TIME_SYNC
  * Fix the return value from virStreamSourceFunc so volume upload works

Version 0.7.0 is available from http://libvirt.org/ruby:

Tarball: http://libvirt.org/ruby/download/ruby-libvirt-0.7.0.tgz
Gem: http://libvirt.org/ruby/download/ruby-libvirt-0.7.0.gem

It is also available from rubygems.org; to get the latest version, run:

$ gem install ruby-libvirt

Packages for it are also built for Fedora, and will be released shortly.

As usual, if you run into questions, problems, or bugs, please feel free to
mail me (clalancette at gmail.com) and the libvirt mailing list.

Thanks to those who contributed to this release.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu: Fix improper indention

[John Ferlan]

Commit id 'ce61c164' indented wrong - not sure how I did that...

Signed-off-by: John Ferlan 
---
 Pushed under the trivial rule

 src/qemu/qemu_command.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f6d26b0..1ac0fa1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5041,20 +5041,20 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
   telnet ? ",telnet" : "",
   dev->data.tcp.listen ? ",server,nowait" : "");
 
-if (cfg->chardevTLS) {
-char *objalias = NULL;
+if (cfg->chardevTLS) {
+char *objalias = NULL;
 
-if (qemuBuildTLSx509CommandLine(cmd, 
cfg->chardevTLSx509certdir,
-dev->data.tcp.listen,
-cfg->chardevTLSx509verify,
-alias, qemuCaps) < 0)
-goto error;
+if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
+dev->data.tcp.listen,
+cfg->chardevTLSx509verify,
+alias, qemuCaps) < 0)
+goto error;
 
-if (!(objalias = qemuAliasTLSObjFromChardevAlias(alias)))
-goto error;
-virBufferAsprintf(, ",tls-creds=%s", objalias);
-VIR_FREE(objalias);
-}
+if (!(objalias = qemuAliasTLSObjFromChardevAlias(alias)))
+goto error;
+virBufferAsprintf(, ",tls-creds=%s", objalias);
+VIR_FREE(objalias);
+}
 break;
 
 case VIR_DOMAIN_CHR_TYPE_UNIX:
-- 
2.7.4

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

[libvirt] [PATCH v2 0/4] libxl: channels support

[Joao Martins]

Hey,

This v2 from channel series. Since v1 path is now autogenerated as needed,
and a few other comments from Jim that were addressed. Difference to qemu
driver would be only the autogenerated path being slightly different.

Channels have been on xen toolstack since Xen 4.5 and this small series
adds support for it, including xenconfig conversion and appropriate tests.
After this series it's possible to do this:
(assuming correct configuration of qemu agent in the guest)

 $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
 
   
   
 

 $ virsh create domain.xml
 $ echo '{"execute":"guest-network-get-interfaces"}' | socat
 stdio,ignoreeof  unix-connect:/tmp/channel

 {"execute":"guest-network-get-interfaces"}
 {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
 "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
 "ip-address": "::1", "prefix": 128}], "hardware-address":
 "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
 [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
 {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
 "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
 "sit0"}]}

Thanks,
Joao


Joao Martins (4):
  conf: add xen type for channels
  libxl: channels support
  xenconfig: channels conversion support
  xlconfigtest: add test for channel conversion

 docs/schemas/domaincommon.rng|  11 ++
 src/conf/domain_conf.c   |  18 +++-
 src/conf/domain_conf.h   |   1 +
 src/libxl/libxl_conf.c   | 120 -
 src/libxl/libxl_conf.h   |   4 +-
 src/libxl/libxl_domain.c |  44 +++-
 src/libxl/libxl_driver.c |   7 ++
 src/qemu/qemu_command.c  |   1 +
 src/xenconfig/xen_xl.c   | 176 +++
 tests/xlconfigdata/test-channel-pty.cfg  |  13 +++
 tests/xlconfigdata/test-channel-pty.xml  |  33 ++
 tests/xlconfigdata/test-channel-unix.cfg |  13 +++
 tests/xlconfigdata/test-channel-unix.xml |  34 ++
 tests/xlconfigtest.c |   4 +
 14 files changed, 471 insertions(+), 8 deletions(-)
 create mode 100644 tests/xlconfigdata/test-channel-pty.cfg
 create mode 100644 tests/xlconfigdata/test-channel-pty.xml
 create mode 100644 tests/xlconfigdata/test-channel-unix.cfg
 create mode 100644 tests/xlconfigdata/test-channel-unix.xml

-- 
2.1.4

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

[libvirt] [PATCH v2 1/4] conf: add xen type for channels

[Joao Martins]

So far only guestfwd and virtio were supported. Add an additional
for Xen as libxl channels create Xen console visible to the guest.

Signed-off-by: Joao Martins 
---
 docs/schemas/domaincommon.rng | 11 +++
 src/conf/domain_conf.c| 18 ++
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_command.c   |  1 +
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 95c7882..6eeb4e9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3690,6 +3690,16 @@
   
 
   
+  
+
+  
+xen
+  
+  
+
+  
+
+  
   
 
   
@@ -3698,6 +3708,7 @@
 
   
   
+  
 
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5fede3d..1b14efd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -434,7 +434,8 @@ VIR_ENUM_IMPL(virDomainChrChannelTarget,
   VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST,
   "none",
   "guestfwd",
-  "virtio")
+  "virtio",
+  "xen")
 
 VIR_ENUM_IMPL(virDomainChrConsoleTarget,
   VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST,
@@ -2068,6 +2069,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
 VIR_FREE(def->target.addr);
 break;
 
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
 case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
 VIR_FREE(def->target.name);
 break;
@@ -9909,10 +9911,12 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def,
 virSocketAddrSetPort(def->target.addr, port);
 break;
 
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
 case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
 def->target.name = virXMLPropString(cur, "name");
 
-if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
+if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
+!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
 (stateStr = virXMLPropString(cur, "state"))) {
 int tmp;
 
@@ -10203,7 +10207,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 /* path can be auto generated */
 if (!path &&
 (!chr_def ||
- chr_def->targetType != 
VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)) {
+ (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
+  chr_def->targetType != 
VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing source path attribute for char device"));
 goto error;
@@ -14403,6 +14408,7 @@ virDomainChrEquals(virDomainChrDefPtr src,
 if (src->targetType != tgt->targetType)
 return false;
 switch ((virDomainChrChannelTargetType) src->targetType) {
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
 case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
 return STREQ_NULLABLE(src->target.name, tgt->target.name);
 break;
@@ -18415,6 +18421,8 @@ virDomainChannelDefCheckABIStability(virDomainChrDefPtr 
src,
 }
 
 switch (src->targetType) {
+
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
 case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
 if (STRNEQ_NULLABLE(src->target.name, dst->target.name)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -21537,11 +21545,13 @@ virDomainChrDefFormat(virBufferPtr buf,
 break;
 }
 
+case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
 case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
 if (def->target.name)
 virBufferEscapeString(buf, " name='%s'", def->target.name);
 
-if (def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT &&
+if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
+def->state != VIR_DOMAIN_CHR_DEVICE_STATE_DEFAULT &&
 !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
 virBufferAsprintf(buf, " state='%s'",
   
virDomainChrDeviceStateTypeToString(def->state));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d4a84c3..a0619f5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1021,6 +1021,7 @@ typedef enum {
 VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE = 0,
 VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD,
 VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO,
+VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN,
 
 VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST
 } virDomainChrChannelTargetType;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f6d26b0..ddfdb85 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9719,6 +9719,7 

[libvirt] [PATCH v2 3/4] xenconfig: channels conversion support

[Joao Martins]

Add support for formating/parsing libxl channels.

Syntax on xen libxl goes as following:
channel=["connection=pty|socket,path=/path/to/socket,name=XXX",...]

Signed-off-by: Joao Martins 
---
Changes since v1:
* Move path to UNIX case.
---
 src/xenconfig/xen_xl.c | 176 +
 1 file changed, 176 insertions(+)

diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index 7774dfc..0c02e1f 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -685,6 +685,93 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def)
 return 0;
 }
 
+static int
+xenParseXLChannel(virConfPtr conf, virDomainDefPtr def)
+{
+virConfValuePtr list = virConfGetValue(conf, "channel");
+virDomainChrDefPtr channel = NULL;
+char *name = NULL;
+char *path = NULL;
+
+if (list && list->type == VIR_CONF_LIST) {
+list = list->list;
+while (list) {
+char type[10];
+char *key;
+
+if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
+goto skipchannel;
+
+key = list->str;
+while (key) {
+char *data;
+char *nextkey = strchr(key, ',');
+
+if (!(data = strchr(key, '=')))
+goto skipchannel;
+data++;
+
+if (STRPREFIX(key, "connection=")) {
+int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
+if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("connection %s too big"), data);
+goto skipchannel;
+}
+} else if (STRPREFIX(key, "name=")) {
+int len = nextkey ? (nextkey - data) : strlen(data);
+VIR_FREE(name);
+if (VIR_STRNDUP(name, data, len) < 0)
+goto cleanup;
+} else if (STRPREFIX(key, "path=")) {
+int len = nextkey ? (nextkey - data) : strlen(data);
+VIR_FREE(path);
+if (VIR_STRNDUP(path, data, len) < 0)
+goto cleanup;
+}
+
+while (nextkey && (nextkey[0] == ',' ||
+   nextkey[0] == ' ' ||
+   nextkey[0] == '\t'))
+nextkey++;
+key = nextkey;
+}
+
+if (!(channel = virDomainChrDefNew()))
+goto cleanup;
+
+if (STRPREFIX(type, "socket")) {
+channel->source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+channel->source.data.nix.path = path;
+channel->source.data.nix.listen = 1;
+} else if (STRPREFIX(type, "pty")) {
+channel->source.type = VIR_DOMAIN_CHR_TYPE_PTY;
+VIR_FREE(path);
+} else {
+goto cleanup;
+}
+
+channel->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL;
+channel->targetType = VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN;
+channel->target.name = name;
+
+if (VIR_APPEND_ELEMENT(def->channels, def->nchannels, channel) < 0)
+goto cleanup;
+
+skipchannel:
+list = list->next;
+}
+}
+
+return 0;
+
+ cleanup:
+virDomainChrDefFree(channel);
+VIR_FREE(path);
+VIR_FREE(name);
+return -1;
+}
+
 virDomainDefPtr
 xenParseXL(virConfPtr conf,
virCapsPtr caps,
@@ -720,6 +807,9 @@ xenParseXL(virConfPtr conf,
 if (xenParseXLUSBController(conf, def) < 0)
 goto cleanup;
 
+if (xenParseXLChannel(conf, def) < 0)
+goto cleanup;
+
 if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
   xmlopt) < 0)
 goto cleanup;
@@ -1347,6 +1437,89 @@ xenFormatXLUSB(virConfPtr conf,
 return -1;
 }
 
+static int
+xenFormatXLChannel(virConfValuePtr list, virDomainChrDefPtr channel)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int sourceType = channel->source.type;
+virConfValuePtr val, tmp;
+
+/* connection */
+virBufferAddLit(, "connection=");
+switch (sourceType) {
+case VIR_DOMAIN_CHR_TYPE_PTY:
+virBufferAddLit(, "pty,");
+break;
+case VIR_DOMAIN_CHR_TYPE_UNIX:
+virBufferAddLit(, "socket,");
+/* path */
+if (channel->source.data.nix.path)
+virBufferAsprintf(, "path=%s,",
+  channel->source.data.nix.path);
+break;
+default:
+goto cleanup;
+}
+
+/* name */
+virBufferAsprintf(, "name=%s", channel->target.name);
+
+if (VIR_ALLOC(val) < 0)
+goto cleanup;
+
+val->type = VIR_CONF_STRING;

[libvirt] [PATCH v2 4/4] xlconfigtest: add test for channel conversion

[Joao Martins]

Signed-off-by: Joao Martins 
---
 tests/xlconfigdata/test-channel-pty.cfg  | 13 
 tests/xlconfigdata/test-channel-pty.xml  | 33 +++
 tests/xlconfigdata/test-channel-unix.cfg | 13 
 tests/xlconfigdata/test-channel-unix.xml | 34 
 tests/xlconfigtest.c |  4 
 5 files changed, 97 insertions(+)
 create mode 100644 tests/xlconfigdata/test-channel-pty.cfg
 create mode 100644 tests/xlconfigdata/test-channel-pty.xml
 create mode 100644 tests/xlconfigdata/test-channel-unix.cfg
 create mode 100644 tests/xlconfigdata/test-channel-unix.xml

diff --git a/tests/xlconfigdata/test-channel-pty.cfg 
b/tests/xlconfigdata/test-channel-pty.cfg
new file mode 100644
index 000..b20e487
--- /dev/null
+++ b/tests/xlconfigdata/test-channel-pty.cfg
@@ -0,0 +1,13 @@
+name = "XenGuest1"
+uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283"
+maxmem = 512
+memory = 512
+vcpus = 1
+localtime = 0
+on_poweroff = "preserve"
+on_reboot = "restart"
+on_crash = "preserve"
+vif = [ "mac=5a:36:0e:be:00:09" ]
+bootloader = "/usr/bin/pygrub"
+disk = [ 
"format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2"
 ]
+channel = [ "connection=pty,name=org.qemu.guest_agent.0" ]
diff --git a/tests/xlconfigdata/test-channel-pty.xml 
b/tests/xlconfigdata/test-channel-pty.xml
new file mode 100644
index 000..17d0c67
--- /dev/null
+++ b/tests/xlconfigdata/test-channel-pty.xml
@@ -0,0 +1,33 @@
+
+  XenGuest1
+  45b60f51-88a9-47a8-a3b3-5e66d71b2283
+  524288
+  524288
+  1
+  /usr/bin/pygrub
+  
+linux
+  
+  
+  preserve
+  restart
+  preserve
+  
+
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+
+
+  
+
diff --git a/tests/xlconfigdata/test-channel-unix.cfg 
b/tests/xlconfigdata/test-channel-unix.cfg
new file mode 100644
index 000..ada7001
--- /dev/null
+++ b/tests/xlconfigdata/test-channel-unix.cfg
@@ -0,0 +1,13 @@
+name = "XenGuest1"
+uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283"
+maxmem = 512
+memory = 512
+vcpus = 1
+localtime = 0
+on_poweroff = "preserve"
+on_reboot = "restart"
+on_crash = "preserve"
+vif = [ "mac=5a:36:0e:be:00:09" ]
+bootloader = "/usr/bin/pygrub"
+disk = [ 
"format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2"
 ]
+channel = [ 
"connection=socket,path=/path/to/socket,name=org.qemu.guest_agent.0" ]
diff --git a/tests/xlconfigdata/test-channel-unix.xml 
b/tests/xlconfigdata/test-channel-unix.xml
new file mode 100644
index 000..8f4eaa2
--- /dev/null
+++ b/tests/xlconfigdata/test-channel-unix.xml
@@ -0,0 +1,34 @@
+
+  XenGuest1
+  45b60f51-88a9-47a8-a3b3-5e66d71b2283
+  524288
+  524288
+  1
+  /usr/bin/pygrub
+  
+linux
+  
+  
+  preserve
+  restart
+  preserve
+  
+
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+  
+
+
+
+  
+
diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c
index 61a6295..8fc73ee 100644
--- a/tests/xlconfigtest.c
+++ b/tests/xlconfigtest.c
@@ -268,6 +268,10 @@ mymain(void)
 DO_TEST_FORMAT("paravirt-cmdline-bogus-extra-root", false);
 DO_TEST("rbd-multihost-noauth");
 
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+DO_TEST("channel-pty");
+DO_TEST("channel-unix");
+#endif
 #ifdef LIBXL_HAVE_BUILDINFO_SERIAL_LIST
 DO_TEST("fullvirt-multiserial");
 #endif
-- 
2.1.4

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

[libvirt] [PATCH v2 2/4] libxl: channels support

[Joao Martins]

Allow libxl to handle channel element which creates a Xen
console visible to the guest as a low-bandwitdh communication
channel. If type is PTY we also fetch the tty after boot using
libxl_channel_getinfo to fetch the tty path. On socket case,
we autogenerate a path if not specified in the XML. Path autogenerated
is slightly different from qemu driver: qemu stores also on
"channels/target" but it creates then a directory per domain with
each channel target name. libxl doesn't appear to have a clear
definition of private files associated with each domain, so for
simplicity we do it slightly different. On qemu each autogenerated
channel goes like:

channels/target//

Whereas for libxl:

channels/target/-

Should note that if path is not specified it won't persist,
existing only on live XML, unless user had initially specified it.
Since support for libxl channels only came on Xen >= 4.5 we therefore
need to conditionally compile it with LIBXL_HAVE_DEVICE_CHANNEL.

After this patch and having a qemu guest agent:
 $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
 
   
   
 

 $ virsh create domain.xml
 $ echo '{"execute":"guest-network-get-interfaces"}' | socat
 stdio,ignoreeof  unix-connect:/tmp/channel

 {"execute":"guest-network-get-interfaces"}
 {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
 "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
 "ip-address": "::1", "prefix": 128}], "hardware-address":
 "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
 [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
 {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
 "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
 "sit0"}]}

Signed-off-by: Joao Martins 
---
Since v1:
* Autogenerated paths, and updated commit message explaning it the different
naming. Despite per domain name being slightly different, parent
directory is same across both drivers.
* Remove the switch case from target type xen and rework the function structure
a bit.
---
 src/libxl/libxl_conf.c   | 120 ++-
 src/libxl/libxl_conf.h   |   4 +-
 src/libxl/libxl_domain.c |  44 -
 src/libxl/libxl_driver.c |   7 +++
 4 files changed, 171 insertions(+), 4 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 306e441..824f2d2 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -88,6 +88,7 @@ libxlDriverConfigDispose(void *obj)
 VIR_FREE(cfg->saveDir);
 VIR_FREE(cfg->autoDumpDir);
 VIR_FREE(cfg->lockManagerName);
+VIR_FREE(cfg->channelDir);
 virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares);
 }
 
@@ -1339,6 +1340,8 @@ libxlDriverConfigNew(void)
 goto error;
 if (VIR_STRDUP(cfg->autoDumpDir, LIBXL_DUMP_DIR) < 0)
 goto error;
+if (VIR_STRDUP(cfg->channelDir, LIBXL_CHANNEL_DIR) < 0)
+goto error;
 
 if (virAsprintf(_file, "%s/libxl-driver.log", cfg->logDir) < 0)
 goto error;
@@ -1490,6 +1493,114 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
 
 }
 
+#ifdef LIBXL_HAVE_DEVICE_CHANNEL
+static int
+libxlPrepareChannel(virDomainChrDefPtr channel,
+const char *channelDir,
+const char *domainName)
+{
+if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
+channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX &&
+!channel->source.data.nix.path) {
+if (virAsprintf(>source.data.nix.path,
+"%s/%s-%s", channelDir, domainName,
+channel->target.name ? channel->target.name
+: "unknown.sock") < 0)
+return -1;
+
+channel->source.data.nix.listen = true;
+}
+
+return 0;
+}
+
+static int
+libxlMakeChannel(virDomainChrDefPtr l_channel,
+ libxl_device_channel *x_channel)
+{
+int ret = -1;
+
+libxl_device_channel_init(x_channel);
+
+if (l_channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("channel target type not supported"));
+return ret;
+}
+
+switch (l_channel->source.type) {
+case VIR_DOMAIN_CHR_TYPE_PTY:
+x_channel->connection = LIBXL_CHANNEL_CONNECTION_PTY;
+break;
+case VIR_DOMAIN_CHR_TYPE_UNIX:
+x_channel->connection = LIBXL_CHANNEL_CONNECTION_SOCKET;
+if (VIR_STRDUP(x_channel->u.socket.path,
+   l_channel->source.data.nix.path) < 0)
+return ret;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("channel source type not supported"));
+break;
+}
+
+if (!l_channel->target.name) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("channel target name missing"));
+

[libvirt] [PATCH] Replacing for (; ; ) with while (1) for better code.

[Nitesh Konkar]

As mentioned in https://libvirt.org/hacking.html,
using while (1) is better than for (;;).
---
 src/libvirt-stream.c| 4 ++--
 src/phyp/phyp_driver.c  | 2 +-
 src/qemu/qemu_migration.c   | 2 +-
 src/remote/remote_driver.c  | 2 +-
 src/rpc/virnetclient.c  | 6 +++---
 src/security/security_selinux.c | 2 +-
 src/util/vircgroup.c| 2 +-
 src/util/vircommand.c   | 2 +-
 src/util/virdbus.c  | 4 ++--
 src/util/virfile.c  | 2 +-
 tests/commandhelper.c   | 2 +-
 11 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
index 8384b37..41daaa1 100644
--- a/src/libvirt-stream.c
+++ b/src/libvirt-stream.c
@@ -349,7 +349,7 @@ virStreamSendAll(virStreamPtr stream,
 if (VIR_ALLOC_N(bytes, want) < 0)
 goto cleanup;
 
-for (;;) {
+while (1) {
 int got, offset = 0;
 got = (handler)(stream, bytes, want, opaque);
 if (got < 0) {
@@ -443,7 +443,7 @@ virStreamRecvAll(virStreamPtr stream,
 if (VIR_ALLOC_N(bytes, want) < 0)
 goto cleanup;
 
-for (;;) {
+while (1) {
 int got, offset = 0;
 got = virStreamRecv(stream, bytes, want);
 if (got < 0)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 3dd8927..50cf1ae 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -187,7 +187,7 @@ phypExec(LIBSSH2_SESSION *session, const char *cmd, int 
*exit_status,
 if (rc != 0)
 goto err;
 
-for (;;) {
+while (1) {
 /* loop until we block */
 do {
 rc = libssh2_channel_read(channel, buffer, buffer_size);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8a220d9..bc7d268 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4398,7 +4398,7 @@ static void qemuMigrationIOFunc(void *arg)
 fds[0].fd = data->sock;
 fds[1].fd = data->wakeupRecvFD;
 
-for (;;) {
+while (1) {
 int ret;
 
 fds[0].events = fds[1].events = POLLIN;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 3b8b796..b0c4440 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -4055,7 +4055,7 @@ remoteAuthSASL(virConnectPtr conn, struct private_data 
*priv,
 /* Loop-the-loop...
  * Even if the server has completed, the client must *always* do at least 
one step
  * in this loop to verify the server isn't lying about something. Mutual 
auth */
-for (;;) {
+while (1) {
 if ((err = virNetSASLSessionClientStep(sasl,
serverin,
serverinlen,
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 361dc1a..f3457be 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -777,7 +777,7 @@ int virNetClientSetTLSSession(virNetClientPtr client,
 
 virNetSocketSetTLSSession(client->sock, client->tls);
 
-for (;;) {
+while (1) {
 ret = virNetTLSSessionHandshake(client->tls);
 
 if (ret < 0)
@@ -1258,7 +1258,7 @@ virNetClientIOHandleInput(virNetClientPtr client)
 /* Read as much data as is available, until we get
  * EAGAIN
  */
-for (;;) {
+while (1) {
 ssize_t ret;
 
 if (client->msg.nfds == 0) {
@@ -1463,7 +1463,7 @@ static int virNetClientIOEventLoop(virNetClientPtr client,
 fds[0].fd = virNetSocketGetFD(client->sock);
 fds[1].fd = client->wakeupReadFD;
 
-for (;;) {
+while (1) {
 char ignore;
 sigset_t oldmask, blockedsigs;
 int timeout = -1;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 4be946d..153e6e9 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -137,7 +137,7 @@ virSecuritySELinuxMCSFind(virSecurityManagerPtr mgr,
 VIR_DEBUG("Using sensitivity level '%s' cat min %d max %d range %d",
   sens, catMin, catMax, catRange);
 
-for (;;) {
+while (1) {
 int c1 = virRandomInt(catRange);
 int c2 = virRandomInt(catRange);
 
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 8b52816..cc83cf7 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1587,7 +1587,7 @@ virCgroupNewMachineSystemd(const char *name,
 goto cleanup;
 
 
-for (;;) {
+while (1) {
 virCgroupPtr tmp;
 char *t = strchr(offset + 1, '/');
 if (t)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 2a59bd1..bfc130d 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -2033,7 +2033,7 @@ virCommandProcessIO(virCommandPtr cmd)
 goto cleanup;
 ret = -1;
 
-for (;;) {
+while (1) {
 size_t i;
 struct pollfd fds[3];
 int nfds = 0;
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index 3f4dbe3..413229c 100644
--- a/src/util/virdbus.c

Re: [libvirt] [RFC v2] libvirt vGPU QEMU integration

[Tian, Kevin]

> From: Kirti Wankhede [mailto:kwankh...@nvidia.com]
> Sent: Wednesday, September 21, 2016 12:23 AM
> >
> >>>  I have
> >>> a hard time believing that a given vendor can even allocate unique type
> >>> ids for their own devices.  Unique type id across vendors is not
> >>> practical.  So which attribute are we actually using to identify which
> >>> type of mdev device we need and why would we ever specify additional
> >>> attributes like fb_length?  Doesn't the vendor guarantee that "GRID
> >>> M60-0B" has a fixed setup of those attributes?
> >>>
> >>
> >> Specifying attributes here is not our requirement. Yes we have fixed set
> >> of attributes for "GRID-M60-0B" and on.
> >> We are defining the attributed here for "display" class for all other
> >> vendor of gpu can use.
> >>

Hi, Kirti, 

We decide to go with above type-based interface for KVMGT, with fixed setup 
of attributes too. If both Intel and NVIDIA solutions use such fixed manner,
can we go with a proposal which doesn't include 'class' extension for now?
Later if there is a concrete usage requiring such class-specific attribute 
setting,
then it's easy to extend following discussion in this thread. I'm thinking how 
we
can converge the discussion here into something simple enough (and extensible)
to accelerate upstreaming of both Intel/NVIDIA solutions...

Thanks
Kevin

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

Re: [libvirt] Cpu Modeling

[Eduardo Habkost]

Hi!

On Thu, Sep 22, 2016 at 02:47:36PM -0400, Jason J. Herne wrote:
> Hi Jiri & Eduardo,
> 
> You might remember a discussion with David Hildenbrand of IBM on the Qemu
> mailing list regarding a new Qemu<->Libvirt interface for cpu modeling. I am
> picking up this work from David and I wanted to confirm that we are still on
> the
> same page as to the direction of that interface.

I am, but I would like to hear Jiri's feedback in case there's
anything that is still lacking in the new interface.

Additionanl comments below:

> 
> For your reference:
> https://www.redhat.com/archives/libvir-list/2016-June/thread.html#01413
> https://lists.gnu.org/archive/html/qemu-devel/2016-09/threads.html#00489
> 
> The first link is to the discussion you were directly involved in. The
> second
> link is to the final patch set posted to qemu-devel. The cover letter gives
> a
> good overview of the interface added to Qemu and the proposed use-case for
> Libvirt to use this new cpu modeling support. I'll paste in the most
> relevant
> section for your convenience:
> 
> Libvirt usecase
> Testing for runability:
> - Simply try to start QEMU with KVM, compat machine, CPU model

The main problem here is for libvirt to get machine-friendly
error information from QEMU in case the VM fails to start. But if
libvirt avoids starting QEMU based on previously-collected
runnability information, we should be OK.

> - Could be done using query-cpu-model-comparison in the future.

I sent a series adding runnability information to
query-cpu-definitions as well.

> 
> Identifying host model, e.g. "virsh capabilities"
> - query-cpu-model-expansion on "host" with "-M none --enable-kvm"

Correct.

> 
> :
> - simply copy the identified host model
> 
> :
> - "-cpu host"

Correct.

> 
> "virsh cpu-baseline":
> - query-cpu-model-baseline on two models with "-M none"
> 
> "virsh cpu-compare":
> - query-cpu-model-comparison on two models with "-M none"

Correct.

> 
> There might be some cenarios where libvirt wants to convert another CPU
> model to a static variant, this can be done using query-cpu-model-expansion
> ---
> 
> Now that I've hopefully refreshed your memory :) I just want to make sure
> that
> you are still on-board with this plan. I realize that this new approach does
> things differently than Libvirt does today for other platforms. Especially
> x86_64. The big differences are as follows:
> 
> 1. We will invoke qemu to gather the host cpu data used for virsh
> capabilities.
> Today this data seems to be collected directly from the host hardware for
> most
> (all?) architectures.

I believe libvirt developers want to change this, because the
current approach is wrong for x86 as well.

> 
> 2. virsh cpu-models {arch} will also use a Qemu invocation to gather cpu
> model
> data.

I believe libvirt can collect this data once, and cache it (like
it does today).

> 
> 3. Most architectures seem to use a "map" (xml file containing cpu model
> data
> that ships with Libvirt) to satisfy #1 and #2 above. Our new method does not
> use
> this map as it gets all of the data directly from Qemu.

I believe the goal is to make cpu_map.xml unnecessary on x86 as
well.

> 
> 4. virsh cpu-baseline and cpu-compare will now invoke qemu directly as well.
> 
> Note: I'm not sure exactly how much all of this will apply just to s390 with
> other architectures moving over to the new interface if/when they want to.
> Or if
> we will want to change all architectures to this new interface at the same
> time.
> Any guidance?

I am not sure about this part, because I don't know what are the
existing use cases of cpu-baseline and cpu-compare. But I believe
this shouldn't be a problem as long as the function is not called
very often and/or results are cached by libvirt.

-- 
Eduardo

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

[libvirt] Cpu Modeling

[Jason J. Herne]

Hi Jiri & Eduardo,

You might remember a discussion with David Hildenbrand of IBM on the Qemu
mailing list regarding a new Qemu<->Libvirt interface for cpu modeling. I am
picking up this work from David and I wanted to confirm that we are 
still on the

same page as to the direction of that interface.

For your reference:
https://www.redhat.com/archives/libvir-list/2016-June/thread.html#01413
https://lists.gnu.org/archive/html/qemu-devel/2016-09/threads.html#00489

The first link is to the discussion you were directly involved in. The 
second
link is to the final patch set posted to qemu-devel. The cover letter 
gives a

good overview of the interface added to Qemu and the proposed use-case for
Libvirt to use this new cpu modeling support. I'll paste in the most 
relevant

section for your convenience:

Libvirt usecase
Testing for runability:
- Simply try to start QEMU with KVM, compat machine, CPU model
- Could be done using query-cpu-model-comparison in the future.

Identifying host model, e.g. "virsh capabilities"
- query-cpu-model-expansion on "host" with "-M none --enable-kvm"

:
- simply copy the identified host model

:
- "-cpu host"

"virsh cpu-baseline":
- query-cpu-model-baseline on two models with "-M none"

"virsh cpu-compare":
- query-cpu-model-comparison on two models with "-M none"

There might be some cenarios where libvirt wants to convert another CPU
model to a static variant, this can be done using query-cpu-model-expansion
---

Now that I've hopefully refreshed your memory :) I just want to make 
sure that

you are still on-board with this plan. I realize that this new approach does
things differently than Libvirt does today for other platforms. Especially
x86_64. The big differences are as follows:

1. We will invoke qemu to gather the host cpu data used for virsh 
capabilities.
Today this data seems to be collected directly from the host hardware 
for most

(all?) architectures.

2. virsh cpu-models {arch} will also use a Qemu invocation to gather cpu 
model

data.

3. Most architectures seem to use a "map" (xml file containing cpu model 
data
that ships with Libvirt) to satisfy #1 and #2 above. Our new method does 
not use

this map as it gets all of the data directly from Qemu.

4. virsh cpu-baseline and cpu-compare will now invoke qemu directly as well.

Note: I'm not sure exactly how much all of this will apply just to s390 with
other architectures moving over to the new interface if/when they want 
to. Or if
we will want to change all architectures to this new interface at the 
same time.

Any guidance?

Thanks for your time and consideration.

--
-- Jason J. Herne (jjhe...@linux.vnet.ibm.com)

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

Re: [libvirt] [PATCH] Fix Multiple Typos

[John Ferlan]

On 09/22/2016 06:33 AM, Nitesh Konkar wrote:
> Signed-off-by: Nitesh Konkar 
> ---
>  src/conf/domain_conf.c | 4 ++--
>  src/conf/domain_conf.h | 2 +-
>  src/conf/storage_conf.c| 2 +-
>  src/esx/esx_driver.c   | 4 ++--
>  src/interface/interface_backend_udev.c | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
> 

ACK and pushed.

Tks,

John

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

Re: [libvirt] [PATCH v2] qemu: enable user-defined cache bypassing while invoking

[John Ferlan]

On 08/26/2016 02:56 AM, fuweiwei wrote:
> in the previous version, I mentioned the scenario of long-term pause while
> writing external memory checkpoints:
> 
> v1: https://www.redhat.com/archives/libvir-list/2016-August/msg01194.html

It would seem the above link has a better explanation as to what's being
fixed.  If this current patch were pushed, it would require the reader
to follow the link in order to get the explanation. Not only that but
the subsequent information would also be kept in the log.

So the "norm" has been when sending patches where there is no cover
letter, one adds "explanation" below the triple hashes (below [1])

When you create your v3, let's try to restore the v1 explanation and
anything else you add here that's pertinent...

> 
> Daniel suggested not to hardcode the flag, but wire this upto the API. When
> the user invokes the snapshot they can request the
> VIR_DOMAIN_SAVE_BYPASS_CACHE flag explicitly. So in this version I
> introduce the --bypass-cache option in libvirt snapshot API. When invoking
> an external VM snapshots, we may use the command like this:
> 
> virsh snapshot-create-as VM snap --memspec /path/to/memsnap --live
>  --bypass-cache
>  
> The VM snapshot can be done with inapparent VM suspend now. Without 

inapparent???

> "--bypass-cache" flag, one may experience long-term VM suspend (so that the 
> implication of "--live" option is not significant), provided that the VM
> has a large amount of dirty pages to save. 
> 
> Signed-off-by: fuweiwei 

We really need a better or full name in order to push this patch.
There's plenty of examples on how to do that.

> ---

[1] here

When you use git send-email you can be dumped into an editor which will
allow you to add your text here.  Anything added here won't be pushed
with the patch, but is meant to help the reviewers understand the
history.  If however you create multiple patches and have a cover
letter, then than can be used.  Hopefully this is clear.

>  include/libvirt/libvirt-domain-snapshot.h |  3 +++
>  src/qemu/qemu_driver.c| 20 ++--
>  tools/virsh-snapshot.c| 12 
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 

Caveat emptor - the snapshot code is not "in my wheelhouse"...

You need to modify tools/virsh.pod in order to describe the flag/option
along with when/how it should be used (for both snapshot-create and
create-as).  See the existing --bypass-cache flags for an example.

You need to modify the API description in src/libvirt-domain-snapshot.c
to describe this new flag just like other @flags are described and of
course similar to virsh.pod's explanation.

FWIW: I'll make use of gitk a lot to find who made a previous/similar
change and see what was changed in that commit to make sure I don't miss
anything (I still miss stuff, but it lessens the chances). In particular
the previous flag was added in commit '5f75bd4bb'.  Prior to that commit
id '922d498e1' added atomic.  You'll also note that those commits
"broke" up the patches into "virsh/libvirt" (API) and "qemu" changes.


> diff --git a/include/libvirt/libvirt-domain-snapshot.h 
> b/include/libvirt/libvirt-domain-snapshot.h
> index 0f73f24..aeff665 100644
> --- a/include/libvirt/libvirt-domain-snapshot.h
> +++ b/include/libvirt/libvirt-domain-snapshot.h
> @@ -70,6 +70,9 @@ typedef enum {
>  VIR_DOMAIN_SNAPSHOT_CREATE_LIVE= (1 << 8), /* create the snapshot
>while the guest is
>running */
> +VIR_DOMAIN_SNAPSHOT_CREATE_BYPASS_CACHE   = (1 << 9), i/* Bypass cache

You already know about the 'i'...

> +  while writing 
> external
> +  checkpoint files. 
> */
>  } virDomainSnapshotCreateFlags;
>  
>  /* Take a snapshot of the current VM state */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2089359..d5f441f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14036,6 +14036,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr 
> conn,
>  bool pmsuspended = false;
>  virQEMUDriverConfigPtr cfg = NULL;
>  int compressed = QEMU_SAVE_FORMAT_RAW;
> +unsigned int save_memory_flag = 0;
>  
>  /* If quiesce was requested, then issue a freeze command, and a
>   * counterpart thaw command when it is actually sent to agent.
> @@ -14116,8 +14117,12 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr 
> conn,
>  if (!(xml = qemuDomainDefFormatLive(driver, vm->def, true, true)))
>  goto cleanup;
>  
> +if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_BYPASS_CACHE)
> +save_memory_flag |= VIR_DOMAIN_SAVE_BYPASS_CACHE;
> +
>  if ((ret = qemuDomainSaveMemory(driver, vm, snap->def->file,
> -

[libvirt] [PATCH 7/8] qemuhelptest: regenerate data for qemu-kvm-0.13.0

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 tests/qemuhelpdata/qemu-kvm-0.13.0| 1 -
 tests/qemuhelpdata/qemu-kvm-0.13.0-device | 4 +---
 tests/qemuhelptest.c  | 3 ---
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/tests/qemuhelpdata/qemu-kvm-0.13.0 
b/tests/qemuhelpdata/qemu-kvm-0.13.0
index be2c8a5..af29d11 100644
--- a/tests/qemuhelpdata/qemu-kvm-0.13.0
+++ b/tests/qemuhelpdata/qemu-kvm-0.13.0
@@ -72,7 +72,6 @@ Display options:
 -ctrl-grab  use Right-Ctrl to grab mouse (instead of Ctrl-Alt)
 -no-quitdisable SDL window close capability
 -sdlenable SDL
--spiceuse spice
 -portrait   rotate graphical output 90 deg left (only PXA LCD)
 -vga [std|cirrus|vmware|xenfb|none]
 select video card type
diff --git a/tests/qemuhelpdata/qemu-kvm-0.13.0-device 
b/tests/qemuhelpdata/qemu-kvm-0.13.0-device
index 4888d18..6dd2d09 100644
--- a/tests/qemuhelpdata/qemu-kvm-0.13.0-device
+++ b/tests/qemuhelpdata/qemu-kvm-0.13.0-device
@@ -22,8 +22,8 @@ name "usb-braille", bus USB
 name "usb-serial", bus USB
 name "usb-net", bus USB
 name "usb-bt-dongle", bus USB
-name "virtconsole", bus virtio-serial-bus
 name "virtserialport", bus virtio-serial-bus
+name "virtconsole", bus virtio-serial-bus
 name "isa-serial", bus ISA
 name "isa-parallel", bus ISA
 name "vt82c686b-usb-uhci", bus PCI
@@ -62,8 +62,6 @@ name "ib700", bus ISA
 name "isa-debugcon", bus ISA
 name "testdev", bus ISA
 name "PIIX4_PM", bus PCI, desc "PM"
-name "qxl", bus PCI, desc "Spice QXL GPU"
-name "spicevmc", bus virtio-serial-bus
 name "pci-assign", bus PCI, desc "pass through host pci devices to the guest"
 pci-assign.host=pci-hostaddr
 pci-assign.iommu=uint32
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 5dc02d4..2fc030e 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -242,10 +242,8 @@ mymain(void)
 QEMU_CAPS_NESTING,
 QEMU_CAPS_NAME_PROCESS,
 QEMU_CAPS_SMBIOS_TYPE,
-QEMU_CAPS_SPICE,
 QEMU_CAPS_VGA_NONE,
 QEMU_CAPS_DRIVE_AIO,
-QEMU_CAPS_DEVICE_SPICEVMC,
 QEMU_CAPS_PCI_MULTIFUNCTION,
 QEMU_CAPS_PIIX3_USB_UHCI,
 QEMU_CAPS_PIIX4_USB_UHCI,
@@ -256,7 +254,6 @@ mymain(void)
 QEMU_CAPS_NO_ACPI,
 QEMU_CAPS_SCSI_LSI,
 QEMU_CAPS_VNC,
-QEMU_CAPS_DEVICE_QXL,
 QEMU_CAPS_DEVICE_VGA,
 QEMU_CAPS_DEVICE_CIRRUS_VGA,
 QEMU_CAPS_DEVICE_VMWARE_SVGA,
-- 
2.10.0

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

[libvirt] [PATCH 8/8] qemuhelptest: regenerate data for qemu-kvm-1.2.0

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 tests/qemuhelpdata/qemu-kvm-1.2.0 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemuhelpdata/qemu-kvm-1.2.0 
b/tests/qemuhelpdata/qemu-kvm-1.2.0
index 372002a..08c3f93 100644
--- a/tests/qemuhelpdata/qemu-kvm-1.2.0
+++ b/tests/qemuhelpdata/qemu-kvm-1.2.0
@@ -173,6 +173,7 @@ Character device options:
 -chardev pipe,id=id,path=path[,mux=on|off]
 -chardev pty,id=id[,mux=on|off]
 -chardev stdio,id=id[,mux=on|off][,signal=on|off]
+-chardev braille,id=id[,mux=on|off]
 -chardev tty,id=id,path=path[,mux=on|off]
 -chardev parport,id=id,path=path[,mux=on|off]
 -chardev spicevmc,id=id,name=name[,debug=debug]
-- 
2.10.0

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

[libvirt] [PATCH 0/8] cleanup and regenerate data for qemuhelptest

[Pavel Hrdina]

Pavel Hrdina (8):
  qemuhelptest: remove downstream test data
  qemuhelptest: regenerate data for qemu-0.12.1
  qemuhelptest: regenerate data for qemu-1.0
  qemuhelptest: regenerate data for qemu-1.1.0
  qemuhelptest: regenerate data for qemu-1.2.0
  qemuhelptest: regenerate data for qemu-kvm-0.12.3
  qemuhelptest: regenerate data for qemu-kvm-0.13.0
  qemuhelptest: regenerate data for qemu-kvm-1.2.0

 tests/qemuhelpdata/qemu-0.12.1 |   7 +-
 tests/qemuhelpdata/qemu-0.12.1-device  |  62 ++
 tests/qemuhelpdata/qemu-1.0|   1 +
 tests/qemuhelpdata/qemu-1.0-device | 146 ++---
 tests/qemuhelpdata/qemu-1.1.0  |   1 +
 tests/qemuhelpdata/qemu-1.1.0-device   |  20 +-
 tests/qemuhelpdata/qemu-1.2.0  |   1 +
 tests/qemuhelpdata/qemu-1.2.0-device   | 226 
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60| 227 
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device |  57 -
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61| 229 
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61-device |  99 -
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel62-beta   | 230 -
 .../qemu-kvm-0.12.1.2-rhel62-beta-device   | 120 ---
 tests/qemuhelpdata/qemu-kvm-0.12.3 |   7 +-
 tests/qemuhelpdata/qemu-kvm-0.12.3-device  |  63 ++
 tests/qemuhelpdata/qemu-kvm-0.13.0 |   1 -
 tests/qemuhelpdata/qemu-kvm-0.13.0-device  |   4 +-
 tests/qemuhelpdata/qemu-kvm-1.2.0  |   1 +
 tests/qemuhelptest.c   | 150 ++
 20 files changed, 234 insertions(+), 1418 deletions(-)
 delete mode 100644 tests/qemuhelpdata/qemu-1.2.0-device
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61-device
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel62-beta
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel62-beta-device

-- 
2.10.0

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

[libvirt] [PATCH 6/8] qemuhelptest: regenerate data for qemu-kvm-0.12.3

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 tests/qemuhelpdata/qemu-kvm-0.12.3|  7 ++--
 tests/qemuhelpdata/qemu-kvm-0.12.3-device | 63 +++
 tests/qemuhelptest.c  | 20 +-
 3 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/tests/qemuhelpdata/qemu-kvm-0.12.3 
b/tests/qemuhelpdata/qemu-kvm-0.12.3
index 54a804f..268d36f 100644
--- a/tests/qemuhelpdata/qemu-kvm-0.12.3
+++ b/tests/qemuhelpdata/qemu-kvm-0.12.3
@@ -66,7 +66,7 @@ Display options:
 -full-screenstart in full screen
 -vnc displaystart a VNC server on display
 
-1 target only:
+i386 target only:
 -win2k-hack use it when installing Windows 2000 to avoid a disk full bug
 -no-fd-bootchk  disable boot signature checking for floppy disks
 -no-acpidisable ACPI
@@ -92,7 +92,7 @@ Network options:
  [,hostfwd=rule][,guestfwd=rule][,smb=dir[,smbserver=addr]]
 connect the user mode network stack to VLAN 'n', configure its
 DHCP server and enabled optional services
--net 
tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h]
+-net 
tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off]
 connect the host TAP network interface to VLAN 'n' and use the
 network scripts 'file' (default=/etc/qemu-ifup)
 and 'dfile' (default=/etc/qemu-ifdown);
@@ -102,8 +102,6 @@ Network options:
 default of 'sndbuf=1048576' can be disabled using 'sndbuf=0'
 use vnet_hdr=off to avoid enabling the IFF_VNET_HDR tap flag; 
use
 vnet_hdr=on to make the lack of IFF_VNET_HDR support an error 
condition
-use vhost=on to enable experimental in kernel accelerator
-use 'vhostfd=h' to connect to an already opened vhost net 
device
 -net socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connect=host:port]
 connect the vlan 'n' to another VLAN using a socket connection
 -net socket[,vlan=n][,name=str][,fd=h][,mcast=maddr:port]
@@ -163,6 +161,7 @@ Debug/Expert options:
 translation (t=none or lba) (usually qemu can guess them)
 -L path set the directory for the BIOS, VGA BIOS and keymaps
 -bios file  set the filename for the BIOS
+-enable-kvm enable KVM full virtualization support
 -no-reboot  exit instead of rebooting
 -no-shutdownstop before shutdown
 -loadvm [tag|id]
diff --git a/tests/qemuhelpdata/qemu-kvm-0.12.3-device 
b/tests/qemuhelpdata/qemu-kvm-0.12.3-device
index e69de29..814dcaf 100644
--- a/tests/qemuhelpdata/qemu-kvm-0.12.3-device
+++ b/tests/qemuhelpdata/qemu-kvm-0.12.3-device
@@ -0,0 +1,63 @@
+name "pci-bridge", bus PCI
+name "virtio-balloon-pci", bus PCI
+name "virtio-console-pci", bus PCI
+name "virtio-net-pci", bus PCI
+name "virtio-blk-pci", bus PCI
+name "i82562", bus PCI
+name "i82559er", bus PCI
+name "i82559c", bus PCI
+name "i82559b", bus PCI
+name "i82559a", bus PCI
+name "i82558b", bus PCI
+name "i82558a", bus PCI
+name "i82557c", bus PCI
+name "i82557b", bus PCI
+name "i82557a", bus PCI
+name "i82551", bus PCI
+name "i82550", bus PCI
+name "pcnet", bus PCI
+name "rtl8139", bus PCI
+name "e1000", bus PCI, desc "Intel Gigabit Ethernet"
+name "ide-drive", bus IDE
+name "isa-ide", bus ISA
+name "piix4-ide", bus PCI, no-user
+name "piix3-ide", bus PCI, no-user
+name "i8042", bus ISA, no-user
+name "sb16", bus ISA, desc "Creative Sound Blaster 16"
+name "ES1370", bus PCI, desc "ENSONIQ AudioPCI ES1370"
+name "AC97", bus PCI, desc "Intel 82801AA AC97 Audio"
+name "VGA", bus PCI
+name "SUNW,fdtwo", bus System
+name "sysbus-fdc", bus System
+name "isa-fdc", bus ISA, no-user
+name "mc146818rtc", bus ISA, no-user
+name "isa-serial", bus ISA
+name "cirrus-vga", bus PCI, desc "Cirrus CLGD 54xx VGA"
+name "isa-parallel", bus ISA
+name "PIIX3", bus PCI, desc "ISA bridge", no-user
+name "i440FX", bus PCI, desc "Host bridge", no-user
+name "i440FX-pcihost", bus System, no-user
+name "piix4-usb-uhci", bus PCI
+name "piix3-usb-uhci", bus PCI
+name "vmware-svga", bus PCI
+name "ib700", bus ISA
+name "ne2k_isa", bus ISA
+name "testdev", bus ISA
+name "smbus-eeprom", bus I2C
+name "scsi-disk", bus SCSI, desc "virtual scsi disk or cdrom"
+name "scsi-generic", bus SCSI, desc "pass through generic scsi device 
(/dev/sg*)"
+name "usb-hub", bus USB
+name "usb-host", bus USB
+name "usb-kbd", bus USB
+name "usb-mouse", bus USB
+name "usb-tablet", bus USB
+name "usb-storage", bus USB
+name "usb-wacom-tablet", bus USB, desc "QEMU PenPartner Tablet"
+name "usb-braille", bus USB
+name "usb-serial", bus USB
+name "usb-net", bus USB
+name "usb-bt-dongle", bus USB
+name "i6300esb", bus PCI
+name "ne2k_pci", bus PCI
+name "lsi53c895a", bus PCI, alias "lsi"
+name "isabus-bridge", bus System, no-user
diff --git a/tests/qemuhelptest.c 

[libvirt] [PATCH 4/8] qemuhelptest: regenerate data for qemu-1.1.0

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 tests/qemuhelpdata/qemu-1.1.0|  1 +
 tests/qemuhelpdata/qemu-1.1.0-device | 20 ++--
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/tests/qemuhelpdata/qemu-1.1.0 b/tests/qemuhelpdata/qemu-1.1.0
index 761eb67..43c189d 100644
--- a/tests/qemuhelpdata/qemu-1.1.0
+++ b/tests/qemuhelpdata/qemu-1.1.0
@@ -172,6 +172,7 @@ Character device options:
 -chardev pipe,id=id,path=path[,mux=on|off]
 -chardev pty,id=id[,mux=on|off]
 -chardev stdio,id=id[,mux=on|off][,signal=on|off]
+-chardev braille,id=id[,mux=on|off]
 -chardev tty,id=id,path=path[,mux=on|off]
 -chardev parport,id=id,path=path[,mux=on|off]
 -chardev spicevmc,id=id,name=name[,debug=debug]
diff --git a/tests/qemuhelpdata/qemu-1.1.0-device 
b/tests/qemuhelpdata/qemu-1.1.0-device
index 7313a34..2186906 100644
--- a/tests/qemuhelpdata/qemu-1.1.0-device
+++ b/tests/qemuhelpdata/qemu-1.1.0-device
@@ -19,19 +19,19 @@ name "scsi-block", bus SCSI, desc "SCSI block device 
passthrough"
 name "usb-serial", bus USB
 name "pc-sysfw", bus System, desc "PC System Firmware"
 name "usb-mouse", bus USB
-name "usb-net", bus USB
 name "usb-hub", bus USB
+name "usb-net", bus USB
 name "ccid-card-emulated", bus ccid-bus, desc "emulated smartcard"
 name "ne2k_isa", bus ISA
 name "scsi-generic", bus SCSI, desc "pass through generic scsi device 
(/dev/sg*)"
 name "pcnet", bus PCI
 name "lsi53c895a", bus PCI, alias "lsi"
 name "scsi-disk", bus SCSI, desc "virtual SCSI disk or CD-ROM (legacy)"
-name "nec-usb-xhci", bus PCI
+name "hda-micro", bus HDA, desc "HDA Audio Codec, duplex (speaker, microphone)"
 name "xio3130-downstream", bus PCI, desc "TI X3130 Downstream Port of PCI 
Express Switch"
-name "pci-ohci", bus PCI, desc "Apple USB Controller"
 name "virtserialport", bus virtio-serial-bus
-name "hda-micro", bus HDA, desc "HDA Audio Codec, duplex (speaker, microphone)"
+name "nec-usb-xhci", bus PCI
+name "pci-ohci", bus PCI, desc "Apple USB Controller"
 name "usb-braille", bus USB
 name "scsi-cd", bus SCSI, desc "virtual SCSI CD-ROM"
 name "usb-wacom-tablet", bus USB, desc "QEMU PenPartner Tablet"
@@ -64,7 +64,6 @@ name "usb-tablet", bus USB
 name "isa-vga", bus ISA
 name "usb-kbd", bus USB
 name "isa-applesmc", bus ISA
-name "ib700", bus ISA
 name "rtl8139", bus PCI
 name "qxl", bus PCI, desc "Spice QXL GPU (secondary)"
 name "i82557a", bus PCI, desc "Intel i82557A Ethernet"
@@ -72,25 +71,26 @@ name "i82557b", bus PCI, desc "Intel i82557B Ethernet"
 name "i82557c", bus PCI, desc "Intel i82557C Ethernet"
 name "usb-audio", bus USB
 name "piix3-usb-uhci", bus PCI
-name "piix4-usb-uhci", bus PCI
 name "ccid-card-passthru", bus ccid-bus, desc "passthrough smartcard"
+name "ib700", bus ISA
 name "i82801", bus PCI, desc "Intel i82801 Ethernet"
 name "smbus-eeprom", bus I2C
 name "vmware-svga", bus PCI
 name "isa-cirrus-vga", bus ISA
+name "piix4-usb-uhci", bus PCI
 name "sb16", bus ISA, desc "Creative Sound Blaster 16"
 name "pci-bridge", bus PCI, desc "Standard PCI Bridge"
 name "usb-ehci", bus PCI
-name "vt82c686b-usb-uhci", bus PCI
 name "i82558a", bus PCI, desc "Intel i82558A Ethernet"
+name "i82558b", bus PCI, desc "Intel i82558B Ethernet"
 name "virtio-net-pci", bus PCI, alias "virtio-net"
 name "virtio-balloon-pci", bus PCI, alias "virtio-balloon"
 name "ich9-usb-ehci1", bus PCI
 name "isa-ide", bus ISA
-name "usb-host", bus USB
-name "ich9-ahci", bus PCI, alias "ahci"
 name "i6300esb", bus PCI
-name "i82558b", bus PCI, desc "Intel i82558B Ethernet"
+name "ich9-ahci", bus PCI, alias "ahci"
+name "usb-host", bus USB
+name "vt82c686b-usb-uhci", bus PCI
 name "virtio-scsi-pci", bus PCI
 virtio-blk-pci.class=hex32
 virtio-blk-pci.drive=drive
-- 
2.10.0

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

[libvirt] [PATCH 5/8] qemuhelptest: regenerate data for qemu-1.2.0

[Pavel Hrdina]

This patch also removes device data for qemu-1.2.0 as it was removed for
qemu-kvm-1.2.0 by commit ae3e29e6e.  They are not required because we
parse only version from help output and return with error that this qemu
is too new to use help parsing.

Signed-off-by: Pavel Hrdina 
---
 tests/qemuhelpdata/qemu-1.2.0|   1 +
 tests/qemuhelpdata/qemu-1.2.0-device | 226 ---
 2 files changed, 1 insertion(+), 226 deletions(-)
 delete mode 100644 tests/qemuhelpdata/qemu-1.2.0-device

diff --git a/tests/qemuhelpdata/qemu-1.2.0 b/tests/qemuhelpdata/qemu-1.2.0
index 78375f3..32a2431 100644
--- a/tests/qemuhelpdata/qemu-1.2.0
+++ b/tests/qemuhelpdata/qemu-1.2.0
@@ -173,6 +173,7 @@ Character device options:
 -chardev pipe,id=id,path=path[,mux=on|off]
 -chardev pty,id=id[,mux=on|off]
 -chardev stdio,id=id[,mux=on|off][,signal=on|off]
+-chardev braille,id=id[,mux=on|off]
 -chardev tty,id=id,path=path[,mux=on|off]
 -chardev parport,id=id,path=path[,mux=on|off]
 -chardev spicevmc,id=id,name=name[,debug=debug]
diff --git a/tests/qemuhelpdata/qemu-1.2.0-device 
b/tests/qemuhelpdata/qemu-1.2.0-device
deleted file mode 100644
index bfc3a4d..000
--- a/tests/qemuhelpdata/qemu-1.2.0-device
+++ /dev/null
@@ -1,226 +0,0 @@
-name "VGA", bus PCI
-name "usb-storage", bus usb-bus
-name "scsi-hd", bus SCSI, desc "virtual SCSI disk"
-name "i82559a", bus PCI, desc "Intel i82559A Ethernet"
-name "i82559b", bus PCI, desc "Intel i82559B Ethernet"
-name "i82559c", bus PCI, desc "Intel i82559C Ethernet"
-name "esp", bus System
-name "sysbus-ohci", bus System, desc "OHCI USB Controller"
-name "virtio-blk-pci", bus PCI, alias "virtio-blk"
-name "usb-uas", bus usb-bus
-name "qxl-vga", bus PCI, desc "Spice QXL GPU (primary, vga compatible)"
-name "ide-drive", bus IDE, desc "virtual IDE disk or CD-ROM (legacy)"
-name "x3130-upstream", bus PCI, desc "TI X3130 Upstream Port of PCI Express 
Switch"
-name "cirrus-vga", bus PCI, desc "Cirrus CLGD 54xx VGA"
-name "ide-hd", bus IDE, desc "virtual IDE disk"
-name "ES1370", bus PCI, desc "ENSONIQ AudioPCI ES1370"
-name "ioh3420", bus PCI, desc "Intel IOH device id 3420 PCIE Root Port"
-name "sga", bus ISA, desc "Serial Graphics Adapter"
-name "scsi-block", bus SCSI, desc "SCSI block device passthrough"
-name "usb-serial", bus usb-bus
-name "pc-sysfw", bus System, desc "PC System Firmware"
-name "usb-mouse", bus usb-bus
-name "usb-net", bus usb-bus
-name "usb-hub", bus usb-bus
-name "ccid-card-emulated", bus ccid-bus, desc "emulated smartcard"
-name "ne2k_isa", bus ISA
-name "scsi-generic", bus SCSI, desc "pass through generic scsi device 
(/dev/sg*)"
-name "pcnet", bus PCI
-name "lsi53c895a", bus PCI, alias "lsi"
-name "scsi-disk", bus SCSI, desc "virtual SCSI disk or CD-ROM (legacy)"
-name "hda-micro", bus HDA, desc "HDA Audio Codec, duplex (speaker, microphone)"
-name "pci-ohci", bus PCI, desc "Apple USB Controller"
-name "nec-usb-xhci", bus PCI
-name "xio3130-downstream", bus PCI, desc "TI X3130 Downstream Port of PCI 
Express Switch"
-name "virtserialport", bus virtio-serial-bus
-name "usb-redir", bus usb-bus
-name "usb-braille", bus usb-bus
-name "scsi-cd", bus SCSI, desc "virtual SCSI CD-ROM"
-name "usb-wacom-tablet", bus usb-bus, desc "QEMU PenPartner Tablet"
-name "isa-serial", bus ISA
-name "i82550", bus PCI, desc "Intel i82550 Ethernet"
-name "i82551", bus PCI, desc "Intel i82551 Ethernet"
-name "isa-debugcon", bus ISA
-name "ide-cd", bus IDE, desc "virtual IDE CD-ROM"
-name "SUNW,fdtwo", bus System
-name "ich9-usb-uhci2", bus PCI
-name "ich9-usb-uhci3", bus PCI
-name "ich9-usb-uhci1", bus PCI
-name "isa-parallel", bus ISA
-name "virtconsole", bus virtio-serial-bus
-name "ne2k_pci", bus PCI
-name "virtio-serial-pci", bus PCI, alias "virtio-serial"
-name "hda-duplex", bus HDA, desc "HDA Audio Codec, duplex (line-out, line-in)"
-name "intel-hda", bus PCI, desc "Intel HD Audio Controller"
-name "megasas", bus PCI, desc "LSI MegaRAID SAS 1078"
-name "i82559er", bus PCI, desc "Intel i82559ER Ethernet"
-name "hda-output", bus HDA, desc "HDA Audio Codec, output-only (line-out)"
-name "i82562", bus PCI, desc "Intel i82562 Ethernet"
-name "sysbus-ahci", bus System
-name "usb-ccid", bus usb-bus, desc "CCID Rev 1.1 smartcard reader"
-name "ivshmem", bus PCI
-name "AC97", bus PCI, desc "Intel 82801AA AC97 Audio"
-name "e1000", bus PCI, desc "Intel Gigabit Ethernet"
-name "sysbus-fdc", bus System
-name "usb-bt-dongle", bus usb-bus
-name "usb-tablet", bus usb-bus
-name "isa-vga", bus ISA
-name "usb-kbd", bus usb-bus
-name "isa-applesmc", bus ISA
-name "rtl8139", bus PCI
-name "qxl", bus PCI, desc "Spice QXL GPU (secondary)"
-name "i82557a", bus PCI, desc "Intel i82557A Ethernet"
-name "i82557c", bus PCI, desc "Intel i82557C Ethernet"
-name "usb-audio", bus usb-bus
-name "ib700", bus ISA
-name "piix3-usb-uhci", bus PCI
-name "i82557b", bus PCI, desc "Intel i82557B Ethernet"
-name "piix4-usb-uhci", bus PCI
-name "ccid-card-passthru", bus ccid-bus, desc 

[libvirt] [PATCH 2/8] qemuhelptest: regenerate data for qemu-0.12.1

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 tests/qemuhelpdata/qemu-0.12.1|  7 +---
 tests/qemuhelpdata/qemu-0.12.1-device | 62 +++
 tests/qemuhelptest.c  | 18 +-
 3 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/tests/qemuhelpdata/qemu-0.12.1 b/tests/qemuhelpdata/qemu-0.12.1
index 748625b..0caa3b3 100644
--- a/tests/qemuhelpdata/qemu-0.12.1
+++ b/tests/qemuhelpdata/qemu-0.12.1
@@ -65,7 +65,7 @@ Display options:
 -full-screenstart in full screen
 -vnc displaystart a VNC server on display
 
-1 target only:
+i386 target only:
 -win2k-hack use it when installing Windows 2000 to avoid a disk full bug
 -no-fd-bootchk  disable boot signature checking for floppy disks
 -no-acpidisable ACPI
@@ -161,11 +161,6 @@ Debug/Expert options:
 -L path set the directory for the BIOS, VGA BIOS and keymaps
 -bios file  set the filename for the BIOS
 -enable-kvm enable KVM full virtualization support
--xen-domid id   specify xen guest domain id
--xen-create create domain using xen hypercalls, bypassing xend
-warning: should not be used when xend is in use
--xen-attach attach to existing xen domain
-xend will use this when starting qemu
 -no-reboot  exit instead of rebooting
 -no-shutdownstop before shutdown
 -loadvm [tag|id]
diff --git a/tests/qemuhelpdata/qemu-0.12.1-device 
b/tests/qemuhelpdata/qemu-0.12.1-device
index e69de29..63c7bef 100644
--- a/tests/qemuhelpdata/qemu-0.12.1-device
+++ b/tests/qemuhelpdata/qemu-0.12.1-device
@@ -0,0 +1,62 @@
+name "pci-bridge", bus PCI
+name "virtio-balloon-pci", bus PCI
+name "virtio-console-pci", bus PCI
+name "virtio-net-pci", bus PCI
+name "virtio-blk-pci", bus PCI
+name "i82562", bus PCI
+name "i82559er", bus PCI
+name "i82559c", bus PCI
+name "i82559b", bus PCI
+name "i82559a", bus PCI
+name "i82558b", bus PCI
+name "i82558a", bus PCI
+name "i82557c", bus PCI
+name "i82557b", bus PCI
+name "i82557a", bus PCI
+name "i82551", bus PCI
+name "i82550", bus PCI
+name "pcnet", bus PCI
+name "rtl8139", bus PCI
+name "e1000", bus PCI, desc "Intel Gigabit Ethernet"
+name "ide-drive", bus IDE
+name "isa-ide", bus ISA
+name "piix4-ide", bus PCI, no-user
+name "piix3-ide", bus PCI, no-user
+name "i8042", bus ISA, no-user
+name "sb16", bus ISA, desc "Creative Sound Blaster 16"
+name "ES1370", bus PCI, desc "ENSONIQ AudioPCI ES1370"
+name "AC97", bus PCI, desc "Intel 82801AA AC97 Audio"
+name "VGA", bus PCI
+name "SUNW,fdtwo", bus System
+name "sysbus-fdc", bus System
+name "isa-fdc", bus ISA, no-user
+name "mc146818rtc", bus ISA, no-user
+name "isa-serial", bus ISA
+name "cirrus-vga", bus PCI, desc "Cirrus CLGD 54xx VGA"
+name "isa-parallel", bus ISA
+name "PIIX3", bus PCI, desc "ISA bridge", no-user
+name "i440FX", bus PCI, desc "Host bridge", no-user
+name "i440FX-pcihost", bus System, no-user
+name "piix4-usb-uhci", bus PCI
+name "piix3-usb-uhci", bus PCI
+name "vmware-svga", bus PCI
+name "ib700", bus ISA
+name "ne2k_isa", bus ISA
+name "smbus-eeprom", bus I2C
+name "scsi-disk", bus SCSI, desc "virtual scsi disk or cdrom"
+name "scsi-generic", bus SCSI, desc "pass through generic scsi device 
(/dev/sg*)"
+name "usb-hub", bus USB
+name "usb-host", bus USB
+name "usb-kbd", bus USB
+name "usb-mouse", bus USB
+name "usb-tablet", bus USB
+name "usb-storage", bus USB
+name "usb-wacom-tablet", bus USB, desc "QEMU PenPartner Tablet"
+name "usb-braille", bus USB
+name "usb-serial", bus USB
+name "usb-net", bus USB
+name "usb-bt-dongle", bus USB
+name "i6300esb", bus PCI
+name "ne2k_pci", bus PCI
+name "lsi53c895a", bus PCI, alias "lsi"
+name "isabus-bridge", bus System, no-user
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 3d0dd8d..4def27b 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -163,9 +163,25 @@ mymain(void)
 QEMU_CAPS_SMBIOS_TYPE,
 QEMU_CAPS_VGA_NONE,
 QEMU_CAPS_DRIVE_AIO,
+QEMU_CAPS_PIIX3_USB_UHCI,
+QEMU_CAPS_PIIX4_USB_UHCI,
+QEMU_CAPS_USB_HUB,
 QEMU_CAPS_NO_SHUTDOWN,
 QEMU_CAPS_NO_ACPI,
-QEMU_CAPS_VNC);
+QEMU_CAPS_SCSI_LSI,
+QEMU_CAPS_VNC,
+QEMU_CAPS_DEVICE_VGA,
+QEMU_CAPS_DEVICE_CIRRUS_VGA,
+QEMU_CAPS_DEVICE_VMWARE_SVGA,
+QEMU_CAPS_DEVICE_USB_SERIAL,
+QEMU_CAPS_DEVICE_USB_NET,
+QEMU_CAPS_DEVICE_PCI_BRIDGE,
+QEMU_CAPS_DEVICE_SCSI_GENERIC,
+QEMU_CAPS_DEVICE_USB_STORAGE,
+QEMU_CAPS_DEVICE_USB_KBD,
+QEMU_CAPS_DEVICE_RTL8139,
+QEMU_CAPS_DEVICE_E1000,
+QEMU_CAPS_DEVICE_VIRTIO_NET);
 DO_TEST("qemu-kvm-0.12.3", 12003, 1, 0,
 QEMU_CAPS_DRIVE_BOOT,
 QEMU_CAPS_KVM,
-- 
2.10.0

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

[libvirt] [PATCH 3/8] qemuhelptest: regenerate data for qemu-1.0

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 tests/qemuhelpdata/qemu-1.0|   1 +
 tests/qemuhelpdata/qemu-1.0-device | 146 ++---
 2 files changed, 74 insertions(+), 73 deletions(-)

diff --git a/tests/qemuhelpdata/qemu-1.0 b/tests/qemuhelpdata/qemu-1.0
index aa93651..d6af0ec 100644
--- a/tests/qemuhelpdata/qemu-1.0
+++ b/tests/qemuhelpdata/qemu-1.0
@@ -161,6 +161,7 @@ Character device options:
 -chardev pipe,id=id,path=path[,mux=on|off]
 -chardev pty,id=id[,mux=on|off]
 -chardev stdio,id=id[,mux=on|off][,signal=on|off]
+-chardev braille,id=id[,mux=on|off]
 -chardev tty,id=id,path=path[,mux=on|off]
 -chardev parport,id=id,path=path[,mux=on|off]
 -chardev spicevmc,id=id,name=name[,debug=debug]
diff --git a/tests/qemuhelpdata/qemu-1.0-device 
b/tests/qemuhelpdata/qemu-1.0-device
index d557f0e..a249dd2 100644
--- a/tests/qemuhelpdata/qemu-1.0-device
+++ b/tests/qemuhelpdata/qemu-1.0-device
@@ -1,8 +1,71 @@
-name "AC97", bus PCI, desc "Intel 82801AA AC97 Audio"
-name "isa-applesmc", bus ISA
-name "ccid-card-emulated", bus ccid-bus, desc "emulated smartcard"
-name "ccid-card-passthru", bus ccid-bus, desc "passthrough smartcard"
-name "e1000", bus PCI, desc "Intel Gigabit Ethernet"
+name "ib700", bus ISA
+name "sga", bus ISA, desc "Serial Graphics Adapter"
+name "qxl", bus PCI, desc "Spice QXL GPU (secondary)"
+name "qxl-vga", bus PCI, desc "Spice QXL GPU (primary, vga compatible)"
+name "ivshmem", bus PCI
+name "isa-debugcon", bus ISA
+name "cirrus-vga", bus PCI, desc "Cirrus CLGD 54xx VGA"
+name "virtio-9p-pci", bus PCI
+name "usb-wacom-tablet", bus USB, desc "QEMU PenPartner Tablet"
+name "usb-braille", bus USB
+name "usb-serial", bus USB
+name "usb-net", bus USB
+name "usb-storage", bus USB
+name "usb-host", bus USB
+name "usb-hub", bus USB
+name "usb-kbd", bus USB
+name "usb-mouse", bus USB
+name "usb-tablet", bus USB
+name "usb-bt-dongle", bus USB
+name "smbus-eeprom", bus I2C
+name "scsi-generic", bus SCSI, desc "pass through generic scsi device 
(/dev/sg*)"
+name "scsi-disk", bus SCSI, desc "virtual SCSI disk or CD-ROM (legacy)"
+name "scsi-block", bus SCSI, desc "SCSI block device passthrough"
+name "scsi-cd", bus SCSI, desc "virtual SCSI CD-ROM"
+name "scsi-hd", bus SCSI, desc "virtual SCSI disk"
+name "x3130-upstream", bus PCI, desc "TI X3130 Upstream Port of PCI Express 
Switch"
+name "xio3130-downstream", bus PCI, desc "TI X3130 Downstream Port of PCI 
Express Switch"
+name "i6300esb", bus PCI
+name "vmware-svga", bus PCI
+name "virtio-balloon-pci", bus PCI, alias "virtio-balloon"
+name "virtio-serial-pci", bus PCI, alias "virtio-serial"
+name "virtio-net-pci", bus PCI, alias "virtio-net"
+name "virtio-blk-pci", bus PCI, alias "virtio-blk"
+name "virtserialport", bus virtio-serial-bus
+name "virtconsole", bus virtio-serial-bus
+name "VGA", bus PCI
+name "isa-vga", bus ISA
+name "ich9-usb-uhci3", bus PCI
+name "ich9-usb-uhci2", bus PCI
+name "ich9-usb-uhci1", bus PCI
+name "vt82c686b-usb-uhci", bus PCI
+name "piix4-usb-uhci", bus PCI
+name "piix3-usb-uhci", bus PCI
+name "sysbus-ohci", bus System, desc "OHCI USB Controller"
+name "pci-ohci", bus PCI, desc "Apple USB Controller"
+name "ich9-usb-ehci1", bus PCI
+name "usb-ehci", bus PCI
+name "usb-ccid", bus USB, desc "CCID Rev 1.1 smartcard reader"
+name "isa-serial", bus ISA
+name "sb16", bus ISA, desc "Creative Sound Blaster 16"
+name "rtl8139", bus PCI
+name "pcnet", bus PCI
+name "isa-parallel", bus ISA
+name "ne2k_pci", bus PCI
+name "ne2k_isa", bus ISA
+name "lsi53c895a", bus PCI, alias "lsi"
+name "ioh3420", bus PCI, desc "Intel IOH device id 3420 PCIE Root Port"
+name "intel-hda", bus PCI, desc "Intel HD Audio Controller"
+name "ide-drive", bus IDE, desc "virtual IDE disk or CD-ROM (legacy)"
+name "ide-cd", bus IDE, desc "virtual IDE CD-ROM"
+name "ide-hd", bus IDE, desc "virtual IDE disk"
+name "isa-ide", bus ISA
+name "ich9-ahci", bus PCI, alias "ahci"
+name "hda-duplex", bus HDA, desc "HDA Audio Codec, duplex"
+name "hda-output", bus HDA, desc "HDA Audio Codec, output-only"
+name "SUNW,fdtwo", bus System
+name "sysbus-fdc", bus System
+name "ES1370", bus PCI, desc "ENSONIQ AudioPCI ES1370"
 name "i82801", bus PCI, desc "Intel i82801 Ethernet"
 name "i82562", bus PCI, desc "Intel i82562 Ethernet"
 name "i82559er", bus PCI, desc "Intel i82559ER Ethernet"
@@ -16,74 +79,11 @@ name "i82557b", bus PCI, desc "Intel i82557B Ethernet"
 name "i82557a", bus PCI, desc "Intel i82557A Ethernet"
 name "i82551", bus PCI, desc "Intel i82551 Ethernet"
 name "i82550", bus PCI, desc "Intel i82550 Ethernet"
-name "ES1370", bus PCI, desc "ENSONIQ AudioPCI ES1370"
-name "SUNW,fdtwo", bus System
-name "sysbus-fdc", bus System
-name "hda-duplex", bus HDA, desc "HDA Audio Codec, duplex"
-name "hda-output", bus HDA, desc "HDA Audio Codec, output-only"
-name "ich9-ahci", bus PCI, alias "ahci"
-name "isa-ide", bus ISA
-name "ide-drive", bus IDE, desc "virtual IDE disk or CD-ROM (legacy)"
-name "ide-cd", bus IDE, desc 

[libvirt] [PATCH 1/8] qemuhelptest: remove downstream test data

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60| 227 
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device |  57 -
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61| 229 
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61-device |  99 -
 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel62-beta   | 230 -
 .../qemu-kvm-0.12.1.2-rhel62-beta-device   | 120 ---
 tests/qemuhelptest.c   | 145 -
 7 files changed, 1107 deletions(-)
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60-device
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel61-device
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel62-beta
 delete mode 100644 tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel62-beta-device

diff --git a/tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60 
b/tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60
deleted file mode 100644
index 3a2ecf3..000
--- a/tests/qemuhelpdata/qemu-kvm-0.12.1.2-rhel60
+++ /dev/null
@@ -1,227 +0,0 @@
-QEMU PC emulator version 0.12.1 (qemu-kvm-0.12.1.2), Copyright (c) 2003-2008 
Fabrice Bellard
-
-WARNING: Direct use of qemu-kvm from the command line is unsupported.
-WARNING: Only use via libvirt.
-WARNING: Some options listed here may not be available in future releases.
-
-usage: qemu [options] [disk_image]
-
-'disk_image' is a raw hard image image for IDE hard disk 0
-
-Standard options:
--h or -help display this help and exit
--versiondisplay version information and exit
--M machine  select emulated machine (-M ? for list)
--cpu cpuselect CPU (-cpu ? for list)
--smp n[,maxcpus=cpus][,cores=cores][,threads=threads][,sockets=sockets]
-set the number of CPUs to 'n' [default=1]
-maxcpus= maximum number of total cpus, including
-  offline CPUs for hotplug etc.
-cores= number of CPU cores on one socket
-threads= number of threads on one CPU core
-sockets= number of discrete sockets in the system
--numa node[,mem=size][,cpus=cpu[-cpu]][,nodeid=node]
--fda/-fdb file  use 'file' as floppy disk 0/1 image
--hda/-hdb file  use 'file' as IDE hard disk 0/1 image
--hdc/-hdd file  use 'file' as IDE hard disk 2/3 image
--cdrom file use 'file' as IDE cdrom image (cdrom is ide1 master)
--drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]
-   [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]
-   [,cache=writethrough|writeback|none][,format=f][,serial=s]
-   [,addr=A][,id=name][,aio=threads|native]
-   [,boot=on|off][,readonly=on|off]
-use 'file' as a drive image
--set group.id.arg=value
-set  parameter for item  of type 
-i.e. -set drive.$id.file=/path/to/image
--global driver.property=value
-set a global default for a driver property
--mtdblock file  use 'file' as on-board Flash memory image
--sd fileuse 'file' as SecureDigital card image
--pflash fileuse 'file' as a parallel flash image
--boot [order=drives][,once=drives][,menu=on|off]
-'drives': floppy (a), hard disk (c), CD-ROM (d), network (n)
--snapshot   write to temporary files instead of disk image files
--m megs set virtual RAM size to megs MB [default=128]
--k language use keyboard layout (for example 'fr' for French)
--audio-help print list of audio drivers and their options
--soundhw c1,... enable audio support
-and only specified sound cards (comma separated list)
-use -soundhw ? to get the list of supported cards
-use -soundhw all to enable all of them
--usbenable the USB driver (will be the default soon)
--usbdevice name add the host or guest USB device 'name'
--device driver[,prop[=value][,...]]
-add device (based on driver)
-prop=value,... sets driver properties
-use -device ? to print all possible drivers
-use -device driver,? to print all possible properties
--name string1[,process=string2]set the name of the guest
-string1 sets the window title and string2 the process name (on 
Linux)
--uuid %08x-%04x-%04x-%04x-%012x
-specify machine UUID
-
-Display options:
--nographic  disable graphical output and redirect serial I/Os to console
--spiceuse spice
--portrait   rotate graphical output 90 deg left (only PXA LCD)
--vga [std|cirrus|vmware|xenfb|qxl|none]
-select video card type
--full-screenstart in full screen
--vnc displaystart a VNC server on display
-
-i386 target only:
--win2k-hack use it when installing Windows 2000 to avoid a disk full bug
--no-fd-bootchk  

[libvirt] [PATCH 1/2] vz: add serial number to disk devices

[Nikolay Shirokovskiy]

vz sdk supports setting serial number only for disk devices.
Getting serial upon cdrom(for example) is error however
setting is just ignored. Let's check for disk device
explicitly for clarity in both cases.

Setting serial number for other devices is ignored
with an info note just as before.

We need usual conversion from "" to NULL in direction
vz sdk -> libvirt, because "" is not valid for libvirt
and "" means unspecifiend in vz sdk which is NULL for libvirt.
---
 src/vz/vz_sdk.c   | 13 +
 src/vz/vz_utils.c |  6 +++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..a38f2af 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -639,6 +639,14 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
 
 disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
 
+if (!isCdrom) {
+if (!(disk->serial = 
prlsdkGetStringParamVar(PrlVmDevHd_GetSerialNumber, prldisk)))
+goto cleanup;
+
+if (*disk->serial == '\0')
+VIR_FREE(disk->serial);
+}
+
 ret = 0;
 
  cleanup:
@@ -3492,6 +3500,11 @@ static int prlsdkConfigureDisk(vzDriverPtr driver,
 pret = PrlVmDev_SetStackIndex(sdkdisk, idx);
 prlsdkCheckRetGoto(pret, cleanup);
 
+if (devType == PDE_HARD_DISK) {
+pret = PrlVmDevHd_SetSerialNumber(sdkdisk, disk->serial);
+prlsdkCheckRetGoto(pret, cleanup);
+}
+
 return 0;
  cleanup:
 PrlHandle_Free(sdkdisk);
diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
index eaf09f2..81429d2 100644
--- a/src/vz/vz_utils.c
+++ b/src/vz/vz_utils.c
@@ -323,9 +323,9 @@ vzCheckDiskUnsupportedParams(virDomainDiskDefPtr disk)
 return -1;
 }
 
-if (disk->serial) {
-VIR_INFO("%s", _("Setting disk serial number is not "
- "supported by vz driver."));
+if (disk->serial && disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
+VIR_INFO("%s", _("Setting disk serial number is "
+ "supported only for disk devices."));
 }
 
 if (disk->wwn) {
-- 
1.8.3.1

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

[libvirt] [PATCH 0/2] vz: add serial number to disk devices

[Nikolay Shirokovskiy]

Only the first patch is really on the subject. The second one is
bugfix that is included mainly because it touches the same place
(and nice to have to test first patch too...)

Nikolay Shirokovskiy (2):
  vz: add serial number to disk devices
  vz: set something in disk driver name

 src/vz/vz_sdk.c   | 16 
 src/vz/vz_utils.c |  6 +++---
 2 files changed, 19 insertions(+), 3 deletions(-)

-- 
1.8.3.1

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

[libvirt] [PATCH 2/2] vz: set something in disk driver name

[Nikolay Shirokovskiy]

Absent driver name attribute is invalid xml. Which in turn makes
unusable 'virsh edit' for example. The value does not make
much sense and ignored on input so nobody will hurt.
---
 src/vz/vz_sdk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index a38f2af..0af450b 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -647,6 +647,9 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
 VIR_FREE(disk->serial);
 }
 
+if (virDomainDiskSetDriver(disk, "vz") < 0)
+goto cleanup;
+
 ret = 0;
 
  cleanup:
-- 
1.8.3.1

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

Re: [libvirt] [RFC v2] libvirt vGPU QEMU integration

[Daniel P. Berrange]

On Thu, Sep 22, 2016 at 08:19:21AM -0600, Alex Williamson wrote:
> On Thu, 22 Sep 2016 09:41:20 +0530
> Kirti Wankhede  wrote:
> 
> > > My concern is that a type id seems arbitrary but we're specifying that
> > > it be unique.  We already have something unique, the name.  So why try
> > > to make the type id unique as well?  A vendor can accidentally create
> > > their vendor driver so that a given name means something very
> > > specific.  On the other hand they need to be extremely deliberate to
> > > coordinate that a type id means a unique thing across all their 
> > > product
> > > lines.
> > >   
> > 
> >  Let me clarify, type id should be unique in the list of
> >  mdev_supported_types. You can't have 2 directories in with same name.  
> >    
> > >>>
> > >>> Of course, but does that mean it's only unique to the machine I'm
> > >>> currently running on?  Let's say I have a Tesla P100 on my system and
> > >>> type-id 11 is named "GRID-M60-0B".  At some point in the future I
> > >>> replace the Tesla P100 with a Q1000 (made up).  Is type-id 11 on that
> > >>> new card still going to be a "GRID-M60-0B"?  If not then we've based
> > >>> our XML on the wrong attribute.  If the new device does not support
> > >>> "GRID-M60-0B" then we should generate an error, not simply initialize
> > >>> whatever type-id 11 happens to be on this new card.
> > >>> 
> > >>
> > >> If there are 2 M60 in the system then you would find '11' type directory
> > >> in mdev_supported_types of both M60. If you have P100, '11' type would
> > >> not be there in its mdev_supported_types, it will have different types.
> > >>
> > >> For example, if you replace M60 with P100, but XML is not updated. XML
> > >> have type '11'. When libvirt would try to create mdev device, libvirt
> > >> would have to find 'create' file in sysfs in following directory format:
> > >>
> > >>  --- mdev_supported_types
> > >>  |-- 11
> > >>  |   |-- create
> > >>
> > >> but now for P100, '11' directory is not there, so libvirt should throw
> > >> error on not able to find '11' directory.  
> > > 
> > > This really seems like an accident waiting to happen.  What happens
> > > when the user replaces their M60 with an Intel XYZ device that happens
> > > to expose a type 11 mdev class gpu device?  How is libvirt supposed to
> > > know that the XML used to refer to a GRID-M60-0B and now it's an
> > > INTEL-IGD-XYZ?  Doesn't basing the XML entry on the name and removing
> > > yet another arbitrary requirement that we have some sort of globally
> > > unique type-id database make a lot of sense?  The same issue applies
> > > for simple debug-ability, if I'm reviewing the XML for a domain and the
> > > name is the primary index for the mdev device, I know what it is.
> > > Seeing type-id='11' is meaningless.
> > >  
> > 
> > Let me clarify again, type '11' is a string that vendor driver would
> > define (see my previous reply below) it could be "11" or "GRID-M60-0B".
> > If 2 vendors used same string we can't control that. right?
> > 
> > 
> >  Lets remove 'id' from type id in XML if that is the concern. Supported
> >  types is going to be defined by vendor driver, so let vendor driver
> >  decide what to use for directory name and same should be used in device
> >  xml file, it could be '11' or "GRID M60-0B":
> > 
> >  
> >    my-vgpu
> >    pci__86_00_0
> >    
> >  

Re: [libvirt] [RFC v2] libvirt vGPU QEMU integration

[Alex Williamson]

On Thu, 22 Sep 2016 09:41:20 +0530
Kirti Wankhede  wrote:

> > My concern is that a type id seems arbitrary but we're specifying that
> > it be unique.  We already have something unique, the name.  So why try
> > to make the type id unique as well?  A vendor can accidentally create
> > their vendor driver so that a given name means something very
> > specific.  On the other hand they need to be extremely deliberate to
> > coordinate that a type id means a unique thing across all their product
> > lines.
> >   
> 
>  Let me clarify, type id should be unique in the list of
>  mdev_supported_types. You can't have 2 directories in with same name.
> >>>
> >>> Of course, but does that mean it's only unique to the machine I'm
> >>> currently running on?  Let's say I have a Tesla P100 on my system and
> >>> type-id 11 is named "GRID-M60-0B".  At some point in the future I
> >>> replace the Tesla P100 with a Q1000 (made up).  Is type-id 11 on that
> >>> new card still going to be a "GRID-M60-0B"?  If not then we've based
> >>> our XML on the wrong attribute.  If the new device does not support
> >>> "GRID-M60-0B" then we should generate an error, not simply initialize
> >>> whatever type-id 11 happens to be on this new card.
> >>> 
> >>
> >> If there are 2 M60 in the system then you would find '11' type directory
> >> in mdev_supported_types of both M60. If you have P100, '11' type would
> >> not be there in its mdev_supported_types, it will have different types.
> >>
> >> For example, if you replace M60 with P100, but XML is not updated. XML
> >> have type '11'. When libvirt would try to create mdev device, libvirt
> >> would have to find 'create' file in sysfs in following directory format:
> >>
> >>  --- mdev_supported_types
> >>  |-- 11
> >>  |   |-- create
> >>
> >> but now for P100, '11' directory is not there, so libvirt should throw
> >> error on not able to find '11' directory.  
> > 
> > This really seems like an accident waiting to happen.  What happens
> > when the user replaces their M60 with an Intel XYZ device that happens
> > to expose a type 11 mdev class gpu device?  How is libvirt supposed to
> > know that the XML used to refer to a GRID-M60-0B and now it's an
> > INTEL-IGD-XYZ?  Doesn't basing the XML entry on the name and removing
> > yet another arbitrary requirement that we have some sort of globally
> > unique type-id database make a lot of sense?  The same issue applies
> > for simple debug-ability, if I'm reviewing the XML for a domain and the
> > name is the primary index for the mdev device, I know what it is.
> > Seeing type-id='11' is meaningless.
> >  
> 
> Let me clarify again, type '11' is a string that vendor driver would
> define (see my previous reply below) it could be "11" or "GRID-M60-0B".
> If 2 vendors used same string we can't control that. right?
> 
> 
>  Lets remove 'id' from type id in XML if that is the concern. Supported
>  types is going to be defined by vendor driver, so let vendor driver
>  decide what to use for directory name and same should be used in device
>  xml file, it could be '11' or "GRID M60-0B":
> 
>  
>    my-vgpu
>    pci__86_00_0
>    
>  

Re: [libvirt] [PATCHv2] qemu: agent: Fix QEMU guest agent is not available due to an error

[Yaowei Bai]

Cc'ed Michal and Nikolay.

On Wed, Sep 21, 2016 at 07:01:37AM -0400, John Ferlan wrote:
> On 07/07/2016 05:53 AM, Xiubo Li wrote:
> > 
> >  src/qemu/qemu_agent.c | 37 -
> >  src/util/virjson.h|  7 +++
> >  2 files changed, 35 insertions(+), 9 deletions(-)
> > 
> 
> Since this has been sitting around for a long time with a response -

Yes, we sent the patch more than 2 months ago,exactly at 07/07/2016.
And then we sent a ping mail wanting to get any response from libvirt
community. But unfortunately there's none.

> figured I'd point out something that was pushed today:

John, thank you for your kindly remainder. We know it just now and find
out that we were even not cc'ed when it happened.

> 
> http://www.redhat.com/archives/libvir-list/2016-September/msg00570.html
> 
> It addresses/resolves the issue in essentially the same manner, although
> it doesn't check the empty {} reply, rather it just makes the else
> condition be a ignored delay reply  There's also a few more patches
> to the series.

Michal, could you please explain why you merged a later-sent patch rather than 
the
first-sent one? We worked very hard to solve this issue and contribute the patch
to fix it. And we always hopefully want to exchange with you guys from the 
libvirt 
community. 

Our company focus on cloud computing technology and we contribute to qemu, linux
kernel, kvm and openstack community. But this situation is the first time we met
in our long-time contribution to these communities. This is the traditional of
libvirt community or something?

We strongly recommend the later patch series rebase on our patch.

Thanks.

> 
> John


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

Re: [libvirt] [PATCH 0/9] Couple of migration fixes

[Michal Privoznik]

On 22.09.2016 15:39, Jiri Denemark wrote:
> On Tue, Sep 20, 2016 at 15:54:54 +0200, Michal Privoznik wrote:
>> Looks like nobody tried migrations lately.
> 
> This statement sounds a bit too strong for such a narrow use case.
> 
>> I just did and found
>> interesting bug. On the destination qemu binary was at different
>> path. So I've dumped domain XML I was trying ti migrate and just
>> changed the emulator path. All of a sudden I observed plenty of
>> errors. Problem is that whilst parsing the user provided
>> migration XML (and doing other operations on it), because of
>> parse callbacks we need qemuCaps for the binary. However, we just
>> take the def->emulator and expect it to exist. Well, it doesn't.
> 
> I think you're just the first one who tried migrating to a host where
> QEMU is installed in a different path. Or at least the first one who
> didn't just make a symlink to make QEMU binary reachable with the same
> path :-)
> 
>> Michal Privoznik (9):
>>   conf: Introduce virDomainDefPostParseOpaque
>>   conf: Introduce virDomainDefParseStringOpaque
>>   qemuDomainDefPostParse: Fetch qemuCaps from domain object
>>   conf: Extend virDomainDeviceDefPostParse for parseOpaque
>>   qemuDomainDeviceDefPostParse: Fetch caps from domain object
>>   conf: Extend virDomainDefAssignAddressesCallback for parseOpaque
>>   qemuDomainDefAssignAddresses: Fetch caps from domain object
>>   domain_conf: Introduce VIR_DOMAIN_DEF_PARSE_SKIP_POST_PARSE
>>   conf: Skip post parse callbacks when creating copy
> 
> Overall, the series looks OK, except for the new *Opaque functions. They
> are all internal APIs and I think just adding an extra parameter to the
> existing functions is much better than introducing new ones with even
> longer and uglier names. And git grep doesn't even show a lot of callers
> so the change to the existing prototypes won't be too invasive.

Ah, okay, I can stick with that approach then. I thought that this is
going to be more acceptable to the upstream. Don't really know why now.

> 
> And do you plan to actually use vm pointers anywhere in the post-parse
> callbacks in the future? If it's not the case, I think passing
> priv->qemuCaps directly would be a bit better.

Well, okay. I think domain objects are more versatile, but if somebody
needs them, we can always change that.

So let me respin version 2.

Michal

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

Re: [libvirt] [PATCH v2 00/45] Enhance guest CPU configuration code

[Jiri Denemark]

On Thu, Sep 22, 2016 at 08:48:16 -0400, John Ferlan wrote:
> [...]
> 
> ACK series,

Pushed, thanks a lot for the review.

Jirka

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

Re: [libvirt] [PATCH 0/9] Couple of migration fixes

[Jiri Denemark]

On Tue, Sep 20, 2016 at 15:54:54 +0200, Michal Privoznik wrote:
> Looks like nobody tried migrations lately.

This statement sounds a bit too strong for such a narrow use case.

> I just did and found
> interesting bug. On the destination qemu binary was at different
> path. So I've dumped domain XML I was trying ti migrate and just
> changed the emulator path. All of a sudden I observed plenty of
> errors. Problem is that whilst parsing the user provided
> migration XML (and doing other operations on it), because of
> parse callbacks we need qemuCaps for the binary. However, we just
> take the def->emulator and expect it to exist. Well, it doesn't.

I think you're just the first one who tried migrating to a host where
QEMU is installed in a different path. Or at least the first one who
didn't just make a symlink to make QEMU binary reachable with the same
path :-)

> Michal Privoznik (9):
>   conf: Introduce virDomainDefPostParseOpaque
>   conf: Introduce virDomainDefParseStringOpaque
>   qemuDomainDefPostParse: Fetch qemuCaps from domain object
>   conf: Extend virDomainDeviceDefPostParse for parseOpaque
>   qemuDomainDeviceDefPostParse: Fetch caps from domain object
>   conf: Extend virDomainDefAssignAddressesCallback for parseOpaque
>   qemuDomainDefAssignAddresses: Fetch caps from domain object
>   domain_conf: Introduce VIR_DOMAIN_DEF_PARSE_SKIP_POST_PARSE
>   conf: Skip post parse callbacks when creating copy

Overall, the series looks OK, except for the new *Opaque functions. They
are all internal APIs and I think just adding an extra parameter to the
existing functions is much better than introducing new ones with even
longer and uglier names. And git grep doesn't even show a lot of callers
so the change to the existing prototypes won't be too invasive.

And do you plan to actually use vm pointers anywhere in the post-parse
callbacks in the future? If it's not the case, I think passing
priv->qemuCaps directly would be a bit better.

Jirka

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

Re: [libvirt] [PATCH v2 00/45] Enhance guest CPU configuration code

[John Ferlan]

On 09/19/2016 09:30 AM, Jiri Denemark wrote:
> Version 2:
> - review comments addressed; see individual patches for details
> 
> This patch series does several things:
> 
> - fixes tests to avoid relying on bugs in our code
> - adds support for advertising supported CPU modes and models in domain
>   capabilities
> - starts adding better and higher level APIs to our cpu driver (the old
>   low level APIs will be removed once this process is over)
> - prepares qemu driver for asking QEMU what a host CPU looks like and
>   what CPU models can be run on it
> - makes QEMU CPU command line builder build command line and nothing
>   else
> 
> The added part of domain capabilities XML looks like this:
> 
> 
> 
> 
> Broadwell
> Intel
> 
> 
> 
> 
> Broadwell
> Broadwell-noTSX
> ...
> 
> 
> 
> and host-passthrough is only advertised as supported for KVM domains,
> host-model is only supported when guest architecture is compatible with
> host and overall it should just work the way one would expect (in
> contrast to the current state of the code).
> 

[...]

ACK series,

John


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

Re: [libvirt] [libvirt-php][PATCH 0/2] Couple of example fixes

[Martin Kletzander]

On Wed, Sep 21, 2016 at 05:33:50PM +0200, Michal Privoznik wrote:

Pushed under "I'm the PHP master" rule.



LOL, ACK series, then, I guess =D


Michal Privoznik (2):
 examples: Sort domains
 examples: Die early if getting a screenshot fails

examples/index.php   | 2 +-
examples/libvirt.php | 8 
2 files changed, 9 insertions(+), 1 deletion(-)

--
2.8.4

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


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

Re: [libvirt] [PATCH] qemu: fix libvirtd crash in migration after vm shutdown

[Jiri Denemark]

On Thu, Sep 22, 2016 at 11:56:38 +0200, Jiri Denemark wrote:
> On Tue, Aug 02, 2016 at 02:20:51 +, weifuqiang wrote:
> > [PATCH] qemu: fix libvirtd crash in migration after vm shutdown
> > 
> > 
> > 
> > 
> > 
> > If we shutdown a guest, then migrate it without the arg XML, libvirtd will 
> > get crashed.
> > 
> > 
> > 
> > The reason is that:
> > 
> > 1 during shutdown callback, qemuProcessStop() , it points vm->def  to 
> > vm->newDef
> > 
> > 2 during migration, it frees persistentDef, which points to vm->newDef when 
> > the arg XML is NULL.
> > 
> >However, because vm->newDef is now vm->def, what we IN FACT freed is 
> > vm->def.
> > 
> > 3 it will refer to vm->def after step2, thus invalid read/write causes 
> > libvirtd crash
> > 
> > 
> > 
> > We needn't to free persistentDef if persist_xml is NULL, because no extra 
> > def was alloced if persistent_xml is NULL.
> > 
> > 
> > ---
> > src/qemu/qemu_migration.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 6a683f7..3636c93 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -4915,7 +4915,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
> >  VIR_WARN("Unable to encode migration cookie");
> >  }
> > -if (persistDef != vm->newDef)
> > +if (persist_xml && persistDef)
> >  virDomainDefFree(persistDef);
> >  qemuMigrationCookieFree(mig);
> 
> If persist_xml != NULL then persistDef is it's parsed version (or NULL)
> and we need to free it. Otherwise it's just a copy of another pointer.
> In other words, your patch is correct (although checking for persist_xml
> is enough).
> 
> An alternative solution would be to make sure we can always free
> persistDef by making a copy of vm->newDef rather than copying just the
> pointer and free it unconditionally.
> 
> Anyway, your patch is good enough. ACK.
> 
> I'll push it later.

Oh, the patch is corrupted and thus it doesn't apply. And it's hard to
see what your real name is. Is it Wei Fuqiang or something else? We need
proper author's name for each commit.

Please, when sending a new version of this patch, don't copy the
patch into your mail client. Use git send-email whenever possible, or
attach the patch generated by git format-patch to the email.

Jirka

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

Re: [libvirt] [PATCH 6/7] qemu: Support newer ivshmem device variants

[Daniel P. Berrange]

On Thu, Sep 22, 2016 at 01:09:20PM +0200, Martin Kletzander wrote:
> On Wed, Sep 21, 2016 at 04:26:31PM +0100, Daniel P. Berrange wrote:
> > On Wed, Sep 21, 2016 at 05:10:09PM +0200, Martin Kletzander wrote:
> > > On Wed, Sep 21, 2016 at 04:01:23PM +0100, Daniel P. Berrange wrote:
> > > > I'm not a fan of the idea of silently picking a different device
> > > > for the guest behind the applications back. By not exposing the
> > > > different device types with a "model" attribute, we miss a way
> > > > to report to the application which models are supported by the
> > > > QEMU they're using - eg via domain capabilities.
> > > >
> > > > This in turn means the application doesn't know whether they're
> > > > getting the new or old version, and so don't know if they're going
> > > > to have working migration or not.
> > > >
> > > > If we expanded the XML to include model, then application can
> > > > explicitly request the new model (which supports migration) and
> > > > know that they'll get a startup failure if not supported, as
> > > > opposed to silently falling back to the non-migratable version.
> > > >
> > > > Also, it makes life hard for us if the ivshmem-plain device wants
> > > > to support use of the 'server' attribute in the future, as we will
> > > > then not know which to create.
> > > >
> > > > We've often been burnt in the past by trying todo magic like this,
> > > > instead of explicitly representing stuff in the XML, so I think we
> > > > should be being explicit about ivshmem models here.
> > > >
> > > > Of course, if we do add  support, we have to allow for it
> > > > to be missing for sake of upgrades. So there's a question of which
> > > > model we should select as the default, if not seen in the XML.
> > > >
> > > 
> > > If selecting the newest one whenever the element is missing is fine,
> > > then I'm OK with that.  But that would change the device when upgrading
> > > libvirt (without user intervention), which you didn't like IIUC.
> > 
> > Yes, I don't think we can do that - at least note exactly in the way
> > you do it in this patch. eg Looking at the ivshmem code in QEMU there
> > is this comment about guest interupt setup:
> > 
> > * Do nothing unless the device actually uses INTx.  Here's how
> > * the device variants signal interrupts, what they put in PCI
> > * config space:
> > * Device variantInterrupt  Interrupt Pin  MSI-X cap.
> > * ivshmem-plain none0 no
> > * ivshmem-doorbell MSI-X1yes(1)
> > * ivshmem,msi=off   INTx1 no
> > * ivshmem,msi=on   MSI-X1(2) yes(1)
> > * (1) if guest enabled MSI-X
> > * (2) the device lies
> > 
> > Note that neither ivshmem-plain or ivshmem-doorbell support use of
> > INTx for interupts. I'm also concerned about the footnote (2) there,
> > as that seems to imply that ivshmem,msi=on, is not in fact the
> > same as ivshmem-doorbell, because ivshmem lies about the interrupt
> > pin (whatever that means).
> > 
> > I'm inclined so that that we should do
> > 
> >  if (ivshmem exists) {
> > use ivshmem
> >  } else {
> > if (server) {
> >if(msi) {
> >use ivshmem-doorbell
> > } else {
> >error config unsupported
> > }
> > } else {
> >use ivshmem-plain
> > }
> >  }
> > 
> > That way if a distro compiles-out 'ivshmem' we'll use one of the
> > new devices if they're available, otherwise we'll stick with the
> > conversative behaviour of using the legacy device for guaranteed
> > bug compatibility.
> > 
> 
> OK, we can do that.  But before I go and do this variant, I would like
> to clarify two more things (so that I can hope that's the last variant I
> have to post) =)  Should we support setting the role to 'peer'/'master'
> (with ivshmem that maps to role=peer/master and with ivshmem-plain that
> maps to master=on/off)?  Basically master means that the domain can
> migrate (with the data being copied) and peer (or master=off) has
> migration disabled in qemu, so we would disable that as well.  That's
> why hotplug is being implemented, basically.  Currently we don't use
> that parameter, which means role=auto.  That is special value that ends
> up being 'master' for non-server, for server it tries to pick correctly.
> Shouldn't be used, but rather explicitly stated (or just peer for
> everyone and copy the data yourself).  New ivshmem defaults to master=off.

Yep, given that the choice of master/peer impacts whether you can migrate
or not, I think it is important to give applications the ability to
set that, to override the potentially incorrect defaults.

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

Re: [libvirt] [PATCH v2 0/3] pass the path of compress prog directly

[Chen Hanxiao]

At 2016-09-22 19:05:33, "John Ferlan"  wrote:
>
>>>
>>> BTW: Patch 2 fails syntax-check
>>>
>> 
>> I make syntax-check again and passed on a centos7 machine.
>> Maybe it's a syntax-check bug?
>> 
>
>Don't think so...
>
>
>cppi: src/qemu/qemu_driver.c: line 3269: not properly indented
>maint.mk: incorrect preprocessor indentation
>cfg.mk:680: recipe for target 'sc_preprocessor_indentation' failed
>make: *** [sc_preprocessor_indentation] Error 1
>make: *** Waiting for unfinished jobs

I didn't install cppi on that centos7 machine.
Sorry for the noise.

>
>Line 3269:
>
>
># define VIR_QEMU_COMPRESS_CHECK(val, type)
>   \
>do {
>   \
>if (cfg->val) {
>
>...
>
>If the "# define" is changed to "#define", then the syntax-check passes.
>The usage of "# define" is done/allowed with an "#if" or "#ifdef" type
>construct in order to 'mark' that we're within some sort of preprocessor
>condition.
>
>Additionally those lines are too long - use 80 character columns

I need to re-read HACKING :)
Thanks for your help.

Regards,
- Chen



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

Re: [libvirt] [PATCH 6/7] qemu: Support newer ivshmem device variants

[Martin Kletzander]

On Wed, Sep 21, 2016 at 04:26:31PM +0100, Daniel P. Berrange wrote:

On Wed, Sep 21, 2016 at 05:10:09PM +0200, Martin Kletzander wrote:

On Wed, Sep 21, 2016 at 04:01:23PM +0100, Daniel P. Berrange wrote:
> I'm not a fan of the idea of silently picking a different device
> for the guest behind the applications back. By not exposing the
> different device types with a "model" attribute, we miss a way
> to report to the application which models are supported by the
> QEMU they're using - eg via domain capabilities.
>
> This in turn means the application doesn't know whether they're
> getting the new or old version, and so don't know if they're going
> to have working migration or not.
>
> If we expanded the XML to include model, then application can
> explicitly request the new model (which supports migration) and
> know that they'll get a startup failure if not supported, as
> opposed to silently falling back to the non-migratable version.
>
> Also, it makes life hard for us if the ivshmem-plain device wants
> to support use of the 'server' attribute in the future, as we will
> then not know which to create.
>
> We've often been burnt in the past by trying todo magic like this,
> instead of explicitly representing stuff in the XML, so I think we
> should be being explicit about ivshmem models here.
>
> Of course, if we do add  support, we have to allow for it
> to be missing for sake of upgrades. So there's a question of which
> model we should select as the default, if not seen in the XML.
>

If selecting the newest one whenever the element is missing is fine,
then I'm OK with that.  But that would change the device when upgrading
libvirt (without user intervention), which you didn't like IIUC.


Yes, I don't think we can do that - at least note exactly in the way
you do it in this patch. eg Looking at the ivshmem code in QEMU there
is this comment about guest interupt setup:

* Do nothing unless the device actually uses INTx.  Here's how
* the device variants signal interrupts, what they put in PCI
* config space:
* Device variantInterrupt  Interrupt Pin  MSI-X cap.
* ivshmem-plain none0 no
* ivshmem-doorbell MSI-X1yes(1)
* ivshmem,msi=off   INTx1 no
* ivshmem,msi=on   MSI-X1(2) yes(1)
* (1) if guest enabled MSI-X
* (2) the device lies

Note that neither ivshmem-plain or ivshmem-doorbell support use of
INTx for interupts. I'm also concerned about the footnote (2) there,
as that seems to imply that ivshmem,msi=on, is not in fact the
same as ivshmem-doorbell, because ivshmem lies about the interrupt
pin (whatever that means).

I'm inclined so that that we should do

 if (ivshmem exists) {
use ivshmem
 } else {
if (server) {
   if(msi) {
   use ivshmem-doorbell
} else {
   error config unsupported
}
} else {
   use ivshmem-plain
}
 }

That way if a distro compiles-out 'ivshmem' we'll use one of the
new devices if they're available, otherwise we'll stick with the
conversative behaviour of using the legacy device for guaranteed
bug compatibility.



OK, we can do that.  But before I go and do this variant, I would like
to clarify two more things (so that I can hope that's the last variant I
have to post) =)  Should we support setting the role to 'peer'/'master'
(with ivshmem that maps to role=peer/master and with ivshmem-plain that
maps to master=on/off)?  Basically master means that the domain can
migrate (with the data being copied) and peer (or master=off) has
migration disabled in qemu, so we would disable that as well.  That's
why hotplug is being implemented, basically.  Currently we don't use
that parameter, which means role=auto.  That is special value that ends
up being 'master' for non-server, for server it tries to pick correctly.
Shouldn't be used, but rather explicitly stated (or just peer for
everyone and copy the data yourself).  New ivshmem defaults to master=off.


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


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/3] pass the path of compress prog directly

[John Ferlan]

>>
>> BTW: Patch 2 fails syntax-check
>>
> 
> I make syntax-check again and passed on a centos7 machine.
> Maybe it's a syntax-check bug?
> 

Don't think so...


cppi: src/qemu/qemu_driver.c: line 3269: not properly indented
maint.mk: incorrect preprocessor indentation
cfg.mk:680: recipe for target 'sc_preprocessor_indentation' failed
make: *** [sc_preprocessor_indentation] Error 1
make: *** Waiting for unfinished jobs

Line 3269:


# define VIR_QEMU_COMPRESS_CHECK(val, type)
   \
do {
   \
if (cfg->val) {

...

If the "# define" is changed to "#define", then the syntax-check passes.
The usage of "# define" is done/allowed with an "#if" or "#ifdef" type
construct in order to 'mark' that we're within some sort of preprocessor
condition.

Additionally those lines are too long - use 80 character columns


John

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

[libvirt] [PATCH] virsh domdisplay: introduce '--all' for showing all possible graphical display

[Chen Hanxiao]

From: Chen Hanxiao 

For one VM, it could have more than one graphical display.
Such as we coud add both vnc and spice display to a VM.

This patch introduces '--all' for showing all
possible type of graphical display for a active VM.

Signed-off-by: Chen Hanxiao 
---
 tools/virsh-domain.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3829b17..7194153 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10648,6 +10648,10 @@ static const vshCmdOptDef opts_domdisplay[] = {
  .help = N_("select particular graphical display "
 "(e.g. \"vnc\", \"spice\", \"rdp\")")
 },
+{.name = "all",
+ .type = VSH_OT_BOOL,
+ .help = N_("show all possible graphical displays")
+},
 {.name = NULL}
 };
 
@@ -10671,6 +10675,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 int tmp;
 int flags = 0;
 bool params = false;
+bool all = false;
 const char *xpath_fmt = "string(/domain/devices/graphics[@type='%s']/%s)";
 virSocketAddr addr;
 
@@ -10685,6 +10690,9 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptBool(cmd, "include-password"))
 flags |= VIR_DOMAIN_XML_SECURE;
 
+if (vshCommandOptBool(cmd, "all"))
+all = true;
+
 if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
 goto cleanup;
 
@@ -10845,7 +10853,15 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
 
 /* We got what we came for so return successfully */
 ret = true;
-break;
+if (!all) {
+break;
+} else {
+VIR_FREE(xpath);
+VIR_FREE(passwd);
+VIR_FREE(listen_addr);
+VIR_FREE(output);
+vshPrint(ctl, "\n");
+}
 }
 
 if (!ret) {
-- 
1.8.3.1


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

Re: [libvirt] [PATCH 0/7] Fix some typos.

[Nitesh Konkar]

Thanks for the feedback. I have now sent them as a single patch.

On Thu, Sep 22, 2016 at 12:52 PM, Peter Krempa  wrote:

> On Thu, Sep 22, 2016 at 03:33:39 +0530, Nitesh Konkar wrote:
> > *** BLURB HERE ***
> >
> > Nitesh Konkar (7):
> >   domain_conf.c:fix a typo
> >   domain_conf.h:fix a typo
> >   domain_conf.c:fix a typo
> >   storage_conf.c:fix a typo
> >   interface_backend_udev.c:fix a typo
> >   esx_driver.c:fix a typo
> >   esx_driver.c:fix a typo
>
> I don't think such changes are worth splitting into separate commits.
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix Multiple Typos

[Nitesh Konkar]

Signed-off-by: Nitesh Konkar 
---
 src/conf/domain_conf.c | 4 ++--
 src/conf/domain_conf.h | 2 +-
 src/conf/storage_conf.c| 2 +-
 src/esx/esx_driver.c   | 4 ++--
 src/interface/interface_backend_udev.c | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5fede3d..dd34cec 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14165,7 +14165,7 @@ void 
virDomainControllerInsertPreAlloced(virDomainDefPtr def,
  virDomainControllerDefPtr controller)
 {
 int idx;
-/* Tenatively plan to insert controller at the end. */
+/* Tentatively plan to insert controller at the end. */
 int insertAt = -1;
 virDomainControllerDefPtr current = NULL;
 
@@ -19847,7 +19847,7 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
 
 /* virDomainSourceDefFormatSeclabel:
  *
- * This function automaticaly closes the  element and formats any
+ * This function automatically closes the  element and formats any
  * possible seclabels.
  */
 static void
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d4a84c3..ce90c27 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -300,7 +300,7 @@ typedef enum {
 
 /* the backend driver used for PCI hostdev devices */
 typedef enum {
-VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, /* detect automaticaly, prefer 
VFIO */
+VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT, /* detect automatically, prefer 
VFIO */
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,/* force legacy kvm style */
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO,   /* force vfio */
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN,/* force legacy xen style, use 
pciback */
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 05a1a49..7e7bb72 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2476,7 +2476,7 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn,
 if (def->type != pool->def->type)
 continue;
 
-/* Don't mach against ourself if re-defining existing pool ! */
+/* Don't match against ourself if re-defining existing pool ! */
 if (STREQ(pool->def->name, def->name))
 continue;
 
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 3d90b69..5ce98b3 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -78,7 +78,7 @@ esxFreePrivate(esxPrivate **priv)
 
 /*
  * Parse a file name from a .vmx file and convert it to datastore path format
- * if possbile. A .vmx file can contain file names in various formats:
+ * if possible. A .vmx file can contain file names in various formats:
  *
  * - A single name referencing a file in the same directory as the .vmx file:
  *
@@ -969,7 +969,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
 STRNEQ(vCenterIPAddress, potentialVCenterIPAddress)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("This host is managed by a vCenter with 
IP "
- "address %s, but a mismachting vCenter 
'%s' "
+ "address %s, but a mismatching vCenter 
'%s' "
  "(%s) has been specified"),
potentialVCenterIPAddress, 
priv->parsedUri->vCenter,
vCenterIPAddress);
diff --git a/src/interface/interface_backend_udev.c 
b/src/interface/interface_backend_udev.c
index 27a772d..5d0fc64 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -884,7 +884,7 @@ udevGetIfaceDefBridge(struct udev *udev,
 goto error;
 ifacedef->data.bridge.nbItf = member_count;
 
-/* Get the interface defintions for each member of the bridge */
+/* Get the interface definitions for each member of the bridge */
 for (i = 0; i < member_count; i++) {
 ifacedef->data.bridge.itf[i] =
 udevGetIfaceDef(udev, member_list[i]->d_name);
-- 
2.1.0

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

Re: [libvirt] [PATCH] qemu: fix libvirtd crash in migration after vm shutdown

[Jiri Denemark]

On Tue, Aug 02, 2016 at 02:20:51 +, weifuqiang wrote:
> [PATCH] qemu: fix libvirtd crash in migration after vm shutdown
> 
> 
> 
> 
> 
> If we shutdown a guest, then migrate it without the arg XML, libvirtd will 
> get crashed.
> 
> 
> 
> The reason is that:
> 
> 1 during shutdown callback, qemuProcessStop() , it points vm->def  to 
> vm->newDef
> 
> 2 during migration, it frees persistentDef, which points to vm->newDef when 
> the arg XML is NULL.
> 
>However, because vm->newDef is now vm->def, what we IN FACT freed is 
> vm->def.
> 
> 3 it will refer to vm->def after step2, thus invalid read/write causes 
> libvirtd crash
> 
> 
> 
> We needn't to free persistentDef if persist_xml is NULL, because no extra def 
> was alloced if persistent_xml is NULL.
> 
> 
> ---
> src/qemu/qemu_migration.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 6a683f7..3636c93 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -4915,7 +4915,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>  VIR_WARN("Unable to encode migration cookie");
>  }
> -if (persistDef != vm->newDef)
> +if (persist_xml && persistDef)
>  virDomainDefFree(persistDef);
>  qemuMigrationCookieFree(mig);

If persist_xml != NULL then persistDef is it's parsed version (or NULL)
and we need to free it. Otherwise it's just a copy of another pointer.
In other words, your patch is correct (although checking for persist_xml
is enough).

An alternative solution would be to make sure we can always free
persistDef by making a copy of vm->newDef rather than copying just the
pointer and free it unconditionally.

Anyway, your patch is good enough. ACK.

I'll push it later.

Jirka

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

Re: [libvirt] [PATCH 2/4] daemon: call free callbacks synchronously in event loop impl

[Nikolay Shirokovskiy]

On 22.09.2016 11:44, Daniel P. Berrange wrote:
> On Thu, Sep 22, 2016 at 11:38:50AM +0300, Nikolay Shirokovskiy wrote:
>> Default event loop impl delays deleting registered handles and
>> timeouts so that deleting is safe from event handlers. But
>> this means deletions after event loop is finished will never
>> occur and particularly assosiated callback objects will not
>> be freed. For this reason network clients that exist at
>> the moment of libvirtd daemon exit are never get disposed
>> and daemon object itself too if there are admin server clients.
>>
>> Let's call free callbacks immediately we don't need to delay
>> this operation, only removing handles from the list.
>>
>> This breaks virnetsocket.c immediately as socket is locked
>> in freeing callback and callback is called synchronously
>> from virNetSocketRemoveIOCallback which holds the lock already.
>> So fix it to pass intermediate callback object to the loop
>> instead of socket itself. I've checked other users of
>> virEventAddHandle, looks like they don't have such problems.
> 
> That is impossible to guarantee. The event loop impl is exposed
> via the public API, so arbitrary external applications get to
> call it.
> 
> So I don't think it is safe to make this change.
> 

Ok, the other I option I think of is to add call to indicate event loop
is finished so that there is no need to defer cleanups. 


Nikolay

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

Re: [libvirt] [PATCH 2/4] daemon: call free callbacks synchronously in event loop impl

[Daniel P. Berrange]

On Thu, Sep 22, 2016 at 11:38:50AM +0300, Nikolay Shirokovskiy wrote:
> Default event loop impl delays deleting registered handles and
> timeouts so that deleting is safe from event handlers. But
> this means deletions after event loop is finished will never
> occur and particularly assosiated callback objects will not
> be freed. For this reason network clients that exist at
> the moment of libvirtd daemon exit are never get disposed
> and daemon object itself too if there are admin server clients.
> 
> Let's call free callbacks immediately we don't need to delay
> this operation, only removing handles from the list.
> 
> This breaks virnetsocket.c immediately as socket is locked
> in freeing callback and callback is called synchronously
> from virNetSocketRemoveIOCallback which holds the lock already.
> So fix it to pass intermediate callback object to the loop
> instead of socket itself. I've checked other users of
> virEventAddHandle, looks like they don't have such problems.

That is impossible to guarantee. The event loop impl is exposed
via the public API, so arbitrary external applications get to
call it.

So I don't think it is safe to make this change.


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

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

[libvirt] [PATCH 0/4] fix some libvirtd cleanup leaks and crashes

[Nikolay Shirokovskiy]

Crash situation of last 2 patches can easily simulated, just
patch libvirt:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 30a2830..f6b71d6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4515,6 +4515,8 @@ processMonitorEOFEvent(virQEMUDriverPtr driver,
 unsigned int stopFlags = 0;
 virObjectEventPtr event = NULL;
 
+sleep(3);
+
 if (qemuProcessBeginStopJob(driver, vm, QEMU_JOB_DESTROY, true) < 0)
 return;

then when it is up and running and there is also a qemu domain running:

kill -9 $QEMU_DOMAIN && kill libvirtd

If first 2 patches are not applied yet then make sure there is no admin
server clients present.

Nikolay Shirokovskiy (4):
  daemon: break cyclic deps between admin clients and daemon
  daemon: call free callbacks synchronously in event loop impl
  daemon: keep daemon until all hypervisors drivers are cleaned up
  qemu: first wait all workers finish on state cleanup

 daemon/libvirtd.c   |  7 -
 src/qemu/qemu_driver.c  |  2 +-
 src/rpc/virnetdaemon.c  |  1 +
 src/rpc/virnetsocket.c  | 70 ++---
 src/util/vireventpoll.c | 24 +++--
 5 files changed, 50 insertions(+), 54 deletions(-)

-- 
1.8.3.1

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

[libvirt] [PATCH 3/4] daemon: keep daemon until all hypervisors drivers are cleaned up

[Nikolay Shirokovskiy]

This fix SEGV with next backtrace (shortened a bit):

1  0x7fd3a791b2e6 in virCondWait (c=, m=) at 
util/virthread.c:154
2  0x7fd3a791bcb0 in virThreadPoolFree (pool=0x7fd38024ee00) at 
util/virthreadpool.c:266
3  0x7fd38edaa00e in qemuStateCleanup () at qemu/qemu_driver.c:1116
4  0x7fd3a79abfeb in virStateCleanup () at libvirt.c:808
5  0x7fd3a85f2c9e in main (argc=, argv=) at 
libvirtd.c:1660

Thread 1 (Thread 0x7fd38722d700 (LWP 32256)):
0  0x7fd3a7900910 in virClassIsDerivedFrom (klass=0xdfd36058d4853, 
parent=0x7fd3a8f394d0) at util/virobject.c:169
1  0x7fd3a7900c4e in virObjectIsClass (anyobj=anyobj@entry=0x7fd3a8f2f850, 
klass=) at util/virobject.c:365
2  0x7fd3a7900c74 in virObjectLock (anyobj=0x7fd3a8f2f850) at 
util/virobject.c:317
3  0x7fd3a7a24d5d in virNetDaemonRemoveShutdownInhibition 
(dmn=0x7fd3a8f2f850) at rpc/virnetdaemon.c:547
4  0x7fd38ed722cf in qemuProcessStop (driver=driver@entry=0x7fd380103810, 
vm=vm@entry=0x7fd38025b6d0, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN, 
asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE,
flags=flags@entry=0) at qemu/qemu_process.c:5786
5  0x7fd38edd9428 in processMonitorEOFEvent (vm=0x7fd38025b6d0, 
driver=0x7fd380103810) at qemu/qemu_driver.c:4588
6  qemuProcessEventHandler (data=, opaque=0x7fd380103810) at 
qemu/qemu_driver.c:4632
7  0x7fd3a791bb55 in virThreadPoolWorker 
(opaque=opaque@entry=0x7fd3a8f1e4c0) at util/virthreadpool.c:145

This happens due to race on simultaneous finishing of libvirtd and
qemu process. We need to keep daemon object until all hypervisor
drivers are cleaned up. The other option is to take reference to
daemon in every hypervisor driver but this will take more work
and we really don't need to. Drivers cleanup procedure is synchronous
thus let's just remove last reference to daemon after drivers cleanup.
---
 daemon/libvirtd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 7fac7b2..97a7587 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1631,7 +1631,6 @@ int main(int argc, char **argv) {
 /* we need to keep servers references up to here
   so that above function will not cause servers cleanup
   which can deadlock */
-virObjectUnref(dmn);
 virObjectUnref(srv);
 virObjectUnref(srvAdm);
 virNetlinkShutdown();
@@ -1661,6 +1660,9 @@ int main(int argc, char **argv) {
 driversInitialized = false;
 virStateCleanup();
 }
+/* unref daemon only here as hypervisor drivers can
+   call shutdown inhibition functions on cleanup */
+virObjectUnref(dmn);
 
 return ret;
 }
-- 
1.8.3.1

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

[libvirt] [PATCH 4/4] qemu: first wait all workers finish on state cleanup

[Nikolay Shirokovskiy]

This fix next crash (bt is shortened a bit):

0  0x77282f2b in virClassIsDerivedFrom (klass=0xdeadbeef, 
parent=0x5581d650) at util/virobject.c:169
1  0x772835fd in virObjectIsClass (anyobj=0x7fffd024f580, 
klass=0x5581d650) at util/virobject.c:365
2  0x77283498 in virObjectLock (anyobj=0x7fffd024f580) at 
util/virobject.c:317
3  0x7722f0a3 in virCloseCallbacksUnset (closeCallbacks=0x7fffd024f580, 
vm=0x7fffd0194db0,
cb=0x7fffdf1af765 ) at util/virclosecallbacks.c:164
4  0x7fffdf1afa7b in qemuProcessAutoDestroyRemove (driver=0x7fffd00f3a60, 
vm=0x7fffd0194db0)
at qemu/qemu_process.c:6365
5  0x7fffdf1adff1 in qemuProcessStop (driver=0x7fffd00f3a60, 
vm=0x7fffd0194db0,
reason=VIR_DOMAIN_SHUTOFF_CRASHED, asyncJob=QEMU_ASYNC_JOB_NONE, flags=0) 
at qemu/qemu_process.c:5877
6  0x7fffdf1f711c in processMonitorEOFEvent (driver=0x7fffd00f3a60, 
vm=0x7fffd0194db0)
at qemu/qemu_driver.c:4545
7  0x7fffdf1f7313 in qemuProcessEventHandler (data=0x55832710, 
opaque=0x7fffd00f3a60)
at qemu/qemu_driver.c:4589
8  0x772a84c4 in virThreadPoolWorker (opaque=0x55805da0) at 
util/virthreadpool.c:167

Thread 1 (Thread 0x77fb1880 (LWP 494472)):
1  0x772a7898 in virCondWait (c=0x7fffd01c21f8, m=0x7fffd01c21a0) at 
util/virthread.c:154
2  0x772a8a22 in virThreadPoolFree (pool=0x7fffd01c2160) at 
util/virthreadpool.c:290
3  0x7fffdf1edd44 in qemuStateCleanup () at qemu/qemu_driver.c:1102
4  0x7736570a in virStateCleanup () at libvirt.c:807
5  0x5556f991 in main (argc=1, argv=0x7fffe458) at libvirtd.c:1660

This happens on race conditions of simultaneous exiting libvirtd and qemu 
process.
Let's just finish all workers that keep driver pointer before disposing
driver object or any of its members.
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e29180d..30a2830 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1068,6 +1068,7 @@ qemuStateCleanup(void)
 if (!qemu_driver)
 return -1;
 
+virThreadPoolFree(qemu_driver->workerPool);
 virNWFilterUnRegisterCallbackDriver();
 virObjectUnref(qemu_driver->config);
 virObjectUnref(qemu_driver->hostdevMgr);
@@ -1099,7 +1100,6 @@ qemuStateCleanup(void)
 virLockManagerPluginUnref(qemu_driver->lockManager);
 
 virMutexDestroy(_driver->lock);
-virThreadPoolFree(qemu_driver->workerPool);
 VIR_FREE(qemu_driver);
 
 return 0;
-- 
1.8.3.1

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

[libvirt] [PATCH 1/4] daemon: break cyclic deps between admin clients and daemon

[Nikolay Shirokovskiy]

Libvirtd daemon cleanup is not correct in case there are admin server
clients connected at the moment libvirtd exits. The reason is
that there is cyclic dependency: daemon holds references to its
network servers, networks servers hold references to its clients,
admin server clients hold refs to daemon. These refs are kept until
holder object dispose thus the above objects are never get disposed.

Let's drop references to severs in daemon object at appropriate moment
on cleanup procedure to break the cycle.

The caller should keep references to servers by itself. Otherwise
dropping refences can cause synchronous servers cleanup which
can cause deadlock (server cleanup blocks until workers are
finished and workers can call daemon functions which will try
to get daemon lock). Other option is to make list cleanup
via temporary list copy.
---
 daemon/libvirtd.c  | 3 +++
 src/rpc/virnetdaemon.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 95c1b1c..7fac7b2 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1628,6 +1628,9 @@ int main(int argc, char **argv) {
 virObjectUnref(qemuProgram);
 virObjectUnref(adminProgram);
 virNetDaemonClose(dmn);
+/* we need to keep servers references up to here
+  so that above function will not cause servers cleanup
+  which can deadlock */
 virObjectUnref(dmn);
 virObjectUnref(srv);
 virObjectUnref(srvAdm);
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index dcc89fa..c6c6522 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -861,6 +861,7 @@ virNetDaemonClose(virNetDaemonPtr dmn)
 virObjectLock(dmn);
 
 virHashForEach(dmn->servers, daemonServerClose, NULL);
+virHashRemoveAll(dmn->servers);
 
 virObjectUnlock(dmn);
 }
-- 
1.8.3.1

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

[libvirt] [PATCH 2/4] daemon: call free callbacks synchronously in event loop impl

[Nikolay Shirokovskiy]

Default event loop impl delays deleting registered handles and
timeouts so that deleting is safe from event handlers. But
this means deletions after event loop is finished will never
occur and particularly assosiated callback objects will not
be freed. For this reason network clients that exist at
the moment of libvirtd daemon exit are never get disposed
and daemon object itself too if there are admin server clients.

Let's call free callbacks immediately we don't need to delay
this operation, only removing handles from the list.

This breaks virnetsocket.c immediately as socket is locked
in freeing callback and callback is called synchronously
from virNetSocketRemoveIOCallback which holds the lock already.
So fix it to pass intermediate callback object to the loop
instead of socket itself. I've checked other users of
virEventAddHandle, looks like they don't have such problems.
---
 src/rpc/virnetsocket.c  | 70 ++---
 src/util/vireventpoll.c | 24 +++--
 2 files changed, 42 insertions(+), 52 deletions(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 405f5ba..732a993 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -69,6 +69,16 @@
 
 VIR_LOG_INIT("rpc.netsocket");
 
+struct _virNetSocketCallbackObject {
+virNetSocketPtr sock;
+virNetSocketIOFunc func;
+void *opaque;
+virFreeCallback ff;
+};
+
+typedef struct _virNetSocketCallbackObject virNetSocketCallbackObject;
+typedef virNetSocketCallbackObject *virNetSocketCallbackObjectPtr;
+
 struct _virNetSocket {
 virObjectLockable parent;
 
@@ -78,11 +88,6 @@ struct _virNetSocket {
 int errfd;
 bool client;
 
-/* Event callback fields */
-virNetSocketIOFunc func;
-void *opaque;
-virFreeCallback ff;
-
 virSocketAddr localAddr;
 virSocketAddr remoteAddr;
 char *localAddrStrSASL;
@@ -1928,38 +1933,19 @@ static void virNetSocketEventHandle(int watch 
ATTRIBUTE_UNUSED,
 int events,
 void *opaque)
 {
-virNetSocketPtr sock = opaque;
-virNetSocketIOFunc func;
-void *eopaque;
-
-virObjectLock(sock);
-func = sock->func;
-eopaque = sock->opaque;
-virObjectUnlock(sock);
-
-if (func)
-func(sock, events, eopaque);
+virNetSocketCallbackObjectPtr o = opaque;
+if (o->func)
+o->func(o->sock, events, o->opaque);
 }
 
 
 static void virNetSocketEventFree(void *opaque)
 {
-virNetSocketPtr sock = opaque;
-virFreeCallback ff;
-void *eopaque;
-
-virObjectLock(sock);
-ff = sock->ff;
-eopaque = sock->opaque;
-sock->func = NULL;
-sock->ff = NULL;
-sock->opaque = NULL;
-virObjectUnlock(sock);
-
-if (ff)
-ff(eopaque);
-
-virObjectUnref(sock);
+virNetSocketCallbackObjectPtr o = opaque;
+if (o->ff)
+o->ff(o->opaque);
+virObjectUnref(o->sock);
+VIR_FREE(o);
 }
 
 int virNetSocketAddIOCallback(virNetSocketPtr sock,
@@ -1969,32 +1955,40 @@ int virNetSocketAddIOCallback(virNetSocketPtr sock,
   virFreeCallback ff)
 {
 int ret = -1;
+virNetSocketCallbackObjectPtr cbobj = NULL;
 
-virObjectRef(sock);
 virObjectLock(sock);
 if (sock->watch >= 0) {
 VIR_DEBUG("Watch already registered on socket %p", sock);
 goto cleanup;
 }
 
+if (VIR_ALLOC(cbobj) < 0)
+goto cleanup;
+
+cbobj->sock = virObjectRef(sock);
+cbobj->func = func;
+cbobj->opaque = opaque;
+cbobj->ff = ff;
+
 if ((sock->watch = virEventAddHandle(sock->fd,
  events,
  virNetSocketEventHandle,
- sock,
+ cbobj,
  virNetSocketEventFree)) < 0) {
 VIR_DEBUG("Failed to register watch on socket %p", sock);
 goto cleanup;
 }
-sock->func = func;
-sock->opaque = opaque;
-sock->ff = ff;
 
 ret = 0;
 
  cleanup:
 virObjectUnlock(sock);
-if (ret != 0)
+if (ret != 0) {
+VIR_FREE(cbobj);
 virObjectUnref(sock);
+}
+
 return ret;
 }
 
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index 81ecab4..e152d23 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -198,6 +198,11 @@ int virEventPollRemoveHandle(int watch)
 if (eventLoop.handles[i].watch == watch) {
 EVENT_DEBUG("mark delete %zu %d", i, eventLoop.handles[i].fd);
 eventLoop.handles[i].deleted = 1;
+if (eventLoop.handles[i].ff) {
+virMutexUnlock();
+eventLoop.handles[i].ff(eventLoop.handles[i].opaque);
+virMutexLock();
+}
 virEventPollInterruptLocked();
 virMutexUnlock();
 return 0;
@@ -315,6 +320,11 @@ int 

Re: [libvirt] [PATCH 0/7] Fix some typos.

[Peter Krempa]

On Thu, Sep 22, 2016 at 03:33:39 +0530, Nitesh Konkar wrote:
> *** BLURB HERE ***
> 
> Nitesh Konkar (7):
>   domain_conf.c:fix a typo
>   domain_conf.h:fix a typo
>   domain_conf.c:fix a typo
>   storage_conf.c:fix a typo
>   interface_backend_udev.c:fix a typo
>   esx_driver.c:fix a typo
>   esx_driver.c:fix a typo

I don't think such changes are worth splitting into separate commits.

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