[libvirt] [PATCH] doc: fix note about Xen credit scheduler

[Jim Fehlig]

Commit 6c504d6a added a note to the virsh man page about the
deprecation of 'cap' and 'weight' settings for the credit
scheduler. To this day, the default scheduler in Xen is credit
and it supports setting 'cap' and 'weight'. Remove the deprecation
notice from the note on the Xen credit scheduler.

Reported-by: Volo M. 
Signed-off-by: Jim Fehlig 
---
 tools/virsh.pod | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 49abda9..971b408 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1959,7 +1959,7 @@ Therefore, -1 is a useful shorthand for 262144. On the 
Linux kernel, the
 values 0 and 1 are automatically converted to a minimal value of 2.
 
 B: The weight and cap parameters are defined only for the
-XEN_CREDIT scheduler and are now I.
+XEN_CREDIT scheduler.
 
 B: The vcpu_period, emulator_period, and iothread_period parameters
 have a valid value range of 1000-100 or 0, and the vcpu_quota,
-- 
2.1.4

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

Re: [libvirt] """virsh schedinfo [--weight number] domain""" doesn't work with virsh(libxl) for managing Xen domains.

[Jim Fehlig]

On 09/27/2016 03:27 AM, Volo M. wrote:
> Hi Devs,
>
> We're using Centos 7 Xen hypervisors and try to use Libvirt(libxl) for
> managing Xen guest domains.
> We noticed that we can't set cpu weight values for Xen domains with libvirt
> (tested for libvirt v.1.3 and v.2.2) even though its possible to do with XL
> toolkit.
> [root@hv1xen ~]# virsh schedinfo --domain rtp6a88apsm3or --cap=5
> Scheduler  : credit
> weight : 1000
> [root@hv1xen ~]# virsh schedinfo --domain rtp6a88apsm3or --weight=50
> Scheduler  : credit
> error: invalid scheduler option: weight

Thanks for the report. That is a bug in the libvirt libxl driver. I've sent a
patch to fix it

https://www.redhat.com/archives/libvir-list/2016-September/msg01284.html

>
> We noticed in documentation for 'virsh' like  this
> https://linux.die.net/man/1/virsh that the cpu weight option is going to be
> deprecated:
> *"""Note*: The weight and cap parameters are defined only for the XEN_CREDIT
> scheduler and are now /DEPRECATED/ . """

That "note" is bogus and was incorrectly introduced all the way back in Oct 2008
via commit 6c504d6a. Even today, the default scheduler in Xen is credit and it
supports setting cap and weight. I'll send a doc patch to remove the "and are
now deprecated".

Regards,
Jim

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

[libvirt] [PATCH] libxl: fix param assignment in domainGetSchedulerParameters

[Jim Fehlig]

Due to a copy and paste error, the scheduler 'cap' parameter
was over-writing the 'weight' parameter when preparing the
return parameters in libxlDomainGetSchedulerParametersFlags.
As a result, the scheduler weight was never shown when getting
schedinfo and setting the weight failed as well

virsh  schedinfo testvm
Scheduler  : credit
cap: 0

virsh  schedinfo testvm --cap 50 --weight 500
Scheduler  : credit
error: invalid scheduler option: weight

The obvious fix is to assign the 'caps' parameter to the correct
item in the parameter list.

