Re: [libvirt] [PATCH RFC] libxl: set serial source path for console type=serial

[Jim Fehlig]

On 06/21/2016 04:20 AM, Joao Martins wrote:
>
> On 06/21/2016 01:38 AM, Jim Fehlig wrote:
>> Joao Martins wrote:
>>> Guests use a  (and sometimes  pair) to represent
>>> the console. On the callback that is called when console is brought up
>>> (NB: before domain starts), we fill the path of the console element with
>>> the appropriate "/dev/pts/X". For PV guests it all works fine, although
>>> for HVM guests it doesn't. Consequently we end up seeing erronous
>>> behaviour on HVM guests where console path is not displayed on the XML
>>> but still can be accessed with virDomainOpenConsole after booting guest.
>>> Consumers of this XML (e.g. Openstack) also won't be able to use the
>>> console path.
>> Can you provide example input domXML config that causes this problem?
>>
> Ah yes - I probably should include that in the commit description?

Yes, I think it helps clarify the problem  being solved by the commit.

>
> So, for PV guests for an input console XML element:
>
> 
>   
> 
>
> Which then on domain start gets filled like:
>
> 
>   
>   
> 
>
> Although for HVM guests we have:
>
> 
>   
> 
> 
>   
> 
>
> And no changes happen i.e. source path isn't written there _even though_ it's
> set on the console element - being the reason why we can access the console 
> in the
> first place. IIUC The expected output should be:
>
> 
>   
>   
> 
> 
>   
>   
> 

I asked for an example after having problems testing your patch, but it turned
out to be an issue which I think was introduced long ago by commit 482e5f15. I
was testing with transient domains and noticed 'virsh create; 'virsh console'
always failed with

error: internal error: character device  is not using a PTY

My config only contained

  

  

so the "really crazy backcompat stuff for consoles" in
virDomainDefAddConsoleCompat() moved the config over to def->serials[0] and
created a new def->consoles[0] without def->consoles[0].source.type set to
VIR_DOMAIN_CHR_TYPE_PTY. Without source.type set, libxlConsoleCallback() would
ignore the console and not copy the PTY path to source.data.file.path. 'virsh
console' would then fail with the error noted above.

I spent too much time debugging this, partly because I was confused by odd
behavior with persistent domains. E.g. 'virsh define; virsh start; virsh
console' would fail with the same error, but all subsequent 'virsh shutdown;
virsh start; virsh console' sequences would work :-). That behavior was due to
vm->newDef being populated with correct console config when calling
virDomainObjSetDefTransient() in libxlDomainStart(). After the first shutdown of
the domain, vm->newDef would be copied over to vm->def, allowing subsequent
'virsh start; virsh console' to work correctly.

Long story short, I found the problem but not sure how to fix it :-). One
approach would be the below patch. Another would be to loosen the restriction in
libxlConsoleCallback, filing in source.data.file.path when the console source
type is also set to VIR_DOMAIN_CHR_TYPE_NULL. Adding Cole and Peter (both of
which have toiled in and likely loathe this code) to see if they have opinions
on a proper fix.

Regards,
Jim


>From a231636e8eec08152220e32af5f4026a50954f41 Mon Sep 17 00:00:00 2001
From: Jim Fehlig 
Date: Tue, 21 Jun 2016 20:25:23 -0600
Subject: [PATCH] domain conf: copy source type from stolen console

When domXML contains only  and no corresponding
, the console is "stolen" [1] and used as the first 
device. A new console is created as an alias to the first 
device, but missed copying some configuration such as the source
'type' attribute.

[1] See comments associated with virDomainDefAddConsoleCompat() in
$LIBVIRT-SRC/src/conf/domain_conf.c:

Signed-off-by: Jim Fehlig 
---
 src/conf/domain_conf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 75ad03f..2fda4fe 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3845,6 +3845,7 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def)
 /* Create an console alias for the serial port */
 def->consoles[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
 def->consoles[0]->targetType =
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL;
+def->consoles[0]->source.type = def->serials[0]->source.type;
 }
 } else if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && def->nserials > 0 &&
def->serials[0]->deviceType == 
VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
-- 
2.1.4


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

Re: [libvirt] [PATCH 0/4] Move ccwaddrs and pciaddrs to domainDef

[Laine Stump]

On 06/20/2016 10:26 PM, Tomasz Flendrich wrote:

Apologies if I'm missing something, I didn't look too closely at this series,
however have you seen this thread?

http://www.redhat.com/archives/libvir-list/2016-May/msg01071.html

I haven’t noticed that some work has been done on that, thank you!


My understanding of the current code is that the cached vioserial/ccw/pciaddrs
lists in qemu aren't actually required…they were at one point to handle
older qemu, but we dropped that support. Maybe you can pick up my patches and
finish off dropping of pciaddrs and ccwaddrs? I suspect the pciaddrs cache in
bhyve can be dropped as well, I don't think it was ever strictly required, the
code just followed the qemu example

If we could do without the caching, it would make the current code simpler.
There wouldn’t be those booleans in qemu_hotplug.c that remember whether
an address has to be deleted or not in case something fails. We could
delete qemuDomainReleaseDeviceAddress() and a few more functions.


I'm completely ignorant about vioserial and ccw. As far as the "cache" 
for pci addresses, I guess whether or not we want the cache depends 
completely on how long it takes to reconstruct vs. how often a device is 
added. There is also the issue of the mismatch between live and config 
devices' address use, and the inconsistency that can be caused by that 
(if you hotplug a device with --live, then hotplug another with --live 
--config, then the 2nd device will have the same address in config as 
the first has in the live state of the guest (more importantly, the 
address of the 2nd device will change the next time the domain is 
shutdown and restarted, which all of this address assignment stuff is 
intended to avoid) - I don't know if that problem would be more easily 
solved by a cache that is used for assigning addresses for both --live 
and --config, or if, as Cole suggests, it would be better just to remove 
the cache and rebuild the allocation table each time a new device is 
added (this would mean that we would need to scan through all the live 
devices *and* persistent devices to re-populate it)




I examined vioserial and pci addresses and it looks like
it could be done. However, I'm not an expert on qemu_hotplug yet
and this is where the interesting stuff happens with addresses,
so I am not entirely sure yet.
I also don't know what the plans are for device addresses in the future.
Perhaps there are some features that will require caching them.

I think that recalculation may change the current behavior of ccw addresses.
Function virDomainCCWAddressReleaseAddr() modifies addrs->next
only when the address being released was the address most recently
assigned.

Laine, you know a lot about PCI addresses and you also mentioned that
you want to modify them in the future. What do you think?



"in the future" sounds so distant :-). Within the next week I will be 
modifiying the PCI address allocation to eliminate the auto-adding of 
pci-bridge devices whenever there aren't enough free slots, and  instead 
add a new controller of the appropriate type for each extra slot that is 
needed. I haven't been able to concentrate enough to determine if that 
is going to cause any merge conflict with what you're going to do.





Martin, should I work on that instead?


Kind regards,
Tomasz



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

Re: [libvirt] [PATCH 0/4] Move ccwaddrs and pciaddrs to domainDef

[Cole Robinson]

On 06/20/2016 10:26 PM, Tomasz Flendrich wrote:
> 
>> Apologies if I'm missing something, I didn't look too closely at this series,
>> however have you seen this thread?
>>
>> http://www.redhat.com/archives/libvir-list/2016-May/msg01071.html
> I haven’t noticed that some work has been done on that, thank you!
> 
>> My understanding of the current code is that the cached 
>> vioserial/ccw/pciaddrs
>> lists in qemu aren't actually required…they were at one point to handle
> 
>> older qemu, but we dropped that support. Maybe you can pick up my patches and
>> finish off dropping of pciaddrs and ccwaddrs? I suspect the pciaddrs cache in
>> bhyve can be dropped as well, I don't think it was ever strictly required, 
>> the
>> code just followed the qemu example
> If we could do without the caching, it would make the current code simpler.
> There wouldn’t be those booleans in qemu_hotplug.c that remember whether
> an address has to be deleted or not in case something fails. We could 
> delete qemuDomainReleaseDeviceAddress() and a few more functions.
> 
> I examined vioserial and pci addresses and it looks like
> it could be done. However, I'm not an expert on qemu_hotplug yet
> and this is where the interesting stuff happens with addresses,
> so I am not entirely sure yet.
> I also don't know what the plans are for device addresses in the future.
> Perhaps there are some features that will require caching them.
> 

In a way they are currently cached twice: once in these lists, and once in the
runtime VM XML. That's what my vioserial series did, was replace the hotplug
usage of the cached list with just fetching it on demand from the running VM
XML, so at least at a high level these cached lists seem redundant because the
info is tracked elsewhere.

> I think that recalculation may change the current behavior of ccw addresses.
> Function virDomainCCWAddressReleaseAddr() modifies addrs->next
> only when the address being released was the address most recently
> assigned.
> 

Hmm, that's interesting. Not sure what that is about. I tried adding this diff:

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 794270d..7d39b69 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -792,14 +792,7 @@ virDomainCCWAddressReleaseAddr(virDomainCCWAddressSetPtr
addrs,
 if (!addr)
 return -1;

-if ((ret = virHashRemoveEntry(addrs->defined, addr)) == 0 &&
-dev->addr.ccw.cssid == addrs->next.cssid &&
-dev->addr.ccw.ssid == addrs->next.ssid &&
-dev->addr.ccw.devno < addrs->next.devno) {
-addrs->next.devno = dev->addr.ccw.devno;
-addrs->next.assigned = false;
-}
-
+ret = virHashRemoveEntry(addrs->defined, addr);
 VIR_FREE(addr);

 return ret;


And the test suite doesn't break, so at least it's not something completely
obvious. But then again this seems to only be via hotunplug call path, and our
test suite coverage for hotplug isn't that great.

This logic may in fact just be an artifact of maintaining a persistent set of
addresses. If we are generating an address set on demand from the runtime XML,
we probably don't need to worry about 'releasing' an address from that set at
all, so it may just side step whatever issue this logic was trying to address.

> Laine, you know a lot about PCI addresses and you also mentioned that
> you want to modify them in the future. What do you think?
> 

Laine did have some patches that did touch some bits in this area, but only as
a side effect.

Thanks,
Cole

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

Re: [libvirt] [PATCH 19/19] qemu: Add luks support for domain disk

[John Ferlan]

On 06/21/2016 09:03 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:58 -0400, John Ferlan wrote:
>> Generate the luks command line using the AES secret key to encrypt the
>> luks secret. A luks secret object will be in addition to a an AES secret.
>>
>> Add tests for sample output
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/qemu/qemu_command.c| 12 ++--
>>  src/qemu/qemu_domain.c | 19 ++--
>>  .../qemuxml2argv-luks-disk-cipher.args | 36 
>> ++
>>  .../qemuxml2argvdata/qemuxml2argv-luks-disks.args  | 36 
>> ++
>>  tests/qemuxml2argvtest.c   | 11 ++-
>>  5 files changed, 109 insertions(+), 5 deletions(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.args
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args
> 
> Note that this won't work with disk hotplug until you add code that will
> add the secret objects too.

Oh right - I had forgotten about the secret object... I had the same
issue with the TLS code, so I know what has to be done...

> 
> Also I've noticed that RBD hotplug is now broken too due to the same
> reason. The code that selects whether to use plaintext password or pass
> it via the secret object does not check whether it's called on the
> hotplug path and thus uses the secret object.

Oh damn...  I think that's a disconnect in qemuDomainSecretSetup (called
from qemuDomainSecretDiskPrepare from hotplug) and the assumption of an
AES secinfo for RBD when there's a secret object available.

> 
> The secret object is not added using the monitor though. You'll need to
> fix that first and if you opt for the correct approach this will then
> work automagically.
> 
> Looks okay but I can't ACK this with hotplug not working.
> 
> Also please note that on hot-unplug you need to remove the secrets as
> you'll possibly be adding a secret with the same name (Since alias will
> be reused)

yep...

> 
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 490260f..7062c17 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -1103,6 +1103,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>>  int actualType = virStorageSourceGetActualType(disk->src);
>>  qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>>  qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
>> +qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo;
>>  bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus);
>>  
>>  if (idx < 0) {
>> @@ -1237,10 +1238,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>>  qemuBufferEscapeComma(, source);
>>  virBufferAddLit(, ",");
>>  
>> -if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
>> +if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES)
>>  virBufferAsprintf(, "password-secret=%s,",
>>secinfo->s.aes.alias);
>> -}
>> +
>> +if (encinfo)
>> +virQEMUBuildLuksOpts(, disk->src->encryption,
>> + encinfo->s.aes.alias);
> 
> This wrapper is not really useful here. It only adds "key-secret=" all
> the other options are necessary only if creating the volume.
> 

I was thinking of the future if/when new things are added.  I'm not
clear yet what would have to be done for block-copy and snapshots.

>>  
>>  if (disk->src->format > 0 &&
>>  disk->src->type != VIR_STORAGE_TYPE_DIR)
>> @@ -1920,6 +1924,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>>  virDomainDiskDefPtr disk = def->disks[i];
>>  qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>>  qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
>> +qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo;
>>  
>>  /* PowerPC pseries based VMs do not support floppy device */
>>  if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) &&
>> @@ -1949,6 +1954,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>>  if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0)
>>  return -1;
>>  
>> +if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
>> +return -1;
>> +
>>  virCommandAddArg(cmd, "-drive");
>>  
>>  optstr = qemuBuildDriveStr(disk,
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index c288fa0..fb3e91f 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
> 
> [...]
> 
>> @@ -1026,7 +1028,6 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
>>   src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
>>  
>>  virSecretUsageType secretUsageType;
>> -qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>>  
>>  if (VIR_ALLOC(secinfo) < 0)
>>  

Re: [libvirt] [PATCH 17/19] storage: Add support to create a luks volume

[John Ferlan]

On 06/21/2016 09:53 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:56 -0400, John Ferlan wrote:
>> If the volume xml was looking to create a luks volume take the necessary
>> steps in order to make that happen.
>>
>> The processing will be:
>>  1. create a temporary file in the storage pool target path
>>1a. the file name will either be $volname_$UUID.tmp or $volname_$USAGE.tmp
>>depending on how the encryption xml specified fetching the secret
>>1b. create/open the file, initially setting mode to 0600 with current
>>effective uid:gid as owner
>>1c. fetch the secret into a buffer and write that into the file
>>1d. change file protections to 0400
>>
>>  2. create a secret object
>>2a. use a similarly crafted alias to the file name
>>2b. add the file to the object
>>
>>  3. create/add luks options to the commandline
>>3a. at the very least a "key-secret" using the secret object alias
>>3b. if found in the XML the various "cipher" and "ivgen" options
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/libvirt_private.syms   |   1 +
>>  src/storage/storage_backend.c  | 285 
>> ++---
>>  src/storage/storage_backend.h  |   3 +-
>>  src/util/virqemu.c |  23 
>>  src/util/virqemu.h |   6 +
>>  tests/storagevolxml2argvtest.c |   3 +-
>>  6 files changed, 300 insertions(+), 21 deletions(-)
>>

[...]

>> +
>>  static int
>>  virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
>>  {
>> @@ -915,12 +940,18 @@ virStorageBackendQEMUImgBackingFormat(const char 
>> *qemuimg)
>>   * out what else we have */
>>  int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
>>  
>> -/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
>> - * understands. Since we still support QEMU 0.12 and newer, we need
>> - * to be able to handle the previous format as can be set via a
>> - * compat=0.10 option. */
>> -if (virStorageBackendQemuImgSupportsCompat(qemuimg))
>> -ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
>> +/* QEMU 2.6 added support for luks - let's check for that.
>> + * If available, then we can also assume OPTIONS_COMPAT is present */
>> +if (virStorageBackendQemuImgSupportsLuks(qemuimg)) {
>> +ret = QEMU_IMG_FORMAT_LUKS;
>> +} else {
>> +/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
>> + * understands. Since we still support QEMU 0.12 and newer, we need
>> + * to be able to handle the previous format as can be set via a
>> + * compat=0.10 option. */
>> +if (virStorageBackendQemuImgSupportsCompat(qemuimg))
>> +ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
>> +}
> 
> This looks like it's becoming a source of technical debt. Heaps of
> comments aren't helping.
> 

It's on the short list of things to deal with.

>>  
>>  return ret;
>>  }
>> @@ -941,21 +972,31 @@ struct _virStorageBackendQemuImgInfo {
>>  const char *inputPath;
>>  const char *inputFormatStr;
>>  int inputFormat;
>> +
>> +char *secretAlias;
>> +const char *secretPath;
>>  };
>>  

[...]

>>  
>> +/* Create a hopefully unique enough name to be used for both the
>> + * secretPath and secretAlias generation
> 
> This won't fly. There's only one way to guarantee that there won't be a
> collision. You need to create a file in a private path.
> 
>> + */
>> +static char *
>> +virStorageBackendCreateQemuImgLuksName(const char *volname,
>> +   virStorageEncryptionPtr enc)
>> +{
>> +char *ret;
>> +
>> +if (enc->secrets[0]->secdef.type == VIR_SECRET_LOOKUP_TYPE_UUID)
>> +ignore_value(virAsprintf(, "%s_%s", volname,
>> + enc->secrets[0]->secdef.u.uuid));
>> +else
>> +ignore_value(virAsprintf(, "%s_%s", volname,
>> + enc->secrets[0]->secdef.u.usage));
> 
> usage is user provided text. Also your example in previous patch uses
> slashes in it. I don't think it will end up well in this case.
> 

Ugh... right.  I'm almost tempted to avoid a file type of secret and
make it be an AES type secret.

This goes away anyway.

>> +return ret;
>> +}
>> +

[...]

>> +static char *
>> +virStorageBackendCreateQemuImgSecretPath(virConnectPtr conn,
>> + virStoragePoolObjPtr pool,
>> + virStorageVolDefPtr vol)
>> +{
>> +virStorageEncryptionPtr enc = vol->target.encryption;
>> +char *str = NULL;
>> +char *secretPath = NULL;
>> +int fd = -1;
>> +uint8_t *secret = NULL;
>> +size_t secretlen = 0;
>> +
>> +if (!enc) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +   _("missing encryption description"));
>> +return NULL;
>> +}
>> +
>> +if (!conn || !conn->secretDriver ||
>> +

Re: [libvirt] [Qemu-devel] [PATCH 1/3] qmp: Add query-host-cpu command

[Eric Blake]

On 06/20/2016 02:12 PM, Eduardo Habkost wrote:
> The command can be used to return host-specific CPU capabilities
> information.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/sysemu/arch_init.h   |  1 +
>  qapi-schema.json | 36 
>  qmp-commands.hx  |  6 ++
>  qmp.c| 13 +
>  stubs/Makefile.objs  |  1 +
>  stubs/arch-query-host-cpu-info.c |  8 
>  6 files changed, 65 insertions(+)
>  create mode 100644 stubs/arch-query-host-cpu-info.c
> 
> diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
> index d690dfa..54215ab 100644
> --- a/include/sysemu/arch_init.h
> +++ b/include/sysemu/arch_init.h
> @@ -35,5 +35,6 @@ int kvm_available(void);
>  int xen_available(void);
>  
>  CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp);
> +void arch_query_host_cpu_info(HostCPUInfo *r, bool migratable, Error **errp);
>  
>  #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 19e3ef2..d2f4879 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3047,6 +3047,42 @@
>  ##
>  { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
>  
> +
> +##
> +# @HostCPUInfo:
> +#
> +# Information on CPU capabilities supported by the current host.
> +#
> +# @qom-properties: #optional Values of CPU QOM properties corresponding
> +#  to CPU capabilities supported by the host.
> +#
> +# Most properties returned in qom-properties are boolean properties
> +# indicating if a feature can be enabled in the current host. Other
> +# non-boolean properties may be returned, the semantics of each property
> +# depend on the architecture-specific code that handle them.
> +#
> +# Since: 2.7.0

Most places in .json files list just 'Since: x.y' rather than 'x.y.z',
but we aren't consistent enough to insist either way on including or
excluding a micro release number.

> +##
> +{ 'struct': 'HostCPUInfo',
> +  'data': { '*qom-properties': 'any' } }

This is a big hammer that makes the properties non-introspectible - a
client can tell that properties will be returned, but cannot tell which
properties to expect nor what format to expect for a given property
name.  I don't know that the interface could be made easily
introspectible or not (it would probably require some QAPI unions, and a
LOT more generated code).  So it would be nice if we could explore how
hard it would be to use a type-safe representation instead of 'any',
before declaring that this is the best we can do.  Or, it may be the
sign of a bigger issue that we have no good way to introspect what qom
properties to expect, in general (and that solving that would also solve
this).

> +
> +##
> +# @query-host-cpu:
> +#
> +# @migratable: #optional If false, unmigratable features will be
> +#  returned as well. If true, only migratable features
> +#  will be returned. Defaults to true.
> +#
> +# Return information about CPU capabilities in the current host.
> +# The returned data may depend on machine and accelerator configuration.
> +#
> +# Returns: A HostCPUInfo object.
> +#
> +# Since: 2.7.0
> +##
> +{ 'command': 'query-host-cpu', 'data': { '*migratable': 'bool' },
> +  'returns': 'HostCPUInfo' }
> +
>  # @AddfdInfo:

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



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

Re: [libvirt] [RFC 00/28] s390x CPU models: exposing features

[Eduardo Habkost]

On Tue, Jun 21, 2016 at 11:09:49PM +0200, Jiri Denemark wrote:
[...]
> > 1) "query-cpu-model-expansion model=host" vs "query-host-cpu":
> > 
> > I still don't think we want to set in stone that "the result the
> > guest sees when using -cpu host" is always the same as "what the
> > host supports running".
> > 
> > For example: let's assume a given architecture have two features
> > (A and B) that are both supported by the host but can never be
> > enabled together. For actual "-cpu host" usage, QEMU would have
> > to choose between enabling A and B. For querying host
> > capabilities, we still want to let management software know that
> > either A or B are supported.
> 
> What libvirt is really interested in is the guest CPU which would be
> used with -cpu host. This is actually what I thought query-host-cpu was
> all about. Perhaps because there's no difference for x86.

In that case, I think it makes sense to just extend
query-cpu-definitions or use "query-cpu-model-expansion
model=host" instead of a query-host-cpu command.

Probably query-cpu-model-expansion is better than just extending
query-cpu-definitions, because it would allow the expansion of
extra CPU options, like "host,migratable=off".

-- 
Eduardo

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

Re: [libvirt] [RFC 00/28] s390x CPU models: exposing features

[Jiri Denemark]

On Tue, Jun 21, 2016 at 17:33:09 -0300, Eduardo Habkost wrote:
> On Tue, Jun 21, 2016 at 07:01:44PM +0200, David Hildenbrand wrote:
> > > (CCing libvirt people)
> > > 
> > > On Tue, Jun 21, 2016 at 03:02:05PM +0200, David Hildenbrand wrote:
> > > > This is our second attempt to implement CPU models for s390x. We 
> > > > realized
> > > > that we also want to have features exposed via the CPU model. While 
> > > > doing
> > > > that we realized that we want to have a better interface for libvirt.  
> > > 
> > > Before getting into the details, I would like to clarify how the
> > > following could be accomplished using the new commands:
> > > 
> > > Example:
> > > 
> > > 1) User configures libvirt with:
> > >
> > >Westmere
> > >
> > >
> > > 2) libvirt will translate that to:
> > >"-cpu Westmere,+aes" or "-cpu Westmere,aes=on"
> > > 3) libvirt wants to know if "-cpu Westmere,aes=on" is usable in
> > >the current host, before trying to start the VM.
> > > 
> > > How exactly would this be done using the new commands?
> > 
> > Hi Eduardo,
> > 
> > thanks for having a look - highly appreciated that you actually map this
> > to libvirt requirements!
> > 
> > That would map to a compare operation between "host" and "Westmere,aes=on".
> > 
> > Host could at that point already be expanded by libvirt. Doesn't matter at 
> > that
> > point.
> > 
> > If the result is "identica"l or "superset", it is runnable. If the result is
> > "subset" or "incompatible", details about the responsible properties is
> > indicated. (I actually took that idea from your patch for indicating
> > runnability).
> 
> So, I have two worries about the proposal:
> 
> 
> 1) "query-cpu-model-expansion model=host" vs "query-host-cpu":
> 
> I still don't think we want to set in stone that "the result the
> guest sees when using -cpu host" is always the same as "what the
> host supports running".
> 
> For example: let's assume a given architecture have two features
> (A and B) that are both supported by the host but can never be
> enabled together. For actual "-cpu host" usage, QEMU would have
> to choose between enabling A and B. For querying host
> capabilities, we still want to let management software know that
> either A or B are supported.

What libvirt is really interested in is the guest CPU which would be
used with -cpu host. This is actually what I thought query-host-cpu was
all about. Perhaps because there's no difference for x86.

> 2) Requiring a running QEMU instance to run
>query-cpu-model-comparison
> 
> With my previous query-host-cpu proposal, the task of comparing
> the configuration requested by the user with host capabilities
> can be done directly by libvirt. This way, no extra QEMU instance
> needs to be run before starting a VM.

I think we can just easily get around this by not comparing a guest CPU
to host (except for the explicit virConnectCompareCPU, which is not very
useful in its current form anyway).

Jirka

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

Re: [libvirt] [RFC 00/28] s390x CPU models: exposing features

[Jiri Denemark]

On Tue, Jun 21, 2016 at 13:44:31 -0300, Eduardo Habkost wrote:
> (CCing libvirt people)
> 
> On Tue, Jun 21, 2016 at 03:02:05PM +0200, David Hildenbrand wrote:
> > This is our second attempt to implement CPU models for s390x. We realized
> > that we also want to have features exposed via the CPU model. While doing
> > that we realized that we want to have a better interface for libvirt.
> 
> Before getting into the details, I would like to clarify how the
> following could be accomplished using the new commands:
> 
> Example:
> 
> 1) User configures libvirt with:
>
>Westmere
>
>
> 2) libvirt will translate that to:
>"-cpu Westmere,+aes" or "-cpu Westmere,aes=on"
> 3) libvirt wants to know if "-cpu Westmere,aes=on" is usable in
>the current host, before trying to start the VM.

While this is what libvirt currently does, I don't think it's necessary
to keep doing that... see below.

> > a) "query-cpu-model-expansion" - tell us what the "host" or a migration
> >unsafe model looks like. Either falling back to a stable model or
> >completely exposing all properties. We are interested in stable models.
> > b) "query-cpu-model-comparison" - tell us how two CPU models compare,
> > indicating which properties were responsible for the decision.
> > c) "query-cpu-model-baseline" - create a new model out of two models,
> > taking a requested level of stability into account.

This looks like it copies current libvirt APIs, which I think is not a
very good idea. Both CPU compare and baseline APIs in libvirt will need
to be changed (or rather new APIs with similar functionality added) to
become useful. I think we should first focus on making guest CPU
configuration work the way it should work. For that I think we need some
higher level commands.

Let me sum up what libvirt is doing (or will be doing) with guest
CPUs... Libvirt supports three guest CPU configuration modes:

- host-passthrough -- this is easy, it's just asking for "-cpu host" and
  no fancy commands are required.

- host-model -- for this we need to know what CPU configuration we need
  to ask for to get as close to the host CPU as possible while being
  able to ask for the exact same CPU on a different host (after
  migration). And we need to be able to ask for this at probing time
  (when libvirtd starts rather than when starting a new domain) using
  "-machine none,accel=kvm:tcg", that is without actually creating any
  guest CPU. This is what Eduardo's query-host-cpu QMP command is useful
  for. In x86 world we could just query the guest CPU after running QEMU
  with "-cpu host", but that's unusable with "-machine none", which is
  why another way of getting this info is needed.

- custom -- the XML specifies an exact guest CPU configuration. We don't
  really need to know if that exact CPU is runnable on the current host
  in advance, we can just try to start the domain, check if the guest
  CPU matches what we asked for, and we may kill QEMU if the CPU does
  not meet the specification. Of course, higher level management wants
  to know a set of host where it can run a given domain for scheduling
  purposes, but since they logically want to avoid tons of different CPU
  configs, they would just stick the CPU models predefined by QEMU. Thus
  giving them a way of checking what CPU models can be run on a given
  host with a given QEMU (using the unavailable features stuff for
  query-cpu-definitions usable with "-machine none") should be enough
  and it's even better than having to ask for every single CPU model,
  which is what they need to do now.

There are configuration options which are somewhere between host-model
and custom, but they don't bring more requirements in addition to what
both of them needs.

This was basically all about starting a new domain. When the domain
successfully starts, we need to make sure the guest CPU does not change
during save/restore or migration to another host. To achieve this, we
need the same checking we need for custom mode, i.e., whether the guest
CPU we got is what we asked for. In addition to this, we need a way to
ask QEMU what the guest CPU looks like so that we can store it in the
domain XML and ask for it during migration.

I think all of this should be pretty much architecture agnostic. Of
course the actual data would be quite different fro each architecture,
but I believe the entry points could fit all. Or did I miss anything?

Jirka

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

Re: [libvirt] [RFC 00/28] s390x CPU models: exposing features

[Eduardo Habkost]

On Tue, Jun 21, 2016 at 07:01:44PM +0200, David Hildenbrand wrote:
> > (CCing libvirt people)
> > 
> > On Tue, Jun 21, 2016 at 03:02:05PM +0200, David Hildenbrand wrote:
> > > This is our second attempt to implement CPU models for s390x. We realized
> > > that we also want to have features exposed via the CPU model. While doing
> > > that we realized that we want to have a better interface for libvirt.  
> > 
> > Before getting into the details, I would like to clarify how the
> > following could be accomplished using the new commands:
> > 
> > Example:
> > 
> > 1) User configures libvirt with:
> >
> >Westmere
> >
> >
> > 2) libvirt will translate that to:
> >"-cpu Westmere,+aes" or "-cpu Westmere,aes=on"
> > 3) libvirt wants to know if "-cpu Westmere,aes=on" is usable in
> >the current host, before trying to start the VM.
> > 
> > How exactly would this be done using the new commands?
> 
> Hi Eduardo,
> 
> thanks for having a look - highly appreciated that you actually map this
> to libvirt requirements!
> 
> That would map to a compare operation between "host" and "Westmere,aes=on".
> 
> Host could at that point already be expanded by libvirt. Doesn't matter at 
> that
> point.
> 
> If the result is "identica"l or "superset", it is runnable. If the result is
> "subset" or "incompatible", details about the responsible properties is
> indicated. (I actually took that idea from your patch for indicating
> runnability).

So, I have two worries about the proposal:


1) "query-cpu-model-expansion model=host" vs "query-host-cpu":

I still don't think we want to set in stone that "the result the
guest sees when using -cpu host" is always the same as "what the
host supports running".

For example: let's assume a given architecture have two features
(A and B) that are both supported by the host but can never be
enabled together. For actual "-cpu host" usage, QEMU would have
to choose between enabling A and B. For querying host
capabilities, we still want to let management software know that
either A or B are supported.

If we don't mind adding multiple new QMP commands in this series,
I don't see why not having a separate "query-host-cpu" command so
the semantics of each command are very clear. Then
query-cpu-definitions/query-cpu-model-expansion would be about
"what you really get when using -cpu ", and
"query-host-cpu" would be about "what this hosts supports
running".


2) Requiring a running QEMU instance to run
   query-cpu-model-comparison

With my previous query-host-cpu proposal, the task of comparing
the configuration requested by the user with host capabilities
can be done directly by libvirt. This way, no extra QEMU instance
needs to be run before starting a VM.

In my example above, if query-cpu-model-comparison were not
required, libvirt could run "query-cpu-expansion model=host" or
"query-host-cpu" once at boot, and just check if "Westmere" is
runnable and "aes" is present in the host data. With
query-cpu-model-comparison, QEMU needs to be run one more time
before starting a VM.

I don't know if requiring running a new QEMU instance just to
check if a CPU config is runnable would be acceptable for
libvirt. I will let the libvirt people answer that.

I still like the query-cpu-model-comparison proposal, because it
would allow us to extend the data returned by
query-cpu-model-expansion without requiring libvirt changes. It
may be the best log-term option, but probably only after we add a
mechanism to allow the VM to be configured after running a few
QMP commands, without re-running QEMU.

-- 
Eduardo

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

Re: [libvirt] [PATCH 2/2] Add new elements source, access and allocation

[Pavel Hrdina]

On Tue, Jun 21, 2016 at 10:33:51AM +, Safka, JaroslavX wrote:
> This change introduces support for preallocated shared file descriptor based 
> memory backing.
> 
> It  allows vhost-user to be used without hugepages. This is achieved by 
> introducing 3 new sub elements to the memoryBacking element: source, access & 
> allocation which will configure qemu commandline during VM startup 
> accordingly.
> 
> 
> 
> First patch adds parsing of elements and storing the values in mem structure.
> 
> Second patch configures the qemu commandline on the base of values in mem 
> structure.

Hi, thanks for contributing to libvirt.  However this is not a good way how to
submit patches and it's also hard to review and provide the best feedback to
your patches.

Please read HACKING, we prefer using "git send-email", it will create a nice
thread where cover-letter is used to describe the patch series like you are
doing in this email.  You may need to install git-email or appropriate package
first (depends on your OS).

Thanks,

Pavel

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

Re: [libvirt] [PATCH 7/7] qemu: Implement virDomainSetGuestVcpus

[Pavel Hrdina]

On Mon, Jun 20, 2016 at 04:34:21PM +0200, Peter Krempa wrote:
> Allow modification of specific vCPU states via the guest agent.
> ---
>  src/qemu/qemu_driver.c | 83 
> ++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5b96ad8..4bd8c92 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -19837,6 +19837,88 @@ qemuDomainGetGuestVcpus(virDomainPtr dom,
>  }
> 
> 
> +static int
> +qemuDomainSetGuestVcpus(virDomainPtr dom,
> +const char *cpumap,
> +int state,
> +unsigned int flags)
> +{
> +virQEMUDriverPtr driver = dom->conn->privateData;
> +virDomainObjPtr vm = NULL;
> +virBitmapPtr map = NULL;
> +qemuAgentCPUInfoPtr info = NULL;
> +int ninfo = 0;
> +size_t i;
> +int ret = -1;
> +
> +virCheckFlags(0, -1);
> +
> +if (state != 0 && state != 1) {
> +virReportInvalidArg(state, "%s", _("unsupported state value"));
> +return -1;
> +}
> +
> +if (virBitmapParse(cpumap, , 4096) < 0)
> +goto cleanup;

s/4096/QEMU_GUEST_VCPU_MAX_ID/

> +
> +if (!(vm = qemuDomObjFromDomain(dom)))
> +goto cleanup;
> +
> +if (virDomainSetGuestVcpusEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +goto cleanup;
> +
> +if (!qemuDomainAgentAvailable(vm, true))
> +goto endjob;

[...]

ACK

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

Re: [libvirt] [PATCH 6/7] qemu: Implement virDomainGetGuestVcpus

[Pavel Hrdina]

On Mon, Jun 20, 2016 at 04:34:20PM +0200, Peter Krempa wrote:
> Allow gathering available vcpu ids, their state and offlinability via
> the qemu guest agent. The maximum id was chosen arbitrarily and ought
> to be enough for everybody.
> ---

[...]

> +#define ADD_BITMAP(name) 
>   \
> +if (!(tmp = virBitmapFormat(name)))  
>   \
> +goto cleanup;
>   \
> +if (virTypedParamsAddString(, , , #name, tmp) < 0)   
>   \
> +goto cleanup;
>   \
> +VIR_FREE(tmp)
> +
> +ADD_BITMAP(vcpus);
> +ADD_BITMAP(online);
> +ADD_BITMAP(offlinable);
> +
> +#undef ADD_BITMAP
> +
> +*params = par;
> +*nparams = npar;

You need to set par to NULL and to make it clear also npar to 0 and call
virTypedParamsFree(par, npar) in the cleanup.

> +ret = 0;
> +
> + cleanup:
> +VIR_FREE(tmp);
> +virBitmapFree(vcpus);
> +virBitmapFree(online);
> +virBitmapFree(offlinable);
> +return ret;
> +}

ACK

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

Re: [libvirt] [PATCH v2] There are several cases where we do not handle transient pool destroy and cleanup correctly. For example:

[Cole Robinson]

On 06/21/2016 12:13 PM, Jovanka Gulicoska wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1227475
> 
> Move the pool cleanup logic to a new function storagePoolSetInactive and
> use it consistently.

Looks like the subject line from the original patch was dropped, please bring
it back.

> ---
>  src/storage/storage_driver.c | 43 +++
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index e2d729f..2f29292 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -77,6 +77,21 @@ static void storageDriverUnlock(void)
>  }
>  
>  static void
> +storagePoolSetInactive(virStoragePoolObjPtr *pool)
> +{
> +(*pool)->active = false;
> +

Hmm, you can probably make this a bit nicer looking by doing

storagePoolSetInactive(virStoragePoolObjPtr *poolptr)
{
virStoragePoolObjPtr pool = *poolptr;
...

That will save having to all the (*pool) stuff, except for the actual bit that
matters, *poolptr = NULL;

A couple other comments below

> +if ((*pool)->configFile == NULL) {
> +virStoragePoolObjRemove(>pools, (*pool));
> +*pool = NULL;
> +} else if ((*pool)->newDef) {
> +virStoragePoolDefFree((*pool)->def);
> +(*pool)->def = (*pool)->newDef;
> +(*pool)->newDef = NULL;
> +}
> +}
> +
> +static void
>  storagePoolUpdateState(virStoragePoolObjPtr pool)
>  {
>  bool active;
> @@ -115,16 +130,19 @@ storagePoolUpdateState(virStoragePoolObjPtr pool)
>  if (backend->refreshPool(NULL, pool) < 0) {
>  if (backend->stopPool)
>  backend->stopPool(NULL, pool);
> +active = false;
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Failed to restart storage pool '%s': %s"),
> pool->def->name, virGetLastErrorMessage());
>  goto error;
>  }
> +active = true;
>  }
>  
> -pool->active = active;
>  ret = 0;
>   error:
> +if (!active)
> +storagePoolSetInactive();
>  if (ret < 0) {
>  if (stateFile)
>  unlink(stateFile);
> @@ -198,6 +216,7 @@ storageDriverAutostart(void)
>  unlink(stateFile);
>  if (backend->stopPool)
>  backend->stopPool(conn, pool);
> +storagePoolSetInactive();
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Failed to autostart storage pool '%s': 
> %s"),
> pool->def->name, virGetLastErrorMessage());

Since this can set pool=NULL; it needs to come after the virReportError call
which derefences pool. Also a few lines below there's a
virStoragePoolObjUnlock, so maybe stick a continue; after virReportError to
avoid issues with that

> @@ -737,8 +756,7 @@ storagePoolCreateXML(virConnectPtr conn,
>  unlink(stateFile);
>  if (backend->stopPool)
>  backend->stopPool(conn, pool);
> -virStoragePoolObjRemove(>pools, pool);
> -pool = NULL;
> +storagePoolSetInactive();
>  goto cleanup;
>  }
>  
> @@ -1068,16 +1086,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
>  VIR_STORAGE_POOL_EVENT_STOPPED,
>  0);
>  
> -pool->active = false;
> -
> -if (pool->configFile == NULL) {
> -virStoragePoolObjRemove(>pools, pool);
> -pool = NULL;
> -} else if (pool->newDef) {
> -virStoragePoolDefFree(pool->def);
> -pool->def = pool->newDef;
> -pool->newDef = NULL;
> -}
> +storagePoolSetInactive();
>  
>  ret = 0;
>  
> @@ -1197,13 +1206,7 @@ storagePoolRefresh(virStoragePoolPtr obj,
>  pool->def->uuid,
>  
> VIR_STORAGE_POOL_EVENT_STOPPED,
>  0);
> -pool->active = false;
> -
> -if (pool->configFile == NULL) {
> -virStoragePoolObjRemove(>pools, pool);
> -pool = NULL;
> -}
> -goto cleanup;
> +storagePoolSetInactive();
>  }
>  

I missed this before, but you need to preserve the goto cleanup; here

Thanks,
Cole

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

Re: [libvirt] [PATCH 5/7] qemu: agent: Make setting of vcpus more robust

[Pavel Hrdina]

On Mon, Jun 20, 2016 at 04:34:19PM +0200, Peter Krempa wrote:
> Documentation for the "guest-set-vcpus" command describes a proper
> algorithm how to set vcpus. This patch makes the following changes:
> 
> - state of cpus that has not changed is not updated
> - if the command was partially successful the command is re-tried with
>   the rest of the arguments to get a proper error message
> - code is more robust against mailicious guest agent

s/mailicious/malicious/

> - fix testsuite to the new semantics
> ---
>  src/qemu/qemu_agent.c  | 83 
> ++
>  src/qemu/qemu_agent.h  |  2 ++
>  src/qemu/qemu_driver.c | 13 
>  tests/qemuagenttest.c  | 44 --
>  4 files changed, 92 insertions(+), 50 deletions(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index cbc0995..5bd767a 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -1523,16 +1523,13 @@ qemuAgentGetVCPUs(qemuAgentPtr mon,
>  return ret;
>  }
> 
> -/**
> - * Set the VCPU state using guest agent.
> - *
> - * Returns -1 on error, ninfo in case everything was successful and less than
> - * ninfo on a partial failure.
> - */
> -int
> -qemuAgentSetVCPUs(qemuAgentPtr mon,
> -  qemuAgentCPUInfoPtr info,
> -  size_t ninfo)
> +
> +/* returns the value provided by the guest agent or -1 on internal error */
> +static int
> +qemuAgentSetVCPUsCommand(qemuAgentPtr mon,
> + qemuAgentCPUInfoPtr info,
> + size_t ninfo,
> + int *nmodified)
>  {
>  int ret = -1;
>  virJSONValuePtr cmd = NULL;
> @@ -1541,6 +1538,8 @@ qemuAgentSetVCPUs(qemuAgentPtr mon,
>  virJSONValuePtr cpu = NULL;
>  size_t i;
> 
> +*nmodified = 0;
> +
>  /* create the key data array */
>  if (!(cpus = virJSONValueNewArray()))
>  goto cleanup;
> @@ -1552,6 +1551,12 @@ qemuAgentSetVCPUs(qemuAgentPtr mon,
>  if (!(cpu = virJSONValueNewObject()))
>  goto cleanup;
> 
> +/* don't set state for cpus that were not touched */
> +if (!in->modified)
> +continue;

This needs to go before the virJSONValueNewObject, otherwise it would leak
memory.

> +
> +(*nmodified)++;
> +
>  if (virJSONValueObjectAppendNumberInt(cpu, "logical-id", in->id) < 0)
>  goto cleanup;
> 
> @@ -1564,6 +1569,11 @@ qemuAgentSetVCPUs(qemuAgentPtr mon,
>  cpu = NULL;
>  }
> 
> +if (*nmodified == 0) {
> +ret = 0;
> +goto cleanup;
> +}
> +
>  if (!(cmd = qemuAgentMakeCommand("guest-set-vcpus",
>   "a:vcpus", cpus,
>   NULL)))
> @@ -1575,9 +1585,17 @@ qemuAgentSetVCPUs(qemuAgentPtr mon,
>   VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
>  goto cleanup;
> 
> -if (virJSONValueObjectGetNumberInt(reply, "return", ) < 0) {
> +if (qemuAgentCheckError(cmd, reply) < 0)
> +goto cleanup;
> +
> +/* All negative values are invalid. Return of 0 is bougs since we 
> wouldn't

s/bougs/bogus/

> + * call the guest agent so that 0 cpus would be set successfully. 
> Reporting
> + * more successfuly set vcpus that we've asked for is invalid */

s/successfuly/successfully/
s/invalid/invalid./

> +if (virJSONValueObjectGetNumberInt(reply, "return", ) < 0 ||
> +ret <= 0 || ret > *nmodified) {
>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("malformed return value"));
> +   _("guest agent returned malformed or invalid return 
> value"));
> +ret = -1;
>  }
> 
>   cleanup:
> @@ -1589,6 +1607,45 @@ qemuAgentSetVCPUs(qemuAgentPtr mon,
>  }

[...]

ACK

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

Re: [libvirt] [PATCH 3/7] lib: Add API to set individual vcpu usage in the guest via guest agent

[Pavel Hrdina]

On Mon, Jun 20, 2016 at 04:34:17PM +0200, Peter Krempa wrote:
> To allow finer-grained control of vcpu state using guest agent this API
> can be used to individually set the state of the vCPU.
> 
> This will allow to better control NUMA enabled guests and/or test
> various vCPU configurations.
> ---

[...]

> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 0971a96..11f20af 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11891,3 +11891,52 @@ virDomainGetGuestVcpus(virDomainPtr domain,
>  virDispatchError(domain->conn);
>  return -1;
>  }
> +
> +
> +/**
> + * virDomainSetGuestVcpus:
> + * @domain: pointer to domain object
> + * @cpumap: text representation of a bitmap of vcpus to set
> + * @state: 0 to disable/1 to enable cpus described by @cpumap
> + * @flags: currently unused, callers shall pass 0
> + *
> + * Sets state of individual vcpus described by @cpumap via guest agent. Other
> + * vcpus are not modified.

It would be worth documenting how the bitmap should looks like as we do in
virsh.  This is a new bitmap representation in our APIs.  And also add a
reference to this description in the virDomainGetGuestVcpus API.

> + *
> + * This API requires the VM to run. Various hypervisors or guest agent
> + * implementation may limit to operate on just 1 vCPU per call.
> + *
> + * Note that OSes (notably Linux) may require vCPU 0 to stay online to 
> support
> + * low-level features a S3 sleep.
> + *
> + * Returns 0 on success, -1 on error.
> + */

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

Re: [libvirt] [RFC 00/28] s390x CPU models: exposing features

[David Hildenbrand]

> (CCing libvirt people)
> 
> On Tue, Jun 21, 2016 at 03:02:05PM +0200, David Hildenbrand wrote:
> > This is our second attempt to implement CPU models for s390x. We realized
> > that we also want to have features exposed via the CPU model. While doing
> > that we realized that we want to have a better interface for libvirt.  
> 
> Before getting into the details, I would like to clarify how the
> following could be accomplished using the new commands:
> 
> Example:
> 
> 1) User configures libvirt with:
>
>Westmere
>
>
> 2) libvirt will translate that to:
>"-cpu Westmere,+aes" or "-cpu Westmere,aes=on"
> 3) libvirt wants to know if "-cpu Westmere,aes=on" is usable in
>the current host, before trying to start the VM.
> 
> How exactly would this be done using the new commands?

Hi Eduardo,

thanks for having a look - highly appreciated that you actually map this
to libvirt requirements!

That would map to a compare operation between "host" and "Westmere,aes=on".

Host could at that point already be expanded by libvirt. Doesn't matter at that
point.

If the result is "identica"l or "superset", it is runnable. If the result is
"subset" or "incompatible", details about the responsible properties is
indicated. (I actually took that idea from your patch for indicating
runnability).

Thanks!

David

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

Re: [libvirt] [PATCH 0/3] qmp: query-host-cpu command

[David Hildenbrand]

> On Tue, Jun 21, 2016 at 02:52:54PM +0200, David Hildenbrand wrote:
> > > On Tue, Jun 21, 2016 at 08:20:40AM +0200, David Hildenbrand wrote:  
> > > > > Add QMP command to allow management software to query for
> > > > > CPU information for the running host.
> > > > > 
> > > > > The data returned by the command is in the form of a dictionary
> > > > > of QOM properties.
> > > > > 
> > > > > This series depends on the "Add runnability info to
> > > > > query-cpu-definitions" series I sent 2 weeks ago.
> > > > > 
> > > > > Git tree:
> > > > >   https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
> > > > > 
> > > > 
> > > > I like that interface, I'm going to post (maybe today? :) ) a similar 
> > > > interface
> > > > that allows to also expand other cpu models, not just the host model.   
> > > >  
> > > 
> > > In x86 I want to avoid exposing the details of other CPU models
> > > to libvirt because the details depend on machine-type.
> > > 
> > > But if it is useful for you, I believe the same "qom-properties"
> > > dict could be returned in query-cpu-definitions.
> > >   
> > > > 
> > > > Maybe we can then decide which one makes sense for all of us. But in 
> > > > general,
> > > > this interface is much better compared to what we had before. 
> > > 
> > > Maybe both? I think it's better to have a separate interface for
> > > querying "what exactly this host supports" and another one for
> > > querying for "what happens if I use -cpu host". In the case of
> > > x86, both are equivalent, but we can't guarantee this on all
> > > architectures.
> > >   
> > 
> > I'll post my patches in a couple of minutes, let's discuss it
> > then.
> > 
> > We might want to avoid having multiple interfaces carrying out
> > the same task.  
> 
> OK, I will wait for the patches before discussing it. My
> assumption is that both look similar, but are actually different
> tasks.
> 

For us, also "-cpu host" and querying the host model is identical. Not sure
if there would for now really be a requirement for some other architecture
to handle this differently. Do you have anything specific already in mind?

But let's see if it indeed makes sense to split this up after looking at
our interface proposal.

David

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

Re: [libvirt] [PATCH 4/7] virsh: Add command 'guestvcpus' implementing virDomain(GS)etGuestVcpus

[Pavel Hrdina]

On Mon, Jun 20, 2016 at 04:34:18PM +0200, Peter Krempa wrote:
> Add a straightforward implementation for using the new APIs.
> ---
>  tools/virsh-domain.c | 93 
> 
>  tools/virsh.pod  | 10 ++
>  2 files changed, 103 insertions(+)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index bd6db47..ff2305f 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -6762,6 +6762,93 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
>  return ret;
>  }
> 
> +
> +/*
> + * "guestvcpus" command
> + */
> +static const vshCmdInfo info_guestvcpus[] = {
> +{.name = "help",
> + .data = N_("query or modify state of vcpu in the guest (via agent)")
> +},
> +{.name = "desc",
> + .data = N_("Use the guest agent to query or set cpu state from guest's "
> +"point of view")
> +},
> +{.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_guestvcpus[] = {
> +VIRSH_COMMON_OPT_DOMAIN_FULL,
> +{.name = "cpumap",
> + .type = VSH_OT_STRING,
> + .help = N_("number of virtual CPUs")
> +},

I would go with "cpulist" and also change the help string, this is not correct.

> +{.name = "enable",
> + .type = VSH_OT_BOOL,
> + .help = N_("enable cpus specified by cpumap")
> +},
> +{.name = "disable",
> + .type = VSH_OT_BOOL,
> + .help = N_("disable cpus specified by cpumap")
> +},
> +{.name = NULL}
> +};

ACK

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

Re: [libvirt] [PATCH 3/7] lib: Add API to set individual vcpu usage in the guest via guest agent

[Pavel Hrdina]

On Mon, Jun 20, 2016 at 04:34:17PM +0200, Peter Krempa wrote:
> To allow finer-grained control of vcpu state using guest agent this API
> can be used to individually set the state of the vCPU.
> 
> This will allow to better control NUMA enabled guests and/or test
> various vCPU configurations.
> ---

ACK

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

Re: [libvirt] [RFC 00/28] s390x CPU models: exposing features

[Eduardo Habkost]

(CCing libvirt people)

On Tue, Jun 21, 2016 at 03:02:05PM +0200, David Hildenbrand wrote:
> This is our second attempt to implement CPU models for s390x. We realized
> that we also want to have features exposed via the CPU model. While doing
> that we realized that we want to have a better interface for libvirt.

Before getting into the details, I would like to clarify how the
following could be accomplished using the new commands:

Example:

1) User configures libvirt with:
   
   Westmere
   
   
2) libvirt will translate that to:
   "-cpu Westmere,+aes" or "-cpu Westmere,aes=on"
3) libvirt wants to know if "-cpu Westmere,aes=on" is usable in
   the current host, before trying to start the VM.

How exactly would this be done using the new commands?

> 
> Unfortunately, CPU models on s390x are special and we have to take care of:
> - A CPU like z13 looks differently in various environments (under different
>   LPAR versions, under different z/VM versions, under different KVM
>   versions, export regulation) - we have _a lot_ of feature variability.
> - We still have certain features that are not published but might be
>   implemented/introduced in the future. As they are a theoretical part
>   of a CPU already, we have to find a way to model these future changes.
> - We still have certain features that are already published, but not
>   implemented. Implementation might be added in the future in KVM.
> - We heavily rely on KVM to tell us which features it can actually
>   virtualize - user space queries like "STFL(e)" give no guarantees.
> - Certain "subfeatures" were introduced in one run. In practice, they are
>   always around, but in theory, subfeatures could be dropped in the future.
> - Just because two CPU models have the same features doesn't mean they
>   are equal - some internal numbers might be different. E.g. we won't allow
>   running a z13 under a zBC12 just by turning off features.
> - We cannot blindly enable all possible features for a CPU generation,
>   the IBC "Instruction Blocking Control" in KVM will try to block
>   instructions introduced with certain features. So a CPU generation always
>   has some maximum feature set that is guaranteed to work.
> 
> It all boils down to a specific released CPU to have.
> a) A fixed feature set that we expect it to be have on every hypervisor.
> b) A variable part that depends on the hypervisor and that could be
>extended in the future (adding not yet implemented features) that we
>always want to enable later on.
> c) A variable part that we want to enable only if requested - nested
>virtualization ("vsie") and assists are one example.
> 
> But, the fixed feature set is not really what we want to use as a default.
> It is just like a really minimum, stable base.
> 
> So we have
> a) A "stable" CPU model for each released CPU that will never change and
>maps to the minimum feature set we expect to be around on all
>hypervisors. e.g. "z13-base" or "z10EC.2-base". These are migration
>safe.
> b) A "default" CPU model for each released CPU, that can change between
>QEMU versions and that will always include the features we expect to
>be around in our currently supported environments and will contain only
>features we expect to be stable. E.g. nested virtualization will not be
>contained in these models. These models are not migration safe, e.g
>"z13" or "z10EC.2". The feature set can differ between QEMU versions.
> c) An internal "maximum" CPU model for each generation that tells us which
>features were supported as a maximum back when the hardware was
>released. This will not be exposed
> 
> To not have to replicate all CPU model changes ("new default fetaures") in
> libvirt, to not duplicate the logic about compatibility and the like,
> our approach tries to keep all the QEMU logic in libvirt and provide
> standardized interfaces for libvirt to e.g. baseline, compare. This
> allows libvirt to not have to care about any model names or feature names,
> it can just pass the data from interface to interface and report it to
> the user.
> 
> Also, libvirt might want to know what the "host" model looks like and
> convert a CPU model to a migration safe variant. For this reason, a QMP
> command is added that can create a migration safe variant of a variable
> CPU model, indicating only the delta changes done to a stable model.
> 
> So we have:
> a) "query-cpu-model-expansion" - tell us what the "host" or a migration
>unsafe model looks like. Either falling back to a stable model or
>completely exposing all properties. We are interested in stable models.
> b) "query-cpu-model-comparison" - tell us how two CPU models compare,
> indicating which properties were responsible for the decision.
> c) "query-cpu-model-baseline" - create a new model out of two models,
> taking a requested level of stability into account.
> 
> As we are aware that e.g. x86 has their own idea of a CPU 