Reported-by: Volo M. 
Signed-off-by: Jim Fehlig 
---
 src/libxl/libxl_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 1adf4c5..b66cb1f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4571,7 +4571,7 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom,
 goto cleanup;
 
 if (*nparams > 1) {
-if (virTypedParameterAssign([0], VIR_DOMAIN_SCHEDULER_CAP,
+if (virTypedParameterAssign([1], VIR_DOMAIN_SCHEDULER_CAP,
 VIR_TYPED_PARAM_UINT, sc_info.cap) < 0)
 goto cleanup;
 }
-- 
2.1.4

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

Re: [libvirt] [PATCH v3 0/4] libxl: channels support

[Jim Fehlig]

Joao Martins wrote:
> Hey,
> 
> This v3 from channel series with latest comments addressed. Difference to
> qemu driver would be only the autogenerated path being slightly different.
> 
> Channels have been on xen toolstack since Xen 4.5 and this small series
> adds support for it, including xenconfig conversion and appropriate tests.
> After this series it's possible to do this:
> (assuming correct configuration of qemu agent in the guest)
> 
>  $ cat domain.xml | grep -a1 channel | head -n 5 | tail -n 4
>  
>
>
>  
> 
>  $ virsh create domain.xml
>  $ echo '{"execute":"guest-network-get-interfaces"}' | socat
>  stdio,ignoreeof  unix-connect:/tmp/channel
> 
>  {"execute":"guest-network-get-interfaces"}
>  {"return": [{"name": "lo", "ip-addresses": [{"ip-address-type": "ipv4",
>  "ip-address": "127.0.0.1", "prefix": 8}, {"ip-address-type": "ipv6",
>  "ip-address": "::1", "prefix": 128}], "hardware-address":
>  "00:00:00:00:00:00"}, {"name": "eth0", "ip-addresses":
>  [{"ip-address-type": "ipv4", "ip-address": "10.100.0.6", "prefix": 24},
>  {"ip-address-type": "ipv6", "ip-address": "fe80::216:3eff:fe40:88eb",
>  "prefix": 64}], "hardware-address": "00:16:3e:40:88:eb"}, {"name":
>  "sit0"}]}
> 
> Thanks,
> Joao
> 
> Joao Martins (4):
>   conf: add xen type for channels
>   libxl: channels support
>   xenconfig: channels conversion support
>   xlconfigtest: add test for channel conversion
> 
>  docs/formatdomain.html.in|  10 ++
>  docs/schemas/domaincommon.rng|  11 ++
>  src/conf/domain_conf.c   |  18 +++-
>  src/conf/domain_conf.h   |   1 +
>  src/libxl/libxl_conf.c   | 110 +++
>  src/libxl/libxl_conf.h   |   3 +
>  src/libxl/libxl_domain.c |  43 +++-
>  src/libxl/libxl_driver.c |   7 ++
>  src/qemu/qemu_command.c  |   1 +
>  src/xenconfig/xen_xl.c   | 176 
> +++
>  tests/xlconfigdata/test-channel-pty.cfg  |  13 +++
>  tests/xlconfigdata/test-channel-pty.xml  |  33 ++
>  tests/xlconfigdata/test-channel-unix.cfg |  13 +++
>  tests/xlconfigdata/test-channel-unix.xml |  34 ++
>  tests/xlconfigtest.c |   4 +
>  15 files changed, 472 insertions(+), 5 deletions(-)
>  create mode 100644 tests/xlconfigdata/test-channel-pty.cfg
>  create mode 100644 tests/xlconfigdata/test-channel-pty.xml
>  create mode 100644 tests/xlconfigdata/test-channel-unix.cfg
>  create mode 100644 tests/xlconfigdata/test-channel-unix.xml
> 

Thanks for your patience, I've pushed this series now.

DV: Sorry for not getting this pushed before the freeze. I was only waiting for
feedback from Joao on some doc text in 1/4 before pushing, but you beat me to
the freeze. The series has been around for a while and through 3 iterations now,
and with exception of the doc text has been ready to go for a few days.
Apologies for stretching the rules a bit.

Regards,
Jim

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

Re: [libvirt] [PATCH 1/3] qemu: store guest visible disk size from qemu monitor block info

[John Ferlan]

On 09/27/2016 04:17 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 26.09.2016 23:07, John Ferlan wrote:
>>
>>
>> On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote:
>>> ---
>>>  src/qemu/qemu_domain.h   |  1 +
>>>  src/qemu/qemu_monitor_json.c | 17 +
>>>  tests/qemumonitorjsontest.c  | 36 
>>>  3 files changed, 54 insertions(+)
>>>
>>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>>> index 13c0372..ea57111 100644
>>> --- a/src/qemu/qemu_domain.h
>>> +++ b/src/qemu/qemu_domain.h
>>> @@ -337,6 +337,7 @@ struct qemuDomainDiskInfo {
>>>  bool tray_open;
>>>  bool empty;
>>>  int io_status;
>>> +unsigned long long guest_size;
>>
>> a/k/a: 'virtual-size' in qemu and 'capacity' in libvirt
>>
>> I probably would stick with capacity or something that says "disk"
>> rather than "guest".
>>
>>>  };
>>>  
>>
>> Trying to determine/decide whether this patch should be "separated" and
>> perhaps made usable with/for the existing callers or whether patch 3
>> should use the block stats (qemuDomainGetStatsBlock) which already
>> handles some details that could be missing here (at least w/r/t backing
>> chains).
> 
> I donno, does backing chain changes anything in this case? I need virtual
> size of top image only and anyway is it possible for images of backing
> chain to have different virtual sizes? It does not make much sense.

I agree... I don't have in depth knowledge of how the backing chains
work, so it's more of a question. Hopefully someone that understands
that logic (pkrempa? eblake?) would be able to be more definitive.

I just wouldn't want something to be missed if backing chains were required.

> 
> qemuDomainGetStatsBlock (and more specifically 
> qemuMonitorBlockStatsUpdateCapacity
> which is more suited to get just size) has rather inconviniet arguments
> more suited for many disks requests.
> 

There's lots of similarities that I see in that code... I spent some
time today working through a mechanism to make one "query-block" call
that 3 consumers (BlockInfo, BlockStats, and DiskNameLookup) could use
(I have it working nominally at least). I would think this could then be
used to more easily add a new lookup function that's only returning the
data you want rather than putting stats data in an BlockInfo...

>>
>> Another option is adjusting the qemuMonitorJSONDiskNameLookup code to be
>> more multi-purpose since it's using the "query-block" for
>> *BlockPullCommon (from Rebase and Pull) as well as from *BlockCommit and
>> it already checks/expects both "inserted" and "image" to be present in
>> order to return that name.
> 
> I would not make function with such specific name provide additionally size 
> info...
> I think it is perfectily fine to have many monitor commands that internally
> use same 'query-block' qemu command because this command is really profound)
> 

Note that I didn't say add a new param/return - I said make it more
multi-purpose with a subtly implied inference that perhaps a function
name change and a different return value (or local/static structure)
could work as well (and hence why I tried to combine things today to see
what was possible).

>>
>>
>>>  typedef struct _qemuDomainHostdevPrivate qemuDomainHostdevPrivate;
>>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>>> index 3d0a120..7903b47 100644
>>> --- a/src/qemu/qemu_monitor_json.c
>>> +++ b/src/qemu/qemu_monitor_json.c
>>> @@ -1800,6 +1800,8 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
>>>  
>>>  for (i = 0; i < virJSONValueArraySize(devices); i++) {
>>>  virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
>>> +virJSONValuePtr inserted;
>>> +virJSONValuePtr image;
>>>  struct qemuDomainDiskInfo *info;
>>>  const char *thisdev;
>>>  const char *status;
>>> @@ -1854,6 +1856,21 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
>>>  if (info->io_status < 0)
>>>  goto cleanup;
>>>  }
>>> +
>>> +if ((inserted = virJSONValueObjectGetObject(dev, "inserted"))) {
>>> +if (!(image = virJSONValueObjectGetObject(inserted, "image"))) 
>>> {
>>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>> +   _("cannot read 'inserted/image' value"));
>>> +goto cleanup;
>>> +}
>>
>> Other code that checks "inserted" and "image" doesn't fail - it just
>> ignores. If there's only going to be one consumer, then I don't believe
>> we want a failure such as this to affect other callers that may not
>> care. That could mean having some sort of "marker" in the returned
>> buffer that the fetch of "virtual-size" did occur (or just using not
>> zero). It would be a shame to have some unexpected failures for a field
>> that nothing else uses especially since the *GetBlockInfo is being used
>> during process launch and reconnect (via 

Re: [libvirt] [PATCH v2] qemu: Fix crash in qemucapsprobe

[John Ferlan]

On 09/27/2016 01:25 PM, Jiri Denemark wrote:
> The qemucapsprobe helper calls virQEMUCapsNewForBinaryInternal with
> caps == NULL, causing the following crash:
> 
> Program received signal SIGSEGV, Segmentation fault.
> #0  0x7788775f in virQEMUCapsInitHostCPUModel
> (qemuCaps=qemuCaps@entry=0x649680, host=host@entry=0x10) at
> src/qemu/qemu_capabilities.c:2969
> #1  0x77889dbf in virQEMUCapsNewForBinaryInternal
> (caps=caps@entry=0x0, binary=,
> libDir=libDir@entry=0x4033f6 "/tmp", cacheDir=cacheDir@entry=0x0,
> runUid=runUid@entry=4294967295, runGid=runGid@entry=4294967295,
> qmpOnly=true) at src/qemu/qemu_capabilities.c:4039
> #2  0x00401702 in main (argc=2, argv=0x7fffd968) at
> tests/qemucapsprobe.c:73
> 
> Caused by v2.2.0-182-g68c7011.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_capabilities.c | 15 +--
>  src/qemu/qemu_capspriv.h |  2 +-
>  tests/qemuxml2argvtest.c |  2 +-
>  3 files changed, 11 insertions(+), 8 deletions(-)
> 

ACK (and save for freeze)

John

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

Re: [libvirt] [PATCH v2 6/8] qemu: Use qemuGetCompressionProgram for error paths

[John Ferlan]

On 09/27/2016 12:53 PM, Ján Tomko wrote:
> On Fri, Sep 23, 2016 at 07:30:54AM -0400, John Ferlan wrote:
>> Let's do some more code reuse - there are 3 other callers that care to
>> check/get the compress program. Each of those though cares whether the
>> requested cfg image is valid and exists. So, add a parameter to handle
>> those cases.
>>
>> NB: We won't need to initialize the returned value in the case where
>> the cfg image doesn't exist since the called program will handle that.
>>
>> Signed-off-by: John Ferlan 
>> ---
>> src/qemu/qemu_driver.c | 103
>> +
>> 1 file changed, 43 insertions(+), 60 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 7dfba50..8a47262 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -3271,6 +3271,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat
>> compress)
>>  * @imageFormat: String representation from qemu.conf for the compression
>>  *   image format being used (dump, save, or snapshot).
>>  * @styleFormat: String representing the style of format (dump, save,
>> snapshot)
>> + * @use_raw_on_fail: Boolean indicating how to handle the error path.
>> For
>> + *   callers that are OK with invalid data or
>> inability to
>> + *   find the compression program, just return a raw
>> format
>>  *
>>  * Returns:
>>  *virQEMUSaveFormat- Integer representation of the compression
>> @@ -3282,7 +3285,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat
>> compress)
>>  */
>> static virQEMUSaveFormat
>> qemuGetCompressionProgram(const char *imageFormat,
>> -  const char *styleFormat)
>> +  const char *styleFormat,
>> +  bool use_raw_on_fail)
>> {
>> virQEMUSaveFormat ret;
>>
>> @@ -3298,18 +3302,34 @@ qemuGetCompressionProgram(const char
>> *imageFormat,
>> return ret;
>>
>>  error:
>> -if (ret < 0)
>> -VIR_WARN("Invalid %s image format specified in "
>> - "configuration file, using raw",
>> - styleFormat);
>> -else
>> -VIR_WARN("Compression program for %s image format in "
>> - "configuration file isn't available, using raw",
>> - styleFormat);
>> +if (ret < 0) {
>> +if (use_raw_on_fail)
>> +VIR_WARN("Invalid %s image format specified in "
>> + "configuration file, using raw",
>> + styleFormat);
>> +else
>> +virReportError(VIR_ERR_OPERATION_FAILED,
>> +   _("Invalid %s image format specified "
>> + "in configuration file"),
>> +   styleFormat);
>> +} else {
>> +if (use_raw_on_fail)
>> +VIR_WARN("Compression program for %s image format in "
>> + "configuration file isn't available, using raw",
>> + styleFormat);
>> +else
>> +virReportError(VIR_ERR_OPERATION_FAILED,
>> +   _("Compression program for %s image format "
>> + "in configuration file isn't available"),
>> +   styleFormat);
>> +}
> 
> This might work for the English language, but constructing a translatable
> message in this matter is unfriendly to translators and not worth
> shaving off a few lines of code.

Hmm... And qemuDomainDiskChangeSupported is any better?  Similarly
qemuMonitorJSONGetBlockInfo?

Instead of 3 (or 4) message pairs that we all similar varying only by
"dump", "save", or "snapshot" there is now 1 message pair which has a %s
parameter no different than 1000's of other libvirt messages. Since
dump, save, and snapshot are all command names - does their translation
really change?

Beyond that I'll note that qemuNodeDeviceDetachFlags uses a const char
*driverName which is just a string and can be messaged using %s. Also
qemuConnectGetDomainCapabilities uses a const char *virttype_str which
is messaged.

> 
> Also, logging the "styleFormat" (sic) only makes sense for the dump,
> which might not have been a result of an API call, otherwise we're
> better off putting the image format in here.
> 

If you don't like the variable name, then change it.  I'm not offended.

> If you call the snapshot API and get an error saying "xz" was not found,
> you know it's not for the core dump format.
> 

If you prefer a different mechanism, I'll review the patch when I see it.


John

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

Re: [libvirt] [PATCH v2 3/8] qemu: Introduce helper qemuGetCompressionProgram

[John Ferlan]

On 09/27/2016 12:40 PM, Ján Tomko wrote:
> On Fri, Sep 23, 2016 at 07:30:51AM -0400, John Ferlan wrote:
>> Split out the guts of getCompressionType to perform the same
>> functionality
>> in the new helper program with a subsequent patch goal to be reusable for
>> other callers making similar checks/calls to ensure the compression type
>> is valid and that the compression program cannot be found.
>>
>> Signed-off-by: John Ferlan 
>> ---
>> src/qemu/qemu_driver.c | 67
>> ++
>> 1 file changed, 46 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 956bddd..3f03576 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -3267,36 +3267,61 @@ qemuCompressProgramAvailable(virQEMUSaveFormat
>> compress)
>> }
>>
>>
>> +/* qemuGetCompressionProgram:
>> + * @imageFormat: String representation from qemu.conf for the
>> compression
>> + *   image format being used (dump, save, or snapshot).
>> + *
>> + * Returns:
>> + *virQEMUSaveFormat- Integer representation of the compression
>> + *   program to be used for particular style
>> + *   (e.g. dump, save, or snapshot).
>> + *QEMU_SAVE_FORMAT_RAW - If there is no qemu.conf imageFormat
>> value or
>> + *   no there was an error, then just return RAW
>> + *   indicating none.
>> + */
>> +static virQEMUSaveFormat
>> +qemuGetCompressionProgram(const char *imageFormat)
>> +{
>> +virQEMUSaveFormat ret;
>> +
>> +if (!imageFormat)
>> +return QEMU_SAVE_FORMAT_RAW;
>> +
>> +if ((ret = qemuSaveCompressionTypeFromString(imageFormat)) < 0)
>> +goto error;
>> +
> 
> The list of supported values is known at compile time.
> 
> If the value is invalid, we should reject it when parsing the config
> file.

I'm sure we could if there was a patch for it, but for each of the 3
different types (dump, save, and snapshot) the "check" for validity was
done within this scope (even before this set of patches).

If one peeks at the virQEMUDriverConfigLoadFile where each of the
cfg->{dump|save|snapshot}ImageFormat values are read, the validation of
what's been read (for other fields) is minimal especially for strings.
It seems it's left for "other" code to handle.

I'll review a patch that validates the various fetches from qemu_conf.c
and messages that "%s_image_format" has an invalid value.

> 
> That way the 'getCompressionProgram' helper would be faithful to its
> name and only try to get the compression program.
> 

If the name is that much of an issue, then feel free to change the name.


John

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

[libvirt] Entering freeze for libvirt-2.3.0

[Daniel Veillard]

  Since nobody complained about my earlier message with the release plan,
I tagged libvirt-2.3.0 candidate release 1 in git and pushed signed tarball
and rpms to the usual place:

   ftp://libvirt.org/libvirt/

 As usual my limited testing is really not sufficient so please give it a try,
I enjoy the view of a completely green 
https://ci.centos.org/view/libvirt-project/ :-)
but that doesn't test portability to other platforms for example !

 Then I will try to push rc2 on Thursday, that way the final release can happen
during the week-end or on Monday if all goes well,

  thanks for testing it !

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

[libvirt] [PATCH] libvirt-storage.c:Lines too long, use 80 character columns.

[Nitesh Konkar]

Signed-off-by: Nitesh Konkar 
---
 src/libvirt-storage.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 48996ba..c4f2a03 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -233,7 +233,8 @@ virConnectNumOfDefinedStoragePools(virConnectPtr conn)
 
 virCheckConnectReturn(conn, -1);
 
-if (conn->storageDriver && 
conn->storageDriver->connectNumOfDefinedStoragePools) {
+if (conn->storageDriver &&
+conn->storageDriver->connectNumOfDefinedStoragePools) {
 int ret;
 ret = conn->storageDriver->connectNumOfDefinedStoragePools(conn);
 if (ret < 0)
@@ -280,7 +281,8 @@ virConnectListDefinedStoragePools(virConnectPtr conn,
 virCheckNonNullArgGoto(names, error);
 virCheckNonNegativeArgGoto(maxnames, error);
 
-if (conn->storageDriver && 
conn->storageDriver->connectListDefinedStoragePools) {
+if (conn->storageDriver &&
+conn->storageDriver->connectListDefinedStoragePools) {
 int ret;
 ret = conn->storageDriver->connectListDefinedStoragePools(conn, names, 
maxnames);
 if (ret < 0)
@@ -332,7 +334,8 @@ virConnectFindStoragePoolSources(virConnectPtr conn,
 virCheckNonNullArgGoto(type, error);
 virCheckReadOnlyGoto(conn->flags, error);
 
-if (conn->storageDriver && 
conn->storageDriver->connectFindStoragePoolSources) {
+if (conn->storageDriver &&
+conn->storageDriver->connectFindStoragePoolSources) {
 char *ret;
 ret = conn->storageDriver->connectFindStoragePoolSources(conn, type, 
srcSpec, flags);
 if (!ret)
@@ -485,7 +488,8 @@ virStoragePoolLookupByVolume(virStorageVolPtr vol)
 
 virCheckStorageVolReturn(vol, NULL);
 
-if (vol->conn->storageDriver && 
vol->conn->storageDriver->storagePoolLookupByVolume) {
+if (vol->conn->storageDriver &&
+vol->conn->storageDriver->storagePoolLookupByVolume) {
 virStoragePoolPtr ret;
 ret = vol->conn->storageDriver->storagePoolLookupByVolume(vol);
 if (!ret)
@@ -1188,7 +1192,8 @@ virStoragePoolNumOfVolumes(virStoragePoolPtr pool)
 
 virCheckStoragePoolReturn(pool, -1);
 
-if (pool->conn->storageDriver && 
pool->conn->storageDriver->storagePoolNumOfVolumes) {
+if (pool->conn->storageDriver &&
+pool->conn->storageDriver->storagePoolNumOfVolumes) {
 int ret;
 ret = pool->conn->storageDriver->storagePoolNumOfVolumes(pool);
 if (ret < 0)
@@ -1230,7 +1235,8 @@ virStoragePoolListVolumes(virStoragePoolPtr pool,
 virCheckNonNullArgGoto(names, error);
 virCheckNonNegativeArgGoto(maxnames, error);
 
-if (pool->conn->storageDriver && 
pool->conn->storageDriver->storagePoolListVolumes) {
+if (pool->conn->storageDriver &&
+pool->conn->storageDriver->storagePoolListVolumes) {
 int ret;
 ret = pool->conn->storageDriver->storagePoolListVolumes(pool, names, 
maxnames);
 if (ret < 0)
@@ -1297,7 +1303,8 @@ virStorageVolLookupByName(virStoragePoolPtr pool,
 virCheckStoragePoolReturn(pool, NULL);
 virCheckNonNullArgGoto(name, error);
 
-if (pool->conn->storageDriver && 
pool->conn->storageDriver->storageVolLookupByName) {
+if (pool->conn->storageDriver &&
+pool->conn->storageDriver->storageVolLookupByName) {
 virStorageVolPtr ret;
 ret = pool->conn->storageDriver->storageVolLookupByName(pool, name);
 if (!ret)
@@ -1471,7 +1478,8 @@ virStorageVolCreateXML(virStoragePoolPtr pool,
 virCheckNonNullArgGoto(xmlDesc, error);
 virCheckReadOnlyGoto(pool->conn->flags, error);
 
-if (pool->conn->storageDriver && 
pool->conn->storageDriver->storageVolCreateXML) {
+if (pool->conn->storageDriver &&
+pool->conn->storageDriver->storageVolCreateXML) {
 virStorageVolPtr ret;
 ret = pool->conn->storageDriver->storageVolCreateXML(pool, xmlDesc, 
flags);
 if (!ret)
-- 
2.1.0

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

[libvirt] [PATCH v2] qemu: Fix crash in qemucapsprobe

[Jiri Denemark]

The qemucapsprobe helper calls virQEMUCapsNewForBinaryInternal with
caps == NULL, causing the following crash:

Program received signal SIGSEGV, Segmentation fault.
#0  0x7788775f in virQEMUCapsInitHostCPUModel
(qemuCaps=qemuCaps@entry=0x649680, host=host@entry=0x10) at
src/qemu/qemu_capabilities.c:2969
#1  0x77889dbf in virQEMUCapsNewForBinaryInternal
(caps=caps@entry=0x0, binary=,
libDir=libDir@entry=0x4033f6 "/tmp", cacheDir=cacheDir@entry=0x0,
runUid=runUid@entry=4294967295, runGid=runGid@entry=4294967295,
qmpOnly=true) at src/qemu/qemu_capabilities.c:4039
#2  0x00401702 in main (argc=2, argv=0x7fffd968) at
tests/qemucapsprobe.c:73

Caused by v2.2.0-182-g68c7011.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_capabilities.c | 15 +--
 src/qemu/qemu_capspriv.h |  2 +-
 tests/qemuxml2argvtest.c |  2 +-
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4d859c4..cc8ec58 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2962,14 +2962,17 @@ virQEMUCapsCPUFilterFeatures(const char *name,
 
 void
 virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
-virCapsHostPtr host)
+virCapsPtr caps)
 {
 virCPUDefPtr cpu = NULL;
 
-if (!virQEMUCapsGuestIsNative(host->arch, qemuCaps->arch))
+if (!caps)
+return;
+
+if (!virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch))
 goto error;
 
-if (host->cpu && host->cpu->model) {
+if (caps->host.cpu && caps->host.cpu->model) {
 if (VIR_ALLOC(cpu) < 0)
 goto error;
 
@@ -2978,7 +2981,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
 cpu->mode = VIR_CPU_MODE_CUSTOM;
 cpu->match = VIR_CPU_MATCH_EXACT;
 
-if (virCPUDefCopyModelFilter(cpu, host->cpu, true,
+if (virCPUDefCopyModelFilter(cpu, caps->host.cpu, true,
  virQEMUCapsCPUFilterFeatures, NULL) < 0)
 goto error;
 }
@@ -3248,7 +3251,7 @@ virQEMUCapsLoadCache(virCapsPtr caps,
 }
 VIR_FREE(nodes);
 
-virQEMUCapsInitHostCPUModel(qemuCaps, >host);
+virQEMUCapsInitHostCPUModel(qemuCaps, caps);
 
 ret = 0;
  cleanup:
@@ -4036,7 +4039,7 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps,
 virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0)
 goto error;
 
-virQEMUCapsInitHostCPUModel(qemuCaps, >host);
+virQEMUCapsInitHostCPUModel(qemuCaps, caps);
 }
 
  cleanup:
diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
index 22c5a8a..fab2c2a 100644
--- a/src/qemu/qemu_capspriv.h
+++ b/src/qemu/qemu_capspriv.h
@@ -64,5 +64,5 @@ virQEMUCapsSetArch(virQEMUCapsPtr qemuCaps,
 
 void
 virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
-virCapsHostPtr host);
+virCapsPtr caps);
 #endif
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0af71a1..4b9ecb8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -357,7 +357,7 @@ testUpdateQEMUCaps(const struct testInfo *info,
 if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0)
 goto cleanup;
 
-virQEMUCapsInitHostCPUModel(info->qemuCaps, >host);
+virQEMUCapsInitHostCPUModel(info->qemuCaps, caps);
 
 virQEMUCapsFilterByMachineType(info->qemuCaps, vm->def->os.machine);
 
-- 
2.10.0

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

Re: [libvirt] [PATCH v2 6/8] qemu: Use qemuGetCompressionProgram for error paths

[Ján Tomko]

On Fri, Sep 23, 2016 at 07:30:54AM -0400, John Ferlan wrote:

Let's do some more code reuse - there are 3 other callers that care to
check/get the compress program. Each of those though cares whether the
requested cfg image is valid and exists. So, add a parameter to handle
those cases.

NB: We won't need to initialize the returned value in the case where
the cfg image doesn't exist since the called program will handle that.

Signed-off-by: John Ferlan 
---
src/qemu/qemu_driver.c | 103 +
1 file changed, 43 insertions(+), 60 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7dfba50..8a47262 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3271,6 +3271,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
 * @imageFormat: String representation from qemu.conf for the compression
 *   image format being used (dump, save, or snapshot).
 * @styleFormat: String representing the style of format (dump, save, snapshot)
+ * @use_raw_on_fail: Boolean indicating how to handle the error path. For
+ *   callers that are OK with invalid data or inability to
+ *   find the compression program, just return a raw format
 *
 * Returns:
 *virQEMUSaveFormat- Integer representation of the compression
@@ -3282,7 +3285,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
 */
static virQEMUSaveFormat
qemuGetCompressionProgram(const char *imageFormat,
-  const char *styleFormat)
+  const char *styleFormat,
+  bool use_raw_on_fail)
{
virQEMUSaveFormat ret;

@@ -3298,18 +3302,34 @@ qemuGetCompressionProgram(const char *imageFormat,
return ret;

 error:
-if (ret < 0)
-VIR_WARN("Invalid %s image format specified in "
- "configuration file, using raw",
- styleFormat);
-else
-VIR_WARN("Compression program for %s image format in "
- "configuration file isn't available, using raw",
- styleFormat);
+if (ret < 0) {
+if (use_raw_on_fail)
+VIR_WARN("Invalid %s image format specified in "
+ "configuration file, using raw",
+ styleFormat);
+else
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("Invalid %s image format specified "
+ "in configuration file"),
+   styleFormat);
+} else {
+if (use_raw_on_fail)
+VIR_WARN("Compression program for %s image format in "
+ "configuration file isn't available, using raw",
+ styleFormat);
+else
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("Compression program for %s image format "
+ "in configuration file isn't available"),
+   styleFormat);
+}


This might work for the English language, but constructing a translatable
message in this matter is unfriendly to translators and not worth
shaving off a few lines of code.

Also, logging the "styleFormat" (sic) only makes sense for the dump,
which might not have been a result of an API call, otherwise we're
better off putting the image format in here.

If you call the snapshot API and get an error saying "xz" was not found,
you know it's not for the core dump format.

Jan


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

[libvirt] [PATCH 4/6] tests: qemumonitorjsontest: Do some actual testing in qemuMonitorJSONTestAttachChardev

[Peter Krempa]

Until now the test was rather useless since it didn't check the
arguments formatted and didn't use properly configured chardev objects.

Add the expected arguments and instrument the test to validate them.
Modify some test cases to actually add valid data.

Note that the UDP test data is currently wrong due to a bug.
---
 tests/qemumonitorjsontest.c | 93 +
 1 file changed, 68 insertions(+), 25 deletions(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 23877f8..61344b7 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -747,6 +747,7 @@ static int
 qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
 const char *label,
 virDomainChrSourceDefPtr chr,
+const char *expectargs,
 const char *reply,
 const char *expectPty,
 bool fail)
@@ -772,7 +773,8 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr 
xmlopt,
 if (!(data.test = qemuMonitorTestNewSimple(true, xmlopt)))
 goto cleanup;

-if (qemuMonitorTestAddItem(data.test, "chardev-add", jsonreply) < 0)
+if (qemuMonitorTestAddItemExpect(data.test, "chardev-add",
+ expectargs, true, jsonreply) < 0)
 goto cleanup;

 if (virTestRun(fulllabel, , ) < 0)
@@ -793,49 +795,90 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr 
xmlopt)
 virDomainChrSourceDef chr;
 int ret = 0;

-#define CHECK(label, fail) \
-if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, , NULL, NULL, \
-fail) < 0) \
+#define CHECK(label, fail, expectargs) 
\
+if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, , expectargs,   
\
+NULL, NULL, fail) < 0) 
\
 ret = -1

 chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_NULL };
-CHECK("null", false);
+CHECK("null", false,
+  "{'id':'alias','backend':{'type':'null','data':{}}}");

 chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC };
-CHECK("vc", false);
+CHECK("vc", false,
+  "{'id':'alias','backend':{'type':'null','data':{}}}");

 chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
 if (qemuMonitorJSONTestAttachOneChardev(xmlopt, "pty", ,
+"{'id':'alias',"
+ "'backend':{'type':'pty',"
+"'data':{}}}",
 "\"pty\" : \"/dev/pts/0\"",
 "/dev/pts/0", false) < 0)
 ret = -1;

 chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
-CHECK("pty missing path", true);
-
-chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_FILE };
-CHECK("file", false);
-
-chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_DEV };
-CHECK("device", false);
-
-chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_TCP };
-CHECK("tcp", false);
-
-chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UDP };
-CHECK("udp", false);
-
-chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_UNIX };
-CHECK("unix", false);
+CHECK("pty missing path", true,
+  "{'id':'alias','backend':{'type':'pty','data':{}}}");
+
+memset(, 0, sizeof(chr));
+chr.type = VIR_DOMAIN_CHR_TYPE_FILE;
+chr.data.file.path = (char *) "/test/path";
+CHECK("file", false,
+  
"{'id':'alias','backend':{'type':'file','data':{'out':'/test/path'}}}");
+
+memset(, 0, sizeof(chr));
+chr.type = VIR_DOMAIN_CHR_TYPE_DEV;
+chr.data.file.path = (char *) "/test/path";
+CHECK("device", false,
+  
"{'id':'alias','backend':{'type':'serial','data':{'device':'/test/path'}}}");
+
+memset(, 0, sizeof(chr));
+chr.type = VIR_DOMAIN_CHR_TYPE_TCP;
+chr.data.tcp.host = (char *) "example.com";
+chr.data.tcp.service = (char *) "1234";
+CHECK("tcp", false,
+  "{'id':'alias',"
+   "'backend':{'type':'socket',"
+  "'data':{'addr':{'type':'inet',"
+  "'data':{'host':'example.com',"
+  "'port':'1234'}},"
+  "'wait':false,"
+  "'telnet':false,"
+  "'server':false}}}");
+
+memset(, 0, sizeof(chr));
+chr.type = VIR_DOMAIN_CHR_TYPE_UDP;
+chr.data.udp.connectHost = (char *) "example.com";
+chr.data.udp.connectService = (char *) "1234";
+CHECK("udp", false,
+  

Re: [libvirt] [PATCH v2 3/8] qemu: Introduce helper qemuGetCompressionProgram

[Ján Tomko]

On Fri, Sep 23, 2016 at 07:30:51AM -0400, John Ferlan wrote:

Split out the guts of getCompressionType to perform the same functionality
in the new helper program with a subsequent patch goal to be reusable for
other callers making similar checks/calls to ensure the compression type
is valid and that the compression program cannot be found.

Signed-off-by: John Ferlan 
---
src/qemu/qemu_driver.c | 67 ++
1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 956bddd..3f03576 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3267,36 +3267,61 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
}


+/* qemuGetCompressionProgram:
+ * @imageFormat: String representation from qemu.conf for the compression
+ *   image format being used (dump, save, or snapshot).
+ *
+ * Returns:
+ *virQEMUSaveFormat- Integer representation of the compression
+ *   program to be used for particular style
+ *   (e.g. dump, save, or snapshot).
+ *QEMU_SAVE_FORMAT_RAW - If there is no qemu.conf imageFormat value or
+ *   no there was an error, then just return RAW
+ *   indicating none.
+ */
+static virQEMUSaveFormat
+qemuGetCompressionProgram(const char *imageFormat)
+{
+virQEMUSaveFormat ret;
+
+if (!imageFormat)
+return QEMU_SAVE_FORMAT_RAW;
+
+if ((ret = qemuSaveCompressionTypeFromString(imageFormat)) < 0)
+goto error;
+


The list of supported values is known at compile time.

If the value is invalid, we should reject it when parsing the config
file.

That way the 'getCompressionProgram' helper would be faithful to its
name and only try to get the compression program.

Jan


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

[libvirt] [PATCH 6/6] qemu: monitor: Properly configure backend for UDP chardevs

[Peter Krempa]

Since introduction of chardev hotplug the code was wrong for the UDP
case and basically created a TCP socket instead. Use proper objects and
type for UDP.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1377602
---
 src/qemu/qemu_monitor_json.c | 12 ++--
 tests/qemumonitorjsontest.c  | 20 
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 126927e..c720376 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6212,12 +6212,20 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
 break;

 case VIR_DOMAIN_CHR_TYPE_UDP:
-backend_type = "socket";
+backend_type = "udp";
 addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.udp.connectHost,
  
chr->data.udp.connectService);
 if (!addr ||
-virJSONValueObjectAppend(data, "addr", addr) < 0)
+virJSONValueObjectAppend(data, "remote", addr) < 0)
 goto error;
+
+if (chr->data.udp.bindHost) {
+addr = 
qemuMonitorJSONBuildInetSocketAddress(chr->data.udp.bindHost,
+ 
chr->data.udp.bindService);
+if (!addr ||
+virJSONValueObjectAppend(data, "local", addr) < 0)
+goto error;
+}
 addr = NULL;
 break;

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 61344b7..0574f8c 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -853,10 +853,22 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr 
xmlopt)
 chr.data.udp.connectService = (char *) "1234";
 CHECK("udp", false,
   "{'id':'alias',"
-   "'backend':{'type':'socket',"
-  "'data':{'addr':{'type':'inet',"
-  "'data':{'host':'example.com',"
-  "'port':'1234'}");
+   "'backend':{'type':'udp',"
+  "'data':{'remote':{'type':'inet',"
+"'data':{'host':'example.com',"
+"'port':'1234'}");
+
+chr.data.udp.bindHost = (char *) "localhost";
+chr.data.udp.bindService = (char *) "4321";
+CHECK("udp", false,
+  "{'id':'alias',"
+   "'backend':{'type':'udp',"
+  "'data':{'remote':{'type':'inet',"
+"'data':{'host':'example.com',"
+"'port':'1234'}},"
+  "'local':{'type':'inet',"
+   "'data':{'host':'localhost',"
+   "'port':'4321'}");

 memset(, 0, sizeof(chr));
 chr.type = VIR_DOMAIN_CHR_TYPE_UNIX;
-- 
2.10.0

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

[libvirt] [PATCH 0/6] qemu: fix UDP chardev hotplug and make qemumonitorjsontest less useless

[Peter Krempa]

Most of the time qemumonitorjson test isn't really testing much. Add support
for actually testing chardev hotplug and fix it for the UDP case.

Peter Krempa (6):
  conf: Sanitize formatting of UDP chardev source
  tests: qemu: Add support for testing aguments on monitor verbatim
  tests: qemumonitorjson: Don't do multiple tests in one virTestRun
  tests: qemumonitorjsontest: Do some actual testing in
qemuMonitorJSONTestAttachChardev
  qemu: monitor: Simplify construction of chardev backends
  qemu: monitor: Properly configure backend for UDP chardevs

 src/conf/domain_conf.c   |  42 
 src/qemu/qemu_monitor_json.c |  51 +-
 tests/qemumonitorjsontest.c  | 222 ---
 tests/qemumonitortestutils.c | 112 ++
 tests/qemumonitortestutils.h |   6 ++
 5 files changed, 326 insertions(+), 107 deletions(-)

-- 
2.10.0

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

[libvirt] [PATCH 3/6] tests: qemumonitorjson: Don't do multiple tests in one virTestRun

[Peter Krempa]

The chardev attach test would do all the tests in one virTestRun
instance. If one sub-test failed then the test would report failure
improperly and the error would be hard to debug since the error pointer
was overwritten.
---
 tests/qemumonitorjsontest.c | 147 ++--
 1 file changed, 102 insertions(+), 45 deletions(-)

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 4e7bb71..23877f8 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -705,87 +705,143 @@ testQemuMonitorJSONGetCommandLineOptionParameters(const 
void *data)
 return ret;
 }

+
+struct qemuMonitorJSONTestAttachChardevData {
+qemuMonitorTestPtr test;
+virDomainChrSourceDefPtr chr;
+const char *expectPty;
+bool fail;
+};
+
 static int
-testQemuMonitorJSONAttachChardev(const void *data)
+testQemuMonitorJSONAttachChardev(const void *opaque)
 {
-virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
-qemuMonitorTestPtr test = qemuMonitorTestNewSimple(true, xmlopt);
-virDomainChrSourceDef chr;
-int ret = 0;
+const struct qemuMonitorJSONTestAttachChardevData *data = opaque;
+int rc;

-if (!test)
+if ((rc = qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(data->test),
+   "alias", data->chr)) < 0)
+goto cleanup;
+
+if (data->chr->type == VIR_DOMAIN_CHR_TYPE_PTY) {
+if (STRNEQ_NULLABLE(data->expectPty, data->chr->data.file.path)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "expected PTY path: %s got: %s",
+   NULLSTR(data->expectPty),
+   NULLSTR(data->chr->data.file.path));
+rc = -1;
+}
+
+VIR_FREE(data->chr->data.file.path);
+}
+
+ cleanup:
+if ((rc != 0) != data->fail)
 return -1;
+else
+return 0;
+}

-#define DO_CHECK(chrID, reply, fail)\
-if (qemuMonitorTestAddItem(test, "chardev-add", reply) < 0) \
-goto cleanup;   \
-if (qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(test),   \
- chrID, ) < 0)  \
-ret = fail ? ret : -1;  \
-else\
-ret = fail ? -1 : ret;  \

-#define CHECK(chrID, reply) \
-DO_CHECK(chrID, reply, false)
+static int
+qemuMonitorJSONTestAttachOneChardev(virDomainXMLOptionPtr xmlopt,
+const char *label,
+virDomainChrSourceDefPtr chr,
+const char *reply,
+const char *expectPty,
+bool fail)

-#define CHECK_FAIL(chrID, reply) \
-DO_CHECK(chrID, reply, true)
+{
+struct qemuMonitorJSONTestAttachChardevData data;
+char *jsonreply = NULL;
+char *fulllabel = NULL;
+int ret = -1;
+
+if (!reply)
+reply = "";
+
+if (virAsprintf(, "{\"return\": {%s}}", reply) < 0)
+goto cleanup;
+
+if (virAsprintf(, "qemuMonitorJSONTestAttachChardev(%s)", label) 
< 0)
+goto cleanup;
+
+data.chr = chr;
+data.fail = fail;
+data.expectPty = expectPty;
+if (!(data.test = qemuMonitorTestNewSimple(true, xmlopt)))
+goto cleanup;
+
+if (qemuMonitorTestAddItem(data.test, "chardev-add", jsonreply) < 0)
+goto cleanup;
+
+if (virTestRun(fulllabel, , ) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+qemuMonitorTestFree(data.test);
+VIR_FREE(jsonreply);
+VIR_FREE(fulllabel);
+return ret;
+}
+
+static int
+qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt)
+{
+virDomainChrSourceDef chr;
+int ret = 0;
+
+#define CHECK(label, fail) \
+if (qemuMonitorJSONTestAttachOneChardev(xmlopt, label, , NULL, NULL, \
+fail) < 0) \
+ret = -1

 chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_NULL };
-CHECK("chr_null", "{\"return\": {}}");
+CHECK("null", false);

 chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC };
-CHECK("chr_vc", "{\"return\": {}}");
+CHECK("vc", false);

-#define PTY_PATH "/dev/ttyS0"
 chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY };
-CHECK("chr_pty", "{\"return\": {\"pty\" : \"" PTY_PATH "\"}}");
-if (STRNEQ_NULLABLE(PTY_PATH, chr.data.file.path)) {
-VIR_FREE(chr.data.file.path);
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "expected PTY path: %s got: %s",
-   PTY_PATH, NULLSTR(chr.data.file.path));
+if (qemuMonitorJSONTestAttachOneChardev(xmlopt, "pty", ,
+"\"pty\" : 

[libvirt] [PATCH 1/6] conf: Sanitize formatting of UDP chardev source

[Peter Krempa]

Use much simpler logic to determine parts of the code to print.
---
 src/conf/domain_conf.c | 42 --
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a06da46..3f34540 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21431,32 +21431,22 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
 break;

 case VIR_DOMAIN_CHR_TYPE_UDP:
-if (def->data.udp.bindService &&
-def->data.udp.bindHost) {
-virBufferEscapeString(buf, "data.udp.bindHost);
-virBufferEscapeString(buf, "service='%s'/>\n",
-  def->data.udp.bindService);
-} else if (def->data.udp.bindHost) {
-virBufferEscapeString(buf, "\n",
-  def->data.udp.bindHost);
-} else if (def->data.udp.bindService) {
-virBufferEscapeString(buf, "\n",
-  def->data.udp.bindService);
-}
-
-if (def->data.udp.connectService &&
-def->data.udp.connectHost) {
-virBufferEscapeString(buf, "data.udp.connectHost);
-virBufferEscapeString(buf, "service='%s'/>\n",
-  def->data.udp.connectService);
-} else if (def->data.udp.connectHost) {
-virBufferEscapeString(buf, "\n",
-  def->data.udp.connectHost);
-} else if (def->data.udp.connectService) {
-virBufferEscapeString(buf, "\n",
-  def->data.udp.connectService);
+if (def->data.udp.bindService || def->data.udp.bindHost) {
+virBufferAddLit(buf, "data.udp.bindService)
+virBufferEscapeString(buf, " host='%s'", 
def->data.udp.bindHost);
+if (def->data.udp.bindService)
+virBufferEscapeString(buf, " service='%s'", 
def->data.udp.bindService);
+virBufferAddLit(buf, "/>\n");
+}
+
+if (def->data.udp.connectService || def->data.udp.connectHost) {
+virBufferAddLit(buf, "data.udp.connectService)
+virBufferEscapeString(buf, " host='%s'", 
def->data.udp.connectHost);
+if (def->data.udp.connectService)
+virBufferEscapeString(buf, " service='%s'", 
def->data.udp.connectService);
+virBufferAddLit(buf, "/>\n");
 }
 break;

-- 
2.10.0

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

[libvirt] [PATCH 2/6] tests: qemu: Add support for testing aguments on monitor verbatim

[Peter Krempa]

Add code that takes a string and matches it against the data passed as
arguments from qemu. This is a simpler version of
qemuMonitorTestAddItemParams.
---
 tests/qemumonitortestutils.c | 112 +++
 tests/qemumonitortestutils.h |   6 +++
 2 files changed, 118 insertions(+)

diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c
index c86a27a..50bf174 100644
--- a/tests/qemumonitortestutils.c
+++ b/tests/qemumonitortestutils.c
@@ -439,6 +439,7 @@ struct qemuMonitorTestHandlerData {
 char *response;
 size_t nargs;
 qemuMonitorTestCommandArgsPtr args;
+char *expectArgs;
 };

 static void
@@ -458,6 +459,7 @@ qemuMonitorTestHandlerDataFree(void *opaque)
 VIR_FREE(data->command_name);
 VIR_FREE(data->response);
 VIR_FREE(data->args);
+VIR_FREE(data->expectArgs);
 VIR_FREE(data);
 }

@@ -668,6 +670,7 @@ qemuMonitorTestProcessCommandWithArgs(qemuMonitorTestPtr 
test,
 }


+
 /* this allows to add a responder that is able to check
  * a (shallow) structure of arguments for a command */
 int
@@ -721,6 +724,115 @@ qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
 }


+static int
+qemuMonitorTestProcessCommandWithArgStr(qemuMonitorTestPtr test,
+qemuMonitorTestItemPtr item,
+const char *cmdstr)
+{
+struct qemuMonitorTestHandlerData *data = item->opaque;
+virJSONValuePtr val = NULL;
+virJSONValuePtr args;
+char *argstr = NULL;
+const char *cmdname;
+int ret = -1;
+
+if (!(val = virJSONValueFromString(cmdstr)))
+return -1;
+
+if (!(cmdname = virJSONValueObjectGetString(val, "execute"))) {
+ret = qemuMonitorReportError(test, "Missing command name in %s", 
cmdstr);
+goto cleanup;
+}
+
+if (STRNEQ(data->command_name, cmdname)) {
+ret = qemuMonitorTestAddUnexpectedErrorResponse(test);
+goto cleanup;
+}
+
+if (!(args = virJSONValueObjectGet(val, "arguments"))) {
+ret = qemuMonitorReportError(test,
+ "Missing arguments section for command 
'%s'",
+ data->command_name);
+goto cleanup;
+}
+
+/* convert the arguments to string */
+if (!(argstr = virJSONValueToString(args, false)))
+goto cleanup;
+
+/* verify that the argument value is expected */
+if (STRNEQ(argstr, data->expectArgs)) {
+ret = qemuMonitorReportError(test,
+ "%s: expected arguments: '%s', got: '%s'",
+ data->command_name,
+ data->expectArgs, argstr);
+goto cleanup;
+}
+
+
+VIR_FREE(argstr);
+
+/* arguments checked out, return the response */
+ret = qemuMonitorTestAddResponse(test, data->response);
+
+ cleanup:
+VIR_FREE(argstr);
+virJSONValueFree(val);
+return ret;
+}
+
+
+/**
+ * qemuMonitorTestAddItemExpect:
+ *
+ * @test: test monitor object
+ * @cmdname: command name
+ * @cmdargs: expected arguments of the command
+ * @apostrophe: convert apostrophes (') in @cmdargs to quotes (")
+ * @response: simulated response of the command
+ *
+ * Simulates a qemu monitor command. Checks that the 'arguments' of the qmp
+ * command are expected. If @apostrophe is true apostrophes are converted to
+ * quotes for simplification of writing the strings into code.
+ */
+int
+qemuMonitorTestAddItemExpect(qemuMonitorTestPtr test,
+ const char *cmdname,
+ const char *cmdargs,
+ bool apostrophe,
+ const char *response)
+{
+struct qemuMonitorTestHandlerData *data;
+
+if (VIR_ALLOC(data) < 0)
+goto error;
+
+if (VIR_STRDUP(data->command_name, cmdname) < 0 ||
+VIR_STRDUP(data->response, response) < 0 ||
+VIR_STRDUP(data->expectArgs, cmdargs) < 0)
+goto error;
+
+if (apostrophe) {
+char *tmp = data->expectArgs;
+
+while (*tmp != '\0') {
+if (*tmp == '\'')
+*tmp = '"';
+
+tmp++;
+}
+}
+
+return qemuMonitorTestAddHandler(test,
+ qemuMonitorTestProcessCommandWithArgStr,
+ data, qemuMonitorTestHandlerDataFree);
+
+ error:
+qemuMonitorTestHandlerDataFree(data);
+return -1;
+}
+
+
 static void
 qemuMonitorTestEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
  virDomainObjPtr vm ATTRIBUTE_UNUSED,
diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h
index 8e2f371..3890cd4 100644
--- a/tests/qemumonitortestutils.h
+++ b/tests/qemumonitortestutils.h
@@ -60,6 +60,12 @@ int qemuMonitorTestAddItemParams(qemuMonitorTestPtr test,
  ...)
 ATTRIBUTE_SENTINEL;

+int 

Re: [libvirt] [PATCH 11/12] conf: Add support for blkiotune "_length" options

[Michal Privoznik]

On 23.09.2016 14:56, John Ferlan wrote:
> Modify _virDomainBlockIoTuneInfo and rng schema to support the _length
> options for bps/iops throttling values. Document the new values.
> 
> Signed-off-by: John Ferlan 
> ---
>  docs/formatdomain.html.in  | 40 -
>  docs/schemas/domaincommon.rng  | 38 +
>  src/conf/domain_conf.c | 45 ++-
>  .../qemuxml2argv-blkdeviotune-max-length.xml   | 65 
> ++
>  .../qemuxml2xmlout-blkdeviotune-max-length.xml |  1 +
>  tests/qemuxml2xmltest.c|  1 +
>  6 files changed, 188 insertions(+), 2 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
> 


> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 861a15d..6e405c4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7134,6 +7134,21 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
> _("disk iotune field '%s_max' must be an integer"),   
>   \
> #val);
>   \
>  return -1;   
>   \
> +}
>   \
> +if (virXPathULongLong("string(./iotune/" #val "_max_length)",
>   \
> +  ctxt, >blkdeviotune.val##_max_length) == -2) 
> {  \
> +virReportError(VIR_ERR_XML_ERROR,
>   \
> +   _("disk iotune field '%s_max_length' must be "
>   \
> + "an integer"), #val);   
>   \
> +return -1;   
>   \
> +}
> +
> +#define CHECK_IOTUNE_MAX_LENGTH(val) 
>   \
> +if (def->blkdeviotune.val##_length && !def->blkdeviotune.val) {  
>   \
> +virReportError(VIR_ERR_XML_ERROR,
>   \
> +   _("disk iotune '%s'_length cannot be set unless " 
>   \
> + "disk iotune '%s' is also set"), #val, #val);   
>   \
> +return -1;   
>   \
>  }
>  
>  static int
> @@ -7155,6 +7170,13 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
>  return -1;
>  }
>  
> +//PARSE_IOTUNE(total_bytes_sec_max_length);
> +//PARSE_IOTUNE(read_bytes_sec_max_length);
> +//PARSE_IOTUNE(write_bytes_sec_max_length);
> +//PARSE_IOTUNE(total_iops_sec_max_length);
> +//PARSE_IOTUNE(read_iops_sec_max_length);
> +//PARSE_IOTUNE(write_iops_sec_max_length);
> +

Whoa, probably a leftover from development? Drop it.

>  if ((def->blkdeviotune.total_bytes_sec &&
>   def->blkdeviotune.read_bytes_sec) ||
>  (def->blkdeviotune.total_bytes_sec &&

Michal

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

Re: [libvirt] [PATCH 08/12] qemu: Add length for bps/iops throttling parameters to driver

[Michal Privoznik]

On 23.09.2016 14:56, John Ferlan wrote:
> Add support for a duration/length for the bps/iops and friends.
> 
> Modify the API in order to add the "blkdeviotune." specific definitions
> for the iotune throttling duration/length options
> 
> total_bytes_sec_max_length
> write_bytes_sec_max_length
> read_bytes_sec_max_length
> total_iops_sec_max_length
> write_iops_sec_max_length
> read_iops_sec_max_length
> 
> Signed-off-by: John Ferlan 
> ---
>  include/libvirt/libvirt-domain.h |  54 
>  src/conf/domain_conf.h   |   6 +++
>  src/qemu/qemu_driver.c   | 106 
> ++-
>  src/qemu/qemu_monitor.c  |   7 ++-
>  src/qemu/qemu_monitor.h  |   3 +-
>  src/qemu/qemu_monitor_json.c |  25 -
>  src/qemu/qemu_monitor_json.h |   3 +-
>  tests/qemumonitorjsontest.c  |  17 ++-
>  8 files changed, 211 insertions(+), 10 deletions(-)
> 

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ee16cb5..3b04754 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -113,7 +113,8 @@ VIR_LOG_INIT("qemu.qemu_driver");
>  #define QEMU_NB_MEM_PARAM  3
>  
>  #define QEMU_NB_BLOCK_IO_TUNE_PARAM  6
> -#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX  13
> +#define QEMU_NB_BLOCK_IO_TUNE_PARAM_LENGTH 12
> +#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX  19
>  
>  #define QEMU_NB_NUMA_PARAM 2
>  
> @@ -17262,7 +17263,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
>  bool set_bytes_max = false;
>  bool set_iops_max = false;
>  bool set_size_iops = false;
> +bool set_bytes_max_length = false;
> +bool set_iops_max_length = false;
>  bool supportMaxOptions = true;
> +bool supportMaxLengthOptions = true;
>  virQEMUDriverConfigPtr cfg = NULL;
>  virObjectEventPtr event = NULL;
>  virTypedParameterPtr eventParams = NULL;
> @@ -17298,6 +17302,18 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> VIR_TYPED_PARAM_ULLONG,
> VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
> VIR_TYPED_PARAM_ULLONG,
> +   
> VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH,
> +   VIR_TYPED_PARAM_ULLONG,
> +   
> VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH,
> +   VIR_TYPED_PARAM_ULLONG,
> +   
> VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH,
> +   VIR_TYPED_PARAM_ULLONG,
> +   
> VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH,
> +   VIR_TYPED_PARAM_ULLONG,
> +   
> VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH,
> +   VIR_TYPED_PARAM_ULLONG,
> +   
> VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH,
> +   VIR_TYPED_PARAM_ULLONG,
> NULL) < 0)
>  return -1;
>  
> @@ -17449,6 +17465,60 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
>  
> VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC,
>  param->value.ul) < 0)
>  goto endjob;
> +} else if (STREQ(param->field,
> + 
> VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH)) {
> +info.total_bytes_sec_max_length = param->value.ul;
> +set_bytes_max_length = true;
> +if (virTypedParamsAddULLong(, ,
> +,
> +
> VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH,
> +param->value.ul) < 0)
> +goto endjob;
> +} else if (STREQ(param->field,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH)) 
> {
> +info.read_bytes_sec_max_length = param->value.ul;
> +set_bytes_max_length = true;
> +if (virTypedParamsAddULLong(, ,
> +,
> +
> VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH,
> +param->value.ul) < 0)
> +goto endjob;
> +} else if (STREQ(param->field,
> + 
> VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH)) {
> +info.write_bytes_sec_max_length = param->value.ul;
> +set_bytes_max_length = true;
> +if (virTypedParamsAddULLong(, ,
> +,
> +
> VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH,
> +param->value.ul) < 0)
> +goto endjob;
> +} else if 

Re: [libvirt] [PATCH 00/12] Add length (duration) params for iotune throttling

[Michal Privoznik]

On 23.09.2016 14:56, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1349898
> 
> Do a little housekeeping and minor adjustments to existing code, then
> add the various "-length" options for the  code.
> 
> 
> John Ferlan (12):
>   docs: Fix typo in libvirt-domain.h parameter description
>   include: Update description for  max params
>   tests: Add blkdeviotune-max xml2xmltest
>   qemu: Convert from shorthand to longer throttling names
>   qemu: Adjust how supportMaxOptions is used.
>   include: Add new definitions for duration for bps/iops throttling
>   caps: Add new capability for the bps/iops throttling length
>   qemu: Add length for bps/iops throttling parameters to driver
>   conf: Add a formatting macro for all the blkiotune values
>   conf: Adjust the PARSE_IOTUNE macro
>   conf: Add support for blkiotune "_length" options
>   qemu: Add the length options to the iotune command line
> 
>  docs/formatdomain.html.in  |  40 +-
>  docs/schemas/domaincommon.rng  |  38 ++
>  include/libvirt/libvirt-domain.h   | 124 --
>  src/conf/domain_conf.c | 142 
> +++--
>  src/conf/domain_conf.h |   6 +
>  src/qemu/qemu_capabilities.c   |   2 +
>  src/qemu/qemu_capabilities.h   |   1 +
>  src/qemu/qemu_command.c|  96 ++
>  src/qemu/qemu_driver.c | 106 ++-
>  src/qemu/qemu_monitor.c|   7 +-
>  src/qemu/qemu_monitor.h|   3 +-
>  src/qemu/qemu_monitor_json.c   |  72 ++-
>  src/qemu/qemu_monitor_json.h   |   3 +-
>  .../caps_2.6.0-gicv2.aarch64.xml   |   1 +
>  .../caps_2.6.0-gicv3.aarch64.xml   |   1 +
>  tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml  |   1 +
>  tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |   1 +
>  tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |   1 +
>  tests/qemumonitorjsontest.c|  17 ++-
>  .../qemuxml2argv-blkdeviotune-max-length.args  |  34 +
>  .../qemuxml2argv-blkdeviotune-max-length.xml   |  65 ++
>  .../qemuxml2argv-blkdeviotune-max.args |  10 +-
>  .../qemuxml2argv-blkdeviotune-max.xml  |  14 +-
>  .../qemuxml2argv-blkdeviotune.args |   5 +-
>  tests/qemuxml2argvtest.c   |   4 +
>  .../qemuxml2xmlout-blkdeviotune-max-length.xml |   1 +
>  .../qemuxml2xmlout-blkdeviotune-max.xml|   1 +
>  tests/qemuxml2xmltest.c|   2 +
>  28 files changed, 616 insertions(+), 182 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max.xml
> 

ACK series.

Michal

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

Re: [libvirt] [PATCH] libxl: find virDomainObj in libxlDomainShutdownThread

[Jim Fehlig]

Martin Kletzander wrote:
> On Wed, Sep 21, 2016 at 03:29:49PM -0600, Jim Fehlig wrote:
>> libxl events are delivered to libvirt via the libxlDomainEventHandler
>> callback registered with libxl. Documenation in
>> $xensrc/tools/libxl/libxl_event.h states that the callback "may occur
>> on any thread in which the application calls libxl". This can result
>> in deadlock since many of the libvirt callees of libxl hold a lock on
>> the virDomainObj they are working on. When the callback is invoked, it
>> attempts to find a virDomainObj corresponding to the domain ID provided
>> by libxl. Searching the domain obj list results in locking each obj
>> before checking if it is active, and its ID equals the requested ID.
>> Deadlock is possible when attempting to lock an obj that is already
>> locked further up the call stack. Indeed, Max Ustermann reported an
>> instance of this deadlock
>>
>> https://www.redhat.com/archives/libvir-list/2015-November/msg00130.html
>>
>> Guido Rossmueller also recently stumbled across it
>>
>> https://www.redhat.com/archives/libvir-list/2016-September/msg00287.html
>>
>> Fix the deadlock by moving the lookup of virDomainObj to the
>> libxlDomainShutdownThread. After this patch, libxl events are
>> enqueued on the libvirt side and processed by dedicated thread,
>> avoiding the described deadlock.
>>
> 
> Makes sense, passing locked object to another thread is not good, the
> references match, so even though I cannot test this out, I'm fairly sure
> when giving it an ACK.

Thanks Martin! I finally got around to pushing this before the freeze.

Regards,
Jim



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

Re: [libvirt] [PATCH 11/12] conf: Add support for blkiotune "_length" options

[Michal Privoznik]

On 23.09.2016 14:56, John Ferlan wrote:
> Modify _virDomainBlockIoTuneInfo and rng schema to support the _length
> options for bps/iops throttling values. Document the new values.
> 
> Signed-off-by: John Ferlan 
> ---
>  docs/formatdomain.html.in  | 40 -
>  docs/schemas/domaincommon.rng  | 38 +
>  src/conf/domain_conf.c | 45 ++-
>  .../qemuxml2argv-blkdeviotune-max-length.xml   | 65 
> ++
>  .../qemuxml2xmlout-blkdeviotune-max-length.xml |  1 +
>  tests/qemuxml2xmltest.c|  1 +
>  6 files changed, 188 insertions(+), 2 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
>  create mode 12 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
> 


> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 861a15d..6e405c4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7134,6 +7134,21 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
> _("disk iotune field '%s_max' must be an integer"),   
>   \
> #val);
>   \
>  return -1;   
>   \
> +}
>   \
> +if (virXPathULongLong("string(./iotune/" #val "_max_length)",
>   \
> +  ctxt, >blkdeviotune.val##_max_length) == -2) 
> {  \
> +virReportError(VIR_ERR_XML_ERROR,
>   \
> +   _("disk iotune field '%s_max_length' must be "
>   \
> + "an integer"), #val);   
>   \
> +return -1;   
>   \
> +}
> +
> +#define CHECK_IOTUNE_MAX_LENGTH(val) 
>   \
> +if (def->blkdeviotune.val##_length && !def->blkdeviotune.val) {  
>   \
> +virReportError(VIR_ERR_XML_ERROR,
>   \
> +   _("disk iotune '%s'_length cannot be set unless " 
>   \
> + "disk iotune '%s' is also set"), #val, #val);   
>   \
> +return -1;   
>   \
>  }
>  
>  static int
> @@ -7155,6 +7170,13 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
>  return -1;
>  }
>  
> +//PARSE_IOTUNE(total_bytes_sec_max_length);
> +//PARSE_IOTUNE(read_bytes_sec_max_length);
> +//PARSE_IOTUNE(write_bytes_sec_max_length);
> +//PARSE_IOTUNE(total_iops_sec_max_length);
> +//PARSE_IOTUNE(read_iops_sec_max_length);
> +//PARSE_IOTUNE(write_iops_sec_max_length);
> +

Whoa, probably a leftover from development? Drop it.

>  if ((def->blkdeviotune.total_bytes_sec &&
>   def->blkdeviotune.read_bytes_sec) ||
>  (def->blkdeviotune.total_bytes_sec &&

Michal

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

Re: [libvirt] [PATCH] qemu: Fix crash in qemucapsprobe

[Jiri Denemark]

On Tue, Sep 27, 2016 at 08:25:05 -0400, John Ferlan wrote:
> 
> 
> On 09/27/2016 07:47 AM, Jiri Denemark wrote:
> > The qemucapsprobe helper calls virQEMUCapsInitHostCPUModel with
> > caps == NULL, causing the following crash:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > #0  0x7788775f in virQEMUCapsInitHostCPUModel
> > (qemuCaps=qemuCaps@entry=0x649680, host=host@entry=0x10) at
> > src/qemu/qemu_capabilities.c:2969
> > #1  0x77889dbf in virQEMUCapsNewForBinaryInternal
> > (caps=caps@entry=0x0, binary=,
> > libDir=libDir@entry=0x4033f6 "/tmp", cacheDir=cacheDir@entry=0x0,
> > runUid=runUid@entry=4294967295, runGid=runGid@entry=4294967295,
> > qmpOnly=true) at src/qemu/qemu_capabilities.c:4039
> > #2  0x00401702 in main (argc=2, argv=0x7fffd968) at
> > tests/qemucapsprobe.c:73
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> >  src/qemu/qemu_capabilities.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> 
> Earlier in this function there's:
> 
> if (!cacheDir)
> rv = 0;
> else if ((rv = virQEMUCapsInitCached(caps, qemuCaps, cacheDir)) < 0)
> goto error;
> 
> 
> virQEMUCapsInitCached will call virQEMUCapsLoadCache which will call
> virQEMUCapsInitHostCPUModel(qemuCaps, >host); as well.

Well, this cannot happen if caps == NULL.

> So wouldn't it better to have virQEMUCapsInitHostCPUModel take "caps" as
> the parameter and then return "silently" if !caps->host ?

But this is the way I should have fixed it, since it will work better
with future changes.

Jirka

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

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

[Michal Privoznik]

On 22.09.2016 12:43, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> For one VM, it could have more than one graphical display.
> Such as we coud add both vnc and spice display to a VM.
> 
> This patch introduces '--all' for showing all
> possible type of graphical display for a active VM.
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  tools/virsh-domain.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)

You need to update the manpage too. Without it I cannot ACK this one.

> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 3829b17..7194153 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -10648,6 +10648,10 @@ static const vshCmdOptDef opts_domdisplay[] = {
>   .help = N_("select particular graphical display "
>  "(e.g. \"vnc\", \"spice\", \"rdp\")")
>  },
> +{.name = "all",
> + .type = VSH_OT_BOOL,
> + .help = N_("show all possible graphical displays")
> +},
>  {.name = NULL}
>  };
>  
> @@ -10671,6 +10675,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
>  int tmp;
>  int flags = 0;
>  bool params = false;
> +bool all = false;

You can initialize this variable right here:

bool all = vshCommandOptBool(cmd, "all"));

>  const char *xpath_fmt = 
> "string(/domain/devices/graphics[@type='%s']/%s)";
>  virSocketAddr addr;
>  
> @@ -10685,6 +10690,9 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
>  if (vshCommandOptBool(cmd, "include-password"))
>  flags |= VIR_DOMAIN_XML_SECURE;
>  
> +if (vshCommandOptBool(cmd, "all"))
> +all = true;
> +
>  if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
>  goto cleanup;
>  
> @@ -10845,7 +10853,15 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
>  
>  /* We got what we came for so return successfully */
>  ret = true;
> -break;
> +if (!all) {
> +break;
> +} else {
> +VIR_FREE(xpath);
> +VIR_FREE(passwd);
> +VIR_FREE(listen_addr);
> +VIR_FREE(output);
> +vshPrint(ctl, "\n");
> +}

I'd prefer if these are spread across the loop body. That is, just
before a variable set, it is prepended with VIR_FREE() call, e.g.

 /* Print out our full URI */
 VIR_FREE(output);
 output = virBufferContentAndReset();

And so on. I like the idea!

Michal

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

[libvirt] [PATCH] mingw: Package cputypes.rng for mingw32 too

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---

Pushed as trivial.

 mingw-libvirt.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index c8476d9..c9bf503 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -210,6 +210,7 @@ rm -rf 
$RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
 %dir %{mingw32_datadir}/libvirt/schemas/
 %{mingw32_datadir}/libvirt/schemas/basictypes.rng
 %{mingw32_datadir}/libvirt/schemas/capability.rng
+%{mingw32_datadir}/libvirt/schemas/cputypes.rng
 %{mingw32_datadir}/libvirt/schemas/domain.rng
 %{mingw32_datadir}/libvirt/schemas/domaincaps.rng
 %{mingw32_datadir}/libvirt/schemas/domaincommon.rng
-- 
2.10.0

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

Re: [libvirt] [libvirt-php][PATCH 05/11] src: Introduce util.h

[Michal Privoznik]

On 27.09.2016 15:11, Michal Privoznik wrote:
> Move out some macros that are shared between multiple source
> files into a separate file called util.h.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/Makefile.am   |  1 +
>  src/libvirt-php.c | 19 +
>  src/libvirt-php.h | 39 --
>  src/sockets.c |  1 +
>  src/util.h| 63 
> +++
>  src/vncfunc.c |  1 +
>  6 files changed, 85 insertions(+), 39 deletions(-)
>  create mode 100644 src/util.h

> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
> index 6b145de..b9256db 100644
> --- a/src/libvirt-php.c
> +++ b/src/libvirt-php.c

> @@ -794,6 +799,20 @@ zend_module_entry libvirt_module_entry = {
>  ZEND_GET_MODULE(libvirt)
>  #endif
>  
> +ZEND_BEGIN_MODULE_GLOBALS(libvirt)
> +char *last_error;
> +char *vnc_location;
> +zend_bool longlong_to_string_ini;
> +char *iso_path_ini;
> +char *image_path_ini;
> +zend_long max_connections_ini;
> +#ifdef DEBUG_SUPPORT
> +int debug;
> +#endif
> +resource_info *binding_resources;
> +int binding_resources_count;
> +ZEND_END_MODULE_GLOBALS(libvirt)
> +

D'oh. In some compilers, forward declaration is not supported so this
breaks. Therefore I am squashing this in:

diff --git c/src/libvirt-php.c i/src/libvirt-php.c
index b9256db..9218f71 100644
--- c/src/libvirt-php.c
+++ i/src/libvirt-php.c
@@ -159,8 +159,6 @@ int le_libvirt_nodedev;
 int le_libvirt_stream;
 int le_libvirt_snapshot;

-ZEND_DECLARE_MODULE_GLOBALS(libvirt)
-
 ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_connect, 0, 0, 0)
 ZEND_ARG_INFO(0, url)
 ZEND_ARG_INFO(0, readonly)
@@ -813,6 +811,8 @@ ZEND_BEGIN_MODULE_GLOBALS(libvirt)
 int binding_resources_count;
 ZEND_END_MODULE_GLOBALS(libvirt)

+ZEND_DECLARE_MODULE_GLOBALS(libvirt)
+
 /* PHP init options */
 PHP_INI_BEGIN()
 STD_PHP_INI_ENTRY("libvirt.longlong_to_string", "1", PHP_INI_ALL,
OnUpdateBool, longlong_to_string_ini, zend_libvirt_globals, libvirt_globals)

Michal

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

[libvirt] [libvirt-php][PATCH 06/11] src: Introduce util.c

[Michal Privoznik]

So far there is just one function shared across our code:
get_datetime().

Signed-off-by: Michal Privoznik 
---
 src/Makefile.am   |  2 +-
 src/config.m4 |  2 +-
 src/libvirt-php.c | 26 --
 src/util.c| 41 +
 src/util.h|  2 ++
 5 files changed, 45 insertions(+), 28 deletions(-)
 create mode 100644 src/util.c

diff --git a/src/Makefile.am b/src/Makefile.am
index 39a47e9..a12e729 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -18,7 +18,7 @@ LIBVIRT_PHP_SYMBOL_FILE = \
 php_plugindir = $(extensiondir)
 php_plugin_LTLIBRARIES = libvirt-php.la
 libvirt_php_la_SOURCES = \
-   util.h  \
+   util.c util.h   \
vncfunc.c \
sockets.c \
libvirt-php.c libvirt-php.h
diff --git a/src/config.m4 b/src/config.m4
index fd76286..ad8adb4 100644
--- a/src/config.m4
+++ b/src/config.m4
@@ -45,7 +45,7 @@ if test "$PHP_LIBVIRT" != "no"; then
 PHP_EVAL_LIBLINE($LIBXML_LIBRARY, LIBVIRT_SHARED_LIBADD)
 
 PHP_SUBST(LIBVIRT_SHARED_LIBADD)
-PHP_NEW_EXTENSION(libvirt, libvirt-php.c sockets.c vncfunc.c, $ext_shared)
+PHP_NEW_EXTENSION(libvirt, libvirt-php.c sockets.c vncfunc.c util.c, 
$ext_shared)
   else
 AC_MSG_ERROR(pkg-config not found)
   fi
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index b9256db..70405f2 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -862,32 +862,6 @@ PHP_RSHUTDOWN_FUNCTION(libvirt)
 }
 
 /*
- * Private function name:   get_datetime
- * Since version:   0.4.2
- * Description: Function can be used to get date and time in the 
`-mm-dd HH:mm:ss` format, internally used for logging when debug logging is 
enabled using libvirt_set_logfile() API function.
- * Arguments:   None
- * Returns: Date/time string in `-mm-dd HH:mm:ss` format
- */
-char *get_datetime(void)
-{
-/* Caution: Function cannot use DPRINTF() macro otherwise the neverending 
loop will be met! */
-char *outstr = NULL;
-time_t t;
-struct tm *tmp;
-
-t = time(NULL);
-tmp = localtime();
-if (tmp == NULL)
-return NULL;
-
-outstr = (char *)malloc(32 * sizeof(char));
-if (strftime(outstr, 32, "%Y-%m-%d %H:%M:%S", tmp) == 0)
-return NULL;
-
-return outstr;
-}
-
-/*
  * Private function name:   set_logfile
  * Since version:   0.4.2
  * Description: Function to set the log file. You can set log file 
to NULL to disable logging (default). Useful for debugging purposes.
diff --git a/src/util.c b/src/util.c
new file mode 100644
index 000..7d2b457
--- /dev/null
+++ b/src/util.c
@@ -0,0 +1,41 @@
+/*
+ * util.c: common, generic utility functions
+ *
+ * See COPYING for the license of this software
+ *
+ * Written by:
+ *  Michal Privoznik 
+ */
+
+#include 
+
+#include 
+#include 
+
+#include "util.h"
+
+/*
+ * Private function name:   get_datetime
+ * Since version:   0.4.2
+ * Description: Function can be used to get date and time in the 
`-mm-dd HH:mm:ss` format, internally used for logging when debug logging is 
enabled using libvirt_set_logfile() API function.
+ * Arguments:   None
+ * Returns: Date/time string in `-mm-dd HH:mm:ss` format
+ */
+char *get_datetime(void)
+{
+/* Caution: Function cannot use DPRINTF() macro otherwise the neverending 
loop will be met! */
+char *outstr = NULL;
+time_t t;
+struct tm *tmp;
+
+t = time(NULL);
+tmp = localtime();
+if (tmp == NULL)
+return NULL;
+
+outstr = (char *)malloc(32 * sizeof(char));
+if (strftime(outstr, 32, "%Y-%m-%d %H:%M:%S", tmp) == 0)
+return NULL;
+
+return outstr;
+}
diff --git a/src/util.h b/src/util.h
index 119d573..e907a49 100644
--- a/src/util.h
+++ b/src/util.h
@@ -60,4 +60,6 @@ extern int gdebug;
((uint32_t)var[2] <<  8) +   \
((uint32_t)var[3]))
 
+char *get_datetime(void);
+
 #endif /* __UTIL_H__ */
-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 05/11] src: Introduce util.h

[Michal Privoznik]

Move out some macros that are shared between multiple source
files into a separate file called util.h.

Signed-off-by: Michal Privoznik 
---
 src/Makefile.am   |  1 +
 src/libvirt-php.c | 19 +
 src/libvirt-php.h | 39 --
 src/sockets.c |  1 +
 src/util.h| 63 +++
 src/vncfunc.c |  1 +
 6 files changed, 85 insertions(+), 39 deletions(-)
 create mode 100644 src/util.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 68b6371..39a47e9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -18,6 +18,7 @@ LIBVIRT_PHP_SYMBOL_FILE = \
 php_plugindir = $(extensiondir)
 php_plugin_LTLIBRARIES = libvirt-php.la
 libvirt_php_la_SOURCES = \
+   util.h  \
vncfunc.c \
sockets.c \
libvirt-php.c libvirt-php.h
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 6b145de..b9256db 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -27,6 +27,7 @@
 #endif
 
 #include "libvirt-php.h"
+#include "util.h"
 
 #ifndef EXTWIN
 // From vncfunc.c
@@ -35,6 +36,10 @@ int vnc_get_dimensions(char *server, char *port, int *width, 
int *height);
 int connect_socket(char *server, char *port, int keepalive, int nodelay, int 
allow_server_override);
 #endif
 
+#ifdef DEBUG_SUPPORT
+int gdebug;
+#endif
+
 #ifdef DEBUG_CORE
 #define DPRINTF(fmt, ...) \
 if (LIBVIRT_G(debug)) \
@@ -794,6 +799,20 @@ zend_module_entry libvirt_module_entry = {
 ZEND_GET_MODULE(libvirt)
 #endif
 
+ZEND_BEGIN_MODULE_GLOBALS(libvirt)
+char *last_error;
+char *vnc_location;
+zend_bool longlong_to_string_ini;
+char *iso_path_ini;
+char *image_path_ini;
+zend_long max_connections_ini;
+#ifdef DEBUG_SUPPORT
+int debug;
+#endif
+resource_info *binding_resources;
+int binding_resources_count;
+ZEND_END_MODULE_GLOBALS(libvirt)
+
 /* PHP init options */
 PHP_INI_BEGIN()
 STD_PHP_INI_ENTRY("libvirt.longlong_to_string", "1", PHP_INI_ALL, 
OnUpdateBool, longlong_to_string_ini, zend_libvirt_globals, libvirt_globals)
diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index ce885e5..39bd520 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -17,14 +17,6 @@
 #ifndef PHP_LIBVIRT_H
 #define PHP_LIBVIRT_H 1
 
-#define DEBUG_SUPPORT
-
-#ifdef DEBUG_SUPPORT
-#define DEBUG_CORE
-#define DEBUG_VNC
-#endif
-
-#define ARRAY_CARDINALITY(array)(sizeof(array) / sizeof(array[0]))
 
 /* Network constants */
 #define VIR_NETWORKS_ACTIVE 1
@@ -151,20 +143,6 @@ typedef struct tTokenizer {
 int numTokens;
 } tTokenizer;
 
-#define IS_BIGENDIAN (*(uint16_t *)"\0\xff" < 0x100)
-
-#define SWAP2_BY_ENDIAN(le, v1, v2) (((le && IS_BIGENDIAN) || (!le && 
!IS_BIGENDIAN)) ? ((v2 << 8) + v1) : ((v1 << 8) + v2))
-#define PUT2_BYTE_ENDIAN(le, val, v1, v2) { if ((le && IS_BIGENDIAN) || (!le 
&& !IS_BIGENDIAN)) { v2 = val >> 8; v1 = val % 256; } else { v1 = val >> 8; v2 
= val % 256; } }
-#define SWAP2_BYTES_ENDIAN(le, a, b) { if ((le && IS_BIGENDIAN) || (!le && 
!IS_BIGENDIAN)) { uint8_t _tmpval; _tmpval = a; a = b; b = _tmpval; } }
-
-#define UINT32STR(var, val) \
-var[0] = (val >> 24) & 0xff;\
-var[1] = (val >> 16) & 0xff;\
-var[2] = (val >>  8) & 0xff;\
-var[3] = (val  ) & 0xff;
-
-#define GETUINT32(var)  (uint32_t)(((uint32_t)var[0] << 24) + 
((uint32_t)var[1] << 16) + ((uint32_t)var[2] << 8) + ((uint32_t)var[3]))
-
 typedef struct _resource_info {
 int type;
 virConnectPtr conn;
@@ -172,20 +150,6 @@ typedef struct _resource_info {
 int overwrite;
 } resource_info;
 
-ZEND_BEGIN_MODULE_GLOBALS(libvirt)
-char *last_error;
-char *vnc_location;
-zend_bool longlong_to_string_ini;
-char *iso_path_ini;
-char *image_path_ini;
-zend_long max_connections_ini;
-#ifdef DEBUG_SUPPORT
-int debug;
-#endif
-resource_info *binding_resources;
-int binding_resources_count;
-ZEND_END_MODULE_GLOBALS(libvirt)
-
 #ifdef ZTS
 #define LIBVIRT_G(v) TSRMG(libvirt_globals_id, zend_libvirt_globals *, v)
 #else
@@ -324,9 +288,6 @@ int set_logfile(char *filename, long maxsize TSRMLS_DC);
 char *get_datetime(void);
 char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal);
 char **get_array_from_xpath(char *xml, char *xpath, int *num);