Re: [libvirt] [PATCH 2/7] lib: Add API to query guest vcpu info using guest agent

[Pavel Hrdina]

On Mon, Jun 20, 2016 at 04:34:16PM +0200, Peter Krempa wrote:
> Add a rather universal API implemented via typed params that will allow
> to query the guest agent for the state and possibly other aspects of
> guest vcpus.
> ---

[...]

> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 73ae369..0971a96 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11835,3 +11835,59 @@ virDomainInterfaceFree(virDomainInterfacePtr iface)
> 
>  VIR_FREE(iface);
>  }
> +
> +
> +/**
> + * virDomainGetGuestVcpus:
> + * @domain: pointer to domain object
> + * @params: Pointer that will be filled with an array of typed parameters

s/Pointer/pointer/

> + * @nparams: pointer filled with number of elements in @params
> + * @flags: currently unused, callers shall pass 0
> + *
> + * Queries the guest agent for state and information regarding vCPUs from
> + * guest's perspective. The reported data depends on the guest agent
> + * implementation.
> + *
> + * Reported fields stored in @params:
> + * 'vcpus': string containing bitmap representing vCPU ids as reported by the
> + *  guest
> + * 'online': string containing bitmap representing online vCPUs as reported
> + *   by the guest agent.
> + * 'offlinable': string containing bitmap representing ids of vCPUs that can 
> be
> + *   offlined

ACK

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

Re: [libvirt] [PATCH 14/17] Assign addresses to USB devices

[Ján Tomko]

On Mon, Jun 20, 2016 at 09:02:54AM +0200, Gerd Hoffmann wrote:

On Fr, 2016-06-17 at 20:07 +0200, Ján Tomko wrote:

Automatically assign addresses to USB devices.

Just like reserving, this is only done for newly defined domains.


What happens if I add a device to a guest (using virsh edit)?




From libvirt's point of view, 'virsh edit' is a define,

so it will assing an address for the new device, as well as the existing
ones.

Jan

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

Re: [libvirt] [PATCH 04/17] Add a test for long USB port paths

[Ján Tomko]

On Mon, Jun 20, 2016 at 08:50:31AM +0200, Gerd Hoffmann wrote:

On Fr, 2016-06-17 at 20:07 +0200, Ján Tomko wrote:

For some reason, we support up to four levels of nested USB devices
in the guest.


FYI: The reason is simply that usb is specified that way.  You can't
chain usb hubs endlessly.



I was under the impression that 7 is the maximum depth.

I see that QEMU errors out at 6 levels for a USB hub:
qemu-git: -device usb-hub,port=1.1.1.1.1.1: usb hub chain too deep

So perhaps up to 6 levels could be allowed for regular devices?

Jan

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

Re: [libvirt] [PATCH 0/3] qmp: query-host-cpu command

[Eduardo Habkost]

On Tue, Jun 21, 2016 at 02:52:54PM +0200, David Hildenbrand wrote:
> > On Tue, Jun 21, 2016 at 08:20:40AM +0200, David Hildenbrand wrote:
> > > > Add QMP command to allow management software to query for
> > > > CPU information for the running host.
> > > > 
> > > > The data returned by the command is in the form of a dictionary
> > > > of QOM properties.
> > > > 
> > > > This series depends on the "Add runnability info to
> > > > query-cpu-definitions" series I sent 2 weeks ago.
> > > > 
> > > > Git tree:
> > > >   https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
> > > >   
> > > 
> > > I like that interface, I'm going to post (maybe today? :) ) a similar 
> > > interface
> > > that allows to also expand other cpu models, not just the host model.  
> > 
> > In x86 I want to avoid exposing the details of other CPU models
> > to libvirt because the details depend on machine-type.
> > 
> > But if it is useful for you, I believe the same "qom-properties"
> > dict could be returned in query-cpu-definitions.
> > 
> > > 
> > > Maybe we can then decide which one makes sense for all of us. But in 
> > > general,
> > > this interface is much better compared to what we had before.   
> > 
> > Maybe both? I think it's better to have a separate interface for
> > querying "what exactly this host supports" and another one for
> > querying for "what happens if I use -cpu host". In the case of
> > x86, both are equivalent, but we can't guarantee this on all
> > architectures.
> > 
> 
> I'll post my patches in a couple of minutes, let's discuss it
> then.
> 
> We might want to avoid having multiple interfaces carrying out
> the same task.

OK, I will wait for the patches before discussing it. My
assumption is that both look similar, but are actually different
tasks.

-- 
Eduardo

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

Re: [libvirt] [PATCH 0/8] Add --in-place and --check to test-wrap-argv

[Ján Tomko]

On Tue, Jun 21, 2016 at 06:41:35AM -0400, John Ferlan wrote:



On 06/15/2016 12:39 PM, Ján Tomko wrote:

The --in-place parameter makes the invocation in testutils slightly nicer
and --check makes syntax-check faster.

Ján Tomko (8):
  test-wrap-argv: split out rewrap_line
  test-wrap-argv: split out rewrap_arg
  test-wrap-argv: return a string in rewrap_arg
  test-wrap-argv: use map and join instead of a for cycle
  test-wrap-argv: return a string in rewrap_line
  test-wrap-argv: hold a copy of the original file in an array
  test-wrap-argv: add --in-place parameter
  test-wrap-argv: add --check parameter

 cfg.mk  |  12 +---
 tests/test-wrap-argv.pl | 170 ++--
 tests/testutils.c   |   8 +--
 3 files changed, 108 insertions(+), 82 deletions(-)



I believe one other difference is that "all" the differences or issues
are generated and displayed not just one at a time (which I actually
prefer).

Changes look OK to me; however, I think an "existing" issue is that the
splitting is off by 1 or 2 characters. If a ",", ":", or " " is the 80th
character and then a " \" is added, that addition is on the next line if
you have a an 80 column display (likewise a "\" when there are more args
to be printed).

Check out the following files and you'll note by visual inspection there
are lines that are longer than 80:

tests/qemuargv2xmldata/qemuargv2xml-disk-drive-cache-unsafe.args
tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-rbd-ipv6.args
tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-http.args
tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-network-tftp.args
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-lun.args
tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args
tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm-addr.args
tests/qemuxml2argvdata/qemuxml2argv-memory-hotplug-dimm.args
tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.args
tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-addr.args
tests/qemuxml2argvdata/qemuxml2argv-pci-autoadd-idx.args
tests/qemuxml2argvdata/qemuxml2argv-pci-many.args
tests/qemuxml2argvdata/qemuxml2argv-pcie-expander-bus.args

All I did was change 79 to 78 and 80 to 79 (it gets worse with 78/77)

ACK to what's done though...



Thanks, I've pushed the series and marked the issues on my TODO list.

Jan

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

[libvirt] [PATCH v2] There are several cases where we do not handle transient pool destroy and cleanup correctly. For example:

[Jovanka Gulicoska]

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

Move the pool cleanup logic to a new function storagePoolSetInactive and
use it consistently.
---
 src/storage/storage_driver.c | 43 +++
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index e2d729f..2f29292 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -77,6 +77,21 @@ static void storageDriverUnlock(void)
 }
 
 static void
+storagePoolSetInactive(virStoragePoolObjPtr *pool)
+{
+(*pool)->active = false;
+
+if ((*pool)->configFile == NULL) {
+virStoragePoolObjRemove(>pools, (*pool));
+*pool = NULL;
+} else if ((*pool)->newDef) {
+virStoragePoolDefFree((*pool)->def);
+(*pool)->def = (*pool)->newDef;
+(*pool)->newDef = NULL;
+}
+}
+
+static void
 storagePoolUpdateState(virStoragePoolObjPtr pool)
 {
 bool active;
@@ -115,16 +130,19 @@ storagePoolUpdateState(virStoragePoolObjPtr pool)
 if (backend->refreshPool(NULL, pool) < 0) {
 if (backend->stopPool)
 backend->stopPool(NULL, pool);
+active = false;
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to restart storage pool '%s': %s"),
pool->def->name, virGetLastErrorMessage());
 goto error;
 }
+active = true;
 }
 
-pool->active = active;
 ret = 0;
  error:
+if (!active)
+storagePoolSetInactive();
 if (ret < 0) {
 if (stateFile)
 unlink(stateFile);
@@ -198,6 +216,7 @@ storageDriverAutostart(void)
 unlink(stateFile);
 if (backend->stopPool)
 backend->stopPool(conn, pool);
+storagePoolSetInactive();
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to autostart storage pool '%s': %s"),
pool->def->name, virGetLastErrorMessage());
@@ -737,8 +756,7 @@ storagePoolCreateXML(virConnectPtr conn,
 unlink(stateFile);
 if (backend->stopPool)
 backend->stopPool(conn, pool);
-virStoragePoolObjRemove(>pools, pool);
-pool = NULL;
+storagePoolSetInactive();
 goto cleanup;
 }
 
@@ -1068,16 +1086,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
 VIR_STORAGE_POOL_EVENT_STOPPED,
 0);
 
-pool->active = false;
-
-if (pool->configFile == NULL) {
-virStoragePoolObjRemove(>pools, pool);
-pool = NULL;
-} else if (pool->newDef) {
-virStoragePoolDefFree(pool->def);
-pool->def = pool->newDef;
-pool->newDef = NULL;
-}
+storagePoolSetInactive();
 
 ret = 0;
 
@@ -1197,13 +1206,7 @@ storagePoolRefresh(virStoragePoolPtr obj,
 pool->def->uuid,
 VIR_STORAGE_POOL_EVENT_STOPPED,
 0);
-pool->active = false;
-
-if (pool->configFile == NULL) {
-virStoragePoolObjRemove(>pools, pool);
-pool = NULL;
-}
-goto cleanup;
+storagePoolSetInactive();
 }
 
 event = virStoragePoolEventLifecycleNew(pool->def->name,
-- 
2.5.5

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

[libvirt] [PATCH 06/18] Use virDirOpen

[Ján Tomko]

Switch from opendir to virDirOpen everywhere we need to report an error.
---
 src/storage/storage_backend_fs.c|  6 +-
 src/storage/storage_backend_iscsi.c |  7 +--
 src/storage/storage_backend_scsi.c  | 19 +++
 src/util/vircgroup.c|  5 +
 src/util/virfile.c  | 16 +++-
 src/util/virhostcpu.c   |  4 +---
 src/util/virnetdev.c|  6 +-
 src/util/virnetdevtap.c |  7 +--
 src/util/virpci.c   | 13 +++--
 src/util/virprocess.c   |  2 +-
 src/util/virscsi.c  | 10 ++
 src/util/virusb.c   |  7 +--
 src/util/virutil.c  | 18 +++---
 src/xen/xen_inotify.c   |  7 ++-
 src/xen/xm_internal.c   |  6 +-
 tests/virschematest.c   |  6 +-
 tools/nss/libvirt_nss.c |  4 +---
 17 files changed, 27 insertions(+), 116 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 152f9f3..2054309 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -900,12 +900,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 int direrr;
 int fd = -1, ret = -1;
 
-if (!(dir = opendir(pool->def->target.path))) {
-virReportSystemError(errno,
- _("cannot open path '%s'"),
- pool->def->target.path);
+if (virDirOpen(, pool->def->target.path) < 0)
 goto cleanup;
-}
 
 while ((direrr = virDirRead(dir, , pool->def->target.path)) > 0) {
 int err;
diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index e50158f..6283837 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -99,13 +99,8 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path,
 
 virFileWaitForDevices();
 
-sysdir = opendir(sysfs_path);
-
-if (sysdir == NULL) {
-virReportSystemError(errno,
- _("Failed to opendir path '%s'"), sysfs_path);
+if (virDirOpen(, sysfs_path) < 0)
 goto cleanup;
-}
 
 while ((direrr = virDirRead(sysdir, , sysfs_path)) > 0) {
 if (STREQLEN(dirent->d_name, "target", strlen("target"))) {
diff --git a/src/storage/storage_backend_scsi.c 
b/src/storage/storage_backend_scsi.c
index b08d960..df683e4 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -265,12 +265,8 @@ getNewStyleBlockDevice(const char *lun_path,
 
 VIR_DEBUG("Looking for block device in '%s'", block_path);
 
-if (!(block_dir = opendir(block_path))) {
-virReportSystemError(errno,
- _("Failed to opendir sysfs path '%s'"),
- block_path);
+if (virDirOpen(_dir, block_path) < 0)
 goto cleanup;
-}
 
 while ((direrr = virDirRead(block_dir, _dirent, block_path)) > 0) {
 if (STREQLEN(block_dirent->d_name, ".", 1))
@@ -353,12 +349,8 @@ getBlockDevice(uint32_t host,
 host, bus, target, lun) < 0)
 goto cleanup;
 
-if (!(lun_dir = opendir(lun_path))) {
-virReportSystemError(errno,
- _("Failed to opendir sysfs path '%s'"),
- lun_path);
+if (virDirOpen(_dir, lun_path) < 0)
 goto cleanup;
-}
 
 while ((direrr = virDirRead(lun_dir, _dirent, lun_path)) > 0) {
 if (STREQLEN(lun_dirent->d_name, "block", 5)) {
@@ -470,13 +462,8 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool,
 
 virFileWaitForDevices();
 
-devicedir = opendir(device_path);
-
-if (devicedir == NULL) {
-virReportSystemError(errno,
- _("Failed to opendir path '%s'"), device_path);
+if (virDirOpen(, device_path) < 0)
 return -1;
-}
 
 snprintf(devicepattern, sizeof(devicepattern), "%u:%%u:%%u:%%u\n", 
scanhost);
 
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index c76c94f..ce9b942 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3957,11 +3957,8 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
 cgroup->controllers[i].placement) < 0)
 goto cleanup;
 
-if (!(dh = opendir(base))) {
-virReportSystemError(errno,
- _("Unable to open dir '%s'"), base);
+if (virDirOpen(, base) < 0)
 goto cleanup;
-}
 
 while ((direrr = virDirRead(dh, , base)) > 0) {
 if (STREQ(de->d_name, ".") ||
diff --git a/src/util/virfile.c b/src/util/virfile.c
index aac0324..7dee3d9 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -614,11 +614,8 @@ static int virFileLoopDeviceOpenSearch(char **dev_name)
 
 VIR_DEBUG("Looking for loop devices in /dev");
 

[libvirt] [PATCH 16/18] Do not skip hidden entries when looking for a stable path

[Ján Tomko]

The device names are unlikely to start with a dot.
'.' and '..' are already skipped by virDirRead.
---
 src/storage/storage_backend.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 31c2974..32f0517 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1951,9 +1951,6 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
  */
  retry:
 while ((direrr = virDirRead(dh, , NULL)) > 0) {
-if (dent->d_name[0] == '.')
-continue;
-
 if (virAsprintf(, "%s/%s",
 pool->def->target.path,
 dent->d_name) == -1) {
-- 
2.7.3

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

[libvirt] [PATCH 04/18] Introduce VIR_DIR_CLOSE

[Ján Tomko]

Introduce a helper that only calls closedir if DIR* is non-NULL
and sets it to NULL afterwards.
---
 cfg.mk  |  4 ++--
 src/conf/network_conf.c |  4 ++--
 src/conf/nwfilter_conf.c|  2 +-
 src/conf/storage_conf.c |  4 ++--
 src/conf/virdomainobjlist.c |  2 +-
 src/conf/virsecretobj.c |  2 +-
 src/libvirt_private.syms|  1 +
 src/network/bridge_driver.c |  2 +-
 src/openvz/openvz_conf.c|  2 +-
 src/qemu/qemu_driver.c  |  3 +--
 src/qemu/qemu_hostdev.c |  4 +---
 src/storage/storage_backend.c   |  6 +++---
 src/storage/storage_backend_fs.c|  6 ++
 src/storage/storage_backend_iscsi.c |  2 +-
 src/storage/storage_backend_scsi.c  |  8 +++-
 src/util/vircgroup.c| 12 
 src/util/virfile.c  | 16 
 src/util/virfile.h  |  3 +++
 src/util/virhostcpu.c   |  6 ++
 src/util/virnetdev.c|  2 +-
 src/util/virnetdevtap.c |  2 +-
 src/util/virnuma.c  |  3 +--
 src/util/virpci.c   | 10 --
 src/util/virprocess.c   |  3 +--
 src/util/virscsi.c  |  6 ++
 src/util/virusb.c   |  4 +---
 src/util/virutil.c  |  6 +++---
 src/xen/xen_inotify.c   |  6 +++---
 src/xen/xm_internal.c   |  4 ++--
 tests/virschematest.c   |  2 +-
 tools/nss/libvirt_nss.c |  6 ++
 31 files changed, 66 insertions(+), 77 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index d82070d..a2576d1 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -421,9 +421,9 @@ sc_prohibit_gethostname:
  $(_sc_search_regexp)
 
 sc_prohibit_readdir:
-   @prohibit='\breaddir *\('   \
+   @prohibit='\b(read|close)dir *\('   \
exclude='exempt from syntax-check'  \
-   halt='use virDirRead, not readdir'  \
+   halt='use virDirRead and VIR_DIR_CLOSE' \
  $(_sc_search_regexp)
 
 sc_prohibit_gettext_noop:
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 02b8cd7..1e4b719 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3258,7 +3258,7 @@ virNetworkLoadAllState(virNetworkObjListPtr nets,
 virNetworkObjEndAPI();
 }
 
-closedir(dir);
+VIR_DIR_CLOSE(dir);
 return ret;
 }
 
@@ -3298,7 +3298,7 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets,
 virNetworkObjEndAPI();
 }
 
-closedir(dir);
+VIR_DIR_CLOSE(dir);
 return ret;
 }
 
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 3f90f65..56f8b86 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -3227,7 +3227,7 @@ virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters,
 virNWFilterObjUnlock(nwfilter);
 }
 
-closedir(dir);
+VIR_DIR_CLOSE(dir);
 return ret;
 }
 
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 6932195..5c044d2 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1964,7 +1964,7 @@ virStoragePoolLoadAllState(virStoragePoolObjListPtr pools,
 virStoragePoolObjUnlock(pool);
 }
 
-closedir(dir);
+VIR_DIR_CLOSE(dir);
 return ret;
 }
 
@@ -2015,7 +2015,7 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr 
pools,
 VIR_FREE(autostartLink);
 }
 
-closedir(dir);
+VIR_DIR_CLOSE(dir);
 return ret;
 }
 
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 485671e..4f7756d 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -616,7 +616,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
 }
 }
 
-closedir(dir);
+VIR_DIR_CLOSE(dir);
 virObjectUnlock(doms);
 return ret;
 }
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index c46d22c..46042eb 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -1000,6 +1000,6 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
 virSecretObjEndAPI();
 }
 
-closedir(dir);
+VIR_DIR_CLOSE(dir);
 return 0;
 }
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 501c23e..f4dc88d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1493,6 +1493,7 @@ saferead;
 safewrite;
 safezero;
 virBuildPathInternal;
+virDirClose;
 virDirCreate;
 virDirRead;
 virFileAbsPath;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 7c8d2cc..7b021d8 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -577,7 +577,7 @@ networkMigrateStateFiles(virNetworkDriverStatePtr driver)
 
 ret = 0;
  cleanup:
-closedir(dir);
+VIR_DIR_CLOSE(dir);
 VIR_FREE(oldPath);
 

[libvirt] [PATCH 13/18] Do not check for '.' and '..' after virDirRead

[Ján Tomko]

It skips those directory entries.
---
 src/conf/virsecretobj.c | 3 ---
 src/network/bridge_driver.c | 4 
 src/util/vircgroup.c| 8 
 src/util/virfile.c  | 4 
 src/util/virpci.c   | 4 
 5 files changed, 23 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index a093258..30a5e80 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -977,9 +977,6 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
 char *path;
 virSecretObjPtr secret;
 
-if (STREQ(de->d_name, ".") || STREQ(de->d_name, ".."))
-continue;
-
 if (!virFileHasSuffix(de->d_name, ".xml"))
 continue;
 
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b108152..58ceaf2 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -525,10 +525,6 @@ networkMigrateStateFiles(virNetworkDriverStatePtr driver)
 entry->d_type != DT_REG)
 continue;
 