-#ifdef DEBUG_SUPPORT
-int gdebug;
-#endif
 
 #define PHP_LIBVIRT_CONNECTION_RES_NAME "Libvirt connection"
 #define PHP_LIBVIRT_DOMAIN_RES_NAME "Libvirt domain"
diff --git a/src/sockets.c b/src/sockets.c
index 71ceb7b..92ea373 100644
--- a/src/sockets.c
+++ b/src/sockets.c
@@ -8,6 +8,7 @@
  */
 
 #include "libvirt-php.h"
+#include "util.h"
 
 #ifdef DEBUG_SOCKETS
 #define DPRINTF(fmt, ...) \
diff --git a/src/util.h b/src/util.h
new file mode 100644
index 000..119d573
--- /dev/null
+++ b/src/util.h
@@ -0,0 +1,63 @@
+/*
+ * util.h: common, generic utility functions
+ *
+ * See COPYING for the license of this software
+ *
+ * Written by:
+ *  Michal Privoznik 

[libvirt] [libvirt-php][PATCH 08/11] src: Introduce sockets.h

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 src/Makefile.am   |  2 +-
 src/libvirt-php.c |  6 +-
 src/libvirt-php.h |  5 -
 src/sockets.c |  1 +
 src/sockets.h | 32 
 src/vncfunc.c |  1 +
 6 files changed, 36 insertions(+), 11 deletions(-)
 create mode 100644 src/sockets.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 5720ddd..bbee667 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -20,7 +20,7 @@ php_plugin_LTLIBRARIES = libvirt-php.la
 libvirt_php_la_SOURCES = \
util.c util.h   \
vncfunc.c vncfunc.h \
-   sockets.c \
+   sockets.c sockets.h \
libvirt-php.c libvirt-php.h
 libvirt_php_la_CFLAGS = \
$(AM_CFLAGS) \
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 01e9b57..faa7200 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -29,11 +29,7 @@
 #include "libvirt-php.h"
 #include "util.h"
 #include "vncfunc.h"
-
-#ifndef EXTWIN
-// From sockets.c
-int connect_socket(char *server, char *port, int keepalive, int nodelay, int 
allow_server_override);
-#endif
+#include "sockets.h"
 
 #ifdef DEBUG_SUPPORT
 int gdebug;
diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index 1225c0b..6b6df21 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -132,11 +132,6 @@ typedef long zend_long;
 typedef unsigned long zend_ulong;
 #endif /* PHP_MAJOR_VERSION < 7 */
 
-int connect_socket(char *server, char *port, int keepalive, int nodelay, int 
allow_server_override);
-int socket_has_data(int sfd, long maxtime, int ignoremsg);
-void socket_read(int sfd, long length);
-int socket_read_and_save(int sfd, char *fn, long length);
-
 typedef struct tTokenizer {
 char **tokens;
 int numTokens;
diff --git a/src/sockets.c b/src/sockets.c
index 92ea373..a175450 100644
--- a/src/sockets.c
+++ b/src/sockets.c
@@ -7,6 +7,7 @@
  *   Michal Novotny 
  */
 
+#include "sockets.h"
 #include "libvirt-php.h"
 #include "util.h"
 
diff --git a/src/sockets.h b/src/sockets.h
new file mode 100644
index 000..52b5866
--- /dev/null
+++ b/src/sockets.h
@@ -0,0 +1,32 @@
+/*
+ * sockets.h: Socket functions for libvirt-php
+ *
+ * See COPYING for the license of this software
+ *
+ * Written by:
+ *   Michal Novotny 
+ *   Michal Privoznik 
+ */
+
+#ifndef __SOCKETS_H__
+# define __SOCKETS_H__
+
+int connect_socket(char *server,
+   char *port,
+   int keepalive,
+   int nodelay,
+   int allow_server_override);
+
+int socket_has_data(int sfd,
+long maxtime,
+int ignoremsg);
+
+void socket_read(int sfd,
+ long length);
+
+int socket_read_and_save(int sfd,
+ char *fn,
+ long length);
+
+#endif /* __SOCKETS_H__ */
+
diff --git a/src/vncfunc.c b/src/vncfunc.c
index b1a994d..447881e 100644
--- a/src/vncfunc.c
+++ b/src/vncfunc.c
@@ -10,6 +10,7 @@
 #include "vncfunc.h"
 #include "libvirt-php.h"
 #include "util.h"
+#include "sockets.h"
 
 #ifdef DEBUG_VNC
 #define DPRINTF(fmt, ...) \
-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 11/11] Clean up debug macros

[Michal Privoznik]

Each single file in our source code redefines the same DPRINTF
macro. What we want instead is to have the macro shared and also
we might to avoid global variable shared across all the code.

Signed-off-by: Michal Privoznik 
---
 src/libvirt-php.c | 15 ++-
 src/sockets.c |  9 +
 src/util.c| 34 +-
 src/util.h| 12 ++--
 src/vncfunc.c |  9 +
 5 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index faa7200..e129481 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -31,18 +31,7 @@
 #include "vncfunc.h"
 #include "sockets.h"
 