-if (STREQ(entry->d_name, ".") ||
-STREQ(entry->d_name, ".."))
-continue;
-
 if (virAsprintf(, "%s/%s",
 oldStateDir, entry->d_name) < 0)
 goto cleanup;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index da20ba5..971894a 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3634,10 +3634,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
 }
 
 while ((direrr = virDirRead(dp, , keypath)) > 0) {
-if (STREQ(ent->d_name, "."))
-continue;
-if (STREQ(ent->d_name, ".."))
-continue;
 if (ent->d_type != DT_DIR)
 continue;
 
@@ -3958,10 +3954,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
 goto cleanup;
 
 while ((direrr = virDirRead(dh, , base)) > 0) {
-if (STREQ(de->d_name, ".") ||
-STREQ(de->d_name, ".."))
-continue;
-
 if (virAsprintf(, "%s/%s", base, de->d_name) < 0)
 goto cleanup;
 
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 2772089..a45279a 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -941,10 +941,6 @@ int virFileDeleteTree(const char *dir)
 while ((direrr = virDirRead(dh, , dir)) > 0) {
 struct stat sb;
 
-if (STREQ(de->d_name, ".") ||
-STREQ(de->d_name, ".."))
-continue;
-
 if (virAsprintf(, "%s/%s",
 dir, de->d_name) < 0)
 goto cleanup;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index c497f02..948fdbf 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2694,10 +2694,6 @@ virPCIGetNetName(char *device_link_sysfs_path, char 
**netname)
 goto out;
 
 while (virDirRead(dir, , pcidev_sysfs_net_path) > 0) {
-if (STREQ(entry->d_name, ".") ||
-STREQ(entry->d_name, ".."))
-continue;
-
 /* Assume a single directory entry */
 if (VIR_STRDUP(*netname, entry->d_name) > 0)
 ret = 0;
-- 
2.7.3

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

[libvirt] [PATCH 08/18] Use virDirOpenIfExists

[Ján Tomko]

Use it instead of opendir everywhere we need to check for ENOENT.
---
 src/conf/network_conf.c | 21 ++---
 src/conf/nwfilter_conf.c| 10 +++---
 src/conf/storage_conf.c | 20 ++--
 src/conf/virdomainobjlist.c | 11 +++
 src/conf/virsecretobj.c |  9 +++--
 src/network/bridge_driver.c | 11 +++
 src/qemu/qemu_driver.c  |  8 +---
 src/util/vircgroup.c| 14 ++
 src/util/virnuma.c  | 14 --
 9 files changed, 35 insertions(+), 83 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 1e4b719..2aa7242 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3236,14 +3236,10 @@ virNetworkLoadAllState(virNetworkObjListPtr nets,
 DIR *dir;
 struct dirent *entry;
 int ret = -1;
+int rc;
 
-if (!(dir = opendir(stateDir))) {
-if (errno == ENOENT)
-return 0;
-
-virReportSystemError(errno, _("Failed to open dir '%s'"), stateDir);
-return -1;
-}
+if ((rc = virDirOpenIfExists(, stateDir)) <= 0)
+return rc;
 
 while ((ret = virDirRead(dir, , stateDir)) > 0) {
 virNetworkObjPtr net;
@@ -3270,15 +3266,10 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets,
 DIR *dir;
 struct dirent *entry;
 int ret = -1;
+int rc;
 
-if (!(dir = opendir(configDir))) {
-if (errno == ENOENT)
-return 0;
-virReportSystemError(errno,
- _("Failed to open dir '%s'"),
- configDir);
-return -1;
-}
+if ((rc = virDirOpenIfExists(, configDir)) <= 0)
+return rc;
 
 while ((ret = virDirRead(dir, , configDir)) > 0) {
 virNetworkObjPtr net;
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 56f8b86..74f2a14 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -3204,14 +3204,10 @@ virNWFilterLoadAllConfigs(virNWFilterObjListPtr 
nwfilters,
 DIR *dir;
 struct dirent *entry;
 int ret = -1;
+int rc;
 
-if (!(dir = opendir(configDir))) {
-if (errno == ENOENT)
-return 0;
-virReportSystemError(errno, _("Failed to open dir '%s'"),
- configDir);
-return -1;
-}
+if ((rc = virDirOpenIfExists(, configDir)) <= 0)
+return rc;
 
 while ((ret = virDirRead(dir, , configDir)) > 0) {
 virNWFilterObjPtr nwfilter;
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 5c044d2..d1ca08b 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1941,14 +1941,10 @@ virStoragePoolLoadAllState(virStoragePoolObjListPtr 
pools,
 DIR *dir;
 struct dirent *entry;
 int ret = -1;
+int rc;
 
-if (!(dir = opendir(stateDir))) {
-if (errno == ENOENT)
-return 0;
-
-virReportSystemError(errno, _("Failed to open dir '%s'"), stateDir);
-return -1;
-}
+if ((rc = virDirOpenIfExists(, stateDir)) <= 0)
+return rc;
 
 while ((ret = virDirRead(dir, , stateDir)) > 0) {
 virStoragePoolObjPtr pool;
@@ -1977,14 +1973,10 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr 
pools,
 DIR *dir;
 struct dirent *entry;
 int ret;
+int rc;
 
-if (!(dir = opendir(configDir))) {
-if (errno == ENOENT)
-return 0;
-virReportSystemError(errno, _("Failed to open dir '%s'"),
- configDir);
-return -1;
-}
+if ((rc = virDirOpenIfExists(, configDir)) <= 0)
+return rc;
 
 while ((ret = virDirRead(dir, , configDir)) > 0) {
 char *path;
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 4f7756d..51753fe 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -566,17 +566,12 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
 DIR *dir;
 struct dirent *entry;
 int ret = -1;
+int rc;
 
 VIR_INFO("Scanning for configs in %s", configDir);
 
-if (!(dir = opendir(configDir))) {
-if (errno == ENOENT)
-return 0;
-virReportSystemError(errno,
- _("Failed to open dir '%s'"),
- configDir);
-return -1;
-}
+if ((rc = virDirOpenIfExists(, configDir)) <= 0)
+return rc;
 
 virObjectLock(doms);
 
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 46042eb..a093258 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -966,13 +966,10 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
 {
 DIR *dir = NULL;
 struct dirent *de;
+int rc;
 
-if (!(dir = opendir(configDir))) {
-if (errno == ENOENT)
-return 0;
-virReportSystemError(errno, _("cannot open '%s'"), configDir);
-return -1;
-}
+if ((rc = virDirOpenIfExists(, 

[libvirt] [PATCH 07/18] Add virDirOpenIfExists

[Ján Tomko]

Just like virDirOpen, but it returns 0 without reporting an error
on ENOENT.
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 21 +++--
 src/util/virfile.h   |  2 ++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 457fe19..2bb1d95 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1496,6 +1496,7 @@ virBuildPathInternal;
 virDirClose;
 virDirCreate;
 virDirOpen;
+virDirOpenIfExists;
 virDirRead;
 virFileAbsPath;
 virFileAccessibleAs;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 7dee3d9..efdb98b 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2727,10 +2727,12 @@ virFileRemove(const char *path,
 #endif /* WIN32 */
 
 static int
-virDirOpenInternal(DIR **dirp, const char *name)
+virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT)
 {
 *dirp = opendir(name);
 if (!*dirp) {
+if (ignoreENOENT && errno == ENOENT)
+return 0;
 virReportSystemError(errno, _("cannot open directory '%s'"), name);
 return -1;
 }
@@ -2748,7 +2750,22 @@ virDirOpenInternal(DIR **dirp, const char *name)
 int
 virDirOpen(DIR **dirp, const char *name)
 {
-return virDirOpenInternal(dirp, name);
+return virDirOpenInternal(dirp, name, false);
+}
+
+/**
+ * virDirOpenIfExists
+ * @dirp: directory stream
+ * @name: path of the directory
+ *
+ * Returns 1 on success.
+ * If opendir returns ENOENT, 0 is returned without reporting an error.
+ * On other errors, -1 is returned and an error is reported.
+ */
+int
+virDirOpenIfExists(DIR **dirp, const char *name)
+{
+return virDirOpenInternal(dirp, name, true);
 }
 
 /**
diff --git a/src/util/virfile.h b/src/util/virfile.h
index c618842..42c65f2 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -232,6 +232,8 @@ int virDirCreate(const char *path, mode_t mode, uid_t uid, 
gid_t gid,
  unsigned int flags) ATTRIBUTE_RETURN_CHECK;
 int virDirOpen(DIR **dirp, const char *dirname)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virDirOpenIfExists(DIR **dirp, const char *dirname)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 void virDirClose(DIR **dirp)
-- 
2.7.3

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

[libvirt] [PATCH 09/18] Introduce virDirOpenQuiet

[Ján Tomko]

A helper function that does not report any errors.
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 25 ++---
 src/util/virfile.h   |  2 ++
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2bb1d95..8ebe6f3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1497,6 +1497,7 @@ virDirClose;
 virDirCreate;
 virDirOpen;
 virDirOpenIfExists;
+virDirOpenQuiet;
 virDirRead;
 virFileAbsPath;
 virFileAccessibleAs;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index efdb98b..f6c43d4 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2727,10 +2727,13 @@ virFileRemove(const char *path,
 #endif /* WIN32 */
 
 static int
-virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT)
+virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet)
 {
 *dirp = opendir(name);
 if (!*dirp) {
+if (quiet)
+return -1;
+
 if (ignoreENOENT && errno == ENOENT)
 return 0;
 virReportSystemError(errno, _("cannot open directory '%s'"), name);
@@ -2750,7 +2753,7 @@ virDirOpenInternal(DIR **dirp, const char *name, bool 
ignoreENOENT)
 int
 virDirOpen(DIR **dirp, const char *name)
 {
-return virDirOpenInternal(dirp, name, false);
+return virDirOpenInternal(dirp, name, false, false);
 }
 
 /**
@@ -2765,7 +2768,23 @@ virDirOpen(DIR **dirp, const char *name)
 int
 virDirOpenIfExists(DIR **dirp, const char *name)
 {
-return virDirOpenInternal(dirp, name, true);
+return virDirOpenInternal(dirp, name, true, false);
+}
+
+/**
+ * virDirOpenQuiet
+ * @dirp: directory stream
+ * @name: path of the directory
+ *
+ * Returns 1 on success.
+ *-1 on failure.
+ *
+ * Does not report any errors and errno is preserved.
+ */
+int
+virDirOpenQuiet(DIR **dirp, const char *name)
+{
+return virDirOpenInternal(dirp, name, false, true);
 }
 
 /**
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 42c65f2..b4ae6ea 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -234,6 +234,8 @@ int virDirOpen(DIR **dirp, const char *dirname)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 int virDirOpenIfExists(DIR **dirp, const char *dirname)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virDirOpenQuiet(DIR **dirp, const char *dirname)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 void virDirClose(DIR **dirp)
-- 
2.7.3

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

[libvirt] [PATCH 15/18] Do not ignore hidden files in /sys and /proc

[Ján Tomko]

The directories we iterate over are unlikely to contain any entries
starting with a dot, other than '.' and '..' which is already skipped
by virDirRead.
---
 src/qemu/qemu_hostdev.c| 4 
 src/storage/storage_backend_scsi.c | 3 ---
 src/util/vircgroup.c   | 1 -
 src/util/virnetdev.c   | 2 --
 src/util/virpci.c  | 7 ---
 src/util/virprocess.c  | 4 
 src/util/virscsi.c | 6 --
 src/util/virusb.c  | 2 +-
 src/util/virutil.c | 8 +---
 9 files changed, 2 insertions(+), 35 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 2fdfae9..dd3a3cf 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -115,10 +115,6 @@ qemuHostdevHostSupportsPassthroughVFIO(void)
 goto cleanup;
 
 while ((direrr = virDirRead(iommuDir, , NULL)) > 0) {
-/* skip ./ ../ */
-if (STRPREFIX(iommuGroup->d_name, "."))
-continue;
-
 /* assume we found a group */
 break;
 }
diff --git a/src/storage/storage_backend_scsi.c 
b/src/storage/storage_backend_scsi.c
index df683e4..558b3cf 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -269,9 +269,6 @@ getNewStyleBlockDevice(const char *lun_path,
 goto cleanup;
 
 while ((direrr = virDirRead(block_dir, _dirent, block_path)) > 0) {
-if (STREQLEN(block_dirent->d_name, ".", 1))
-continue;
-
 if (VIR_STRDUP(*block_device, block_dirent->d_name) < 0)
 goto cleanup;
 
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 971894a..04f3818 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3391,7 +3391,6 @@ virCgroupRemoveRecursively(char *grppath)
 while ((direrr = virDirRead(grpdir, , NULL)) > 0) {
 char *path;
 
-if (ent->d_name[0] == '.') continue;
 if (ent->d_type != DT_DIR) continue;
 
 if (virAsprintf(, "%s/%s", grppath, ent->d_name) == -1) {
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 84406de..cdbc41b 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3168,8 +3168,6 @@ virNetDevRDMAFeature(const char *ifname,
 goto cleanup;
 
 while (virDirRead(dirp, , SYSFS_INFINIBAND_DIR) > 0) {
-if (dp->d_name[0] == '.')
-continue;
 if (virAsprintf(_devpath, SYSFS_INFINIBAND_DIR "%s/device/resource",
 dp->d_name) < 0)
 continue;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 948fdbf..04ff68b 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -468,10 +468,6 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate 
predicate,
 virPCIDevicePtr check;
 char *tmp;
 
-/* Ignore '.' and '..' */
-if (entry->d_name[0] == '.')
-continue;
-
 /* expected format: ::. */
 if (/* domain */
 virStrToLong_ui(entry->d_name, , 16, ) < 0 || *tmp != 
':' ||
@@ -2024,9 +2020,6 @@ 
virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig,
 while ((direrr = virDirRead(groupDir, , groupPath)) > 0) {
 virPCIDeviceAddress newDev;
 
-if (ent->d_name[0] == '.')
-continue;
-
 if (virPCIDeviceAddressParse(ent->d_name, ) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Found invalid device link '%s' in '%s'"),
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index a3e7f4e..09dd3c9 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -619,10 +619,6 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t 
**pids)
 long long tmp;
 pid_t tmp_pid;
 
-/* Skip . and .. */
-if (STRPREFIX(ent->d_name, "."))
-continue;
-
 if (virStrToLong_ll(ent->d_name, NULL, 10, ) < 0)
 goto cleanup;
 tmp_pid = tmp;
diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 0f57494..4843367 100644
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -131,9 +131,6 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix,
 goto cleanup;
 
 while (virDirRead(dir, , path) > 0) {
-if (entry->d_name[0] == '.')
-continue;
-
 /* Assume a single directory entry */
 ignore_value(VIR_STRDUP(sg, entry->d_name));
 break;
@@ -174,9 +171,6 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix,
 goto cleanup;
 
 while (virDirRead(dir, , path) > 0) {
-if (entry->d_name[0] == '.')
-continue;
-
 ignore_value(VIR_STRDUP(name, entry->d_name));
 break;
 }
diff --git a/src/util/virusb.c b/src/util/virusb.c
index 1db8173..6a001a7 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -145,7 +145,7 @@ virUSBDeviceSearch(unsigned int vendor,
 unsigned int found_prod, found_vend, found_bus, found_devno;
 char *tmpstr = 

[libvirt] [PATCH 10/18] Use virDirOpenQuiet

[Ján Tomko]

Remove all the remaining usage of opendir.
---
 src/openvz/openvz_conf.c  | 3 +--
 src/qemu/qemu_hostdev.c   | 2 +-
 src/storage/storage_backend.c | 2 +-
 src/util/vircgroup.c  | 3 +--
 src/util/virhostcpu.c | 2 +-
 src/util/virpci.c | 2 +-
 6 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index ff5e5b8..b678456 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -1057,8 +1057,7 @@ static int openvzAssignUUIDs(void)
 if (conf_dir == NULL)
 return -1;
 
-dp = opendir(conf_dir);
-if (dp == NULL) {
+if (virDirOpenQuiet(, conf_dir) < 0) {
 VIR_FREE(conf_dir);
 return 0;
 }
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 84f3615..2fdfae9 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -111,7 +111,7 @@ qemuHostdevHostSupportsPassthroughVFIO(void)
 int direrr;
 
 /* condition 1 - /sys/kernel/iommu_groups/ contains entries */
-if (!(iommuDir = opendir("/sys/kernel/iommu_groups/")))
+if (virDirOpenQuiet(, "/sys/kernel/iommu_groups/") < 0)
 goto cleanup;
 
 while ((direrr = virDirRead(iommuDir, , NULL)) > 0) {
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 955d983..31c2974 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1928,7 +1928,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
  * get created.
  */
  reopen:
-if ((dh = opendir(pool->def->target.path)) == NULL) {
+if (virDirOpenQuiet(, pool->def->target.path) < 0) {
 opentries++;
 if (loop && errno == ENOENT && opentries < 50) {
 usleep(100 * 1000);
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 634f659..da20ba5 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3378,8 +3378,7 @@ virCgroupRemoveRecursively(char *grppath)
 int rc = 0;
 int direrr;
 
-grpdir = opendir(grppath);
-if (grpdir == NULL) {
+if (virDirOpenQuiet(, grppath) < 0) {
 if (errno == ENOENT)
 return 0;
 rc = -errno;
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 087ce22..f38fbec 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -641,7 +641,7 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
 if (virAsprintf(_nodedir, "%s/node", sysfs_system_path) < 0)
 goto cleanup;
 
-if (!(nodedir = opendir(sysfs_nodedir))) {
+if (virDirOpenQuiet(, sysfs_nodedir) < 0) {
 /* the host isn't probably running a NUMA architecture */
 goto fallback;
 }
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 77ae9b4..c497f02 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2015,7 +2015,7 @@ 
virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig,
 orig->domain, orig->bus, orig->slot, orig->function) < 0)
 goto cleanup;
 
-if (!(groupDir = opendir(groupPath))) {
+if (virDirOpenQuiet(, groupPath) < 0) {
 /* just process the original device, nothing more */
 ret = (actor)(orig, opaque);
 goto cleanup;
-- 
2.7.3

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

[libvirt] [PATCH 14/18] Fix comment in virStorageBackendFileSystemRefresh

[Ján Tomko]

'.' and '..' are now skipped by virDirRead, there's no need to mention
them in the comment.
---
 src/storage/storage_backend_fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 2054309..44dabf4 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -932,7 +932,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 >target.encryption)) < 0) 
{
 if (err == -2) {
 /* Silently ignore non-regular files,
- * eg '.' '..', 'lost+found', dangling symbolic link */
+ * eg 'lost+found', dangling symbolic link */
 virStorageVolDefFree(vol);
 vol = NULL;
 continue;
-- 
2.7.3

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

[libvirt] [PATCH 12/18] Skip '.' and '..' in virDirRead

[Ján Tomko]

All of the callers either skip these explicitly, skip all entries
starting with a dot or match the entry name against stricter patterns.
---
 src/util/virfile.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 1820e80..2772089 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2809,14 +2809,17 @@ virDirOpenQuiet(DIR **dirp, const char *name)
  */
 int virDirRead(DIR *dirp, struct dirent **ent, const char *name)
 {
-errno = 0;
-*ent = readdir(dirp); /* exempt from syntax-check */
-if (!*ent && errno) {
-if (name)
-virReportSystemError(errno, _("Unable to read directory '%s'"),
- name);
-return -1;
-}
+do {
+errno = 0;
+*ent = readdir(dirp); /* exempt from syntax-check */
+if (!*ent && errno) {
+if (name)
+virReportSystemError(errno, _("Unable to read directory '%s'"),
+ name);
+return -1;
+}
+} while (*ent && (STREQ((*ent)->d_name, ".") ||
+  STREQ((*ent)->d_name, "..")));
 return !!*ent;
 }
 
-- 
2.7.3

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

[libvirt] [PATCH 17/18] Allow configs to start with a dot

[Ján Tomko]

This fixes the disappearance of domains and networks starting with a
dot.

https://bugzilla.redhat.com/show_bug.cgi?id=1333248
---
 src/conf/network_conf.c | 6 --
 src/conf/nwfilter_conf.c| 3 ---
 src/conf/storage_conf.c | 6 --
 src/conf/virdomainobjlist.c | 3 ---
 src/qemu/qemu_driver.c  | 3 ---
 5 files changed, 21 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 2aa7242..dfa851b 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3244,9 +3244,6 @@ virNetworkLoadAllState(virNetworkObjListPtr nets,
 while ((ret = virDirRead(dir, , stateDir)) > 0) {
 virNetworkObjPtr net;
 
-if (entry->d_name[0] == '.')
-continue;
-
 if (!virFileStripSuffix(entry->d_name, ".xml"))
 continue;
 
@@ -3274,9 +3271,6 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets,
 while ((ret = virDirRead(dir, , configDir)) > 0) {
 virNetworkObjPtr net;
 
-if (entry->d_name[0] == '.')
-continue;
-
 if (!virFileStripSuffix(entry->d_name, ".xml"))
 continue;
 
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 74f2a14..2cdcfa7 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -3212,9 +3212,6 @@ virNWFilterLoadAllConfigs(virNWFilterObjListPtr nwfilters,
 while ((ret = virDirRead(dir, , configDir)) > 0) {
 virNWFilterObjPtr nwfilter;
 
-if (entry->d_name[0] == '.')
-continue;
-
 if (!virFileStripSuffix(entry->d_name, ".xml"))
 continue;
 
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index d1ca08b..05a1a49 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1949,9 +1949,6 @@ virStoragePoolLoadAllState(virStoragePoolObjListPtr pools,
 while ((ret = virDirRead(dir, , stateDir)) > 0) {
 virStoragePoolObjPtr pool;
 
-if (entry->d_name[0] == '.')
-continue;
-
 if (!virFileStripSuffix(entry->d_name, ".xml"))
 continue;
 
@@ -1983,9 +1980,6 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr 
pools,
 char *autostartLink;
 virStoragePoolObjPtr pool;
 
-if (entry->d_name[0] == '.')
-continue;
-
 if (!virFileHasSuffix(entry->d_name, ".xml"))
 continue;
 
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 51753fe..41c9910 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -578,9 +578,6 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
 while ((ret = virDirRead(dir, , configDir)) > 0) {
 virDomainObjPtr dom;
 
-if (entry->d_name[0] == '.')
-continue;
-
 if (!virFileStripSuffix(entry->d_name, ".xml"))
 continue;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7769272..1174317 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -502,9 +502,6 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
 goto cleanup;
 
 while ((direrr = virDirRead(dir, , NULL)) > 0) {
-if (entry->d_name[0] == '.')
-continue;
-
 /* NB: ignoring errors, so one malformed config doesn't
kill the whole process */
 VIR_INFO("Loading snapshot file '%s'", entry->d_name);
-- 
2.7.3

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

[libvirt] [PATCH 11/18] Prohibit opendir in syntax-check

[Ján Tomko]

Prefer virDirOpen.
---
 cfg.mk | 7 +--
 src/util/virfile.c | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index a2576d1..de1c1da 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -421,9 +421,9 @@ sc_prohibit_gethostname:
  $(_sc_search_regexp)
 
 sc_prohibit_readdir:
-   @prohibit='\b(read|close)dir *\('   \
+   @prohibit='\b(read|close|open)dir *\('  \
exclude='exempt from syntax-check'  \
-   halt='use virDirRead and VIR_DIR_CLOSE' \
+   halt='use virDirOpen, virDirRead and VIR_DIR_CLOSE' \
  $(_sc_search_regexp)
 
 sc_prohibit_gettext_noop:
@@ -1293,3 +1293,6 @@ exclude_file_name_regexp--sc_prohibit_dt_without_code = \
 
 exclude_file_name_regexp--sc_prohibit_always-defined_macros = \
   ^tests/virtestmock.c$$
+
+exclude_file_name_regexp--sc_prohibit_readdir = \
+  ^tests/.*mock\.c$$
diff --git a/src/util/virfile.c b/src/util/virfile.c
index f6c43d4..1820e80 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2729,7 +2729,7 @@ virFileRemove(const char *path,
 static int
 virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet)
 {
-*dirp = opendir(name);
+*dirp = opendir(name); /* exempt from syntax-check */
 if (!*dirp) {
 if (quiet)
 return -1;
-- 
2.7.3

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

[libvirt] [PATCH 05/18] Introduce virDirOpen

[Ján Tomko]

A helper that calls opendir and reports an error if it fails.
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 33 +
 src/util/virfile.h   |  2 ++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f4dc88d..457fe19 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1495,6 +1495,7 @@ safezero;
 virBuildPathInternal;
 virDirClose;
 virDirCreate;
+virDirOpen;
 virDirRead;
 virFileAbsPath;
 virFileAccessibleAs;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index ce8f7fd..aac0324 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2736,6 +2736,31 @@ virFileRemove(const char *path,
 }
 #endif /* WIN32 */
 
+static int
+virDirOpenInternal(DIR **dirp, const char *name)
+{
+*dirp = opendir(name);
+if (!*dirp) {
+virReportSystemError(errno, _("cannot open directory '%s'"), name);
+return -1;
+}
+return 1;
+}
+
+/**
+ * virDirOpen
+ * @dirp: directory stream
+ * @name: path of the directory
+ *
+ * Returns 1 on success.
+ * On failure, -1 is returned and an error is reported.
+ */
+int
+virDirOpen(DIR **dirp, const char *name)
+{
+return virDirOpenInternal(dirp, name);
+}
+
 /**
  * virDirRead:
  * @dirp: directory to read
@@ -2744,13 +2769,13 @@ virFileRemove(const char *path,
  *
  * Wrapper around readdir. Typical usage:
  *   struct dirent ent;
- *   int value;
+ *   int rc;
  *   DIR *dir;
- *   if (!(dir = opendir(name)))
+ *   if (virDirOpen(, name) < 0)
  *   goto error;
- *   while ((value = virDirRead(dir, , name)) > 0)
+ *   while ((rc = virDirRead(dir, , name)) > 0)
  *   process ent;
- *   if (value < 0)
+ *   if (rc < 0)
  *   goto error;
  *
  * Returns -1 on error, with error already reported if @name was
diff --git a/src/util/virfile.h b/src/util/virfile.h
index ab9eeba..c618842 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -230,6 +230,8 @@ enum {
 };
 int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid,
  unsigned int flags) ATTRIBUTE_RETURN_CHECK;
+int virDirOpen(DIR **dirp, const char *dirname)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 int virDirRead(DIR *dirp, struct dirent **ent, const char *dirname)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
 void virDirClose(DIR **dirp)
-- 
2.7.3

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

[libvirt] [PATCH 18/18] Do not skip files starting with a dot in leases directory

[Ján Tomko]

'.' and '..' are skipped by virDirRead already.
---
 tools/nss/libvirt_nss.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
index 724cb06..66e338a 100644
--- a/tools/nss/libvirt_nss.c
+++ b/tools/nss/libvirt_nss.c
@@ -137,9 +137,6 @@ findLease(const char *name,
 while ((ret = virDirRead(dir, , leaseDir)) > 0) {
 char *path;
 
-if (entry->d_name[0] == '.')
-continue;
-
 if (!virFileHasSuffix(entry->d_name, ".status"))
 continue;
 
-- 
2.7.3

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

[libvirt] [PATCH 03/18] Do not check the return value of closedir

[Ján Tomko]

The only possible error is EBADFD.
Since we only use the directory stream returned by opendir,
this should never happen.
---
 src/util/virhostcpu.c | 15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 4ed7b21..0cdba0a 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -477,11 +477,8 @@ virHostCPUParseNode(const char *node,
 ret = processors;
 
  cleanup:
-/* don't shadow a more serious error */
-if (cpudir && closedir(cpudir) < 0 && ret >= 0) {
-virReportSystemError(errno, _("problem closing %s"), node);
-ret = -1;
-}
+if (cpudir)
+closedir(cpudir);
 if (cores_maps)
 for (i = 0; i < sock_max; i++)
 virBitmapFree(cores_maps[i]);
@@ -777,12 +774,8 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
 ret = 0;
 
  cleanup:
-/* don't shadow a more serious error */
-if (nodedir && closedir(nodedir) < 0 && ret >= 0) {
-virReportSystemError(errno, _("problem closing %s"), sysfs_nodedir);
-ret = -1;
-}
-
+if (nodedir)
+closedir(nodedir);
 virBitmapFree(present_cpus_map);
 virBitmapFree(online_cpus_map);
 VIR_FREE(sysfs_nodedir);
-- 
2.7.3

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

[libvirt] [PATCH 00/18] Allow domains to start with a dot

[Ján Tomko]

Also introduce virDirOpen* and VIR_DIR_CLOSE helpers.

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

Ján Tomko (18):
  Do not save errno in virUSBDeviceSearch
  Fix error detection in virStorageBackendISCSIGetHostNumber
  Do not check the return value of closedir
  Introduce VIR_DIR_CLOSE
  Introduce virDirOpen
  Use virDirOpen
  Add virDirOpenIfExists
  Use virDirOpenIfExists
  Introduce virDirOpenQuiet
  Use virDirOpenQuiet
  Prohibit opendir in syntax-check
  Skip '.' and '..' in virDirRead
  Do not check for '.' and '..' after virDirRead
  Fix comment in virStorageBackendFileSystemRefresh
  Do not ignore hidden files in /sys and /proc
  Do not skip hidden entries when looking for a stable path
  Allow configs to start with a dot
  Do not skip files starting with a dot in leases directory

 cfg.mk  |   7 +-
 src/conf/network_conf.c |  31 +++--
 src/conf/nwfilter_conf.c|  15 ++---
 src/conf/storage_conf.c |  30 +++--
 src/conf/virdomainobjlist.c |  16 ++---
 src/conf/virsecretobj.c |  14 ++--
 src/libvirt_private.syms|   4 ++
 src/network/bridge_driver.c |  17 ++---
 src/openvz/openvz_conf.c|   5 +-
 src/qemu/qemu_driver.c  |  14 +---
 src/qemu/qemu_hostdev.c |  10 +--
 src/storage/storage_backend.c   |  11 ++--
 src/storage/storage_backend_fs.c|  14 ++--
 src/storage/storage_backend_iscsi.c |  47 +++---
 src/storage/storage_backend_scsi.c  |  30 ++---
 src/util/vircgroup.c|  43 -
 src/util/virfile.c  | 124 ++--
 src/util/virfile.h  |   9 +++
 src/util/virhostcpu.c   |  19 ++
 src/util/virnetdev.c|  10 +--
 src/util/virnetdevtap.c |   9 +--
 src/util/virnuma.c  |  17 ++---
 src/util/virpci.c   |  36 +++
 src/util/virprocess.c   |   9 +--
 src/util/virscsi.c  |  22 ++-
 src/util/virusb.c   |  16 +
 src/util/virutil.c  |  32 ++
 src/xen/xen_inotify.c   |  13 ++--
 src/xen/xm_internal.c   |  10 +--
 tests/virschematest.c   |   8 +--
 tools/nss/libvirt_nss.c |  13 +---
 31 files changed, 245 insertions(+), 410 deletions(-)

-- 
2.7.3

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

[libvirt] [PATCH 01/18] Do not save errno in virUSBDeviceSearch

[Ján Tomko]

The virUSBDeviceFind* callers do not check errno after calling
this function.
---
 src/util/virusb.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/util/virusb.c b/src/util/virusb.c
index 520610b..5c39667 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -202,11 +202,8 @@ virUSBDeviceSearch(unsigned int vendor,
 ret = list;
 
  cleanup:
-if (dir) {
-int saved_errno = errno;
+if (dir)
 closedir(dir);
-errno = saved_errno;
-}
 
 if (!ret)
 virObjectUnref(list);
-- 
2.7.3

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

[libvirt] [PATCH 02/18] Fix error detection in virStorageBackendISCSIGetHostNumber

[Ján Tomko]

In the unlikely case the iSCSI session path exists, but does not
contain an entry starting with "target", we would silently use
an initialized value.

Rewrite the function to correctly report errors.
---
 src/storage/storage_backend_iscsi.c | 38 +++--
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index bccfba3..876c14c 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -90,7 +90,7 @@ static int
 virStorageBackendISCSIGetHostNumber(const char *sysfs_path,
 uint32_t *host)
 {
-int retval = 0;
+int ret = -1;
 DIR *sysdir = NULL;
 struct dirent *dirent = NULL;
 int direrr;
@@ -104,26 +104,33 @@ virStorageBackendISCSIGetHostNumber(const char 
*sysfs_path,
 if (sysdir == NULL) {
 virReportSystemError(errno,
  _("Failed to opendir path '%s'"), sysfs_path);
-retval = -1;
-goto out;
+goto cleanup;
 }
 
 while ((direrr = virDirRead(sysdir, , sysfs_path)) > 0) {
 if (STREQLEN(dirent->d_name, "target", strlen("target"))) {
-if (sscanf(dirent->d_name,
-   "target%u:", host) != 1) {
-VIR_DEBUG("Failed to parse target '%s'", dirent->d_name);
-retval = -1;
-break;
+if (sscanf(dirent->d_name, "target%u:", host) == 1) {
+ret = 0;
+goto cleanup;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to parse target '%s'"), 
dirent->d_name);
+goto cleanup;
 }
 }
 }
-if (direrr < 0)
-retval = -1;
 
+if (direrr == 0) {
+virReportSystemError(errno,
+ _("Failed to get host number for iSCSI session "
+   "with path '%s'"),
+ sysfs_path);
+goto cleanup;
+}
+
+ cleanup:
 closedir(sysdir);
- out:
-return retval;
+return ret;
 }
 
 static int
@@ -138,13 +145,8 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
 "/sys/class/iscsi_session/session%s/device", session) < 0)
 goto cleanup;
 
-if (virStorageBackendISCSIGetHostNumber(sysfs_path, ) < 0) {
-virReportSystemError(errno,
- _("Failed to get host number for iSCSI session "
-   "with path '%s'"),
- sysfs_path);
+if (virStorageBackendISCSIGetHostNumber(sysfs_path, ) < 0)
 goto cleanup;
-}
 
 if (virStorageBackendSCSIFindLUs(pool, host) < 0)
 goto cleanup;
-- 
2.7.3

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

Re: [libvirt] [PATCH 1/7] rpcgen: Add support for generating funcs returning alloc'd typed params

[Pavel Hrdina]

On Mon, Jun 20, 2016 at 04:34:15PM +0200, Peter Krempa wrote:
> Since it's rather tedious to write the dispatchers for functions that
> return an array of typed parameters (which are rather common) let's add
> some rpcgen code to generate them.
> ---
>  src/remote/remote_protocol.x |  5 +
>  src/rpc/gendispatch.pl   | 45 
> +++-
>  2 files changed, 49 insertions(+), 1 deletion(-)

ACK

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

Re: [libvirt] [PATCH 09/17] Add newDomain parameter to qemuDomainAssignAddresses

[Peter Krempa]

On Fri, Jun 17, 2016 at 20:07:11 +0200, Ján Tomko wrote:
> Pass 'true' if we are not dealing with a migration.
> ---
>  src/qemu/qemu_domain.c | 3 ++-
>  src/qemu/qemu_domain_address.c | 3 ++-
>  src/qemu/qemu_domain_address.h | 3 ++-
>  src/qemu/qemu_process.c| 7 ---
>  tests/qemuhotplugtest.c| 2 +-
>  5 files changed, 11 insertions(+), 7 deletions(-)

ACK

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

Re: [libvirt] [PATCH 08/17] Do not call postParse with ABI_UPDATE when parsing cmdline

[Peter Krempa]

On Fri, Jun 17, 2016 at 20:07:10 +0200, Ján Tomko wrote:
> So far this is only useful for recalculating NUMA memory size,
> which this function cannot parse.
> 
> This will let us generate USB addresses based on this flag.
> ---
>  src/qemu/qemu_parse_command.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
> index 927bd79..1d54a68 100644
> --- a/src/qemu/qemu_parse_command.c
> +++ b/src/qemu/qemu_parse_command.c
> @@ -2626,8 +2626,7 @@ qemuParseCommandLine(virCapsPtr caps,
>  
>  VIR_FREE(nics);
>  
> -if (virDomainDefPostParse(def, caps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
> -  xmlopt) < 0)
> +if (virDomainDefPostParse(def, caps, 0, xmlopt) < 0)
>  goto error;

Additionally ABI update is not desired in case when this function is
used in the qemu-attach API. Thankfully that doesn't work in most cases
either so I didn't manage to break much.

ACK

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

Re: [libvirt] [PATCH 06/17] Use for instead of code duplication when parsing USB port

[Peter Krempa]

On Fri, Jun 17, 2016 at 20:07:08 +0200, Ján Tomko wrote:
> We are done if the string ends and move to another nesting
> level if we find a dot.
> ---
>  src/conf/device_conf.h |  3 +++
>  src/conf/domain_conf.c | 19 +++
>  2 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
> index 847564b..4903839 100644
> --- a/src/conf/device_conf.h
> +++ b/src/conf/device_conf.h
> @@ -118,6 +118,9 @@ typedef struct _virDomainDeviceCcidAddress {
>  unsigned int slot;
>  } virDomainDeviceCcidAddress, *virDomainDeviceCcidAddressPtr;
>  
> +/* chosen by fair dice roll */

Remove this line.

> +# define VIR_DOMAIN_DEVICE_USB_MAX_PORT_DEPTH 4
> +
>  typedef struct _virDomainDeviceUSBAddress {
>  unsigned int bus;
>  char *port;

ACK

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

Re: [libvirt] [PATCH 03/17] Add a USB hub to controller order test

[Peter Krempa]

On Fri, Jun 17, 2016 at 20:07:05 +0200, Ján Tomko wrote:
> The test has too many USB devices.
> ---
>  tests/qemuxml2argvdata/qemuxml2argv-controller-order.args | 1 +
>  tests/qemuxml2argvdata/qemuxml2argv-controller-order.xml  | 1 +
>  tests/qemuxml2argvtest.c  | 3 ++-
>  3 files changed, 4 insertions(+), 1 deletion(-)

ACK

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

Re: [libvirt] [PATCH 04/17] Add a test for long USB port paths

[Peter Krempa]

On Fri, Jun 17, 2016 at 20:07:06 +0200, Ján Tomko wrote:
> For some reason, we support up to four levels of nested USB devices
> in the guest.

This was explained by Gerd, so please fix it.

> 
> Add a test for a domain using all four and a negative test for a domain
> using five.
> ---

ACK

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

Re: [libvirt] [PATCH 05/17] Split out USB port parsing

[Peter Krempa]

On Fri, Jun 17, 2016 at 20:07:07 +0200, Ján Tomko wrote:
> Make rewriting it easier.
> ---
>  src/conf/domain_conf.c | 32 ++--
>  1 file changed, 22 insertions(+), 10 deletions(-)

ACK

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

Re: [libvirt] [PATCH 02/17] Fix USB port in input-usbmouse test

[Peter Krempa]

On Fri, Jun 17, 2016 at 20:07:04 +0200, Ján Tomko wrote:
> The default USB controller only has two ports.
> ---
>  tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.args | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse-addr.xml  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

ACK

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

Re: [libvirt] [PATCH 14/19] conf: Add new secret type "key"

[John Ferlan]

On 06/21/2016 08:08 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:53 -0400, John Ferlan wrote:
>> Add a new secret type known as "key" - it will handle adding the secret
>> objects that need a key (or passphrase), such as will soon be the case
> 
> This may be misleading a "key" is not equal to a "passprhase" in usual
> encryption terminology. Key usually refers to the actual encryption key
> used to encrypt the data whereas passprhase is usually a human readable
> secret string (which may not be random at all) used to access the key
> later.
> 
> The cryptsetup man page tends to treat them interchangably to some
> extent (eg a key slot equals to passprhase, but the master key refers to
> the actual encryption key used for the data).
> 
> To avoid confusion I'd rather stick with "passphrase".
> 
>> for a luks volume for both storage driver create and libvirt domain usage.
>>
>> Signed-off-by: John Ferlan 
>> ---

While replying to review comments from 6/19, I realized another reason I
went with "key" over "passphrase".

Consider the existing/old qcow encryption format
(http://libvirt.org/formatsecret.html)

The  XML looks like:

  
Super secret name of my first puppy
0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f

  /var/lib/libvirt/images/puppyname.img

  

while the  XML has:

  
   
  

or once patch 11 hits:

  

  

where 'usage' matches 'volume'

Using something other than passphrase allowed me to distinguish between
that 'old' format and this new style...

Using "passphrase" will then have  format of:

  
/

And a  format of

  
Sample
0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f

  somestring

  

where "somestring" is just a 'usage' string and not the actual
passphrase which would be set by the 'secret-set-value' command.

I could have the  XML use something different than passphrase,
but key just seemed to be the most reasonable beyond passphrase. Unless
you have a different suggestion for a better name.

John

Hopefully this was clear...

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

Re: [libvirt] [PATCH 06/19] util: Move and rename virStorageAuthDefParseSecret

[John Ferlan]

On 06/21/2016 10:05 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:45 -0400, John Ferlan wrote:
>> Move to virsecret.c and rename to virSecretParseSecret. Also convert to
> 
> NACK to the naming. It implies that it parses  xmls.
> 

The "Secret" just replaced the "StorageAuthDef" since no longer was a
 going to be only an element of .

>> usage xmlNodePtr and virXMLPropString rather than virXPathString.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  po/POTFILES.in|  1 +
>>  src/libvirt_private.syms  |  1 +
>>  src/util/virsecret.c  | 44 +
>>  src/util/virsecret.h  |  5 +++-
>>  src/util/virstoragefile.c | 71 
>> ---
>>  5 files changed, 67 insertions(+), 55 deletions(-)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 32d5179..ca65885 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -2211,6 +2211,7 @@ virSecurityLabelDefNew;
>>  # util/virsecret.h
>>  virSecretLookupDefClear;
>>  virSecretLookupDefCopy;
>> +virSecretParseSecret;
> 
> It should be named similarly to the above or to the original name.
> 
> ACK to the code as long as you name it reasonably
> 

OK - is virSecretLookupParseSecret more palatable? Or an even longer
name virSecretLookupParseDomainSecret.

Up through this point it's only a subelement of , but adding to
 short term and to  for TLS secrets in another series.

Also, As I was working in the TLS code - I began to realize using
'secdef' for variable names became confusing, so I changed to
'seclookupdef'...  That was posted as a separate series

I also note that patch 5 doesn't yet have any review, but this patch
relies on that.

Tks -

John



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

Re: [libvirt] libvirt and VIR_WARN("cannot parse process status data")

[Martin Kletzander]

On Tue, Jun 21, 2016 at 04:40:45PM +0300, Vasiliy Tolstov wrote:

I have system with 3.19.3 kernel and libvirt 1.3.3 (version not matter
because part of code not changed).
When i'm try to get cpu stats via libvirt i have  error in libvirt
log, that says "cannot parse process status data"
relevant code in src/qemu/qemu_driver.c
in function qemuGetProcessInfo



Honestly, I have only very rough idea about what you're trying to say.
But you see that warning mentioned.  Let's go from that.


i'm add debut test to this function and in my case i have always fscanf != 4



That's weird indeed, but what are you getting as a result if not 4?  Are
you getting it for each of the PIDs below or just for some?  Are you
getting bad statistics or you just don't like the warning?  What is the
debug message few lines below that saying (the "Got status for ...")?

This could be caused by some weird libC, I guess.  What are you using on
your system?

If you were able to modify the code to add more debugging, then try
reading the file, print it out for yourself, then after the fscanf see
what's in those variables.  I can only guess without more detail.


Does somebody have time to fix this ?



The problem here is we don't know what is there to fix.


--
Vasiliy Tolstov,
e-mail: v.tols...@yoctocloud.net

--
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 14/19] conf: Add new secret type "key"

[John Ferlan]

On 06/21/2016 08:08 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:53 -0400, John Ferlan wrote:
>> Add a new secret type known as "key" - it will handle adding the secret
>> objects that need a key (or passphrase), such as will soon be the case
> 
> This may be misleading a "key" is not equal to a "passprhase" in usual
> encryption terminology. Key usually refers to the actual encryption key
> used to encrypt the data whereas passprhase is usually a human readable
> secret string (which may not be random at all) used to access the key
> later.
> 
> The cryptsetup man page tends to treat them interchangably to some
> extent (eg a key slot equals to passprhase, but the master key refers to
> the actual encryption key used for the data).
> 
> To avoid confusion I'd rather stick with "passphrase".
> 

That was my other choice... 'key' was just shorter and easier to type.

I'll make that adjustment, so it'll be:


  f52a81b2-424e-490c-823d-6bd4235bc572
  Sample Passphrase Secret
  
mumblyfratz
  



John
>> for a luks volume for both storage driver create and libvirt domain usage.
>>
>> Signed-off-by: John Ferlan 
>> ---

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

Re: [libvirt] [PATCH 1/3] Remove unused SOL_NETLINK macro

[Laine Stump]

On 06/21/2016 04:17 AM, Ján Tomko wrote:

On Mon, Jun 20, 2016 at 01:43:30PM -0400, Laine Stump wrote:

On 06/20/2016 11:28 AM, Ján Tomko wrote:

Introduced by commit d575679, unused at the time.
---
  src/util/virnetlink.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 5491ece..513f36e 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -42,10 +42,6 @@
  #include "virmacaddr.h"
  #include "virerror.h"

-#ifndef SOL_NETLINK
-# define SOL_NETLINK 270
-#endif
-
  #define VIR_FROM_THIS VIR_FROM_NET

  VIR_LOG_INIT("util.netlink");


Provisional ACK since you are correct that it isn't visibly used, but I
can't help thinking the author had some reason for adding it.


An earlier version of the patchset used it to call:
 setsockopt(fd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP,
https://www.redhat.com/archives/libvir-list/2012-August/msg01457.html

In a later version it was changed to nl_socket_add_membership without
removing the macro (since we disable -Wunused-macros by default):
https://www.redhat.com/archives/libvir-list/2012-August/msg01541.html


Okay, so that explains how it got there, and proves that it's unneeded, 
so the provisional ACK is now full :-)




Sadly, this function is also present in libnl 1.1, which is our minimum
required version, so we can't drop the #ifdev HAVE_LIBNL1 code yet.


I don't understand the relationship. LIBNL1 is used in RHEL6 because for 
a very long time libnl3 wasn't available there.  At some point 
relatively far into the life of RHEL6 the libnl-3 package was added (but 
appears to only be used by libiwpm and sssd-common; I had *thought* that 
NetworkManager in RHEL6 had been the reason for the addition, but I 
guess I remembered wrong). At any rate it's probably not a good idea to 
switch even upstream RHEL6/CentOS6 builds (which are likely the only 
reason left to keep libnl1) over to the RHEL6 libnl-3 library, because 
it was added so later, is used almost nowhere else, and is such an old 
version. So unfortunately we will have to keep the libnl1 support around 
for a long while yet.





It looks
like SOL_NETLINK wasn't present in kernel 2.6 (RHEL6 vintage - although
it is mentioned in a comment of one of the system #includes), but was
there by the time of kernel 3.10 (RHEL7).



It was introduced by commit 9a4595bc7e released in kernel v2.6.14,
so RHEL6 based on 2.6.32 had it from the start.


It may seem that way, but actually isn't. The problem is that whatever 
file it was originally put in didn't get into the kernel-headers 
package. So "find /etc -name \*.h | xargs grep SOL_NETLINK" finds 
nothing, and if you try to use SOL_NETLINK in a RHEL6 build of libvirt 
with these extra lines removed, it will fail. (Yes, I tried it)


(That's not a reason to not rip out these lines, just illustrating that 
all is not always as it seems :-)


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

Re: [libvirt] [PATCH 15/19] encryption: Add luks parsing for storageencryption

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:54 -0400, John Ferlan wrote:
> Add parse and format of the luks/key secret including tests for
> volume XML parsing.
> 
> Signed-off-by: John Ferlan 
> ---
>  docs/formatsecret.html.in  |  7 +++-
>  docs/formatstorageencryption.html.in   | 24 +++-
>  docs/schemas/storagecommon.rng |  3 ++
>  src/qemu/qemu_process.c|  6 +++
>  src/storage/storage_backend.c  |  3 +-
>  src/storage/storage_backend_fs.c   |  7 +++-
>  src/storage/storage_backend_gluster.c  |  2 +
>  src/util/virstorageencryption.c|  4 +-
>  src/util/virstorageencryption.h|  2 +
>  tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml | 41 
>  .../qemuxml2xmlout-luks-disks.xml  | 45 
> ++
>  tests/qemuxml2xmltest.c|  1 +
>  tests/storagevolxml2xmlin/vol-luks.xml | 21 ++
>  tests/storagevolxml2xmlout/vol-luks.xml| 21 ++
>  tests/storagevolxml2xmltest.c  |  1 +
>  15 files changed, 181 insertions(+), 7 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-luks-disks.xml
>  create mode 100644 tests/storagevolxml2xmlin/vol-luks.xml
>  create mode 100644 tests/storagevolxml2xmlout/vol-luks.xml

[]

> diff --git a/docs/formatstorageencryption.html.in 
> b/docs/formatstorageencryption.html.in
> index 048cc8e..ae2e815 100644
> --- a/docs/formatstorageencryption.html.in
> +++ b/docs/formatstorageencryption.html.in
> @@ -59,8 +59,20 @@
>the secret element is not present during volume creation,
>a secret is automatically generated and attached to the volume.
>  
> +"luks" format
> +
> +  The luks format is specific to a luks encrypted volume
> +  and the secret used in order to either encrypt or decrypt the volume.
> +  A single secret type='key' element is expected.

I've explained in some other patch why 'key' is not a desired name.

> +  The secret may be referenced via either a uuid or
> +  usage attribute. One of the two must be present. When
> +  present for volume creation, the secret will be used in order for
> +  volume encryption.  When present for domain usage, the secret will
> +  be used as the key to decrypt the volume.
> +  Since 1.3.6.
> +
>  
> -Example
> +Examples
>  
>  
>Here is a simple example, specifying use of the qcow 
> format:

I'll like to see a updated version.

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

Re: [libvirt] [PATCH 13/19] util: Add 'luks' to the FileTypeInfo

[John Ferlan]

On 06/21/2016 08:41 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:52 -0400, John Ferlan wrote:
>> Add the ability to detect a luks encrypted device.
>>
>> This also adding new 16 bit big/little endian macros since the
>> version of a luks device is stored in a uint16_t.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/util/virendian.h  | 24 
>>  src/util/virstoragefile.c | 38 --
>>  src/util/virstoragefile.h |  1 +
>>  tests/virendiantest.c | 18 ++
>>  4 files changed, 75 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/util/virendian.h b/src/util/virendian.h
>> index eefe48c..97940bd 100644
>> --- a/src/util/virendian.h
>> +++ b/src/util/virendian.h
>> @@ -90,4 +90,28 @@
>>   ((uint32_t)(uint8_t)((buf)[2]) << 16) | \
>>   ((uint32_t)(uint8_t)((buf)[3]) << 24))
>>  
>> +/**
>> + * virReadBufInt16BE:
>> + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*');
>> + *   evaluating buf must not have any side effects
>> + *
>> + * Read 2 bytes at BUF as a big-endian 16-bit number.  Caller is
>> + * responsible to avoid reading beyond array bounds.
>> + */
>> +# define virReadBufInt16BE(buf)  \
>> +(((uint16_t)(uint8_t)((buf)[0]) << 8) |  \
>> + (uint16_t)(uint8_t)((buf)[1]))
>> +
>> +/**
>> + * virReadBufInt16LE:
>> + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*');
>> + *   evaluating buf must not have any side effects
>> + *
>> + * Read 2 bytes at BUF as a little-endian 16-bit number.  Caller is
>> + * responsible to avoid reading beyond array bounds.
>> + */
>> +# define virReadBufInt16LE(buf)  \
>> +((uint16_t)(uint8_t)((buf)[0]) | \
>> + ((uint16_t)(uint8_t)((buf)[1]) << 8))
>> +
>>  #endif /* __VIR_ENDIAN_H__ */
> 
> Since you are adding this code and also the tests below it looks like a
> job for a separate patch.
> 

OK - easily done

>> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
>> index 5c2519c..f7a9632 100644
>> --- a/src/util/virstoragefile.c
>> +++ b/src/util/virstoragefile.c
> 
> [...]
> 
>> @@ -193,6 +194,14 @@ qedGetBackingStore(char **, int *, const char *, 
>> size_t);
>>  #define PLOOP_IMAGE_SIZE_OFFSET 36
>>  #define PLOOP_SIZE_MULTIPLIER 512
>>  
>> +#define LUKS_HDR_MAGIC_LEN 6
>> +#define LUKS_HDR_VERSION_LEN 2
>> +
>> +/* Format described by qemu commit id '3e308f20e' */
>> +#define LUKS_HDR_VERSION_OFFSET LUKS_HDR_MAGIC_LEN
>> +#define LUKS_HDR_CRYPT_OFFSET LUKS_HDR_MAGIC_LEN + LUKS_HDR_VERSION_LEN
>> +
>> +
>>  static struct FileTypeInfo const fileTypeInfo[] = {
>>  [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
>>  -1, {0}, 0, 0, 0, FI_CRYPT_NONE, 0, NULL, 
>> NULL },
>> @@ -249,6 +258,13 @@ static struct FileTypeInfo const fileTypeInfo[] = {
>>   PLOOP_SIZE_MULTIPLIER,
>>   FI_CRYPT_NONE, -1, NULL, NULL },
>>  
>> +/* Magic is 'L','U','K','S', 0xBA, 0xBE
>> + * Set sizeOffset = -1 and let hypervisor handle */
>> +[VIR_STORAGE_FILE_LUKS] = {
>> +0, "\x4c\x55\x4b\x53\xba\xbe", NULL,
>> +LV_BIG_ENDIAN, LUKS_HDR_VERSION_OFFSET, {1},
>> +-1, 0, 0, FI_CRYPT_LUKS, LUKS_HDR_CRYPT_OFFSET, NULL, NULL
> 
> The encryption header offset is not used here. You are not extracting
> the cipher name from the header.
> 

Hmm... I think at one time I had thought it might need to be. I can drop
LUKS_HDR_CRYPT_OFFSET


>> +},
>>  /* All formats with a backing store probe below here */
>>  [VIR_STORAGE_FILE_COW] = {
>>  0, "OOOM", NULL,
>> @@ -634,7 +650,7 @@ virStorageFileMatchesVersion(int format,
>>   char *buf,
>>   size_t buflen)
>>  {
>> -int version;
>> +int version = 0;
>>  size_t i;
>>  
>>  /* Validate version number info */
>> @@ -648,10 +664,16 @@ virStorageFileMatchesVersion(int format,
>>  if ((fileTypeInfo[format].versionOffset + 4) > buflen)
>>  return false;
>>  
>> -if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN)
>> +if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) {
>>  version = virReadBufInt32LE(buf + 
>> fileTypeInfo[format].versionOffset);
>> -else
>> -version = virReadBufInt32BE(buf + 
>> fileTypeInfo[format].versionOffset);
>> +} else {
>> +if (format == VIR_STORAGE_FILE_LUKS)
> 
> This should be selected with a different property than the file type.
> The idea of the structure above was to avoid having to tweak this code
> with individual cases for every single file type. You'll either need to
> pass a function pointer for extraction or switch it according to the
> passed size.
> 

Ewww.  I think going with versionSize will be easier, although
versionFunc would be just as 

Re: [libvirt] [PATCH 09/19] qemu: Remove authdef from secret setup

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:48 -0400, John Ferlan wrote:
> Rather than pass authdef, pass the 'authdef->username' and the
> '>secdef'
> 
> Note that a username may be NULL.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_domain.c | 37 ++---
>  1 file changed, 22 insertions(+), 15 deletions(-)

ACK

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

Re: [libvirt] [PATCH 07/19] util: Introduce virSecretFormatSecret

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:46 -0400, John Ferlan wrote:
> Add utility to format the virSecretLookupTypeDefPtr in XML
> 
> Signed-off-by: John Ferlan 
> ---
>  src/libvirt_private.syms  |  1 +
>  src/util/virsecret.c  | 26 ++
>  src/util/virsecret.h  |  3 +++
>  src/util/virstoragefile.c | 19 +--
>  4 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index ca65885..fdf06ae 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2209,6 +2209,7 @@ virSecurityLabelDefNew;
>  
>  
>  # util/virsecret.h
> +virSecretFormatSecret;

ACK if you pick a more reasonable name.

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

Re: [libvirt] [PATCH 12/19] util: Modify the FileTypeInfo for meta data checks

[Peter Krempa]

On Tue, Jun 21, 2016 at 10:01:50 -0400, John Ferlan wrote:
> 
> 
> On 06/21/2016 08:20 AM, Peter Krempa wrote:
> > On Mon, Jun 13, 2016 at 20:27:51 -0400, John Ferlan wrote:
> >> Currently the assumption is there is one type of disk encryption - in
> >> some qcow format which is old and crusty... But there's a new sheriff
> >> in town known as 'luks' and we'll need to handle that shortly
> >>
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  src/util/virstoragefile.c | 54 
> >> ---
> >>  1 file changed, 32 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> >> index 6d7e5d9..5c2519c 100644
> >> --- a/src/util/virstoragefile.c
> >> +++ b/src/util/virstoragefile.c
> > 
> > [...]
> > 
> >> @@ -111,6 +111,11 @@ enum {
> >>  BACKING_STORE_ERROR,
> >>  };
> >>  
> >> +enum fi_crypt {
> >> +FI_CRYPT_NONE = 0,
> >> +FI_CRYPT_QCOW
> > 
> > This lacks the "VIR_" prefix. Also I don't really see a point in adding
> > this. Currently it's used to distinguish between an encrypted QCOW and
> > an unencrypted QCOW. With LUKS (as you note later in a comment) it's
> > implied that they are encrypted and thus we don't need a side band to
> > store the same data.
> > 
> 
> OK I can drop this... It would be replaced in "a" subsequent patch with
> a more direct "meta->format == VIR_STORAGE_FILE_LUKS" type check in
> order to allocate meta->encryption

I concluded that it might be desired to keep this as long as you want to
parse more data from the LUKS header. The name change is desired though.

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

Re: [libvirt] [PATCH 06/19] util: Move and rename virStorageAuthDefParseSecret

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:45 -0400, John Ferlan wrote:
> Move to virsecret.c and rename to virSecretParseSecret. Also convert to

NACK to the naming. It implies that it parses  xmls.

> usage xmlNodePtr and virXMLPropString rather than virXPathString.
> 
> Signed-off-by: John Ferlan 
> ---
>  po/POTFILES.in|  1 +
>  src/libvirt_private.syms  |  1 +
>  src/util/virsecret.c  | 44 +
>  src/util/virsecret.h  |  5 +++-
>  src/util/virstoragefile.c | 71 
> ---
>  5 files changed, 67 insertions(+), 55 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 32d5179..ca65885 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2211,6 +2211,7 @@ virSecurityLabelDefNew;
>  # util/virsecret.h
>  virSecretLookupDefClear;
>  virSecretLookupDefCopy;
> +virSecretParseSecret;

It should be named similarly to the above or to the original name.

ACK to the code as long as you name it reasonably

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

Re: [libvirt] [PATCH 12/19] util: Modify the FileTypeInfo for meta data checks

[John Ferlan]

On 06/21/2016 08:20 AM, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:51 -0400, John Ferlan wrote:
>> Currently the assumption is there is one type of disk encryption - in
>> some qcow format which is old and crusty... But there's a new sheriff
>> in town known as 'luks' and we'll need to handle that shortly
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/util/virstoragefile.c | 54 
>> ---
>>  1 file changed, 32 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
>> index 6d7e5d9..5c2519c 100644
>> --- a/src/util/virstoragefile.c
>> +++ b/src/util/virstoragefile.c
> 
> [...]
> 
>> @@ -111,6 +111,11 @@ enum {
>>  BACKING_STORE_ERROR,
>>  };
>>  
>> +enum fi_crypt {
>> +FI_CRYPT_NONE = 0,
>> +FI_CRYPT_QCOW
> 
> This lacks the "VIR_" prefix. Also I don't really see a point in adding
> this. Currently it's used to distinguish between an encrypted QCOW and
> an unencrypted QCOW. With LUKS (as you note later in a comment) it's
> implied that they are encrypted and thus we don't need a side band to
> store the same data.
> 

OK I can drop this... It would be replaced in "a" subsequent patch with
a more direct "meta->format == VIR_STORAGE_FILE_LUKS" type check in
order to allocate meta->encryption

John

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

Re: [libvirt] [PATCH 03/19] storage: Create helper to set options for CreateQemuImg code

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:42 -0400, John Ferlan wrote:
> Create a helper virStorageBackendCreateQemuImgSetOptions to set either
> the qemu-img -o options or the previous mechanism using -F
> 
> Signed-off-by: John Ferlan 
> ---
>  src/storage/storage_backend.c | 30 +-
>  1 file changed, 21 insertions(+), 9 deletions(-)
>

ACK

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

Re: [libvirt] [PATCH 04/19] storage: Use virSecretGetSecretString

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:43 -0400, John Ferlan wrote:
> Rather than inline code secret lookup for rbd/iscsi, use the common function.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/Makefile.am |  1 +
>  src/storage/storage_backend_iscsi.c | 49 
> +
>  src/storage/storage_backend_rbd.c   | 48 +++-
>  3 files changed, 10 insertions(+), 88 deletions(-)

ACK

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

Re: [libvirt] [PATCH 02/19] storage: Create helper to set backing for CreateQemuImg code

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:41 -0400, John Ferlan wrote:
> Create a helper virStorageBackendCreateQemuImgSetBacking to perform the
> backing store set
> 
> Signed-off-by: John Ferlan 
> ---
>  src/storage/storage_backend.c | 116 
> +++---
>  1 file changed, 64 insertions(+), 52 deletions(-)

ACK

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

Re: [libvirt] [PATCH 01/19] storage: Adjust qemu-img switches check

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:40 -0400, John Ferlan wrote:
> Since we support QEMU 0.12 and later, checking for support of specific flags
> added prior to that isn't necessary.
> 
> Thus start with the base of having the "-o options" available for the
> qemu-img create option and then determine whether we have the compat
> option for qcow2 files (which would be necessary up through qemu 2.0
> where the default changes to compat 0.11).
> 
> Adjust test to no long check for NONE and FLAG options as well was removing
> results of tests that would use that option.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/storage/storage_backend.c  | 91 
> +++---
>  tests/storagevolxml2argvdata/qcow2-flag.argv   |  2 -
>  .../qcow2-nobacking-convert-flag.argv  |  2 -
>  .../qcow2-nobacking-convert-none.argv  |  2 -
>  .../qcow2-nobacking-flag.argv  |  1 -
>  .../qcow2-nobacking-none.argv  |  1 -
>  tests/storagevolxml2argvdata/qcow2-none.argv   |  1 -
>  tests/storagevolxml2argvtest.c | 22 +-
>  8 files changed, 29 insertions(+), 93 deletions(-)
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv
>  delete mode 100644 
> tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv
>  delete mode 100644 
> tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv
>  delete mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv

I still think the code should be refactored sooner or later, but ACK.

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

Re: [libvirt] [PATCH 17/19] storage: Add support to create a luks volume

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:56 -0400, John Ferlan wrote:
> If the volume xml was looking to create a luks volume take the necessary
> steps in order to make that happen.
> 
> The processing will be:
>  1. create a temporary file in the storage pool target path
>1a. the file name will either be $volname_$UUID.tmp or $volname_$USAGE.tmp
>depending on how the encryption xml specified fetching the secret
>1b. create/open the file, initially setting mode to 0600 with current
>effective uid:gid as owner
>1c. fetch the secret into a buffer and write that into the file
>1d. change file protections to 0400
> 
>  2. create a secret object
>2a. use a similarly crafted alias to the file name
>2b. add the file to the object
> 
>  3. create/add luks options to the commandline
>3a. at the very least a "key-secret" using the secret object alias
>3b. if found in the XML the various "cipher" and "ivgen" options
> 
> Signed-off-by: John Ferlan 
> ---
>  src/libvirt_private.syms   |   1 +
>  src/storage/storage_backend.c  | 285 
> ++---
>  src/storage/storage_backend.h  |   3 +-
>  src/util/virqemu.c |  23 
>  src/util/virqemu.h |   6 +
>  tests/storagevolxml2argvtest.c |   3 +-
>  6 files changed, 300 insertions(+), 21 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fdf06ae..0d6d080 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2153,6 +2153,7 @@ virProcessWait;
>  
>  
>  # util/virqemu.h
> +virQEMUBuildLuksOpts;
>  virQEMUBuildObjectCommandlineFromJSON;
>  
>  
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 4965c9e..fc50807 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -55,11 +55,14 @@
>  #include "viralloc.h"
>  #include "internal.h"
>  #include "secret_conf.h"
> +#include "secret_util.h"
>  #include "viruuid.h"
>  #include "virstoragefile.h"
>  #include "storage_backend.h"
>  #include "virlog.h"
>  #include "virfile.h"
> +#include "virjson.h"
> +#include "virqemu.h"
>  #include "stat-time.h"
>  #include "virstring.h"
>  #include "virxml.h"
> @@ -880,6 +883,7 @@ virStoragePloopResize(virStorageVolDefPtr vol,
>  enum {
>  QEMU_IMG_BACKING_FORMAT_OPTIONS = 0,
>  QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT,
> +QEMU_IMG_FORMAT_LUKS,
>  };
>  
>  static bool
> @@ -907,6 +911,27 @@ virStorageBackendQemuImgSupportsCompat(const char 
> *qemuimg)
>  return ret;
>  }
>  
> +
> +static bool
> +virStorageBackendQemuImgSupportsLuks(const char *qemuimg)
> +{
> +bool ret = false;
> +int exitstatus = -1;
> +virCommandPtr cmd = virCommandNewArgList(qemuimg, "create", "-o", "?",
> + "-f", "luks", "/dev/null", 
> NULL);
> +
> +if (virCommandRun(cmd, ) < 0)
> +goto cleanup;
> +
> +if (exitstatus == 0)
> +ret = true;
> +
> + cleanup:
> +virCommandFree(cmd);
> +return ret;
> +}
> +
> +
>  static int
>  virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
>  {
> @@ -915,12 +940,18 @@ virStorageBackendQEMUImgBackingFormat(const char 
> *qemuimg)
>   * out what else we have */
>  int ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
>  
> -/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
> - * understands. Since we still support QEMU 0.12 and newer, we need
> - * to be able to handle the previous format as can be set via a
> - * compat=0.10 option. */
> -if (virStorageBackendQemuImgSupportsCompat(qemuimg))
> -ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
> +/* QEMU 2.6 added support for luks - let's check for that.
> + * If available, then we can also assume OPTIONS_COMPAT is present */
> +if (virStorageBackendQemuImgSupportsLuks(qemuimg)) {
> +ret = QEMU_IMG_FORMAT_LUKS;
> +} else {
> +/* QEMU 2.0 changed to using a format that only QEMU 1.1 and newer
> + * understands. Since we still support QEMU 0.12 and newer, we need
> + * to be able to handle the previous format as can be set via a
> + * compat=0.10 option. */
> +if (virStorageBackendQemuImgSupportsCompat(qemuimg))
> +ret = QEMU_IMG_BACKING_FORMAT_OPTIONS_COMPAT;
> +}

This looks like it's becoming a source of technical debt. Heaps of
comments aren't helping.

>  
>  return ret;
>  }
> @@ -941,21 +972,31 @@ struct _virStorageBackendQemuImgInfo {
>  const char *inputPath;
>  const char *inputFormatStr;
>  int inputFormat;
> +
> +char *secretAlias;
> +const char *secretPath;
>  };
>  
> +
>  static int
> -virStorageBackendCreateQemuImgOpts(char **opts,
> +virStorageBackendCreateQemuImgOpts(virStorageEncryptionPtr enc,
> +   char **opts,
> struct _virStorageBackendQemuImgInfo info)
>  {
>

Re: [libvirt] [PATCH v5 3/3] qemu: return balloon statistics when all domain statistics reported

[John Ferlan]

On 06/16/2016 02:38 PM, Maxim Nestratov wrote:
> From: Derbyshev Dmitry 
> 
> To collect all balloon statistics for all guests it was necessary to make
> several libvirt requests. Now it's possible to get all balloon statiscs via
> single connectGetAllDomainStats call.
> 
> Signed-off-by: Derbyshev Dmitry 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/qemu/qemu_driver.c | 95 
> +++---
>  1 file changed, 74 insertions(+), 21 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2885936..675ac5a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11073,32 +11073,23 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
>  return ret;
>  }
>  
> +/* This functions assumes that job  QEMU_JOB_QUERY is started by a caller */
 ^
Extra space
> +
>  static int
> -qemuDomainMemoryStats(virDomainPtr dom,
> -  virDomainMemoryStatPtr stats,
> -  unsigned int nr_stats,
> -  unsigned int flags)
> +qemuDomainMemoryStatsInternal(virQEMUDriverPtr driver,
> +  virDomainObjPtr vm,
> +  virDomainMemoryStatPtr stats,
> +  unsigned int nr_stats)
> +
>  {
> -virQEMUDriverPtr driver = dom->conn->privateData;
> -virDomainObjPtr vm;
>  int ret = -1;
> +qemuDomainObjPrivatePtr priv;

This fails to compile as 'priv' isn't used.

>  long rss;
>  
> -virCheckFlags(0, -1);
> -
> -if (!(vm = qemuDomObjFromDomain(dom)))
> -goto cleanup;
> -
> -if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0)
> -goto cleanup;
> -
> -if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> -goto cleanup;
> -
>  if (!virDomainObjIsActive(vm)) {
>  virReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("domain is not running"));
> -goto endjob;
> +goto cleanup;

No need to cleanup, just return -1

>  }
>  
>  if (vm->def->memballoon &&
> @@ -11109,7 +11100,7 @@ qemuDomainMemoryStats(virDomainPtr dom,
>  ret = -1;
>  
>  if (ret < 0 || ret >= nr_stats)
> -goto endjob;
> +goto cleanup;

Likewise, return ret

>  } else {
>  ret = 0;
>  }
> @@ -11123,7 +4,33 @@ qemuDomainMemoryStats(virDomainPtr dom,
>  ret++;
>  }
>  
> - endjob:
> + cleanup:
> +return ret;

Rendering cleanup: unnecessary

> +}
> +
> +static int
> +qemuDomainMemoryStats(virDomainPtr dom,
> +  virDomainMemoryStatPtr stats,
> +  unsigned int nr_stats,
> +  unsigned int flags)
> +{
> +virQEMUDriverPtr driver = dom->conn->privateData;
> +virDomainObjPtr vm;
> +int ret = -1;
> +
> +virCheckFlags(0, -1);
> +
> +if (!(vm = qemuDomObjFromDomain(dom)))
> +goto cleanup;
> +
> +if (virDomainMemoryStatsEnsureACL(dom->conn, vm->def) < 0)
> +goto cleanup;
> +
> +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
> +goto cleanup;
> +
> +ret = qemuDomainMemoryStatsInternal(driver, vm, stats, nr_stats);
> +
>  qemuDomainObjEndJob(driver, vm);
>  
>   cleanup:
> @@ -18608,15 +18625,29 @@ qemuDomainGetStatsCpu(virQEMUDriverPtr driver 
> ATTRIBUTE_UNUSED,
>  return 0;
>  }
>  
> +
> +#define STORE_MEM_RECORD(TAG, NAME) {  \
> +if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ ##TAG) \
> +if (virTypedParamsAddULLong(>params,   \
> +>nparams,  \
> +maxparams, \
> +"balloon." NAME,   \
> +stats[i].val) < 0) \
> +return -1; \
> +}
> +

All of the rest feels like a separate patch...

If I understand correctly this is for 'domstats' to display, correct?
If so, then virsh.pod could use an update.

>  static int
>  qemuDomainGetStatsBalloon(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,

This is now used too.

>virDomainObjPtr dom,
>virDomainStatsRecordPtr record,
>int *maxparams,
> -  unsigned int privflags ATTRIBUTE_UNUSED)
> +  unsigned int privflags)
>  {
>  qemuDomainObjPrivatePtr priv = dom->privateData;
> +virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];
> +int nr_stats;
>  unsigned long long cur_balloon = 0;
> +size_t i;
>  int err = 0;
>  
>  if (!virDomainDefHasMemballoon(dom->def)) {
> @@ -18641,8 +18672,30 @@ qemuDomainGetStatsBalloon(virQEMUDriverPtr driver 
> ATTRIBUTE_UNUSED,
>  

Re: [libvirt] [PATCH v5 2/3] qemu: expand domain memory statistics with 'last-update' timestamp

[John Ferlan]

On 06/16/2016 02:38 PM, Maxim Nestratov wrote:
> From: Derbyshev Dmitry 
> 
> QEMU reports timestamp along with other memory statistics, but this
> information is not reported by libvirt statistics API.
> It could be useful to determine if the data reported is fresh or not.
> 
> Signed-off-by: Derbyshev Dmitry 
> ---
>  include/libvirt/libvirt-domain.h |  5 -
>  src/libvirt-domain.c |  2 ++
>  src/qemu/qemu_monitor_json.c | 22 --
>  tools/virsh-domain-monitor.c |  2 ++
>  4 files changed, 20 insertions(+), 11 deletions(-)
> 

Likewise from my 1/3 comments - virsh.pod updates.

Personally, I'm not so sure just a plain timestamp is all that useful to
print out as the number displayed doesn't have much meaning without
format or knowing what the current timestamp is. That is, what visual
cue is there that the timestamp displayed is from within 10 seconds, 10
minutes, 10 hours, or 10 days ago?

For someone that's using the API (and not virsh) - having a timestamp
would allow for the ability to generate "more real" statistics.

I'm not opposed to it being displayed, but unlike other stats provided
that are essentially "sized" - this one is less a stat and more of
something that "could" be used. It also only has meaning for certain
values (rss not being one of those IIRC).

John

> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index 1554198..f953897 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -609,11 +609,14 @@ typedef enum {
>   */
>  VIR_DOMAIN_MEMORY_STAT_USABLE  = 8,
>  
> +/* Timestamp of the last update of statistics */
> +VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE = 9,
> +
>  /*
>   * The number of statistics supported by this version of the interface.
>   * To add new statistics, add them to the enum and increase this value.
>   */
> -VIR_DOMAIN_MEMORY_STAT_NR  = 9,
> +VIR_DOMAIN_MEMORY_STAT_NR  = 10,
>  
>  # ifdef VIR_ENUM_SENTINELS
>  VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index fabd4a6..29e0c72 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -5991,6 +5991,8 @@ virDomainGetInterfaceParameters(virDomainPtr domain,
>   * to swap, corresponds to 'Available' in /proc/meminfo
>   * VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON:
>   * Current balloon value (in kb).
> + * VIR_DOMAIN_MEMORY_STAT_LAST_UPDATE
> + * Timestamp of the last statistic
>   *
>   * Returns: The number of stats provided or -1 in case of failure.
>   */
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index d10e758..6f0d187 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1633,10 +1633,10 @@ qemuMonitorJSONGetBalloonInfo(qemuMonitorPtr mon,
>   * rates and/or whether data has been collected since a previous cycle.
>   * It's currently unused.
>   */
> -#define GET_BALLOON_STATS(FIELD, TAG, DIVISOR)   
>  \
> -if (virJSONValueObjectHasKey(statsdata, FIELD) &&
>  \
> +#define GET_BALLOON_STATS(OBJECT, FIELD, TAG, DIVISOR)   
>  \
> +if (virJSONValueObjectHasKey(OBJECT, FIELD) &&   
>  \
> (got < nr_stats)) {   
>  \
> -if (virJSONValueObjectGetNumberUlong(statsdata, FIELD, ) < 0) {  
>  \
> +if (virJSONValueObjectGetNumberUlong(OBJECT, FIELD, ) < 0) { 
>  \
>  VIR_DEBUG("Failed to get '%s' value", FIELD);
>  \
>  } else { 
>  \
>  /* Not being collected? No point in providing bad data */
>  \
> @@ -1707,20 +1707,22 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
>  goto cleanup;
>  }
>  
> -GET_BALLOON_STATS("stat-swap-in",
> +GET_BALLOON_STATS(statsdata, "stat-swap-in",
>VIR_DOMAIN_MEMORY_STAT_SWAP_IN, 1024);
> -GET_BALLOON_STATS("stat-swap-out",
> +GET_BALLOON_STATS(statsdata, "stat-swap-out",
>VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, 1024);
> -GET_BALLOON_STATS("stat-major-faults",
> +GET_BALLOON_STATS(statsdata, "stat-major-faults",
>VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, 1);
> -GET_BALLOON_STATS("stat-minor-faults",
> +GET_BALLOON_STATS(statsdata, "stat-minor-faults",
>VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, 1);
> -GET_BALLOON_STATS("stat-free-memory",
> +GET_BALLOON_STATS(statsdata, "stat-free-memory",
>VIR_DOMAIN_MEMORY_STAT_UNUSED, 1024);
> -GET_BALLOON_STATS("stat-total-memory",
> +GET_BALLOON_STATS(statsdata, "stat-total-memory",
>

Re: [libvirt] [PATCH v5 1/3] qemu: expand domain memory statistics with 'usable'

[John Ferlan]

On 06/16/2016 02:38 PM, Maxim Nestratov wrote:
> From: Derbyshev Dmitry 
> 
> Currently 'memtotal' in virtio drivers and qemu corresponds
> to 'available' in libvirt. Because of that we introduce libvirt 'usable'
> parameter, which maps to 'stat-available-memory' balloon statistics.
> As balloon statistics isn't reported in hmp, so no modification is made
> in qemu_monitor_text.c.
> 
> Signed-off-by: Derbyshev Dmitry 
> ---
>  include/libvirt/libvirt-domain.h | 8 +++-
>  src/libvirt-domain.c | 3 +++
>  src/qemu/qemu_monitor_json.c | 4 
>  tools/virsh-domain-monitor.c | 2 ++
>  4 files changed, 16 insertions(+), 1 deletion(-)
> 

Existing, but sure would be nice to have a brief description in
virsh.pod of the various statistics to be displayed. Adding that would
mean generating a patch before this to "catch up" and describe the
current stats, then adding this new stat. That way when someone adds in
the future a reviewer may actually catch that the new stat isn't
described. The format could be similar to domblkstat and the text could
just borrow liberally from libvirt-domain.h

ACK for what's here though.

John

> diff --git a/include/libvirt/libvirt-domain.h 
> b/include/libvirt/libvirt-domain.h
> index cba4fa5..1554198 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -604,10 +604,16 @@ typedef enum {
>  VIR_DOMAIN_MEMORY_STAT_RSS = 7,
>  
>  /*
> + * How big the balloon can be inflated without pushing the guest system
> + * to swap, corresponds to 'Available' in /proc/meminfo
> + */
> +VIR_DOMAIN_MEMORY_STAT_USABLE  = 8,
> +
> +/*
>   * The number of statistics supported by this version of the interface.
>   * To add new statistics, add them to the enum and increase this value.
>   */
> -VIR_DOMAIN_MEMORY_STAT_NR  = 8,
> +VIR_DOMAIN_MEMORY_STAT_NR  = 9,
>  
>  # ifdef VIR_ENUM_SENTINELS
>  VIR_DOMAIN_MEMORY_STAT_LAST = VIR_DOMAIN_MEMORY_STAT_NR
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 73ae369..fabd4a6 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -5986,6 +5986,9 @@ virDomainGetInterfaceParameters(virDomainPtr domain,
>   * The amount of memory which is not being used for any purpose (in kb).
>   * VIR_DOMAIN_MEMORY_STAT_AVAILABLE:
>   * The total amount of memory available to the domain's OS (in kb).
> + * VIR_DOMAIN_MEMORY_STAT_USABLE:
> + * How big the balloon can be inflated without pushing the guest system
> + * to swap, corresponds to 'Available' in /proc/meminfo
>   * VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON:
>   * Current balloon value (in kb).
>   *
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 380ddab..d10e758 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1719,7 +1719,11 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon,
>VIR_DOMAIN_MEMORY_STAT_UNUSED, 1024);
>  GET_BALLOON_STATS("stat-total-memory",
>VIR_DOMAIN_MEMORY_STAT_AVAILABLE, 1024);
> +GET_BALLOON_STATS("stat-available-memory",
> +  VIR_DOMAIN_MEMORY_STAT_USABLE, 1024);
> +
>  ret = got;
> +
>   cleanup:
>  virJSONValueFree(cmd);
>  virJSONValueFree(reply);
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 0a93949..1921ff5 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -369,6 +369,8 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
>  vshPrint(ctl, "unused %llu\n", stats[i].val);
>  if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_AVAILABLE)
>  vshPrint(ctl, "available %llu\n", stats[i].val);
> +if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_USABLE)
> +vshPrint(ctl, "usable %llu\n", stats[i].val);
>  if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON)
>  vshPrint(ctl, "actual %llu\n", stats[i].val);
>  if (stats[i].tag == VIR_DOMAIN_MEMORY_STAT_RSS)
> 

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

[libvirt] libvirt and VIR_WARN("cannot parse process status data")

[Vasiliy Tolstov]

I have system with 3.19.3 kernel and libvirt 1.3.3 (version not matter
because part of code not changed).
When i'm try to get cpu stats via libvirt i have  error in libvirt
log, that says "cannot parse process status data"
relevant code in src/qemu/qemu_driver.c
in function qemuGetProcessInfo

i'm add debut test to this function and in my case i have always fscanf != 4

qemu pid 7977

cat /proc/7977/stat
7977 (qemu-system-x86) S 1 7976 7976 0 -1 138420608 12728 0 0 0 4819
8249 0 0 20 0 7 0 51378692 4783886336 75779 18446744073709551615
139849472311296 139849478294132 140732912418880 140732912417952
139849400618495 0 268444224 4096 16963 18446744073709551615 0 0 17 5 0
0 15624 744 0 139849480394624 139849482248448 139849496641536
140732912421083 140732912422775 140732912422775 140732912422876 0

cat /proc/7977/task/7977/stat
7977 (qemu-system-x86) S 1 7976 7976 0 -1 138420608 12537 0 0 0 3917
7799 0 0 20 0 7 0 51378692 4783886336 75779 18446744073709551615
139849472311296 139849478294132 140732912418880 140732912417952
139849400618495 0 268444224 4096 16963 0 0 0 17 3 0 0 15624 0 0
139849480394624 139849482248448 139849496641536 140732912421083
140732912422775 140732912422775 140732912422876 0

cat /proc/7977/task/7979/stat
7979 (qemu-system-x86) S 1 7976 7976 0 -1 1077944384 20 0 0 0 0 0 0 0
20 0 7 0 51378696 4783886336 75779 18446744073709551615
139849472311296 139849478294132 140732912418880 139849260591832
139849400637449 0 2147221247 4096 16963 0 0 0 -1 6 0 0 0 0 0
139849480394624 139849482248448 139849496641536 140732912421083
140732912422775 140732912422775 140732912422876 0

cat /proc/7977/task/7982/stat
7982 (CPU 0/KVM) S 1 7976 7976 0 -1 138420416 22 0 0 0 226 118 0 0 20
0 7 0 51378697 4783886336 75779 18446744073709551615 139849472311296
139849478294132 140732912418880 139849222842936 139849400624151 0
2147220671 4096 16963 0 0 0 -1 5 0 0 0 196 0 139849480394624
139849482248448 139849496641536 140732912421083 140732912422775
140732912422775 140732912422876 0

Does somebody have time to fix this ?

-- 
Vasiliy Tolstov,
e-mail: v.tols...@yoctocloud.net

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

Re: [libvirt] [PATCH 19/19] qemu: Add luks support for domain disk

[Peter Krempa]

On Tue, Jun 21, 2016 at 15:03:51 +0200, Peter Krempa wrote:
> On Mon, Jun 13, 2016 at 20:27:58 -0400, John Ferlan wrote:
> > Generate the luks command line using the AES secret key to encrypt the
> > luks secret. A luks secret object will be in addition to a an AES secret.
> > 
> > Add tests for sample output
> > 
> > Signed-off-by: John Ferlan 
> > ---

[...]

> > @@ -1237,10 +1238,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
> >  qemuBufferEscapeComma(, source);
> >  virBufferAddLit(, ",");
> >  
> > -if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
> > +if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES)
> >  virBufferAsprintf(, "password-secret=%s,",
> >secinfo->s.aes.alias);
> > -}
> > +
> > +if (encinfo)
> > +virQEMUBuildLuksOpts(, disk->src->encryption,
> > + encinfo->s.aes.alias);
> 
> This wrapper is not really useful here. It only adds "key-secret=" all
> the other options are necessary only if creating the volume.

Okay, in the end this might be a reasonable idea if we'll want to add
support for block-copy-ing into a luks volume.

On the other hand, you'll need to disallow snapshots if the disk is
LUKS until we add support for full backing chain tracking since you'll
lose the definitions for the key once you take a snapshot. A second
start of that VM will not be possible then.

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

Re: [libvirt] [Qemu-devel] [PATCH v2 0/6] Add runnability info to query-cpu-definitions

[Jiri Denemark]

On Mon, Jun 20, 2016 at 17:09:18 -0300, Eduardo Habkost wrote:
> 
> Ping? No other feedback on this?

The interface is fine from my point of view and I even have a working
libvirt code that consumes this.

Jirka

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

Re: [libvirt] [PATCH 18/19] qemu: Add new secret info type

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:57 -0400, John Ferlan wrote:
> Add 'encinfo' to the extended disk structure. This will contain the
> encryption secret (if present).
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_domain.c | 1 +
>  src/qemu/qemu_domain.h | 5 +
>  2 files changed, 6 insertions(+)

ACK

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

Re: [libvirt] [PATCH 19/19] qemu: Add luks support for domain disk

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:58 -0400, John Ferlan wrote:
> Generate the luks command line using the AES secret key to encrypt the
> luks secret. A luks secret object will be in addition to a an AES secret.
> 
> Add tests for sample output
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_command.c| 12 ++--
>  src/qemu/qemu_domain.c | 19 ++--
>  .../qemuxml2argv-luks-disk-cipher.args | 36 
> ++
>  .../qemuxml2argvdata/qemuxml2argv-luks-disks.args  | 36 
> ++
>  tests/qemuxml2argvtest.c   | 11 ++-
>  5 files changed, 109 insertions(+), 5 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disk-cipher.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks.args

Note that this won't work with disk hotplug until you add code that will
add the secret objects too.

Also I've noticed that RBD hotplug is now broken too due to the same
reason. The code that selects whether to use plaintext password or pass
it via the secret object does not check whether it's called on the
hotplug path and thus uses the secret object.

The secret object is not added using the monitor though. You'll need to
fix that first and if you opt for the correct approach this will then
work automagically.

Looks okay but I can't ACK this with hotplug not working.

Also please note that on hot-unplug you need to remove the secrets as
you'll possibly be adding a secret with the same name (Since alias will
be reused)

> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 490260f..7062c17 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1103,6 +1103,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>  int actualType = virStorageSourceGetActualType(disk->src);
>  qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>  qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
> +qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo;
>  bool emitDeviceSyntax = qemuDiskBusNeedsDeviceArg(disk->bus);
>  
>  if (idx < 0) {
> @@ -1237,10 +1238,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
>  qemuBufferEscapeComma(, source);
>  virBufferAddLit(, ",");
>  
> -if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) {
> +if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES)
>  virBufferAsprintf(, "password-secret=%s,",
>secinfo->s.aes.alias);
> -}
> +
> +if (encinfo)
> +virQEMUBuildLuksOpts(, disk->src->encryption,
> + encinfo->s.aes.alias);

This wrapper is not really useful here. It only adds "key-secret=" all
the other options are necessary only if creating the volume.

>  
>  if (disk->src->format > 0 &&
>  disk->src->type != VIR_STORAGE_TYPE_DIR)
> @@ -1920,6 +1924,7 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>  virDomainDiskDefPtr disk = def->disks[i];
>  qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>  qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo;
> +qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo;
>  
>  /* PowerPC pseries based VMs do not support floppy device */
>  if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) &&
> @@ -1949,6 +1954,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
>  if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0)
>  return -1;
>  
> +if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
> +return -1;
> +
>  virCommandAddArg(cmd, "-drive");
>  
>  optstr = qemuBuildDriveStr(disk,
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c288fa0..fb3e91f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c

[...]

> @@ -1026,7 +1028,6 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
>   src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
>  
>  virSecretUsageType secretUsageType;
> -qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>  
>  if (VIR_ALLOC(secinfo) < 0)
>  return -1;
> @@ -1044,6 +1045,20 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
>  diskPriv->secinfo = secinfo;
>  }
>  
> +if (conn && !virStorageSourceIsEmpty(src) &&

This part is the same with the block above. It may be worth extracting
it ...

> +src->encryption && src->format == VIR_STORAGE_FILE_LUKS) {
> +
> +if (VIR_ALLOC(secinfo) < 0)
> +return -1;
> +
> +if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias,
> +  VIR_SECRET_USAGE_TYPE_KEY, NULL,
> +  >encryption->secrets[0]->secdef) < 0)
> +goto 

Re: [libvirt] [PATCH 0/3] qmp: query-host-cpu command

[David Hildenbrand]

> On Tue, Jun 21, 2016 at 08:20:40AM +0200, David Hildenbrand wrote:
> > > Add QMP command to allow management software to query for
> > > CPU information for the running host.
> > > 
> > > The data returned by the command is in the form of a dictionary
> > > of QOM properties.
> > > 
> > > This series depends on the "Add runnability info to
> > > query-cpu-definitions" series I sent 2 weeks ago.
> > > 
> > > Git tree:
> > >   https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
> > >   
> > 
> > I like that interface, I'm going to post (maybe today? :) ) a similar 
> > interface
> > that allows to also expand other cpu models, not just the host model.  
> 
> In x86 I want to avoid exposing the details of other CPU models
> to libvirt because the details depend on machine-type.
> 
> But if it is useful for you, I believe the same "qom-properties"
> dict could be returned in query-cpu-definitions.
> 
> > 
> > Maybe we can then decide which one makes sense for all of us. But in 
> > general,
> > this interface is much better compared to what we had before.   
> 
> Maybe both? I think it's better to have a separate interface for
> querying "what exactly this host supports" and another one for
> querying for "what happens if I use -cpu host". In the case of
> x86, both are equivalent, but we can't guarantee this on all
> architectures.
> 

I'll post my patches in a couple of minutes, let's discuss it then.

We might want to avoid having multiple interfaces carrying out the same task.

David

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

Re: [libvirt] [PATCH 0/3] qmp: query-host-cpu command

[Eduardo Habkost]

On Tue, Jun 21, 2016 at 08:20:40AM +0200, David Hildenbrand wrote:
> > Add QMP command to allow management software to query for
> > CPU information for the running host.
> > 
> > The data returned by the command is in the form of a dictionary
> > of QOM properties.
> > 
> > This series depends on the "Add runnability info to
> > query-cpu-definitions" series I sent 2 weeks ago.
> > 
> > Git tree:
> >   https://github.com/ehabkost/qemu-hacks.git work/query-host-cpu
> > 
> 
> I like that interface, I'm going to post (maybe today? :) ) a similar 
> interface
> that allows to also expand other cpu models, not just the host model.

In x86 I want to avoid exposing the details of other CPU models
to libvirt because the details depend on machine-type.

But if it is useful for you, I believe the same "qom-properties"
dict could be returned in query-cpu-definitions.

> 
> Maybe we can then decide which one makes sense for all of us. But in general,
> this interface is much better compared to what we had before. 

Maybe both? I think it's better to have a separate interface for
querying "what exactly this host supports" and another one for
querying for "what happens if I use -cpu host". In the case of
x86, both are equivalent, but we can't guarantee this on all
architectures.

-- 
Eduardo

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

Re: [libvirt] [PATCH 10/19] tests: Adjust tests for encrypted storage

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:49 -0400, John Ferlan wrote:
> Make them work again...  The xml2xml had been working, but the xml2argv
> were not working. Making the xml2argv work required a few adjustments to
> the xml to update to more recent times.
> 
> Signed-off-by: John Ferlan 
> ---
>  .../qemuxml2argv-encrypted-disk.args   | 26 
> +-
>  .../qemuxml2argv-encrypted-disk.xml|  4 ++--
>  tests/qemuxml2argvtest.c   |  2 ++
>  .../qemuxml2xmlout-encrypted-disk.xml  |  4 ++--
>  4 files changed, 17 insertions(+), 19 deletions(-)

ACK

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

Re: [libvirt] [PATCH 13/19] util: Add 'luks' to the FileTypeInfo

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:52 -0400, John Ferlan wrote:
> Add the ability to detect a luks encrypted device.
> 
> This also adding new 16 bit big/little endian macros since the
> version of a luks device is stored in a uint16_t.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virendian.h  | 24 
>  src/util/virstoragefile.c | 38 --
>  src/util/virstoragefile.h |  1 +
>  tests/virendiantest.c | 18 ++
>  4 files changed, 75 insertions(+), 6 deletions(-)
> 
> diff --git a/src/util/virendian.h b/src/util/virendian.h
> index eefe48c..97940bd 100644
> --- a/src/util/virendian.h
> +++ b/src/util/virendian.h
> @@ -90,4 +90,28 @@
>   ((uint32_t)(uint8_t)((buf)[2]) << 16) | \
>   ((uint32_t)(uint8_t)((buf)[3]) << 24))
>  
> +/**
> + * virReadBufInt16BE:
> + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*');
> + *   evaluating buf must not have any side effects
> + *
> + * Read 2 bytes at BUF as a big-endian 16-bit number.  Caller is
> + * responsible to avoid reading beyond array bounds.
> + */
> +# define virReadBufInt16BE(buf)  \
> +(((uint16_t)(uint8_t)((buf)[0]) << 8) |  \
> + (uint16_t)(uint8_t)((buf)[1]))
> +
> +/**
> + * virReadBufInt16LE:
> + * @buf: byte to start reading at (can be 'char*' or 'unsigned char*');
> + *   evaluating buf must not have any side effects
> + *
> + * Read 2 bytes at BUF as a little-endian 16-bit number.  Caller is
> + * responsible to avoid reading beyond array bounds.
> + */
> +# define virReadBufInt16LE(buf)  \
> +((uint16_t)(uint8_t)((buf)[0]) | \
> + ((uint16_t)(uint8_t)((buf)[1]) << 8))
> +
>  #endif /* __VIR_ENDIAN_H__ */

Since you are adding this code and also the tests below it looks like a
job for a separate patch.

> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 5c2519c..f7a9632 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c

[...]

> @@ -193,6 +194,14 @@ qedGetBackingStore(char **, int *, const char *, size_t);
>  #define PLOOP_IMAGE_SIZE_OFFSET 36
>  #define PLOOP_SIZE_MULTIPLIER 512
>  
> +#define LUKS_HDR_MAGIC_LEN 6
> +#define LUKS_HDR_VERSION_LEN 2
> +
> +/* Format described by qemu commit id '3e308f20e' */
> +#define LUKS_HDR_VERSION_OFFSET LUKS_HDR_MAGIC_LEN
> +#define LUKS_HDR_CRYPT_OFFSET LUKS_HDR_MAGIC_LEN + LUKS_HDR_VERSION_LEN
> +
> +
>  static struct FileTypeInfo const fileTypeInfo[] = {
>  [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN,
>  -1, {0}, 0, 0, 0, FI_CRYPT_NONE, 0, NULL, 
> NULL },
> @@ -249,6 +258,13 @@ static struct FileTypeInfo const fileTypeInfo[] = {
>   PLOOP_SIZE_MULTIPLIER,
>   FI_CRYPT_NONE, -1, NULL, NULL },
>  
> +/* Magic is 'L','U','K','S', 0xBA, 0xBE
> + * Set sizeOffset = -1 and let hypervisor handle */
> +[VIR_STORAGE_FILE_LUKS] = {
> +0, "\x4c\x55\x4b\x53\xba\xbe", NULL,
> +LV_BIG_ENDIAN, LUKS_HDR_VERSION_OFFSET, {1},
> +-1, 0, 0, FI_CRYPT_LUKS, LUKS_HDR_CRYPT_OFFSET, NULL, NULL

The encryption header offset is not used here. You are not extracting
the cipher name from the header.

> +},
>  /* All formats with a backing store probe below here */
>  [VIR_STORAGE_FILE_COW] = {
>  0, "OOOM", NULL,
> @@ -634,7 +650,7 @@ virStorageFileMatchesVersion(int format,
>   char *buf,
>   size_t buflen)
>  {
> -int version;
> +int version = 0;
>  size_t i;
>  
>  /* Validate version number info */
> @@ -648,10 +664,16 @@ virStorageFileMatchesVersion(int format,
>  if ((fileTypeInfo[format].versionOffset + 4) > buflen)
>  return false;
>  
> -if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN)
> +if (fileTypeInfo[format].endian == LV_LITTLE_ENDIAN) {
>  version = virReadBufInt32LE(buf + 
> fileTypeInfo[format].versionOffset);
> -else
> -version = virReadBufInt32BE(buf + 
> fileTypeInfo[format].versionOffset);
> +} else {
> +if (format == VIR_STORAGE_FILE_LUKS)

This should be selected with a different property than the file type.
The idea of the structure above was to avoid having to tweak this code
with individual cases for every single file type. You'll either need to
pass a function pointer for extraction or switch it according to the
passed size.

> +version = virReadBufInt16BE(buf +
> +fileTypeInfo[format].versionOffset);
> +else
> +version = virReadBufInt32BE(buf +
> +fileTypeInfo[format].versionOffset);
> +}
>  
>  for (i = 0;
>   i < FILE_TYPE_VERSIONS_LAST && 
> fileTypeInfo[format].versionNumbers[i];
> @@ -832,6 

Re: [libvirt] qemu: cpu model definition problem

[Maxim Nestratov]

21.06.2016 15:21, Jiri Denemark пишет:


On Sat, Jun 18, 2016 at 23:24:56 +0300, Maxim Nestratov wrote:

Hi all,

I've just encountered a problem with ability to live migrate a VM
between hosts using the following cpu section in VM's xml:


  Haswell-noTSX
  ...
  


The problem is that a VM with such a config crashes when it migrates
from a host where 'rdrand' presents to a host where it is absent.
This happens due to the fact that QEMU and libvirt have pretty different
view on what 'Haswell-noTSX' is. Libvirt's description lacks some
features, which present in QEMU description of this cpu model (e.g.
'rdrand'). It is impossible to disable features present on the host and
different from QEMU's ones by current libvirt code, i.e. libvirt thinks
that no use to disable something that doesn't exist.
Of course, the problem in this case can be fixed just by adding missed
features to 'Haswell-noTSX' model in cpu_map.xml to match what QEMU has,
but it doesn't seem right because it will help just in this particular
case, and in general, it will not be a solution because QEMU can change
its cpu model definitions at any time and it does this from time to time.
What libvirt might have done is:
   - either ask QEMU for cpu model definition and work with it,

We can't do this since QEMU is not able to provide us the data the way
we need them. There were several attempts to change that, but it's not
an easy task and the current consensus says it's not even necessary now.


- or construct a command line for QEMU with required and disabled
features without taking into account libvirt cpu_map.xml, i.e. always
add '+xx' and '-yy' for features mentioned explicitly in domain xml.

Yes, we should do this. And we will do this once I finish my series so
that I can (at least partially) send it to the list. Stay tuned :-)

Jirka


Ok. Thank you.

Maxim

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

Re: [libvirt] [PATCH 12/19] util: Modify the FileTypeInfo for meta data checks

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:51 -0400, John Ferlan wrote:
> Currently the assumption is there is one type of disk encryption - in
> some qcow format which is old and crusty... But there's a new sheriff
> in town known as 'luks' and we'll need to handle that shortly
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virstoragefile.c | 54 
> ---
>  1 file changed, 32 insertions(+), 22 deletions(-)
> 
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 6d7e5d9..5c2519c 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c

[...]

> @@ -111,6 +111,11 @@ enum {
>  BACKING_STORE_ERROR,
>  };
>  
> +enum fi_crypt {
> +FI_CRYPT_NONE = 0,
> +FI_CRYPT_QCOW

This lacks the "VIR_" prefix. Also I don't really see a point in adding
this. Currently it's used to distinguish between an encrypted QCOW and
an unencrypted QCOW. With LUKS (as you note later in a comment) it's
implied that they are encrypted and thus we don't need a side band to
store the same data.

Peter

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

Re: [libvirt] qemu: cpu model definition problem

[Jiri Denemark]

On Sat, Jun 18, 2016 at 23:24:56 +0300, Maxim Nestratov wrote:
> Hi all,
> 
> I've just encountered a problem with ability to live migrate a VM 
> between hosts using the following cpu section in VM's xml:
> 
>
>  Haswell-noTSX
>  ...
>  
>
> 
> The problem is that a VM with such a config crashes when it migrates 
> from a host where 'rdrand' presents to a host where it is absent.
> This happens due to the fact that QEMU and libvirt have pretty different 
> view on what 'Haswell-noTSX' is. Libvirt's description lacks some 
> features, which present in QEMU description of this cpu model (e.g. 
> 'rdrand'). It is impossible to disable features present on the host and 
> different from QEMU's ones by current libvirt code, i.e. libvirt thinks 
> that no use to disable something that doesn't exist.
> Of course, the problem in this case can be fixed just by adding missed 
> features to 'Haswell-noTSX' model in cpu_map.xml to match what QEMU has, 
> but it doesn't seem right because it will help just in this particular 
> case, and in general, it will not be a solution because QEMU can change 
> its cpu model definitions at any time and it does this from time to time.

> What libvirt might have done is:
>   - either ask QEMU for cpu model definition and work with it,

We can't do this since QEMU is not able to provide us the data the way
we need them. There were several attempts to change that, but it's not
an easy task and the current consensus says it's not even necessary now.

> - or construct a command line for QEMU with required and disabled 
> features without taking into account libvirt cpu_map.xml, i.e. always 
> add '+xx' and '-yy' for features mentioned explicitly in domain xml.

Yes, we should do this. And we will do this once I finish my series so
that I can (at least partially) send it to the list. Stay tuned :-)

Jirka

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

Re: [libvirt] [PATCH 14/19] conf: Add new secret type "key"

[Peter Krempa]

On Mon, Jun 13, 2016 at 20:27:53 -0400, John Ferlan wrote:
> Add a new secret type known as "key" - it will handle adding the secret
> objects that need a key (or passphrase), such as will soon be the case

This may be misleading a "key" is not equal to a "passprhase" in usual
encryption terminology. Key usually refers to the actual encryption key
used to encrypt the data whereas passprhase is usually a human readable
secret string (which may not be random at all) used to access the key
later.

The cryptsetup man page tends to treat them interchangably to some
extent (eg a key slot equals to passprhase, but the master key refers to
the actual encryption key used for the data).

To avoid confusion I'd rather stick with "passphrase".

> for a luks volume for both storage driver create and libvirt domain usage.
> 
> Signed-off-by: John Ferlan 
> ---

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

[libvirt] [PATCH 06/11] test: Rework qemuMonitorJSONGetMigrationParams test

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 tests/qemumonitorjsontest.c | 44 +++-
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index d0b5d9e..a0079dd 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1641,32 +1641,26 @@ 
testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data)
   ) < 0)
 goto cleanup;
 
-if (!params.compressLevel_set ||
-!params.compressThreads_set ||
-!params.decompressThreads_set) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   "One of level, threads or dthreads flags is not set");
-return -1;
-}
+#define CHECK(VAR, FIELD, VALUE)\
+do {\
+if (!params.VAR ## _set) {  \
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s is not set", FIELD); \
+goto cleanup;   \
+}   \
+if (params.VAR != VALUE) {  \
+virReportError(VIR_ERR_INTERNAL_ERROR,  \
+   "Invalid %s: %d, expected %d",   \
+   FIELD, params.VAR, VALUE);   \
+goto cleanup;   \
+}   \
+} while (0)
+
+CHECK(compressLevel, "compress-level", 1);
+CHECK(compressThreads, "compress-threads", 8);
+CHECK(decompressThreads, "decompress-threads", 2);
+
+#undef CHECK
 
-if (params.compressLevel != 1) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "Invalid decompress-threads: %d, expected 1",
-   params.compressLevel);
-goto cleanup;
-}
-if (params.compressThreads != 8) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "Invalid decompress-threads: %d, expected 8",
-   params.compressThreads);
-goto cleanup;
-}
-if (params.decompressThreads != 2) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "Invalid decompress-threads: %d, expected 2",
-   params.decompressThreads);
-goto cleanup;
-}
 ret = 0;
 
  cleanup:
-- 
2.9.0

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

[libvirt] [PATCH 09/11] qemu: Add support for cpu throttling parameters

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_monitor.c  | 11 ---
 src/qemu/qemu_monitor.h  |  6 ++
 src/qemu/qemu_monitor_json.c |  4 
 tests/qemumonitorjsontest.c  |  6 +-
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2862cbc..098e654 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2173,16 +2173,21 @@ qemuMonitorSetMigrationParams(qemuMonitorPtr mon,
   qemuMonitorMigrationParamsPtr params)
 {
 VIR_DEBUG("compressLevel=%d:%d compressThreads=%d:%d "
-  "decompressThreads=%d:%d",
+  "decompressThreads=%d:%d cpuThrottleInitial=%d:%d "
+  "cpuThrottleIncrement=%d:%d",
   params->compressLevel_set, params->compressLevel,
   params->compressThreads_set, params->compressThreads,
-  params->decompressThreads_set, params->decompressThreads);
+  params->decompressThreads_set, params->decompressThreads,
+  params->cpuThrottleInitial_set, params->cpuThrottleInitial,
+  params->cpuThrottleIncrement_set, params->cpuThrottleIncrement);
 
 QEMU_CHECK_MONITOR_JSON(mon);
 
 if (!params->compressLevel_set &&
 !params->compressThreads_set &&
-!params->decompressThreads_set)
+!params->decompressThreads_set &&
+!params->cpuThrottleInitial_set &&
+!params->cpuThrottleIncrement_set)
 return 0;
 
 return qemuMonitorJSONSetMigrationParams(mon, params);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 28cffc9..6fecca7 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -496,6 +496,12 @@ struct _qemuMonitorMigrationParams {
 
 bool decompressThreads_set;
 int decompressThreads;
+
+bool cpuThrottleInitial_set;
+int cpuThrottleInitial;
+
+bool cpuThrottleIncrement_set;
+int cpuThrottleIncrement;
 };
 
 int qemuMonitorGetMigrationParams(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 0434522..66b9c4c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2560,6 +2560,8 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon,
 PARSE(compressLevel, "compress-level");
 PARSE(compressThreads, "compress-threads");
 PARSE(decompressThreads, "decompress-threads");
+PARSE(cpuThrottleInitial, "cpu-throttle-initial");
+PARSE(cpuThrottleIncrement, "cpu-throttle-increment");
 
 #undef PARSE
 
@@ -2600,6 +2602,8 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon,
 APPEND(compressLevel, "compress-level");
 APPEND(compressThreads, "compress-threads");
 APPEND(decompressThreads, "decompress-threads");
+APPEND(cpuThrottleInitial, "cpu-throttle-initial");
+APPEND(cpuThrottleIncrement, "cpu-throttle-increment");
 
 #undef APPEND
 
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index a0079dd..f698c14 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1630,8 +1630,10 @@ 
testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data)
"{"
"\"return\": {"
"\"decompress-threads\": 2,"
+   "\"cpu-throttle-increment\": 10,"
"\"compress-threads\": 8,"
-   "\"compress-level\": 1"
+   "\"compress-level\": 1,"
+   "\"cpu-throttle-initial\": 20"
"}"
"}") < 0) {
 goto cleanup;
@@ -1658,6 +1660,8 @@ 
testQemuMonitorJSONqemuMonitorJSONGetMigrationParams(const void *data)
 CHECK(compressLevel, "compress-level", 1);
 CHECK(compressThreads, "compress-threads", 8);
 CHECK(decompressThreads, "decompress-threads", 2);
+CHECK(cpuThrottleInitial, "cpu-throttle-initial", 20);
+CHECK(cpuThrottleIncrement, "cpu-throttle-increment", 10);
 
 #undef CHECK
 
-- 
2.9.0

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

[libvirt] [PATCH 08/11] qemu: Introduce qemuMigrationSetParams

[Jiri Denemark]

Several places in the code update qemuMonitorMigrationParams structure
and qemuMigrationSetParams is then used to set them all at once.

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

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 433c00c..8cfbb9f 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3432,11 +3432,11 @@ static int
 qemuMigrationSetCompression(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
 qemuDomainAsyncJob job,
-qemuMigrationCompressionPtr compression)
+qemuMigrationCompressionPtr compression,
+qemuMonitorMigrationParamsPtr migParams)
 {
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
-qemuMonitorMigrationParams migParams = { 0 };
 
 if (qemuMigrationSetOption(driver, vm,
QEMU_MONITOR_MIGRATION_CAPS_XBZRLE,
@@ -3455,17 +3455,14 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
 if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0)
 return -1;
 
-migParams.compressLevel_set = compression->level_set;
-migParams.compressLevel = compression->level;
+migParams->compressLevel_set = compression->level_set;
+migParams->compressLevel = compression->level;
 
-migParams.compressThreads_set = compression->threads_set;
-migParams.compressThreads = compression->threads;
+migParams->compressThreads_set = compression->threads_set;
+migParams->compressThreads = compression->threads;
 
-migParams.decompressThreads_set = compression->dthreads_set;
-migParams.decompressThreads = compression->dthreads;
-
-if (qemuMonitorSetMigrationParams(priv->mon, ) < 0)
-goto cleanup;
+migParams->decompressThreads_set = compression->dthreads_set;
+migParams->decompressThreads = compression->dthreads;
 
 if (compression->xbzrle_cache_set &&
 qemuMonitorSetMigrationCacheSize(priv->mon,
@@ -3481,6 +3478,32 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
 return ret;
 }
 
+
+static int
+qemuMigrationSetParams(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   qemuDomainAsyncJob job,
+   qemuMonitorMigrationParamsPtr migParams)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret = -1;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0)
+return -1;
+
+if (qemuMonitorSetMigrationParams(priv->mon, migParams) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+return ret;
+}
+
+
 static int
 qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 virConnectPtr dconn,
@@ -3516,6 +3539,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 bool stopProcess = false;
 bool relabel = false;
 int rv;
+qemuMonitorMigrationParams migParams = { 0 };
 
 virNWFilterReadLockFilterUpdates();
 
@@ -3698,7 +3722,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 }
 
 if (qemuMigrationSetCompression(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
-compression) < 0)
+compression, ) < 0)
 goto stopjob;
 
 if (STREQ_NULLABLE(protocol, "rdma") &&
@@ -3717,6 +3741,10 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
  QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
 goto stopjob;
 
+if (qemuMigrationSetParams(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
+   ) < 0)
+goto stopjob;
+
 if (mig->nbd &&
 flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) &&
 virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) {
@@ -4554,6 +4582,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 virDomainDefPtr persistDef = NULL;
 char *timestamp;
 int rc;
+qemuMonitorMigrationParams migParams = { 0 };
 
 VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
   "cookieout=%p, cookieoutlen=%p, flags=%lx, resource=%lu, "
@@ -4634,7 +4663,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 }
 
 if (qemuMigrationSetCompression(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
-compression) < 0)
+compression, ) < 0)
 goto cleanup;
 
 if (qemuMigrationSetOption(driver, vm,
@@ -4654,6 +4683,10 @@ qemuMigrationRun(virQEMUDriverPtr driver,
  QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
 goto cleanup;
 
+if (qemuMigrationSetParams(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
+   ) < 0)
+goto cleanup;
+
 if 

[libvirt] [PATCH 03/11] qemu: Rename qemuMonitorMigrationCompression

[Jiri Denemark]

qemuMonitorMigrationParams is a better name for a structure which
contains various migration parameters. While doing that, we should use
full names for individual parameters.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c| 16 
 src/qemu/qemu_monitor.c  | 25 ++---
 src/qemu/qemu_monitor.h  | 27 ++-
 src/qemu/qemu_monitor_json.c | 32 
 src/qemu/qemu_monitor_json.h |  8 
 tests/qemumonitorjsontest.c  | 28 ++--
 6 files changed, 70 insertions(+), 66 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ec2b3dd..433c00c 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3436,7 +3436,7 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
 {
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
-qemuMonitorMigrationCompression params = { 0 };
+qemuMonitorMigrationParams migParams = { 0 };
 
 if (qemuMigrationSetOption(driver, vm,
QEMU_MONITOR_MIGRATION_CAPS_XBZRLE,
@@ -3455,16 +3455,16 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
 if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0)
 return -1;
 
-params.level_set = compression->level_set;
-params.level = compression->level;
+migParams.compressLevel_set = compression->level_set;
+migParams.compressLevel = compression->level;
 
-params.threads_set = compression->threads_set;
-params.threads = compression->threads;
+migParams.compressThreads_set = compression->threads_set;
+migParams.compressThreads = compression->threads;
 
-params.dthreads_set = compression->dthreads_set;
-params.dthreads = compression->dthreads;
+migParams.decompressThreads_set = compression->dthreads_set;
+migParams.decompressThreads = compression->dthreads;
 
-if (qemuMonitorSetMigrationCompression(priv->mon, ) < 0)
+if (qemuMonitorSetMigrationParams(priv->mon, ) < 0)
 goto cleanup;
 
 if (compression->xbzrle_cache_set &&
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f7609e9..2862cbc 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2160,29 +2160,32 @@ qemuMonitorSetMigrationCacheSize(qemuMonitorPtr mon,
 
 
 int
-qemuMonitorGetMigrationCompression(qemuMonitorPtr mon,
-   qemuMonitorMigrationCompressionPtr compress)
+qemuMonitorGetMigrationParams(qemuMonitorPtr mon,
+  qemuMonitorMigrationParamsPtr params)
 {
 QEMU_CHECK_MONITOR_JSON(mon);
 
-return qemuMonitorJSONGetMigrationCompression(mon, compress);
+return qemuMonitorJSONGetMigrationParams(mon, params);
 }
 
 int
-qemuMonitorSetMigrationCompression(qemuMonitorPtr mon,
-   qemuMonitorMigrationCompressionPtr compress)
+qemuMonitorSetMigrationParams(qemuMonitorPtr mon,
+  qemuMonitorMigrationParamsPtr params)
 {
-VIR_DEBUG("level=%d threads=%d dthreads=%d",
-  compress->level, compress->threads, compress->dthreads);
+VIR_DEBUG("compressLevel=%d:%d compressThreads=%d:%d "
+  "decompressThreads=%d:%d",
+  params->compressLevel_set, params->compressLevel,
+  params->compressThreads_set, params->compressThreads,
+  params->decompressThreads_set, params->decompressThreads);
 
 QEMU_CHECK_MONITOR_JSON(mon);
 
-if (!compress->level_set &&
-!compress->threads_set &&
-!compress->dthreads_set)
+if (!params->compressLevel_set &&
+!params->compressThreads_set &&
+!params->decompressThreads_set)
 return 0;
 
-return qemuMonitorJSONSetMigrationCompression(mon, compress);
+return qemuMonitorJSONSetMigrationParams(mon, params);
 }
 
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index dd3587f..28cffc9 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -485,22 +485,23 @@ int qemuMonitorGetMigrationCacheSize(qemuMonitorPtr mon,
 int qemuMonitorSetMigrationCacheSize(qemuMonitorPtr mon,
  unsigned long long cacheSize);
 
-typedef struct _qemuMonitorMigrationCompression 
qemuMonitorMigrationCompression;
-typedef qemuMonitorMigrationCompression *qemuMonitorMigrationCompressionPtr;
-struct _qemuMonitorMigrationCompression {
-bool level_set;
-bool threads_set;
-bool dthreads_set;
+typedef struct _qemuMonitorMigrationParams qemuMonitorMigrationParams;
+typedef qemuMonitorMigrationParams *qemuMonitorMigrationParamsPtr;
+struct _qemuMonitorMigrationParams {
+bool compressLevel_set;
+int compressLevel;
 
-int level;
-int threads;
-int dthreads;
+bool compressThreads_set;
+int compressThreads;
+
+bool decompressThreads_set;
+int decompressThreads;
 };
 
-int 

[libvirt] [PATCH 10/11] qemu: Implement auto convergence migration parameters

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_driver.c|  13 --
 src/qemu/qemu_migration.c | 101 +-
 src/qemu/qemu_migration.h |   8 
 3 files changed, 99 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d065e45..2f2eecb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11775,6 +11775,7 @@ qemuDomainMigratePerform(virDomainPtr dom,
 int ret = -1;
 const char *dconnuri = NULL;
 qemuMigrationCompressionPtr compression = NULL;
+qemuMonitorMigrationParams migParams = { 0 };
 
 virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
 
@@ -11809,7 +11810,7 @@ qemuDomainMigratePerform(virDomainPtr dom,
  */
 ret = qemuMigrationPerform(driver, dom->conn, vm, NULL,
NULL, dconnuri, uri, NULL, NULL, 0, NULL, 0,
-   compression, cookie, cookielen,
+   compression, , cookie, cookielen,
NULL, NULL, /* No output cookies in v2 */
flags, dname, resource, false);
 
@@ -12198,6 +12199,7 @@ qemuDomainMigratePerform3(virDomainPtr dom,
 virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm;
 qemuMigrationCompressionPtr compression = NULL;
+qemuMonitorMigrationParams migParams = { 0 };
 int ret = -1;
 
 virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
@@ -12215,7 +12217,7 @@ qemuDomainMigratePerform3(virDomainPtr dom,
 
 ret = qemuMigrationPerform(driver, dom->conn, vm, xmlin, NULL,
dconnuri, uri, NULL, NULL, 0, NULL, 0,
-   compression,
+   compression, ,
cookiein, cookieinlen,
cookieout, cookieoutlen,
flags, dname, resource, true);
@@ -12249,6 +12251,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
 unsigned long long bandwidth = 0;
 int nbdPort = 0;
 qemuMigrationCompressionPtr compression = NULL;
+qemuMonitorMigrationParamsPtr migParams = NULL;
 int ret = -1;
 
 virCheckFlags(QEMU_MIGRATION_FLAGS, -1);
@@ -12288,6 +12291,9 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
 if (nmigrate_disks < 0)
 goto cleanup;
 
+if (!(migParams = qemuMigrationParams(params, nparams, flags)))
+goto cleanup;
+
 if (!(compression = qemuMigrationCompressionParse(params, nparams, flags)))
 goto cleanup;
 
@@ -12302,11 +12308,12 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
 ret = qemuMigrationPerform(driver, dom->conn, vm, dom_xml, persist_xml,
dconnuri, uri, graphicsuri, listenAddress,
nmigrate_disks, migrate_disks, nbdPort,
-   compression,
+   compression, migParams,
cookiein, cookieinlen, cookieout, cookieoutlen,
flags, dname, bandwidth, true);
  cleanup:
 VIR_FREE(compression);
+VIR_FREE(migParams);
 VIR_FREE(migrate_disks);
 return ret;
 }
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8cfbb9f..b26f363 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3479,6 +3479,52 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
 }
 
 
+qemuMonitorMigrationParamsPtr
+qemuMigrationParams(virTypedParameterPtr params,
+int nparams,
+unsigned long flags)
+{
+qemuMonitorMigrationParamsPtr migParams;
+
+if (VIR_ALLOC(migParams) < 0)
+return NULL;
+
+if (!params)
+return migParams;
+
+#define GET(PARAM, VAR) \
+do {\
+int rc; \
+if ((rc = virTypedParamsGetInt(params, nparams, \
+   VIR_MIGRATE_PARAM_ ## PARAM, \
+   >VAR)) < 0)   \
+goto error; \
+\
+if (rc == 1)\
+migParams->VAR ## _set = true;  \
+} while (0)
+
+GET(AUTO_CONVERGE_INITIAL, cpuThrottleInitial);
+GET(AUTO_CONVERGE_INCREMENT, cpuThrottleIncrement);
+
+#undef GET
+
+if ((migParams->cpuThrottleInitial_set ||
+ migParams->cpuThrottleIncrement_set) &&
+!(flags & VIR_MIGRATE_AUTO_CONVERGE)) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("Turn auto convergence on to 

[libvirt] [PATCH 05/11] qemu: Rework qemuMonitorJSONSetMigrationParams

[Jiri Denemark]

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

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 99678c1..0434522 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2589,20 +2589,19 @@ qemuMonitorJSONSetMigrationParams(qemuMonitorPtr mon,
 if (!(args = virJSONValueNewObject()))
 goto cleanup;
 
-if (params->compressLevel_set &&
-virJSONValueObjectAppendNumberInt(args, "compress-level",
-  params->compressLevel) < 0)
-goto cleanup;
+#define APPEND(VAR, FIELD)  \
+do {\
+if (params->VAR ## _set &&  \
+virJSONValueObjectAppendNumberInt(args, FIELD,  \
+  params->VAR) < 0) \
+goto cleanup;   \
+} while (0)
 
-if (params->compressThreads_set &&
-virJSONValueObjectAppendNumberInt(args, "compress-threads",
-  params->compressThreads) < 0)
-goto cleanup;
+APPEND(compressLevel, "compress-level");
+APPEND(compressThreads, "compress-threads");
+APPEND(decompressThreads, "decompress-threads");
 
-if (params->decompressThreads_set &&
-virJSONValueObjectAppendNumberInt(args, "decompress-threads",
-  params->decompressThreads) < 0)
-goto cleanup;
+#undef APPEND
 
 if (virJSONValueObjectAppend(cmd, "arguments", args) < 0)
 goto cleanup;
-- 
2.9.0

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

[libvirt] [PATCH 11/11] Report auto convergence throttle rate in migration stats

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 include/libvirt/libvirt-domain.h | 8 
 src/qemu/qemu_domain.c   | 6 ++
 src/qemu/qemu_migration.c| 6 ++
 src/qemu/qemu_monitor.h  | 2 ++
 src/qemu/qemu_monitor_json.c | 3 +++
 tools/virsh-domain.c | 9 +
 6 files changed, 34 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 0ca621b..5f41602 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2985,6 +2985,14 @@ int virDomainAbortJob(virDomainPtr dom);
  */
 # define VIR_DOMAIN_JOB_COMPRESSION_OVERFLOW "compression_overflow"
 
+/**
+ * VIR_DOMAIN_JOB_AUTO_CONVERGE_THROTTLE:
+ *
+ * virDomainGetJobStats field: current percentage guest CPUs are throttled
+ * to when auto-convergence decided migration was not converging, as
+ * VIR_TYPED_PARAM_INT.
+ */
+# define VIR_DOMAIN_JOB_AUTO_CONVERGE_THROTTLE  "auto_converge_throttle"
 
 
 /**
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 29f6b93..ab20e52 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -458,6 +458,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
 goto error;
 }
 
+if (stats->cpu_throttle_percentage &&
+virTypedParamsAddInt(, , ,
+ VIR_DOMAIN_JOB_AUTO_CONVERGE_THROTTLE,
+ stats->cpu_throttle_percentage) < 0)
+goto error;
+
 *type = jobInfo->type;
 *params = par;
 *nparams = npar;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b26f363..c411dab 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -807,6 +807,10 @@ qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf,
   stats->xbzrle_overflow);
 }
 
+virBufferAsprintf(buf, "<%1$s>%2$d\n",
+  VIR_DOMAIN_JOB_AUTO_CONVERGE_THROTTLE,
+  stats->cpu_throttle_percentage);
+
 virBufferAdjustIndent(buf, -2);
 virBufferAddLit(buf, "\n");
 }
@@ -1152,6 +1156,8 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr 
ctxt)
 virXPathULongLong("string(./" VIR_DOMAIN_JOB_COMPRESSION_OVERFLOW "[1])",
   ctxt, >xbzrle_overflow);
 
+virXPathInt("string(./" VIR_DOMAIN_JOB_AUTO_CONVERGE_THROTTLE "[1])",
+ctxt, >cpu_throttle_percentage);
  cleanup:
 ctxt->node = save_ctxt;
 return jobInfo;
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 6fecca7..cb4cca8 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -562,6 +562,8 @@ struct _qemuMonitorMigrationStats {
 unsigned long long xbzrle_pages;
 unsigned long long xbzrle_cache_miss;
 unsigned long long xbzrle_overflow;
+
+int cpu_throttle_percentage;
 };
 
 int qemuMonitorGetMigrationStats(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 66b9c4c..bb426dc 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2673,6 +2673,9 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr 
reply,
  >setup_time) == 0)
 stats->setup_time_set = true;
 
+ignore_value(virJSONValueObjectGetNumberInt(ret, "cpu-throttle-percentage",
+
>cpu_throttle_percentage));
+
 switch ((qemuMonitorMigrationStatus) stats->status) {
 case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE:
 case QEMU_MONITOR_MIGRATION_STATUS_SETUP:
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f4d1156..81b1956 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5759,6 +5759,7 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
 int nparams = 0;
 unsigned long long value;
 unsigned int flags = 0;
+int ivalue;
 int rc;
 
 if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
@@ -5994,6 +5995,14 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd)
 vshPrint(ctl, "%-17s %-13llu\n", _("Compression overflows:"), value);
 }
 
+if ((rc = virTypedParamsGetInt(params, nparams,
+   VIR_DOMAIN_JOB_AUTO_CONVERGE_THROTTLE,
+   )) < 0) {
+goto save_error;
+} else if (rc) {
+vshPrint(ctl, "%-17s %-13d\n", _("Auto converge throttle:"), ivalue);
+}
+
 ret = true;
 
  cleanup:
-- 
2.9.0

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

[libvirt] [PATCH 04/11] qemu: Rework qemuMonitorJSONGetMigrationParams

[Jiri Denemark]

We should not require any parameters to be present. After all we have
the *_set bools to express that some parameters were not set.

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

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index eea32ab..99678c1 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2537,6 +2537,8 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon,
 virJSONValuePtr cmd = NULL;
 virJSONValuePtr reply = NULL;
 
+memset(params, 0, sizeof(*params));
+
 if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate-parameters", NULL)))
 return -1;
 
@@ -2548,32 +2550,18 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon,
 
 result = virJSONValueObjectGet(reply, "return");
 
-if (virJSONValueObjectGetNumberInt(result, "compress-level",
-   >compressLevel) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("malformed/missing compress-level "
- "in migrate parameters"));
-goto cleanup;
-}
-params->compressLevel_set = true;
+#define PARSE(VAR, FIELD)   \
+do {\
+if (virJSONValueObjectGetNumberInt(result, FIELD,   \
+   >VAR) == 0)  \
+params->VAR ## _set = true; \
+} while (0)
 
-if (virJSONValueObjectGetNumberInt(result, "compress-threads",
-   >compressThreads) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("malformed/missing compress-threads "
- "in migrate parameters"));
-goto cleanup;
-}
-params->compressThreads_set = true;
+PARSE(compressLevel, "compress-level");
+PARSE(compressThreads, "compress-threads");
+PARSE(decompressThreads, "decompress-threads");
 
-if (virJSONValueObjectGetNumberInt(result, "decompress-threads",
-   >decompressThreads) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("malformed/missing decompress-threads "
- "in migrate parameters"));
-goto cleanup;
-}
-params->decompressThreads_set = true;
+#undef PARSE
 
 ret = 0;
  cleanup:
-- 
2.9.0

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

[libvirt] [PATCH 00/11] Add auto convergence migration parameters

[Jiri Denemark]

This series adds support for a couple of migration parameters which can
be used to tune the auto-convergence algorithm in QEMU.

Jiri Denemark (11):
  qemu: Make qemuMonitorSetMigrationCompression saner
  qemu: Decouple migration parameters from compression settings
  qemu: Rename qemuMonitorMigrationCompression
  qemu: Rework qemuMonitorJSONGetMigrationParams
  qemu: Rework qemuMonitorJSONSetMigrationParams
  test: Rework qemuMonitorJSONGetMigrationParams test
  Add auto convergence migration parameters
  qemu: Introduce qemuMigrationSetParams
  qemu: Add support for cpu throttling parameters
  qemu: Implement auto convergence migration parameters
  Report auto convergence throttle rate in migration stats

 include/libvirt/libvirt-domain.h |  27 ++
 src/qemu/qemu_domain.c   |   6 ++
 src/qemu/qemu_driver.c   |  13 ++-
 src/qemu/qemu_migration.c| 195 ++-
 src/qemu/qemu_migration.h|  17 +++-
 src/qemu/qemu_monitor.c  |  29 --
 src/qemu/qemu_monitor.h  |  37 +---
 src/qemu/qemu_monitor_json.c |  74 +++
 src/qemu/qemu_monitor_json.h |   8 +-
 tests/qemumonitorjsontest.c  |  60 ++--
 tools/virsh-domain.c |  35 +++
 tools/virsh.pod  |  16 ++--
 12 files changed, 364 insertions(+), 153 deletions(-)

-- 
2.9.0

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

[libvirt] [PATCH 02/11] qemu: Decouple migration parameters from compression settings

[Jiri Denemark]

Compression parameters are not the only migration parameters.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c | 52 +--
 src/qemu/qemu_migration.h |  9 +++-
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 575d367..ec2b3dd 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3436,7 +3436,7 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
 {
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
-qemuMonitorMigrationCompressionPtr params = >params;
+qemuMonitorMigrationCompression params = { 0 };
 
 if (qemuMigrationSetOption(driver, vm,
QEMU_MONITOR_MIGRATION_CAPS_XBZRLE,
@@ -3455,7 +3455,16 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
 if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0)
 return -1;
 
-if (qemuMonitorSetMigrationCompression(priv->mon, params) < 0)
+params.level_set = compression->level_set;
+params.level = compression->level;
+
+params.threads_set = compression->threads_set;
+params.threads = compression->threads;
+
+params.dthreads_set = compression->dthreads_set;
+params.dthreads = compression->dthreads;
+
+if (qemuMonitorSetMigrationCompression(priv->mon, ) < 0)
 goto cleanup;
 
 if (compression->xbzrle_cache_set &&
@@ -6684,7 +6693,6 @@ qemuMigrationCompressionParse(virTypedParameterPtr params,
 {
 size_t i;
 qemuMigrationCompressionPtr compression = NULL;
-qemuMonitorMigrationCompressionPtr cparams;
 
 if (VIR_ALLOC(compression) < 0)
 return NULL;
@@ -6713,34 +6721,31 @@ qemuMigrationCompressionParse(virTypedParameterPtr 
params,
 compression->methods |= 1ULL << method;
 }
 
-#define GET_PARAM(PARAM, TYPE, PARENT, VALUE)   \
+#define GET_PARAM(PARAM, TYPE, VALUE)   \
 do {\
 int rc; \
+const char *par = VIR_MIGRATE_PARAM_COMPRESSION_ ## PARAM;  \
 \
 if ((rc = virTypedParamsGet ## TYPE(params, nparams,\
-PARAM, >VALUE)) < 0)\
+par, >VALUE)) < 0) \
 goto error; \
 \
 if (rc == 1)\
-PARENT->VALUE ## _set = true;   \
+compression->VALUE ## _set = true;  \
 } while (0)
 
-cparams = >params;
-
 if (params) {
-GET_PARAM(VIR_MIGRATE_PARAM_COMPRESSION_MT_LEVEL, Int,
-  cparams, level);
-GET_PARAM(VIR_MIGRATE_PARAM_COMPRESSION_MT_THREADS, Int,
-  cparams, threads);
-GET_PARAM(VIR_MIGRATE_PARAM_COMPRESSION_MT_DTHREADS, Int,
-  cparams, dthreads);
-GET_PARAM(VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE, ULLong,
-  compression, xbzrle_cache);
+GET_PARAM(MT_LEVEL, Int, level);
+GET_PARAM(MT_THREADS, Int, threads);
+GET_PARAM(MT_DTHREADS, Int, dthreads);
+GET_PARAM(XBZRLE_CACHE, ULLong, xbzrle_cache);
 }
 
 #undef GET_PARAM
 
-if ((cparams->level_set || cparams->threads_set || cparams->dthreads_set) 
&&
+if ((compression->level_set ||
+ compression->threads_set ||
+ compression->dthreads_set) &&
 !(compression->methods & (1ULL << QEMU_MIGRATION_COMPRESS_MT))) {
 virReportError(VIR_ERR_INVALID_ARG, "%s",
_("Turn multithread compression on to tune it"));
@@ -6772,7 +6777,6 @@ qemuMigrationCompressionDump(qemuMigrationCompressionPtr 
compression,
  unsigned long *flags)
 {
 size_t i;
-qemuMonitorMigrationCompressionPtr cparams = >params;
 
 if (compression->methods == 1ULL << QEMU_MIGRATION_COMPRESS_XBZRLE &&
 !compression->xbzrle_cache_set) {
@@ -6788,22 +6792,22 @@ 
qemuMigrationCompressionDump(qemuMigrationCompressionPtr compression,
 return -1;
 }
 
-if (cparams->level_set &&
+if (compression->level_set &&
 virTypedParamsAddInt(params, nparams, maxparams,
  VIR_MIGRATE_PARAM_COMPRESSION_MT_LEVEL,
- cparams->level) < 0)
+ compression->level) < 0)
 return -1;
 
-if (cparams->threads_set &&
+if (compression->threads_set &&
 virTypedParamsAddInt(params, nparams, maxparams,
 

[libvirt] [PATCH 07/11] Add auto convergence migration parameters

[Jiri Denemark]

They can be used to tune auto-convergence algorithm (which is enabled
with VIR_MIGRATE_AUTO_CONVERGE).

Signed-off-by: Jiri Denemark 
---
 include/libvirt/libvirt-domain.h | 19 +++
 tools/virsh-domain.c | 26 ++
 tools/virsh.pod  | 16 ++--
 3 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index cba4fa5..0ca621b 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -845,6 +845,25 @@ typedef enum {
  */
 # define VIR_MIGRATE_PARAM_COMPRESSION_XBZRLE_CACHE "compression.xbzrle.cache"
 
+/**
+ * VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL:
+ *
+ * virDomainMigrate* params field: the initial percentage guest CPUs are
+ * throttled to when auto-convergence decides migration is not converging.
+ * As VIR_TYPED_PARAM_INT.
+ */
+# define VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL"auto_converge.initial"
+
+/**
+ * VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT:
+ *
+ * virDomainMigrate* params field: the increment added to
+ * VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL whenever the hypervisor decides
+ * the current rate is not enough to ensure convergence of the migration.
+ * As VIR_TYPED_PARAM_INT.
+ */
+# define VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT  "auto_converge.increment"
+
 /* Domain migration. */
 virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn,
unsigned long flags, const char *dname,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 7c3fc86..f4d1156 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9873,6 +9873,14 @@ static const vshCmdOptDef opts_migrate[] = {
  .type = VSH_OT_INT,
  .help = N_("page cache size for xbzrle compression")
 },
+{.name = "auto-converge-initial",
+ .type = VSH_OT_INT,
+ .help = N_("initial CPU throttling rate for auto-convergence")
+},
+{.name = "auto-converge-increment",
+ .type = VSH_OT_INT,
+ .help = N_("CPU throttling rate increment for auto-convergence")
+},
 {.name = NULL}
 };
 
@@ -10033,6 +10041,24 @@ doMigrate(void *opaque)
 VIR_FREE(xml);
 }
 
+if ((rv = vshCommandOptInt(ctl, cmd, "auto-converge-initial", )) < 
0) {
+goto out;
+} else if (rv > 0) {
+if (virTypedParamsAddInt(, , ,
+ VIR_MIGRATE_PARAM_AUTO_CONVERGE_INITIAL,
+ intOpt) < 0)
+goto save_error;
+}
+
+if ((rv = vshCommandOptInt(ctl, cmd, "auto-converge-increment", )) 
< 0) {
+goto out;
+} else if (rv > 0) {
+if (virTypedParamsAddInt(, , ,
+ VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT,
+ intOpt) < 0)
+goto save_error;
+}
+
 if (vshCommandOptBool(cmd, "live"))
 flags |= VIR_MIGRATE_LIVE;
 if (vshCommandOptBool(cmd, "p2p"))
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 8e3ba69..73edd82 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1550,14 +1550,14 @@ to the I namespace is displayed instead of being 
modified.
 =item B [I<--live>] [I<--offline>] [I<--direct>] [I<--p2p> 
[I<--tunnelled>]]
 [I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>]
 [I<--copy-storage-inc>] [I<--change-protection>] [I<--unsafe>] [I<--verbose>]
-[I<--abort-on-error>] [I<--auto-converge>] [I<--postcopy>]
-[I<--postcopy-after-precopy>] I I [I]
-[I] [I] [I]
+[I<--abort-on-error>] [I<--postcopy>] [I<--postcopy-after-precopy>]
+I I [I] [I] [I] 
[I]
 [I<--timeout> B [I<--timeout-suspend> | I<--timeout-postcopy>]]
 [I<--xml> B] [I<--migrate-disks> B] [I<--disks-port> B]
 [I<--compressed>] [I<--comp-methods> B]
 [I<--comp-mt-level>] [I<--comp-mt-threads>] [I<--comp-mt-dthreads>]
-[I<--comp-xbzrle-cache>]
+[I<--comp-xbzrle-cache>] [I<--auto-converge>] [I]
+[I]
 
 Migrate domain to another host.  Add I<--live> for live migration; <--p2p>
 for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled>
@@ -1582,14 +1582,18 @@ by the hypervisor, but can be explicitly used to reject 
the migration if the
 hypervisor lacks change protection support.  I<--verbose> displays the progress
 of migration.  I<--abort-on-error> cancels
 the migration if a soft error (for example I/O error) happens during the
-migration. I<--auto-converge> forces convergence during live migration.
-I<--postcopy> enables post-copy logic in migration, but does not
+migration. I<--postcopy> enables post-copy logic in migration, but does not
 actually start post-copy, i.e., migration is started in pre-copy mode.
 Once migration is running, the user may switch to post-copy using the
 B command sent from another virsh instance or use
 I<--postcopy-after-precopy> to let libvirt automatically switch to
 post-copy after the first pass of pre-copy is finished.
 

[libvirt] [PATCH 01/11] qemu: Make qemuMonitorSetMigrationCompression saner

[Jiri Denemark]

Checking whether the function has anything to do is better done in the
function rather then requiring callers to do that.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration.c | 3 +--
 src/qemu/qemu_monitor.c   | 5 +
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a1ce84e..575d367 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3455,8 +3455,7 @@ qemuMigrationSetCompression(virQEMUDriverPtr driver,
 if (qemuDomainObjEnterMonitorAsync(driver, vm, job) < 0)
 return -1;
 
-if ((params->level_set || params->threads_set || params->dthreads_set) &&
-qemuMonitorSetMigrationCompression(priv->mon, params) < 0)
+if (qemuMonitorSetMigrationCompression(priv->mon, params) < 0)
 goto cleanup;
 
 if (compression->xbzrle_cache_set &&
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d8e33a98..f7609e9 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2177,6 +2177,11 @@ qemuMonitorSetMigrationCompression(qemuMonitorPtr mon,
 
 QEMU_CHECK_MONITOR_JSON(mon);
 
+if (!compress->level_set &&
+!compress->threads_set &&
+!compress->dthreads_set)
+return 0;
+
 return qemuMonitorJSONSetMigrationCompression(mon, compress);
 }
 
-- 
2.9.0

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

[libvirt] [PATCH] qemu: Fix typo pci-extender-bus -> pci-expander-bus

[Andrea Bolognani]

---
Pushed as trivial.

 src/qemu/qemu_domain_address.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 883264a..f42e7c0 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1382,7 +1382,7 @@ qemuDomainAddressFindNewBusNr(virDomainDefPtr def)
  *bus and busNr of the next highest bus for the guest to assign a
  *unique bus number to each PCI bus that is a child of this
  *bus. Each PCI controller. On top of this, the pxb device (which
- *implements the pci-extender-bus) includes a pci-bridge within
+ *implements the pci-expander-bus) includes a pci-bridge within
  *it, and that bridge also uses one bus number (so each pxb device
  *requires at least 2 bus numbers).
  *
-- 
2.7.4

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