-#ifdef DEBUG_SUPPORT
-int gdebug;
-#endif
-
-#ifdef DEBUG_CORE
-#define DPRINTF(fmt, ...) \
-if (LIBVIRT_G(debug)) \
-do { fprintf(stderr, "[%s ", get_datetime()); fprintf(stderr, 
"libvirt-php/core   ]: " fmt , ## __VA_ARGS__); fflush(stderr); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-do {} while(0)
-#endif
+DEBUG_INIT("core");
 
 /* PHP functions are prefixed with `zif_` so strip it */
 #define PHPFUNC (__FUNCTION__ + 4)
@@ -819,7 +808,7 @@ PHP_INI_END()
 void change_debug(int val TSRMLS_DC)
 {
 LIBVIRT_G(debug) = val;
-gdebug = val;
+setDebug(val);
 }
 
 /* PHP requires to have this function defined */
diff --git a/src/sockets.c b/src/sockets.c
index 6620e17..0a3e3c2 100644
--- a/src/sockets.c
+++ b/src/sockets.c
@@ -26,14 +26,7 @@
 #include "sockets.h"
 #include "util.h"
 
-#ifdef DEBUG_SOCKETS
-#define DPRINTF(fmt, ...) \
-if (gdebug) \
-do { fprintf(stderr, "[%s ", get_datetime()); fprintf(stderr, 
"libvirt-php/sockets]: " fmt , ## __VA_ARGS__); fflush(stderr); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-do {} while(0)
-#endif
+DEBUG_INIT("sockets");
 
 /* Function macro */
 #define PHPFUNC __FUNCTION__
diff --git a/src/util.c b/src/util.c
index 7d2b457..53096ae 100644
--- a/src/util.c
+++ b/src/util.c
@@ -9,11 +9,15 @@
 
 #include 
 
+#include 
+#include 
 #include 
 #include 
 
 #include "util.h"
 
+int gdebug;
+
 /*
  * Private function name:   get_datetime
  * Since version:   0.4.2
@@ -21,7 +25,8 @@
  * Arguments:   None
  * Returns: Date/time string in `-mm-dd HH:mm:ss` format
  */
-char *get_datetime(void)
+static char *
+get_datetime(void)
 {
 /* Caution: Function cannot use DPRINTF() macro otherwise the neverending 
loop will be met! */
 char *outstr = NULL;
@@ -39,3 +44,30 @@ char *get_datetime(void)
 
 return outstr;
 }
+
+void debugPrint(const char *source,
+const char *fmt, ...)
+{
+char *datetime;
+va_list args;
+
+if (!gdebug)
+return;
+
+datetime = get_datetime();
+fprintf(stderr, "[%s libvirt-php/%s ]: ", datetime, source);
+free(datetime);
+
+if (fmt) {
+va_start(args, fmt);
+vfprintf(stderr, fmt, args);
+va_end(args);
+}
+fprintf(stderr, "\n");
+fflush(stderr);
+}
+
+void setDebug(int level)
+{
+gdebug = level;
+}
diff --git a/src/util.h b/src/util.h
index e907a49..c2b7324 100644
--- a/src/util.h
+++ b/src/util.h
@@ -17,9 +17,14 @@
 # ifdef DEBUG_SUPPORT
 #  define DEBUG_CORE
 #  define DEBUG_VNC
-extern int gdebug;
 # endif
 
+# define DEBUG_INIT(source) \
+static const char *debugSource = "" source ""
+
+# define DPRINTF(fmt, ...)  \
+debugPrint(debugSource, fmt, __VA_ARGS__)
+
 # define ARRAY_CARDINALITY(array) (sizeof(array) / sizeof(array[0]))
 
 # define IS_BIGENDIAN (*(uint16_t *)"\0\xff" < 0x100)
@@ -60,6 +65,9 @@ extern int gdebug;
((uint32_t)var[2] <<  8) +   \
((uint32_t)var[3]))
 
-char *get_datetime(void);
+void debugPrint(const char *source,
+const char *fmt, ...);
+
+void setDebug(int level);
 
 #endif /* __UTIL_H__ */
diff --git a/src/vncfunc.c b/src/vncfunc.c
index cb98341..45f4007 100644
--- a/src/vncfunc.c
+++ b/src/vncfunc.c
@@ -23,14 +23,7 @@
 #include "util.h"
 #include "sockets.h"
 
-#ifdef DEBUG_VNC
-#define DPRINTF(fmt, ...) \
-if (gdebug) \
-do { fprintf(stderr, "[%s ", get_datetime()); fprintf(stderr, "libvirt-php/vnc 
   ]: " fmt , ## __VA_ARGS__); fflush(stderr); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-do {} while(0)
-#endif
+DEBUG_INIT("vncfunc");
 
 /* Function macro */
 #define PHPFUNC __FUNCTION__
-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 03/11] Move VNC_MAX_AUTH_ATTEMPTS to vncfunc.c

[Michal Privoznik]

There is no need to have this macro in the header file. It's used
in one place after all.

Signed-off-by: Michal Privoznik 
---
 src/libvirt-php.h | 3 ---
 src/vncfunc.c | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index 0cd160f..ce885e5 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -26,9 +26,6 @@
 
 #define ARRAY_CARDINALITY(array)(sizeof(array) / sizeof(array[0]))
 
-/* Maximum number of authentication attempts */
-#define VNC_MAX_AUTH_ATTEMPTS   10
-
 /* Network constants */
 #define VIR_NETWORKS_ACTIVE 1
 #define VIR_NETWORKS_INACTIVE   2
diff --git a/src/vncfunc.c b/src/vncfunc.c
index 17cf050..6e2f220 100644
--- a/src/vncfunc.c
+++ b/src/vncfunc.c
@@ -78,6 +78,9 @@ vnc_write_client_version(int sfd)
 return 0;
 }
 
+/* Maximum number of authentication attempts */
+# define VNC_MAX_AUTH_ATTEMPTS  10
+
 /*
  * Private function name:   vnc_authorize
  * Since version:   0.4.3
-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 09/11] vncfunc: Drop include of libvirt-php.h

[Michal Privoznik]

The point is, libvirt-php.c uses vnc functions implemented in
vncfunc.c, but for some weird historical reasons, it was
vncfunc.c who included libvirt-php.h.

Signed-off-by: Michal Privoznik 
---
 src/libvirt-php.h | 21 -
 src/vncfunc.c | 32 +++-
 2 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index 6b6df21..6b34581 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -194,27 +194,6 @@ typedef struct tVMNetwork {
 char *model;
 } tVMNetwork;
 
-#ifndef EXTWIN
-typedef struct tBMPFile {
-uint32_t filesz;
-uint16_t creator1;
-uint16_t creator2;
-uint32_t bmp_offset;
-
-uint32_t header_sz;
-int32_t height;
-int32_t width;
-uint16_t nplanes;
-uint16_t bitspp;
-uint32_t compress_type;
-uint32_t bmp_bytesz;
-int32_t hres;
-int32_t vres;
-uint32_t ncolors;
-uint32_t nimpcolors;
-} tBMPFile;
-#endif
-
 /* Libvirt-php types */
 typedef struct _php_libvirt_connection {
 virConnectPtr conn;
diff --git a/src/vncfunc.c b/src/vncfunc.c
index 447881e..cb98341 100644
--- a/src/vncfunc.c
+++ b/src/vncfunc.c
@@ -7,8 +7,19 @@
  *   Michal Novotny 
  */
 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
 #include "vncfunc.h"
-#include "libvirt-php.h"
 #include "util.h"
 #include "sockets.h"
 
@@ -44,6 +55,25 @@ typedef struct tServerFBParams {
 unsigned char *desktopName;
 } tServerFBParams;
 
+typedef struct tBMPFile {
+uint32_t filesz;
+uint16_t creator1;
+uint16_t creator2;
+uint32_t bmp_offset;
+
+uint32_t header_sz;
+int32_t height;
+int32_t width;
+uint16_t nplanes;
+uint16_t bitspp;
+uint32_t compress_type;
+uint32_t bmp_bytesz;
+int32_t hres;
+int32_t vres;
+uint32_t ncolors;
+uint32_t nimpcolors;
+} tBMPFile;
+
 /*
  * Private function name:   vnc_write_client_version
  * Since version:   0.4.3
-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 07/11] src: Introduce vncfunc.h

[Michal Privoznik]

Instead of forward declaration of some vnc_* functions from
vncfunc.c lets have a separate header file for that purpose.

Signed-off-by: Michal Privoznik 
---
 src/Makefile.am   |  2 +-
 src/libvirt-php.c |  3 +--
 src/libvirt-php.h |  5 -
 src/vncfunc.c |  1 +
 src/vncfunc.h | 38 ++
 5 files changed, 41 insertions(+), 8 deletions(-)
 create mode 100644 src/vncfunc.h

diff --git a/src/Makefile.am b/src/Makefile.am
index a12e729..5720ddd 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -19,7 +19,7 @@ php_plugindir = $(extensiondir)
 php_plugin_LTLIBRARIES = libvirt-php.la
 libvirt_php_la_SOURCES = \
util.c util.h   \
-   vncfunc.c \
+   vncfunc.c vncfunc.h \
sockets.c \
libvirt-php.c libvirt-php.h
 libvirt_php_la_CFLAGS = \
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 70405f2..01e9b57 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -28,10 +28,9 @@
 
 #include "libvirt-php.h"
 #include "util.h"
+#include "vncfunc.h"
 
 #ifndef EXTWIN
-// From vncfunc.c
-int vnc_get_dimensions(char *server, char *port, int *width, int *height);
 // From sockets.c
 int connect_socket(char *server, char *port, int keepalive, int nodelay, int 
allow_server_override);
 #endif
diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index 39bd520..1225c0b 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -136,7 +136,6 @@ int connect_socket(char *server, char *port, int keepalive, 
int nodelay, int all
 int socket_has_data(int sfd, long maxtime, int ignoremsg);
 void socket_read(int sfd, long length);
 int socket_read_and_save(int sfd, char *fn, long length);
-int vnc_get_bitmap(char *server, char *port, char *fn);
 
 typedef struct tTokenizer {
 char **tokens;
@@ -280,10 +279,6 @@ typedef struct _php_libvirt_hash_key_info {
 } php_libvirt_hash_key_info;
 
 /* Private definitions */
-int vnc_refresh_screen(char *server, char *port, int scancode);
-int vnc_send_keys(char *server, char *port, char *keys);
-int vnc_send_pointer_event(char *server, char *port, int pos_x, int pos_y, int 
clicked, int release);
-
 int set_logfile(char *filename, long maxsize TSRMLS_DC);
 char *get_datetime(void);
 char *get_string_from_xpath(char *xml, char *xpath, zval **val, int *retVal);
diff --git a/src/vncfunc.c b/src/vncfunc.c
index 393624f..b1a994d 100644
--- a/src/vncfunc.c
+++ b/src/vncfunc.c
@@ -7,6 +7,7 @@
  *   Michal Novotny 
  */
 
+#include "vncfunc.h"
 #include "libvirt-php.h"
 #include "util.h"
 
diff --git a/src/vncfunc.h b/src/vncfunc.h
new file mode 100644
index 000..4758f95
--- /dev/null
+++ b/src/vncfunc.h
@@ -0,0 +1,38 @@
+/*
+ * vncfunc.h: VNC Client functions to be used for the graphical VNC console of 
libvirt-php
+ *
+ * See COPYING for the license of this software
+ *
+ * Written by:
+ *   Michal Novotny 
+ *   Michal Privoznik 
+ */
+
+#ifndef __VNCFUNC_H__
+# define __VNCFUNC_H__
+
+int vnc_get_bitmap(char *server,
+   char *port,
+   char *fn);
+
+int vnc_get_dimensions(char *server,
+   char *port,
+   int *width,
+   int *height);
+
+int vnc_refresh_screen(char *server,
+   char *port,
+   int scancode);
+
+int vnc_send_keys(char *server,
+  char *port,
+  char *keys);
+
+int vnc_send_pointer_event(char *server,
+   char *port,
+   int pos_x,
+   int pos_y,
+   int clicked,
+   int release);
+
+#endif /* __VNCFUNC_H__ */
-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 01/11] vncfunc: Honour static function

[Michal Privoznik]

In order to bring some order to the source code, lets start with
marking static functions as static.

Signed-off-by: Michal Privoznik 
---
 src/vncfunc.c | 36 
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/src/vncfunc.c b/src/vncfunc.c
index 70eaa8b..17cf050 100644
--- a/src/vncfunc.c
+++ b/src/vncfunc.c
@@ -48,7 +48,8 @@ typedef struct tServerFBParams {
  * Arguments:   @sfd [int]: socket descriptor connected to the VNC 
server
  * Returns: 0 on success, -errno on error
  */
-int vnc_write_client_version(int sfd)
+static int
+vnc_write_client_version(int sfd)
 {
 unsigned char buf[12];
 
@@ -84,7 +85,8 @@ int vnc_write_client_version(int sfd)
  * Arguments:   @sfd [int]: socket descriptor connected to the VNC 
server
  * Returns: 0 on success, -errno on error (incl. -EIO on 
invalid authorization)
  */
-int vnc_authorize(int sfd)
+static int
+vnc_authorize(int sfd)
 {
 unsigned char buf[4] = { 0 };
 unsigned char buf2[32] = { 0 };
@@ -158,7 +160,8 @@ int vnc_authorize(int sfd)
  *  @len [int]: length of the buffer
  * Returns: parameters structure of tServerFBParams
  */
-tServerFBParams vnc_parse_fb_params(unsigned char *buf, int len)
+static tServerFBParams
+vnc_parse_fb_params(unsigned char *buf, int len)
 {
 int nlen, little_endian = 0;
 int w1, w2, h1, h2, h, w;
@@ -222,7 +225,8 @@ tServerFBParams vnc_parse_fb_params(unsigned char *buf, int 
len)
  *  @release [bool]: flag to release the key 
immediately
  * Returns: 0 on success, -errno otherwise
  */
-int vnc_send_key(int sfd, unsigned char key, int modifier, int release)
+static int
+vnc_send_key(int sfd, unsigned char key, int modifier, int release)
 {
 unsigned char buf[8];
 
@@ -262,7 +266,8 @@ int vnc_send_key(int sfd, unsigned char key, int modifier, 
int release)
  *  @pos_y [int]: Y position of mouse cursor
  * Returns: 0 on success, -errno otherwise
  */
-int vnc_send_client_pointer(int sfd, int clicked, int pos_x, int pos_y)
+static int
+vnc_send_client_pointer(int sfd, int clicked, int pos_x, int pos_y)
 {
 unsigned char buf[6] = { 0 };
 
@@ -299,7 +304,8 @@ int vnc_send_client_pointer(int sfd, int clicked, int 
pos_x, int pos_y)
  *  @params [struct]: structure of parameters to set 
the data
  * Returns: 0 on success, -errno otherwise
  */
-int vnc_set_pixel_format(int sfd, tServerFBParams params)
+static int
+vnc_set_pixel_format(int sfd, tServerFBParams params)
 {
 unsigned char buf[20];
 
@@ -355,7 +361,8 @@ int vnc_set_pixel_format(int sfd, tServerFBParams params)
  * Arguments:   @sfd [int]: socket descriptor for existing VNC 
client socket
  * Returns: 0 on success, -errno otherwise
  */
-int vnc_set_encoding(int sfd)
+static int
+vnc_set_encoding(int sfd)
 {
 unsigned char buf[8];
 
@@ -400,7 +407,8 @@ int vnc_set_encoding(int sfd)
  *  @h [int]: height of frame
  * Returns: 0 on success, -errno otherwise
  */
-int vnc_send_framebuffer_update(int sfd, int incrementalUpdate, int x, int y, 
int w, int h)
+static int
+vnc_send_framebuffer_update(int sfd, int incrementalUpdate, int x, int y, int 
w, int h)
 {
 unsigned char buf[10];
 
@@ -440,7 +448,8 @@ int vnc_send_framebuffer_update(int sfd, int 
incrementalUpdate, int x, int y, in
  *  @params [struct]: structure of parameters to 
request the update for full screen
  * Returns: 0 on success, -errno otherwise
  */
-int vnc_send_framebuffer_update_request(int sfd, int incrementalUpdate, 
tServerFBParams params)
+static int
+vnc_send_framebuffer_update_request(int sfd, int incrementalUpdate, 
tServerFBParams params)
 {
 return vnc_send_framebuffer_update(sfd, incrementalUpdate, 0, 0, 
params.width, params.height);
 }
@@ -454,7 +463,8 @@ int vnc_send_framebuffer_update_request(int sfd, int 
incrementalUpdate, tServerF
  *  @share [bool]: flag whether to share desktop or not
  * Returns: socket descriptor on success, -errno otherwise
  */
-int vnc_connect(char *server, char *port, int share)
+static int
+vnc_connect(char *server, char *port, int share)
 {
 int sfd, err;
 unsigned char buf[1024] = { 0 };
@@ -498,7 +508,8 @@ int vnc_connect(char *server, char *port, int share)
  * Arguments:   @sfd [int]: socket file descriptor acquired by 
vnc_connect() call
  * Returns: parameters block
  */
-tServerFBParams vnc_read_server_init(int sfd)
+static tServerFBParams
+vnc_read_server_init(int sfd)
 {
 unsigned char *buf = NULL;
 unsigned char tmpbuf[25] = { 0 };
@@ -597,7 +608,8 @@ int vnc_get_dimensions(char *server, char *port, int 
*width, int *height)
  *  

[libvirt] [libvirt-php][PATCH 02/11] Move DEFAULT_LOG_MAXSIZE to libvirt-php.c

[Michal Privoznik]

There is no need to have this macro in the header file. It's used
in one place after all.

Signed-off-by: Michal Privoznik 
---
 src/libvirt-php.c | 3 +++
 src/libvirt-php.h | 3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 693f911..951375f 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -1097,6 +1097,9 @@ int has_builtin(char *name)
 return 0;
 }
 
+/* Maximum size (in KiB) of log file when DEBUG_SUPPORT is enabled */
+#define DEFAULT_LOG_MAXSIZE 1024
+
 /* Information function for phpinfo() */
 PHP_MINFO_FUNCTION(libvirt)
 {
diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index a429ed8..0cd160f 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -29,9 +29,6 @@
 /* Maximum number of authentication attempts */
 #define VNC_MAX_AUTH_ATTEMPTS   10
 
-/* Maximum size (in KiB) of log file when DEBUG_SUPPORT is enabled */
-#define DEFAULT_LOG_MAXSIZE 1024
-
 /* Network constants */
 #define VIR_NETWORKS_ACTIVE 1
 #define VIR_NETWORKS_INACTIVE   2
-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 04/11] libvirt-php.c: Move system headers together

[Michal Privoznik]

And at the same time switch from "header.h" notation to
.

Signed-off-by: Michal Privoznik 
---
 configure.ac  | 1 +
 src/libvirt-php.c | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 8d52862..f232756 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4,6 +4,7 @@ AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([-Wall -Werror -Wno-portability])
 AM_MAINTAINER_MODE([enable])
+AC_GNU_SOURCE
 
 m4_ifndef([LT_INIT], [
   AM_PROG_LIBTOOL
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index 951375f..6b145de 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -18,14 +18,15 @@
 #define EXTWIN
 #endif
 
-#include "stdafx.h"
+#include 
+#include 
+#include 
 
 #ifdef EXTWIN
 #define PHP_COMPILER_ID  "VC9"
 #endif
 
 #include "libvirt-php.h"
-#include "stdio.h"
 
 #ifndef EXTWIN
 // From vncfunc.c
-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 00/11] Resolve reverse include order

[Michal Privoznik]

These are not pushed yet as they might be somewhat controversial.
I'll wait if somebody wants to review them.

The ultimate goal is to, well drop libvirt-php.h completely. It
is needless. But before going there we must distribute the
interesting parts from it around. Therefore I'm introducing some
modules (like it should have been done from the start).

Anyway, happy coding!

Michal Privoznik (11):
  vncfunc: Honour static function
  Move DEFAULT_LOG_MAXSIZE to libvirt-php.c
  Move VNC_MAX_AUTH_ATTEMPTS to vncfunc.c
  libvirt-php.c: Move system headers together
  src: Introduce util.h
  src: Introduce util.c
  src: Introduce vncfunc.h
  src: Introduce sockets.h
  vncfunc: Drop include of libvirt-php.h
  sockets: Drop include of libvirt-php.h
  Clean up debug macros

 configure.ac  |  1 +
 src/Makefile.am   |  5 ++--
 src/config.m4 |  2 +-
 src/libvirt-php.c | 69 +
 src/libvirt-php.h | 81 -
 src/sockets.c | 24 +++-
 src/sockets.h | 32 +
 src/util.c| 73 
 src/util.h| 73 
 src/vncfunc.c | 83 +--
 src/vncfunc.h | 38 +
 11 files changed, 325 insertions(+), 156 deletions(-)
 create mode 100644 src/sockets.h
 create mode 100644 src/util.c
 create mode 100644 src/util.h
 create mode 100644 src/vncfunc.h

-- 
2.8.4

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

[libvirt] [libvirt-php][PATCH 10/11] sockets: Drop include of libvirt-php.h

[Michal Privoznik]

Just like in the previous commit, socket is offering its
functions to the rest of the code and therefore it should not
include libvirt-php.h.

Signed-off-by: Michal Privoznik 
---
 src/libvirt-php.h |  5 -
 src/sockets.c | 17 -
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/libvirt-php.h b/src/libvirt-php.h
index 6b34581..9978e27 100644
--- a/src/libvirt-php.h
+++ b/src/libvirt-php.h
@@ -84,11 +84,6 @@
 #include 
 #include 
 
-#ifdef __APPLE__
-#include 
-#else
-#include 
-#endif
 #else
 
 #define PRIx32   "I32x"
diff --git a/src/sockets.c b/src/sockets.c
index a175450..6620e17 100644
--- a/src/sockets.c
+++ b/src/sockets.c
@@ -7,8 +7,23 @@
  *   Michal Novotny 
  */
 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#ifdef __APPLE__
+#include 
+#else
+#include 
+#endif
+
 #include "sockets.h"
-#include "libvirt-php.h"
 #include "util.h"
 
 #ifdef DEBUG_SOCKETS
-- 
2.8.4

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

[libvirt] [PATCH] mingw: Package cputypes.rng

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---

Pushed as trivial.

 mingw-libvirt.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 5c7183f..c8476d9 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -292,6 +292,7 @@ rm -rf 
$RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
 %dir %{mingw64_datadir}/libvirt/schemas/
 %{mingw64_datadir}/libvirt/schemas/basictypes.rng
 %{mingw64_datadir}/libvirt/schemas/capability.rng
+%{mingw64_datadir}/libvirt/schemas/cputypes.rng
 %{mingw64_datadir}/libvirt/schemas/domain.rng
 %{mingw64_datadir}/libvirt/schemas/domaincaps.rng
 %{mingw64_datadir}/libvirt/schemas/domaincommon.rng
-- 
2.10.0

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

Re: [libvirt] [PATCH] qemu: Fix crash in qemucapsprobe

[Martin Kletzander]

On Tue, Sep 27, 2016 at 01:47:10PM +0200, Jiri Denemark wrote:

The qemucapsprobe helper calls virQEMUCapsInitHostCPUModel with
caps == NULL, causing the following crash:

   Program received signal SIGSEGV, Segmentation fault.
   #0  0x7788775f in virQEMUCapsInitHostCPUModel
   (qemuCaps=qemuCaps@entry=0x649680, host=host@entry=0x10) at
   src/qemu/qemu_capabilities.c:2969
   #1  0x77889dbf in virQEMUCapsNewForBinaryInternal
   (caps=caps@entry=0x0, binary=,
   libDir=libDir@entry=0x4033f6 "/tmp", cacheDir=cacheDir@entry=0x0,
   runUid=runUid@entry=4294967295, runGid=runGid@entry=4294967295,
   qmpOnly=true) at src/qemu/qemu_capabilities.c:4039
   #2  0x00401702 in main (argc=2, argv=0x7fffd968) at
   tests/qemucapsprobe.c:73

Signed-off-by: Jiri Denemark 
---


Some would say that you should put the causing commit
(68c7011856ad0033dff55a1b2c7b9749ce98e439) in the message, but I'm not
one of them.  So just ACK.


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

[libvirt] [PATCH 8/8] qemu: Add support for hot/cold-(un)plug of shmem devices

[Martin Kletzander]

This is needed in order to migrate a domain with shmem devices as that
is not allowed to migrate.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_driver.c |  39 +++-
 src/qemu/qemu_hotplug.c| 247 -
 src/qemu/qemu_hotplug.h|   6 +
 tests/qemuhotplugtest.c|  21 ++
 .../qemuhotplug-ivshmem-doorbell-detach.xml|   7 +
 .../qemuhotplug-ivshmem-doorbell.xml   |   4 +
 .../qemuhotplug-ivshmem-plain-detach.xml   |   6 +
 .../qemuhotplug-ivshmem-plain.xml  |   3 +
 ...muhotplug-base-live+ivshmem-doorbell-detach.xml |   1 +
 .../qemuhotplug-base-live+ivshmem-doorbell.xml |  65 ++
 .../qemuhotplug-base-live+ivshmem-plain-detach.xml |   1 +
 .../qemuhotplug-base-live+ivshmem-plain.xml|  58 +
 12 files changed, 453 insertions(+), 5 deletions(-)
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-doorbell-detach.xml
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-doorbell.xml
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-plain-detach.xml
 create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-plain.xml
 create mode 12 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-doorbell-detach.xml
 create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-doorbell.xml
 create mode 12 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-plain-detach.xml
 create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-plain.xml

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7873a6814ee7..c78db761c112 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7528,6 +7528,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 dev->data.memory = NULL;
 break;

+case VIR_DOMAIN_DEVICE_SHMEM:
+ret = qemuDomainAttachShmemDevice(driver, vm,
+  dev->data.shmem);
+if (!ret) {
+alias = dev->data.shmem->info.alias;
+dev->data.shmem = NULL;
+}
+break;
+
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_FS:
 case VIR_DOMAIN_DEVICE_INPUT:
@@ -7539,7 +7548,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_SMARTCARD:
 case VIR_DOMAIN_DEVICE_MEMBALLOON:
 case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_SHMEM:
 case VIR_DOMAIN_DEVICE_TPM:
 case VIR_DOMAIN_DEVICE_PANIC:
 case VIR_DOMAIN_DEVICE_IOMMU:
@@ -7617,6 +7625,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_MEMORY:
 ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory);
 break;
+case VIR_DOMAIN_DEVICE_SHMEM:
+ret = qemuDomainDetachShmemDevice(driver, vm, dev);
+break;

 case VIR_DOMAIN_DEVICE_FS:
 case VIR_DOMAIN_DEVICE_INPUT:
@@ -7628,7 +7639,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_SMARTCARD:
 case VIR_DOMAIN_DEVICE_MEMBALLOON:
 case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_SHMEM:
 case VIR_DOMAIN_DEVICE_REDIRDEV:
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_TPM:
@@ -7775,6 +7785,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 virDomainControllerDefPtr controller;
 virDomainFSDefPtr fs;
 virDomainRedirdevDefPtr redirdev;
+virDomainShmemDefPtr shmem;

 switch ((virDomainDeviceType) dev->type) {
 case VIR_DOMAIN_DEVICE_DISK:
@@ -7899,6 +7910,18 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 dev->data.redirdev = NULL;
 break;

+case VIR_DOMAIN_DEVICE_SHMEM:
+shmem = dev->data.shmem;
+if (virDomainShmemDefFind(vmdef, shmem) >= 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("device is already in the domain configuration"));
+return -1;
+}
+if (virDomainShmemDefInsert(vmdef, shmem) < 0)
+return -1;
+dev->data.shmem = NULL;
+break;
+
 case VIR_DOMAIN_DEVICE_INPUT:
 case VIR_DOMAIN_DEVICE_SOUND:
 case VIR_DOMAIN_DEVICE_VIDEO:
@@ -7908,7 +7931,6 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 case VIR_DOMAIN_DEVICE_SMARTCARD:
 case VIR_DOMAIN_DEVICE_MEMBALLOON:
 case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_SHMEM:
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_TPM:
 case VIR_DOMAIN_DEVICE_PANIC:
@@ -8055,6 +8077,16 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
 virDomainRedirdevDefFree(virDomainRedirdevDefRemove(vmdef, idx));
 break;

+case VIR_DOMAIN_DEVICE_SHMEM:
+if ((idx = virDomainShmemDefFind(vmdef, dev->data.shmem)) < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, 

Re: [libvirt] [PATCH] qemu: Fix crash in qemucapsprobe

[John Ferlan]

On 09/27/2016 07:47 AM, Jiri Denemark wrote:
> The qemucapsprobe helper calls virQEMUCapsInitHostCPUModel with
> caps == NULL, causing the following crash:
> 
> Program received signal SIGSEGV, Segmentation fault.
> #0  0x7788775f in virQEMUCapsInitHostCPUModel
> (qemuCaps=qemuCaps@entry=0x649680, host=host@entry=0x10) at
> src/qemu/qemu_capabilities.c:2969
> #1  0x77889dbf in virQEMUCapsNewForBinaryInternal
> (caps=caps@entry=0x0, binary=,
> libDir=libDir@entry=0x4033f6 "/tmp", cacheDir=cacheDir@entry=0x0,
> runUid=runUid@entry=4294967295, runGid=runGid@entry=4294967295,
> qmpOnly=true) at src/qemu/qemu_capabilities.c:4039
> #2  0x00401702 in main (argc=2, argv=0x7fffd968) at
> tests/qemucapsprobe.c:73
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_capabilities.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Earlier in this function there's:

if (!cacheDir)
rv = 0;
else if ((rv = virQEMUCapsInitCached(caps, qemuCaps, cacheDir)) < 0)
goto error;


virQEMUCapsInitCached will call virQEMUCapsLoadCache which will call
virQEMUCapsInitHostCPUModel(qemuCaps, >host); as well.

So wouldn't it better to have virQEMUCapsInitHostCPUModel take "caps" as
the parameter and then return "silently" if !caps->host ?

John

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 4d859c4..73a7fd4 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -4036,7 +4036,8 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps,
>  virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0)
>  goto error;
>  
> -virQEMUCapsInitHostCPUModel(qemuCaps, >host);
> +if (caps)
> +virQEMUCapsInitHostCPUModel(qemuCaps, >host);
>  }
>  
>   cleanup:
> 

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

[libvirt] [PATCH 4/8] conf, qemu: Add newer shmem models

[Martin Kletzander]

The old ivshmem is deprecated in QEMU, so let's use the better
ivshmem-{plain,doorbell} variants instead.

Signed-off-by: Martin Kletzander 
---
 docs/schemas/domaincommon.rng | 2 ++
 src/conf/domain_conf.c| 4 +++-
 src/conf/domain_conf.h| 2 ++
 src/qemu/qemu_command.c   | 7 +++
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 43c8b6696ac7..5d591ccc6bdd 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3591,6 +3591,8 @@
 
   
 ivshmem
+ivshmem-plain
+ivshmem-doorbell
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ed9a86af0f4e..a94266603811 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -844,7 +844,9 @@ VIR_ENUM_IMPL(virDomainMemoryModel, 
VIR_DOMAIN_MEMORY_MODEL_LAST,
   "", "dimm")

 VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST,
-  "ivshmem")
+  "ivshmem",
+  "ivshmem-plain",
+  "ivshmem-doorbell")

 static virClassPtr virDomainObjClass;
 static virClassPtr virDomainXMLOptionClass;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4e74c5611b2e..9f38cea831e6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1556,6 +1556,8 @@ struct _virDomainNVRAMDef {

 typedef enum {
 VIR_DOMAIN_SHMEM_MODEL_IVSHMEM,
+VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN,
+VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL,

 VIR_DOMAIN_SHMEM_MODEL_LAST
 } virDomainShmemModel;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4e4a1c9cafe9..aa8cff80e8b1 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8517,6 +8517,13 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
 devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps);
 break;

+case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
+case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("%s device is not supported with this QEMU binary"),
+   virDomainShmemModelTypeToString(shmem->model));
+break;
+
 case VIR_DOMAIN_SHMEM_MODEL_LAST:
 break;
 }
-- 
2.10.0

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

[libvirt] [PATCH 3/8] conf, qemu: Add support for shmem model

[Martin Kletzander]

Just the default one now, new ones will be added in following commits.

Signed-off-by: Martin Kletzander 
---
 docs/schemas/domaincommon.rng |  9 +
 src/conf/domain_conf.c| 44 +--
 src/conf/domain_conf.h|  8 +
 src/libvirt_private.syms  |  2 ++
 src/qemu/qemu_command.c   | 11 +-
 tests/qemuxml2argvdata/qemuxml2argv-shmem.xml |  2 ++
 tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml |  8 +
 7 files changed, 72 insertions(+), 12 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 95c7882cb73d..43c8b6696ac7 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3587,6 +3587,15 @@
   
   
 
+  
+
+  
+ivshmem
+  
+
+  
+
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dbf6eca57153..ed9a86af0f4e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -843,6 +843,9 @@ VIR_ENUM_IMPL(virDomainBlockJob, 
VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
 VIR_ENUM_IMPL(virDomainMemoryModel, VIR_DOMAIN_MEMORY_MODEL_LAST,
   "", "dimm")

+VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST,
+  "ivshmem")
+
 static virClassPtr virDomainObjClass;
 static virClassPtr virDomainXMLOptionClass;
 static void virDomainObjDispose(void *obj);
@@ -12244,6 +12247,20 @@ virDomainShmemDefParseXML(xmlNodePtr node,

 ctxt->node = node;

+tmp = virXPathString("string(./model/@type)", ctxt);
+if (tmp) {
+/* If there's none, we will automatically have the first one
+ * (as default).  Unfortunately this has to be done for
+ * compatibility reasons. */
+if ((def->model = virDomainShmemModelTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Unknown shmem model type '%s'"), tmp);
+goto cleanup;
+}
+
+VIR_FREE(tmp);
+}
+
 if (!(def->name = virXMLPropString(node, "name"))) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("shmem element must contain 'name' attribute"));
@@ -14824,6 +14841,9 @@ virDomainShmemDefEquals(virDomainShmemDefPtr src,
 if (src->size != dst->size)
 return false;

+if (src->model != dst->model)
+return false;
+
 if (src->server.enabled != dst->server.enabled)
 return false;

@@ -18811,6 +18831,15 @@ 
virDomainShmemDefCheckABIStability(virDomainShmemDefPtr src,
 return false;
 }

+if (src->model != dst->model) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target shared memory model '%s' does not match "
+ "source model '%s'"),
+   virDomainShmemModelTypeToString(dst->model),
+   virDomainShmemModelTypeToString(src->model));
+return false;
+}
+
 if (src->size != dst->size) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target shared memory size '%llu' does not match "
@@ -21888,20 +21917,13 @@ virDomainShmemDefFormat(virBufferPtr buf,
 virDomainShmemDefPtr def,
 unsigned int flags)
 {
-virBufferEscapeString(buf, "name);
-
-if (!def->size &&
-!def->server.enabled &&
-!def->msi.enabled &&
-!virDomainDeviceInfoNeedsFormat(>info, flags)) {
-virBufferAddLit(buf, "/>\n");
-return 0;
-} else {
-virBufferAddLit(buf, ">\n");
-}
+virBufferEscapeString(buf, "\n", def->name);

 virBufferAdjustIndent(buf, 2);

+virBufferAsprintf(buf, "\n",
+  virDomainShmemModelTypeToString(def->model));
+
 if (def->size)
 virBufferAsprintf(buf, "%llu\n", def->size >> 
20);

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d2065cf3edea..4e74c5611b2e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1554,9 +1554,16 @@ struct _virDomainNVRAMDef {
 virDomainDeviceInfo info;
 };

+typedef enum {
+VIR_DOMAIN_SHMEM_MODEL_IVSHMEM,
+
+VIR_DOMAIN_SHMEM_MODEL_LAST
+} virDomainShmemModel;
+
 struct _virDomainShmemDef {
 char *name;
 unsigned long long size;
+int model; /* enum virDomainShmemModel */
 struct {
 bool enabled;
 virDomainChrSourceDef chr;
@@ -3074,6 +3081,7 @@ VIR_ENUM_DECL(virDomainTPMBackend)
 VIR_ENUM_DECL(virDomainMemoryModel)
 VIR_ENUM_DECL(virDomainMemoryBackingModel)
 VIR_ENUM_DECL(virDomainIOMMUModel)
+VIR_ENUM_DECL(virDomainShmemModel)
 /* from libvirt.h */
 VIR_ENUM_DECL(virDomainState)
 VIR_ENUM_DECL(virDomainNostateReason)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 

[libvirt] [PATCH 2/8] qemu: Disable migration with ivshmem

[Martin Kletzander]

It was never safe anyway and as such shouldn't have been enabled in the
first place.  Future patches will allow hot-(un)pluging of some ivshmem
devices as a workaround.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_migration.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index e2ca3303efd3..0d7fec8360f3 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2346,6 +2346,12 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver,
 return false;
 }
 }
+
+if (vm->def->nshmems) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("Migration with shmem device is not supported"));
+return false;
+}
 }

 return true;
-- 
2.10.0

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

[libvirt] [PATCH 5/8] qemu: Add capabilities for ivshmem-{plain, doorbell}

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_capabilities.c| 4 
 src/qemu/qemu_capabilities.h| 2 ++
 tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 2 ++
 tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 2 ++
 tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml   | 2 ++
 tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml| 2 ++
 tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml| 2 ++
 7 files changed, 16 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4d859c4dacac..b1d56cebd419 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -344,6 +344,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "query-hotpluggable-cpus",

   "virtio-net.rx_queue_size", /* 235 */
+  "ivshmem-plain",
+  "ivshmem-doorbell",
 );


@@ -1587,6 +1589,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "pxb-pcie", QEMU_CAPS_DEVICE_PXB_PCIE },
 { "tls-creds-x509", QEMU_CAPS_OBJECT_TLS_CREDS_X509 },
 { "intel-iommu", QEMU_CAPS_DEVICE_INTEL_IOMMU },
+{ "ivshmem-plain", QEMU_CAPS_DEVICE_IVSHMEM_PLAIN },
+{ "ivshmem-doorbell", QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL },
 };

 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index ba0ef4859d42..f2c356b0cef3 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -378,6 +378,8 @@ typedef enum {

 /* 235 */
 QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */
+QEMU_CAPS_DEVICE_IVSHMEM_PLAIN, /* -device ivshmem-plain */
+QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL, /* -device ivshmem-doorbell */

 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
index fd14665d5397..0644718ee081 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
@@ -159,6 +159,8 @@
   
   
   
+  
+  
   2005094
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
index eb708f8a983a..9526ae8b70b9 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
@@ -159,6 +159,8 @@
   
   
   
+  
+  
   2005094
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
index 482b3849b50f..f97d2b05ffa7 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
@@ -153,6 +153,8 @@
   
   
   
+  
+  
   2005094
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
index 60f1fcfe85d5..845a98bafe30 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
@@ -196,6 +196,8 @@
   
   
   
+  
+  
   2006000
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
index 98f37622bff0..cb6a55a9ec0c 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
@@ -197,6 +197,8 @@
   
   
   
+  
+  
   2007000
   0
(v2.7.0)
-- 
2.10.0

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

[libvirt] [PATCH 1/8] conf: Fix virDomainShmemDefFind

[Martin Kletzander]

Due to the switch of parameters in a call to virDomainShmemDefEquals()
no device was found when looking for device with all the information
except address.  Also fix the indentation.

Signed-off-by: Martin Kletzander 
---
 src/conf/domain_conf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a06da46fabe4..dbf6eca57153 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14858,8 +14858,8 @@ virDomainShmemDefFind(virDomainDefPtr def,
 size_t i;

 for (i = 0; i < def->nshmems; i++) {
- if (virDomainShmemDefEquals(def->shmems[i], shmem))
- break;
+if (virDomainShmemDefEquals(shmem, def->shmems[i]))
+break;
 }

 if (i < def->nshmems)
-- 
2.10.0

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

[libvirt] [PATCH 6/8] qemu: Save various defaults for shmem

[Martin Kletzander]

We're keeping some things at default and that's not something we want to
do intentionaly.  Let's save some sensible defaults upfront then having
problems later.  The details for the defaults (of the newer
implementation) can be found in qemu's commit 5400c02b90bb:

  http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_domain.c| 49 ++-
 tests/qemuxml2argvdata/qemuxml2argv-shmem.args|  2 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml |  1 +
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9b1a32ec3897..83b1f98d9a43 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2551,12 +2551,55 @@ qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
 STRPREFIX(chr->source.data.nix.path, cfg->channelTargetDir)) {
 VIR_FREE(chr->source.data.nix.path);
 }
-
 virObjectUnref(cfg);
 }


 static int
+qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm)
+{
+if (!shm->size)
+shm->size = 4 << 20;
+
+/* Nothing more to check/change for IVSHMEM */
+if (shm->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM)
+return 0;
+
+if (!shm->server.enabled) {
+if (shm->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("shmem model '%s' is supported "
+ "only with server option enabled"),
+   virDomainShmemModelTypeToString(shm->model));
+return -1;
+}
+
+if (shm->msi.enabled) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("shmem model '%s' doesn't support "
+ "msi"),
+   virDomainShmemModelTypeToString(shm->model));
+}
+} else {
+if (shm->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("shmem model '%s' is supported "
+ "only with server option disabled"),
+   virDomainShmemModelTypeToString(shm->model));
+return -1;
+}
+
+shm->size = 0;
+shm->msi.enabled = true;
+if (!shm->msi.ioeventfd)
+shm->msi.ioeventfd = VIR_TRISTATE_SWITCH_ON;
+}
+
+return 0;
+}
+
+
+static int
 qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
  const virDomainDef *def,
  virCapsPtr caps,
@@ -2760,6 +2803,10 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 }
 }

+if (dev->type == VIR_DOMAIN_DEVICE_SHMEM &&
+qemuDomainShmemDefPostParse(dev->data.shmem) < 0)
+goto cleanup;
+
 ret = 0;

  cleanup:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args 
b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args
index 99fac119b04c..bdf660a3c435 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args
@@ -17,7 +17,7 @@ QEMU_AUDIO_DRV=none \
 -no-acpi \
 -boot c \
 -usb \
--device ivshmem,id=shmem0,shm=shmem0,bus=pci.0,addr=0x3 \
+-device ivshmem,id=shmem0,size=4m,shm=shmem0,bus=pci.0,addr=0x3 \
 -device ivshmem,id=shmem1,size=128m,shm=shmem1,bus=pci.0,addr=0x5 \
 -device ivshmem,id=shmem2,size=256m,shm=shmem2,bus=pci.0,addr=0x4 \
 -device ivshmem,id=shmem3,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml
index 5602913648bc..04b463a27892 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml
@@ -23,6 +23,7 @@
 
 
   
+  4
   
 
 
-- 
2.10.0

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

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

[Martin Kletzander]

QEMU added support for ivshmem-plain and ivshmem-doorbell.  Those are
reworked varians of legacy ivshmem that are compatible from the guest
POV, but not from host's POV and have sane specification and handling.

Details about the newer device type can be found in qemu's commit
5400c02b90bb:

  http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_command.c| 88 +-
 src/qemu/qemu_command.h| 10 +++
 .../qemuxml2argv-shmem-plain-doorbell.args | 43 +++
 .../qemuxml2argv-shmem-plain-doorbell.xml  | 63 
 tests/qemuxml2argvtest.c   |  3 +
 5 files changed, 204 insertions(+), 3 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aa8cff80e8b1..29f7130ef911 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8456,6 +8456,39 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def,
 return NULL;
 }

+char *
+qemuBuildShmemDevStr(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ virQEMUCapsPtr qemuCaps)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+virBufferAsprintf(, "%s", 
virDomainShmemModelTypeToString(shmem->model));
+virBufferAsprintf(, ",id=%s", shmem->info.alias);
+
+if (shmem->server.enabled)
+virBufferAsprintf(, ",chardev=char%s", shmem->info.alias);
+else
+virBufferAsprintf(, ",memdev=shmmem-%s", shmem->info.alias);
+
+if (shmem->msi.vectors)
+virBufferAsprintf(, ",vectors=%u", shmem->msi.vectors);
+if (shmem->msi.ioeventfd) {
+virBufferAsprintf(, ",ioeventfd=%s",
+  virTristateSwitchTypeToString(shmem->msi.ioeventfd));
+}
+
+if (qemuBuildDeviceAddressStr(, def, >info, qemuCaps) < 0) {
+virBufferFreeAndReset();
+return NULL;
+}
+
+if (virBufferCheckError() < 0)
+return NULL;
+
+return virBufferContentAndReset();
+}
+
 static char *
 qemuBuildShmemBackendChrStr(virLogManagerPtr logManager,
 virCommandPtr cmd,
@@ -8476,6 +8509,50 @@ qemuBuildShmemBackendChrStr(virLogManagerPtr logManager,
 return devstr;
 }

+
+virJSONValuePtr
+qemuBuildShmemBackendMemProps(virDomainShmemDefPtr shmem)
+{
+char *mem_path = NULL;
+virJSONValuePtr ret = NULL;
+
+if (virAsprintf(_path, "/dev/shm/%s", shmem->name) < 0)
+return NULL;
+
+virJSONValueObjectCreate(,
+ "s:mem-path", mem_path,
+ "U:size", shmem->size,
+ NULL);
+
+VIR_FREE(mem_path);
+return ret;
+}
+
+
+static char *
+qemuBuildShmemBackendMemStr(virDomainShmemDefPtr shmem)
+{
+char *ret = NULL;
+char *alias = NULL;
+virJSONValuePtr props = qemuBuildShmemBackendMemProps(shmem);
+
+if (!props)
+return NULL;
+
+if (virAsprintf(, "shmmem-%s", shmem->info.alias) < 0)
+goto cleanup;
+
+ret = virQEMUBuildObjectCommandlineFromJSON("memory-backend-file",
+alias,
+props);
+ cleanup:
+VIR_FREE(alias);
+virJSONValueFree(props);
+
+return ret;
+}
+
+
 static int
 qemuBuildShmemCommandLine(virLogManagerPtr logManager,
   virCommandPtr cmd,
@@ -8518,10 +8595,15 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
 break;

 case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
+if (!(devstr = qemuBuildShmemBackendMemStr(shmem)))
+return -1;
+
+virCommandAddArgList(cmd, "-object", devstr, NULL);
+VIR_FREE(devstr);
+
+/* fall-through */
 case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("%s device is not supported with this QEMU binary"),
-   virDomainShmemModelTypeToString(shmem->model));
+devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps);
 break;

 case VIR_DOMAIN_SHMEM_MODEL_LAST:
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2f2a6ff877e7..facc833bf886 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -186,4 +186,14 @@ bool qemuCheckCCWS390AddressSupport(const virDomainDef 
*def,
 virJSONValuePtr qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu)
 ATTRIBUTE_NONNULL(1);

+virJSONValuePtr qemuBuildShmemBackendMemProps(virDomainShmemDefPtr shmem)
+ATTRIBUTE_NONNULL(1);
+
+char *qemuBuildShmemDevStr(virDomainDefPtr def,
+   virDomainShmemDefPtr shmem,
+   virQEMUCapsPtr qemuCaps)
+ATTRIBUTE_NONNULL(1) 

[libvirt] [PATCH] qemu: Fix crash in qemucapsprobe

[Jiri Denemark]

The qemucapsprobe helper calls virQEMUCapsInitHostCPUModel with
caps == NULL, causing the following crash:

Program received signal SIGSEGV, Segmentation fault.
#0  0x7788775f in virQEMUCapsInitHostCPUModel
(qemuCaps=qemuCaps@entry=0x649680, host=host@entry=0x10) at
src/qemu/qemu_capabilities.c:2969
#1  0x77889dbf in virQEMUCapsNewForBinaryInternal
(caps=caps@entry=0x0, binary=,
libDir=libDir@entry=0x4033f6 "/tmp", cacheDir=cacheDir@entry=0x0,
runUid=runUid@entry=4294967295, runGid=runGid@entry=4294967295,
qmpOnly=true) at src/qemu/qemu_capabilities.c:4039
#2  0x00401702 in main (argc=2, argv=0x7fffd968) at
tests/qemucapsprobe.c:73

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_capabilities.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4d859c4..73a7fd4 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4036,7 +4036,8 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps,
 virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0)
 goto error;
 
-virQEMUCapsInitHostCPUModel(qemuCaps, >host);
+if (caps)
+virQEMUCapsInitHostCPUModel(qemuCaps, >host);
 }
 
  cleanup:
-- 
2.10.0

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

[libvirt] [PATCH] spec: Package cputypes.rng

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---

Pushed as trivial.

 libvirt.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index a389e88..00b95b8 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1814,6 +1814,7 @@ exit 0
 
 %{_datadir}/libvirt/schemas/basictypes.rng
 %{_datadir}/libvirt/schemas/capability.rng
+%{_datadir}/libvirt/schemas/cputypes.rng
 %{_datadir}/libvirt/schemas/domain.rng
 %{_datadir}/libvirt/schemas/domaincaps.rng
 %{_datadir}/libvirt/schemas/domaincommon.rng
-- 
2.10.0

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

Re: [libvirt] [PATCH] qemu: make qemuGetCompressionProgram return int not an enum

[Martin Kletzander]

On Tue, Sep 27, 2016 at 09:51:41AM +0100, Daniel P. Berrange wrote:

enum types are unsigned and the qemuGetCompressionProgram
function can return -1 on error. It is therefore inappropriate
to return an enum type. This fixes a build error where the
internal 'ret' variable was used in a comparison with -1

../../src/qemu/qemu_driver.c: In function 'qemuGetCompressionProgram':
../../src/qemu/qemu_driver.c:3280:5: error: comparison of unsigned expression < 
0 is always false [-Werror=type-limits]
../../src/qemu/qemu_driver.c:3289:5: error: comparison of unsigned expression < 
0 is always false [-Werror=type-limits]
cc1: all warnings being treated as errors

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

Pushed as a CI broken build fix.



I wanted to push this with one slight modification, basically mentioning
in the comment above the function that it can also return -1.  But
that's fine.

Ideally, I felt like doCoreDump() shouldn't call this function because
just for this one caller it adds unnecessary error handling which no
other caller uses, but since that was the reason for the broken commits,
I didn't want a long debate on that just prolonging the fix.  And the
long rant mail about that series is safely stored in drafts while my
rage is cooling down slowly.

Have a nice day,
Martin


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

[libvirt] """virsh schedinfo [--weight number] domain""" doesn't work with virsh(libxl) for managing Xen domains.

[Volo M.]

Hi Devs,

We're using Centos 7 Xen hypervisors and try to use Libvirt(libxl) for
managing Xen guest domains.
We noticed that we can't set cpu weight values for Xen domains with libvirt
(tested for libvirt v.1.3 and v.2.2) even though its possible to do with XL
toolkit.
[root@hv1xen ~]# virsh schedinfo --domain rtp6a88apsm3or --cap=5
Scheduler  : credit
weight : 1000
[root@hv1xen ~]# virsh schedinfo --domain rtp6a88apsm3or --weight=50
Scheduler  : credit
error: invalid scheduler option: weight

We noticed in documentation for 'virsh' like  this
https://linux.die.net/man/1/virsh that the cpu weight option is going to be
deprecated:
*"""Note*: The weight and cap parameters are defined only for the XEN_CREDIT
scheduler and are now *DEPRECATED* . """

Can you please clarify why it's been deprecated and what should be used
instead for scheduling and limitation of CPU for Xen domains with Libvirt?
We like Libvirt very much and don't want to apply some awkward workarounds
like simultaneous using Libvirt and XL toolkit.
Any suggestion here?

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

Re: [libvirt] [PATCH 2/3] qemu: special error code in case of no job on cancel block job

[Nikolay Shirokovskiy]

On 26.09.2016 23:07, John Ferlan wrote:
> 
> 
> On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote:
>> Special error code helps gracefully handle race conditions on
>> blockjob cancelling. Consider for example pull backup. We want
>> to stop it and in the process we cancel blockjob for some of the
>> exported disks. But at the time this command reaches qemu blockjob
>> fail for some reason and cancel result and result of stopping
>> will be an error with quite unexpecte message - no such job (sort of).
>> Instead we can detect this race and treat as correct cancelling
>> and wait until fail event will arrive. Then we can report corect
>> error message with some details from qemu probably.
>>
>> Users of domainBlockJobAbort can be affected as -2 is new possible
>> return code. Not sure if I should stick to the original behaviour in
>> this case.
>> ---
>>  src/qemu/qemu_monitor.c  |  5 +
>>  src/qemu/qemu_monitor_json.c | 11 +++
>>  2 files changed, 12 insertions(+), 4 deletions(-)
>>
> 
> I haven't looked forward to patch 3/3 yet, but perhaps it'd be better in
> the "caller" that cares to :
> 
> if (qemu*() < 0) {
> virErrorPtr err = virGetLastError();
> if (err && err->code == VIR_ERR_OPERATION_INVALID) {
> /* Do something specific */
> }
> }
> 
> Returning -2 alters virDomainBlockJobAbort expected return values and
> well that's probably not a good idea since we don't know if some caller
> has said (if virDomainBlockJobAbort() == -1) instead of <= -1
> 

It is common place in libvirt to have special error codes instead of 0/-1
only (qemuMonitorJSONCheckCPUx86) and it feels bulky to have checks that
involve virGetLastError. It looks like other places that use similar 
construction do not have other choice because of rpc call or smth.

I think when function spawns error when it comes to error code
value is does not take much care as they are not really semantic
(at least such common error as invalid operation) 
so one day the caller can catch 'operation invalid' for different
reason from different place on the stack.

As to virDomainBlockJobAbort it is easy to patch)

I think I should also remove reporting error from qemuMonitorJSONBlockJobError 
in case of DeviceNotActive however it is used from several places even
like setting speed. It is legacy of commit b976165c when
a lot of qemu commands share common error checking code. I think I'd
better copy this code to qemuMonitorJSONBlockJobCancel and change it there
instead of common place. What do you think?

Nikolay

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

[libvirt] [PATCH] qemu: make qemuGetCompressionProgram return int not an enum

[Daniel P. Berrange]

enum types are unsigned and the qemuGetCompressionProgram
function can return -1 on error. It is therefore inappropriate
to return an enum type. This fixes a build error where the
internal 'ret' variable was used in a comparison with -1

../../src/qemu/qemu_driver.c: In function 'qemuGetCompressionProgram':
../../src/qemu/qemu_driver.c:3280:5: error: comparison of unsigned expression < 
0 is always false [-Werror=type-limits]
../../src/qemu/qemu_driver.c:3289:5: error: comparison of unsigned expression < 
0 is always false [-Werror=type-limits]
cc1: all warnings being treated as errors

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

Pushed as a CI broken build fix.

 src/qemu/qemu_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index eaea96f..7873a68 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3264,13 +3264,13 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
  *   no there was an error, then just return RAW
  *   indicating none.
  */
-static virQEMUSaveFormat ATTRIBUTE_NONNULL(2)
+static int ATTRIBUTE_NONNULL(2)
 qemuGetCompressionProgram(const char *imageFormat,
   char **compresspath,
   const char *styleFormat,
   bool use_raw_on_fail)
 {
-virQEMUSaveFormat ret;
+int ret;
 
 *compresspath = NULL;
 
-- 
2.7.4

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

Re: [libvirt] [PATCH 1/3] qemu: store guest visible disk size from qemu monitor block info

[Nikolay Shirokovskiy]

On 26.09.2016 23:07, John Ferlan wrote:
> 
> 
> On 09/07/2016 05:24 AM, Nikolay Shirokovskiy wrote:
>> ---
>>  src/qemu/qemu_domain.h   |  1 +
>>  src/qemu/qemu_monitor_json.c | 17 +
>>  tests/qemumonitorjsontest.c  | 36 
>>  3 files changed, 54 insertions(+)
>>
>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>> index 13c0372..ea57111 100644
>> --- a/src/qemu/qemu_domain.h
>> +++ b/src/qemu/qemu_domain.h
>> @@ -337,6 +337,7 @@ struct qemuDomainDiskInfo {
>>  bool tray_open;
>>  bool empty;
>>  int io_status;
>> +unsigned long long guest_size;
> 
> a/k/a: 'virtual-size' in qemu and 'capacity' in libvirt
> 
> I probably would stick with capacity or something that says "disk"
> rather than "guest".
> 
>>  };
>>  
> 
> Trying to determine/decide whether this patch should be "separated" and
> perhaps made usable with/for the existing callers or whether patch 3
> should use the block stats (qemuDomainGetStatsBlock) which already
> handles some details that could be missing here (at least w/r/t backing
> chains).

I donno, does backing chain changes anything in this case? I need virtual
size of top image only and anyway is it possible for images of backing
chain to have different virtual sizes? It does not make much sense.

qemuDomainGetStatsBlock (and more specifically 
qemuMonitorBlockStatsUpdateCapacity
which is more suited to get just size) has rather inconviniet arguments
more suited for many disks requests.

> 
> Another option is adjusting the qemuMonitorJSONDiskNameLookup code to be
> more multi-purpose since it's using the "query-block" for
> *BlockPullCommon (from Rebase and Pull) as well as from *BlockCommit and
> it already checks/expects both "inserted" and "image" to be present in
> order to return that name.

I would not make function with such specific name provide additionally size 
info...
I think it is perfectily fine to have many monitor commands that internally
use same 'query-block' qemu command because this command is really profound)

> 
> 
>>  typedef struct _qemuDomainHostdevPrivate qemuDomainHostdevPrivate;
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 3d0a120..7903b47 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -1800,6 +1800,8 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
>>  
>>  for (i = 0; i < virJSONValueArraySize(devices); i++) {
>>  virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
>> +virJSONValuePtr inserted;
>> +virJSONValuePtr image;
>>  struct qemuDomainDiskInfo *info;
>>  const char *thisdev;
>>  const char *status;
>> @@ -1854,6 +1856,21 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
>>  if (info->io_status < 0)
>>  goto cleanup;
>>  }
>> +
>> +if ((inserted = virJSONValueObjectGetObject(dev, "inserted"))) {
>> +if (!(image = virJSONValueObjectGetObject(inserted, "image"))) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +   _("cannot read 'inserted/image' value"));
>> +goto cleanup;
>> +}
> 
> Other code that checks "inserted" and "image" doesn't fail - it just
> ignores. If there's only going to be one consumer, then I don't believe
> we want a failure such as this to affect other callers that may not
> care. That could mean having some sort of "marker" in the returned
> buffer that the fetch of "virtual-size" did occur (or just using not
> zero). It would be a shame to have some unexpected failures for a field
> that nothing else uses especially since the *GetBlockInfo is being used
> during process launch and reconnect (via qemuProcessRefreshDisks).
> 

I doubt there can be 'inserted' without 'image', it doesn't make sense,
inserted what?)) I guess qemuDomainGetStatsBlock ignores missing 'image'
because it can afford it, while qemuMonitorJSONDiskNameLookup does not
really ignores, if name is not found then function return error eventually.
So if this is true, there should be no 'inserted' without 'image', then shame 
will go
to qemu)

> Futhermore, what if there's a "backing-image" for "this" particular
> path? Will that be important for the pull backups support? Without
> looking at patch 3 I would think so...

Not really. Backing chain has nothing to do with backup, there is just no
difference from backup POV.


Nikolay

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

Re: [libvirt] Cpu Modeling

[David Hildenbrand]

CCing Christian

> On Thu, Sep 22, 2016 at 14:47:36 -0400, Jason J. Herne wrote:
> > Testing for runability:
> > - Simply try to start QEMU with KVM, compat machine, CPU model  
> 
> Yes, if the domain XML explicitly requests a specific CPU model.
> Additionally, we need to make sure a CPU model chosen by libvirt
> (host-model) is runnable, but that should be handled by
> query-cpu-model-expansion.
> 
> > Identifying host model, e.g. "virsh capabilities"
> > - query-cpu-model-expansion on "host" with "-M none --enable-kvm"  
> 
> Right, except that it doesn't seem to work on x86_64 now.
> 
> It's not critical for query-cpu-model-expansion, but do we have an
> interface we can use to check whether the new commands are supported by
> a QEMU binary? Trying and checking for
> 
> {"error": {"class": "GenericError", "desc": "this feature or command
> is not currently supported"}}
> 
> is not really possible. At least we'd need a specific error class.
> 
> > :
> > - simply copy the identified host model  
> 
> Yes.
> 
> > :
> > - "-cpu host"  
> 
> Exactly.
> 
> ...
> > 1. We will invoke qemu to gather the host cpu data used for virsh
> > capabilities. Today this data seems to be collected directly from the
> > host hardware for most (all?) architectures.  
> 
> Not really, virsh capabilities will still contain data gathered directly
> from the host hardware. But, we have virsh domcapabilities for
> displaying data tight to a specific QEMU binary. Since yesterday
> afternoon we have support for displaying CPU related data in the domain
> capabilities XML; see
> http://libvirt.org/formatdomaincaps.html#elementsCPU
> 
> The host-model part of the XML will show the result of
> query-cpu-model-expansion on "host" model, or the result of querying the
> hardware directly if we can't ask QEMU for that (which is the current
> state).
> 
> > 2. virsh cpu-models {arch} will also use a Qemu invocation to gather
> > cpu model data.  
> 
> No, virsh cpu-models will just list CPU models supported by libvirt (or
> an empty list if libvirt supports all models supported by QEMU). The CPU
> model data from QEMU can be found in domain capabilities XML.
> 
> > 3. Most architectures seem to use a "map" (xml file containing cpu
> > model data that ships with Libvirt) to satisfy #1 and #2 above. Our
> > new method does not use this map as it gets all of the data directly
> > from Qemu.  
> 
> Even if we switch some CPU operations to work through QEMU, we may still
> need to use the cpu_map.xml file for some other operations, or other
> hypervisor driver. It all depends on what we need to do for a given
> architecture. For example, ARM does not use cpu_map.xml at all even now.
> 
> Slightly related, I don't think we have a way to list CPU features
> supported by QEMU over QMP, do we? "-cpu ?" will show them all, but I
> couldn't find a QMP command that would give me the same list.
> 
> > 4. virsh cpu-baseline and cpu-compare will now invoke qemu directly as
> > well.  
> 
> Perhaps, but before we can do that, we'll probably need to come up with
> new APIs. It don't think it's critical, though.
> 
> > Note: I'm not sure exactly how much all of this will apply just to
> > s390 with other architectures moving over to the new interface if/when
> > they want to. Or if we will want to change all architectures to this
> > new interface at the same time.  
> 
> Well, IIUC the new commands are not supported on any other architecture
> right now, are they? Anyway, I don't think we need to switch everything
> at the same time. If we have a way to detect what commands are supported
> for a specific QEMU binary, we can implement the code in libvirt and use
> when we can or falling back to the current way.
> 
> Jirka
> 



David

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

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

[Joao Martins]

On 09/27/2016 05:05 AM, Jim Fehlig wrote:
> On 09/26/2016 11:33 AM, Joao Martins wrote:
>> So far only guestfwd and virtio were supported. Add an additional
>> for Xen as libxl channels create Xen console visible to the guest.
>>
>> Signed-off-by: Joao Martins 
>> ---
>> Changes since v2:
>>  * Add relevant documentation about target type xen.
>> ---
>>  docs/formatdomain.html.in | 10 ++
>>  docs/schemas/domaincommon.rng | 11 +++
>>  src/conf/domain_conf.c| 18 ++
>>  src/conf/domain_conf.h|  1 +
>>  src/qemu/qemu_command.c   |  1 +
>>  5 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index f48a4d8..129ba62 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -5980,6 +5980,16 @@ qemu-kvm -net nic,model=? /dev/null
>>  Possible values for the state attribute are
>>  connected and disconnected.
>>
>> +  xen
>> +   Paravirtualized xen channel. Channel is exposed in the guest as a
>> +xen console but identified with a name. The setup of the channel
>> +depends to guest own rules and can live in a arbitrary path (for 
>> more
>> +info, please see > href="http://xenbits.xen.org/docs/unstable/misc/channel.txt;>http://xenbits.xen.org/docs/unstable/misc/channel.txt).
> 
> The last sentence is not clear IMO and I'd like to improve it before pushing
> this series. What do you think of the below diff? Or feel free to propose
> something better :-).
Much cleared indeed, it looks good to me. Specially the changes to the first
sentences.. I was sort of searching for that kind of working when making that 
text.

Joao

> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 129ba62..7008005 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5981,12 +5981,12 @@ qemu-kvm -net nic,model=? /dev/null
>  connected and disconnected.
>
>xen
> -   Paravirtualized xen channel. Channel is exposed in the guest as a
> -xen console but identified with a name. The setup of the channel
> -depends to guest own rules and can live in a arbitrary path (for more
> -info, please see  href="http://xenbits.xen.org/docs/unstable/misc/channel.txt;>http://xenbits.xen.org/docs/unstable/misc/channel.txt).
> +   Paravirtualized Xen channel. Channel is exposed in the guest as a
> +Xen console but identified with a name. Setup and consumption of a 
> Xen
> +channel depends on software and configuration in the guest
> +(for more info, please see  href="http://xenbits.xen.org/docs/unstable/misc/channel.txt;>http://xenbits.xen.org/docs/unstable/misc/channel.txt).
>  Channel source path semantics are the same as the virtio target type.
> -Although state attribute is not provided as xen channels
> +The state attribute is not supported since Xen channels
>  lack the necessary probing mechanism.
>  Since 2.3.0
>
> 
> 
> Regards,
> Jim
> 

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

[libvirt] Plan for next release

[Daniel Veillard]

  Time flies, I didn't realized we were that close from the end of the month.
So if we want to have 2.3.0 for next Monday I think we should start the freeze
today, possibly tonight.

   Does this work ? Sorry for late notice,

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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