Re: [libvirt] [PATCH v5 3/3] gluster: cache glfs connection object per volume

[Peter Krempa]

On Thu, Dec 15, 2016 at 13:37:05 +0530, Prasanna Kumar Kalever wrote:
> Currently, in case if we have 4 extra attached disks, then for each disk
> we need to call 'glfs_init' (over network) and friends which could be costly.
> 
> Additionally snapshot(external) scenario will further complex the situation.
> 
> This patch maintain a cache of glfs objects per volume, hence the
> all disks/snapshots belonging to the volume whose glfs object is cached
> can avoid calls to 'glfs_init' and friends by simply taking a ref on
> existing object.
> 
> The glfs object is shared only if volume name and all the volfile servers 
> match
> (includes hostname, transport and port number).
> 
> Thanks to 'Peter Krempa' for all the inputs.
> 
> Signed-off-by: Prasanna Kumar Kalever 
> ---
>  src/storage/storage_backend_gluster.c | 302 
> +++---
>  1 file changed, 283 insertions(+), 19 deletions(-)
> 
> diff --git a/src/storage/storage_backend_gluster.c 
> b/src/storage/storage_backend_gluster.c
> index 779a3bd..71426c8 100644
> --- a/src/storage/storage_backend_gluster.c
> +++ b/src/storage/storage_backend_gluster.c
> @@ -36,6 +36,20 @@
>  
>  VIR_LOG_INIT("storage.storage_backend_gluster");
>  
> +
> +typedef struct _virStorageBackendGlusterState virStorageBackendGlusterState;
> +typedef virStorageBackendGlusterState *virStorageBackendGlusterStatePtr;
> +
> +typedef struct _virStorageFileBackendGlusterPriv 
> virStorageFileBackendGlusterPriv;
> +typedef virStorageFileBackendGlusterPriv 
> *virStorageFileBackendGlusterPrivPtr;
> +
> +typedef struct _virStorageBackendGlusterCache virStorageBackendGlusterCache;
> +typedef virStorageBackendGlusterCache *virStorageBackendGlusterCachePtr;
> +
> +typedef struct _virStorageBackendGlusterCacheConn 
> virStorageBackendGlusterCacheConn;
> +typedef virStorageBackendGlusterCacheConn 
> *virStorageBackendGlusterCacheConnPtr;
> +
> +
>  struct _virStorageBackendGlusterState {
>  glfs_t *vol;
>  
> @@ -47,8 +61,206 @@ struct _virStorageBackendGlusterState {
>  char *dir; /* dir from URI, or "/"; always starts and ends in '/' */
>  };
>  
> -typedef struct _virStorageBackendGlusterState virStorageBackendGlusterState;
> -typedef virStorageBackendGlusterState *virStorageBackendGlusterStatePtr;
> +struct _virStorageFileBackendGlusterPriv {
> +glfs_t *vol;
> +char *canonpath;
> +};
> +
> +struct _virStorageBackendGlusterCacheConn {
> +virObjectLockable parent;
> +virCond cond;
> +bool quit;

The name 'quit' is wrong. The flag is used to mark that the connection is valid.

> +
> +glfs_t *fs;
> +size_t nhosts;
> +virStorageNetHostDefPtr hosts;
> +char *volname;
> +};
> +
> +struct _virStorageBackendGlusterCache {
> +virMutex lock;
> +size_t nConn;
> +virStorageBackendGlusterCacheConnPtr *conn;
> +};
> +
> +static bool
> +virStorageBackendGlusterCompareHosts(size_t nSrcHosts,
> + virStorageNetHostDefPtr srcHosts,
> + size_t nDstHosts,
> + virStorageNetHostDefPtr dstHosts)
> +{
> +bool match = false;
> +size_t i;
> +size_t j;
> +
> +if (nSrcHosts != nDstHosts)
> +return false;
> +for (i = 0; i < nSrcHosts; i++) {
> +for (j = 0; j < nDstHosts; j++) {
> +if (srcHosts[i].type != dstHosts[j].type)
> +continue;
> +switch (srcHosts[i].type) {
> +case VIR_STORAGE_NET_HOST_TRANS_UNIX:
> +if (STREQ(srcHosts[i].u.uds.path, 
> dstHosts[j].u.uds.path))
> +match = true;
> +break;
> +case VIR_STORAGE_NET_HOST_TRANS_TCP:
> +case VIR_STORAGE_NET_HOST_TRANS_RDMA:
> +if (STREQ(srcHosts[i].u.inet.addr, 
> dstHosts[j].u.inet.addr)
> +   &&
> +STREQ(srcHosts[i].u.inet.port, 
> dstHosts[j].u.inet.port))
> +match = true;
> +break;
> +case VIR_STORAGE_NET_HOST_TRANS_LAST:
> +break;
> +}
> +if (match)
> +break; /* out of inner for loop */
> +}
> +if (!match)
> +return false;
> +match = false; /* reset */
> +}
> +return true;
> +}
> +
> +static int
> +virStorageBackendGlusterCacheAdd(virStorageSourcePtr src, glfs_t *fs)

Since need access to the cache entry after calling this function and you
repeatedly try to look it up, this function should return the cache
entry so that the code does not have to look it up afterwards.

> +{
> +virStorageBackendGlusterCacheConnPtr newConn = NULL;
> +virStorageBackendGlusterCacheConnPtr conn = NULL;
> +size_t i;
> +
> +for (i = 0; i < virStorageBackendGlusterGlobalCacheObj->nConn; i++) {
> +conn = 

Re: [libvirt] [PATCH v5 0/3] gluster: cache glfs connection object per volume

[Prasanna Kalever]

Hi Virt Team,


Please review this patch series.


Thanks,
--
Prasanna


On Thu, Dec 15, 2016 at 1:37 PM, Prasanna Kumar Kalever
 wrote:
> v5: Address review comments from Peter on v4
> Dropping Patch 3 from v3
> Patch 1: fix transport type in all drivers, possible switch case converts
> Patch 2: use virObject with src->drv to avoid 'initFlag' for
>  remembering 'virStorageFileInit' status
> Patch 3:
> * change virStorageBackendGlusterCacheQuery to *Lookup
> * change virStorageBackendGlusterCacheRefresh to *Release
> * Avoid the global Flags
> * Thread safe measure by using virCond*
> Thanks Peter!
>
> v4: Address review comments from Peter on v3
> Patch 1: changes to fix test failures
>  (qemuargv2xmltest, qemuxml2argvtest, qemuxml2xmltest and 
> virstoragetest)
> Patch 2: fix hung in qemuxml2argvtest
> Patch 3 v3: Split this Patch to 2 parts as below
> Patch 3 v4: don't free metadata used for storage driver access
> Patch 4:
> * update variable naming as per standards
> * virStorageBackendGlusterPreopened* -> virStorageBackendGlusterCache*
> Thanks Peter!
>
> v3: Address comments by Daniel and Peter on v2
> * Split the patch to 3 parts
> Patch 1: change the virStorageNetHostDef type
> Patch 2: optimize calls to virStorageFileInit and friends
> Patch 3: add the caching for glfs
> * Thanks to Daniel, this version make all the methods as thread safe
> * Thanks to Peter for pointing use of virObjectLockable, this had simplified
>   rest of the parts a lot.
>
> v2: Address review comments from Peter on v1
> * Rebased on latest master
> * Changes to commit msg
> * Introduce storage API's for Register and Unregister of volume
> * During qemu process Start/Stop and snapshot create
> * Check Transport and Port type
> * Atomic element add/del to list and ref counting
> Pending: Treating IP and FQDN belong to same host
>
> v1: Initial patch
>
> Prasanna Kumar Kalever (3):
>   util: change the virStorageNetHostDef type
>   storage: optimize calls to virStorageFileInit and friends
>   gluster: cache glfs connection object per volume
>
>  src/conf/domain_conf.c   |  60 +++--
>  src/libxl/libxl_conf.c   |  10 +-
>  src/qemu/qemu_command.c  |  97 ---
>  src/qemu/qemu_domain.c   |  25 +-
>  src/qemu/qemu_domain.h   |   8 +-
>  src/qemu/qemu_driver.c   |  26 +-
>  src/qemu/qemu_parse_command.c|  50 ++--
>  src/qemu/qemu_process.c  |  49 
>  src/qemu/qemu_process.h  |   4 +
>  src/storage/storage_backend.h|   1 +
>  src/storage/storage_backend_fs.c |   5 +
>  src/storage/storage_backend_gluster.c| 329 
> ---
>  src/storage/storage_driver.c |  40 ++-
>  src/storage/storage_driver.h |   2 +
>  src/util/virstoragefile.c|  93 ---
>  src/util/virstoragefile.h|  21 +-
>  src/xenconfig/xen_xl.c   |  10 +-
>  tests/domainsnapshotxml2xmlin/disk_snapshot.xml  |   2 +-
>  tests/domainsnapshotxml2xmlout/disk_snapshot.xml |   2 +-
>  tests/virstoragetest.c   |   2 +-
>  20 files changed, 614 insertions(+), 222 deletions(-)
>
> --
> 2.7.4
>

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

[libvirt] [PATCH 0/9] i386: query-cpu-model-expansion test script

[Eduardo Habkost]

This is a follow-up to the series that implements
query-cpu-model-expansion. Before including the test script, the
series has some fixes to allow the results of
query-cpu-model-expansion to be used in the QEMU command-line.

The script probably will work on s390x too, but I couldn't test
it yet.

This series and its dependencies can be pulled from the branch:
  https://github.com/ehabkost/qemu-hacks.git work/x86-query-cpu-expansion-test

---
Cc: Cornelia Huck 
Cc: Christian Borntraeger 
Cc: David Hildenbrand 
Cc: libvir-list@redhat.com
Cc: Jiri Denemark 
Cc: "Jason J. Herne" 
Cc: Markus Armbruster 
Cc: Richard Henderson 
Cc: Igor Mammedov 
Cc: Eric Blake 

Eduardo Habkost (9):
  target-i386: Move "host" properties to base class
  target-i386: Allow short strings to be used as vendor ID
  cpu: Support comma escaping when parsing -cpu
  qemu.py: Make logging optional
  qtest.py: Support QTEST_LOG environment variable
  qtest.py: make logging optional
  qtest.py: Make 'binary' parameter optional
  tests: Add rules to non-gtester qtest test cases
  tests: Test case for query-cpu-model-expansion

 scripts/qemu.py   |  25 ++-
 scripts/qtest.py  |  15 +-
 qom/cpu.c |  32 ++--
 target/i386/cpu.c |  83 -
 tests/test-x86-cpuid-compat.c |  19 ++
 tests/Makefile.include|  40 -
 tests/query-cpu-model-test.py | 398 ++
 7 files changed, 551 insertions(+), 61 deletions(-)
 create mode 100755 tests/query-cpu-model-test.py

-- 
2.11.0.259.g40922b1

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

[libvirt] [PATCH v2 0/4] target-i386: Implement query-cpu-model-expansion

[Eduardo Habkost]

This series implements query-cpu-model-expansion on target-i386.

Changes v1 -> v2:
-

This version is highly simplified compared to v1. It contains
only an implementation that will return a limited set of
properties. I have a follow-up series that will expend type=full
expansion to return every single QOM property, but this version
will return the same data for type=static and type=full expansion
for simplicity (except that type=static expansion will use the
"base" CPU model as base).

This means this version also won't include "pmu" and
"host-cache-info" in full expansion, and won't require special
code for those properties.

The unit test code was also removed in this version, to keep the
series simple and easier to review. Most of the patches on the
previous series were changes just to make the test case work. I
will send the test-case-related changes as a follow-up series.

---
Cc: Cornelia Huck 
Cc: Christian Borntraeger 
Cc: David Hildenbrand 
Cc: libvir-list@redhat.com
Cc: Jiri Denemark 
Cc: "Jason J. Herne" 
Cc: Markus Armbruster 
Cc: Richard Henderson 
Cc: Igor Mammedov 
Cc: Eric Blake 

Eduardo Habkost (4):
  target-i386: Reorganize and document CPUID initialization steps
  qapi-schema: Comment about full expansion of non-migration-safe models
  target-i386: Define static "base" CPU model
  target-i386: Implement query-cpu-model-expansion QMP command

 qapi-schema.json  |   9 ++
 target/i386/cpu-qom.h |   2 +
 monitor.c |   4 +-
 target/i386/cpu.c | 317 --
 4 files changed, 298 insertions(+), 34 deletions(-)

-- 
2.11.0.259.g40922b1

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

[libvirt] [PATCH v2 4/4] target-i386: Implement query-cpu-model-expansion QMP command

[Eduardo Habkost]

Implement query-cpu-model-expansion for target-i386.

This should meet all the requirements while being simple. In the
case of static expansion, it will use the new "base" CPU model,
and in the case of full expansion, it will keep the original CPU
model name+props, and append extra properties.

A future follow-up should improve the implementation of
type=full, so that it returns more detailed data, including every
writable QOM property in the CPU object.

Cc: libvir-list@redhat.com
Cc: Jiri Denemark 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Fix typos
  * Reported-by: Markus Armbruster 
* Removed static/migration-safe field from return value
  (they will be added by a separate patch)
* Use a dictionary to keep track of the properties that
  should be included in the expansion, instead of hardcoding them
* Removed test case to keep series simple (test case will be
  submitted as a separate series)
---
 monitor.c |   4 +-
 target/i386/cpu.c | 191 +-
 2 files changed, 193 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index 0841d436b0..90c12b3cd5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -983,8 +983,10 @@ static void qmp_unregister_commands_hack(void)
 #ifndef TARGET_ARM
 qmp_unregister_command("query-gic-capabilities");
 #endif
-#if !defined(TARGET_S390X)
+#if !defined(TARGET_S390X) && !defined(TARGET_I386)
 qmp_unregister_command("query-cpu-model-expansion");
+#endif
+#if !defined(TARGET_S390X)
 qmp_unregister_command("query-cpu-model-baseline");
 qmp_unregister_command("query-cpu-model-comparison");
 #endif
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1a1bb75a75..0b68c2d423 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -29,10 +29,16 @@
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "qapi/qmp/qerror.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qbool.h"
+#include "qapi/qmp/qint.h"
+#include "qapi/qmp/qfloat.h"
 
 #include "qapi-types.h"
 #include "qapi-visit.h"
 #include "qapi/visitor.h"
+#include "qom/qom-qobject.h"
 #include "sysemu/arch_init.h"
 
 #if defined(CONFIG_KVM)
@@ -2261,7 +2267,7 @@ static void x86_cpu_apply_props(X86CPU *cpu, PropValue 
*props)
 }
 }
 
-/* Load data from X86CPUDefinition
+/* Load data from X86CPUDefinition into a X86CPU object
  */
 static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
 {
@@ -2270,6 +2276,11 @@ static void x86_cpu_load_def(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 char host_vendor[CPUID_VENDOR_SZ + 1];
 FeatureWord w;
 
+/*NOTE: any property set by this function should be returned by
+ * x86_cpu_static_props(), so static expansion of
+ * query-cpu-model-expansion is always complete.
+ */
+
 /* CPU models only set _minimum_ values for level/xlevel: */
 object_property_set_int(OBJECT(cpu), def->level, "min-level", errp);
 object_property_set_int(OBJECT(cpu), def->xlevel, "min-xlevel", errp);
@@ -2314,6 +2325,184 @@ static void x86_cpu_load_def(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 
 }
 
+/* Return a QDict containing keys for all properties that can be included
+ * in static expansion of CPU models. All properties set by x86_cpu_load_def()
+ * must be included in the dictionary.
+ */
+static QDict *x86_cpu_static_props(void)
+{
+FeatureWord w;
+int i;
+static const char *props[] = {
+"min-level",
+"min-xlevel",
+"family",
+"model",
+"stepping",
+"model-id",
+"vendor",
+"lmce",
+NULL,
+};
+static QDict *d;
+
+if (d) {
+return d;
+}
+
+d = qdict_new();
+for (i = 0; props[i]; i++) {
+qdict_put_obj(d, props[i], qnull());
+}
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+FeatureWordInfo *fi = _word_info[w];
+int bit;
+for (bit = 0; bit < 32; bit++) {
+if (!fi->feat_names[bit]) {
+continue;
+}
+qdict_put_obj(d, fi->feat_names[bit], qnull());
+}
+}
+
+return d;
+}
+
+/* Add an entry to @props dict, with the value for property. */
+static void x86_cpu_expand_prop(X86CPU *cpu, QDict *props, const char *prop)
+{
+QObject *value = object_property_get_qobject(OBJECT(cpu), prop,
+ _abort);
+
+qdict_put_obj(props, prop, value);
+}
+
+/* Convert CPU model data from X86CPU object to a property dictionary
+ * that can recreate exactly the same CPU model.
+ */
+static void x86_cpu_to_dict(X86CPU *cpu, QDict *props)
+{
+QDict *sprops = x86_cpu_static_props();
+const QDictEntry *e;
+
+for (e = qdict_first(sprops); e; e = qdict_next(sprops, e)) {
+const char *prop = qdict_entry_key(e);
+x86_cpu_expand_prop(cpu, props, prop);
+}
+}
+
+static void 

Re: [libvirt] Performance about x-data-plane

[Weiwei Jia]

On Mon, Jan 16, 2017 at 8:15 AM, Stefan Hajnoczi  wrote:
> On Tue, Jan 03, 2017 at 12:02:14PM -0500, Weiwei Jia wrote:
>> > The expensive part is the virtqueue kick.  Recently we tried polling the
>> > virtqueue instead of waiting for the ioeventfd file descriptor and got
>> > double-digit performance improvements:
>> > https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00148.html
>> >
>> > If you want to understand the performance of your benchmark you'll have
>> > compare host/guest disk stats (e.g. request lifetime, disk utilization,
>> > queue depth, average request size) to check that the bare metal and
>> > guest workloads are really sending comparable I/O patterns to the
>> > physical disk.
>> >
>> > Then you using Linux and/or QEMU tracing to analyze the request latency
>> > by looking at interesting points in the request lifecycle like virtqueue
>> > kick, host Linux AIO io_submit(2), etc.
>> >
>>
>> Thank you. I will look into "polling the virtqueue" as you said above.
>> Currently, I just use blktrace to see disk stats and add logs in the
>> I/O workload to see the time latency for each request. What kind of
>> tools are you using to analyze request lifecycle like virtqueue kick,
>> host Linux AIO iosubmit, etc.
>>
>> Do you trace the lifecycle like this
>> (http://www.linux-kvm.org/page/Virtio/Block/Latency#Performance_data)
>> but it seems to be out of date. Does it
>> (http://repo.or.cz/qemu-kvm/stefanha.git/shortlog/refs/heads/tracing-dev-0.12.4)
>> still work on QEMU 2.4.1?
>
> The details are out of date but the general approach to tracing the I/O
> request lifecycle still apply.
>
> There are multiple tracing tools that can do what you need.  I've CCed
> Karl Rister who did the latest virtio-blk dataplane tracing.
>
> "perf record -a -e kvm:\*" is a good start.  You can use "perf probe" to
> trace QEMU's trace events (recent versions have sdt support, which means
> SystemTap tracepoints work) and also trace any function in QEMU:
> http://blog.vmsplice.net/2011/03/how-to-use-perf-probe.html

Thank you. I will try it.


Best,
Weiwei Jia

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

Re: [libvirt] Performance about x-data-plane

[Weiwei Jia]

On Mon, Jan 16, 2017 at 10:00 AM, Karl Rister  wrote:
> On 01/16/2017 07:15 AM, Stefan Hajnoczi wrote:
>> On Tue, Jan 03, 2017 at 12:02:14PM -0500, Weiwei Jia wrote:
 The expensive part is the virtqueue kick.  Recently we tried polling the
 virtqueue instead of waiting for the ioeventfd file descriptor and got
 double-digit performance improvements:
 https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00148.html

 If you want to understand the performance of your benchmark you'll have
 compare host/guest disk stats (e.g. request lifetime, disk utilization,
 queue depth, average request size) to check that the bare metal and
 guest workloads are really sending comparable I/O patterns to the
 physical disk.

 Then you using Linux and/or QEMU tracing to analyze the request latency
 by looking at interesting points in the request lifecycle like virtqueue
 kick, host Linux AIO io_submit(2), etc.

>>>
>>> Thank you. I will look into "polling the virtqueue" as you said above.
>>> Currently, I just use blktrace to see disk stats and add logs in the
>>> I/O workload to see the time latency for each request. What kind of
>>> tools are you using to analyze request lifecycle like virtqueue kick,
>>> host Linux AIO iosubmit, etc.
>>>
>>> Do you trace the lifecycle like this
>>> (http://www.linux-kvm.org/page/Virtio/Block/Latency#Performance_data)
>>> but it seems to be out of date. Does it
>>> (http://repo.or.cz/qemu-kvm/stefanha.git/shortlog/refs/heads/tracing-dev-0.12.4)
>>> still work on QEMU 2.4.1?
>>
>> The details are out of date but the general approach to tracing the I/O
>> request lifecycle still apply.
>>
>> There are multiple tracing tools that can do what you need.  I've CCed
>> Karl Rister who did the latest virtio-blk dataplane tracing.
>
> I roughly followed this guide by Luiz Capitulino:
>
> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg00887.html
>
> I tweaked his trace-host-and-guest script to avoid doing IO while
> tracing is enabled, my version is available here:
>
> http://people.redhat.com/~krister/tracing/trace-host-and-guest
>
> I built QEMU with --enable-trace-backends=ftrace and then turned on the
> QEMU trace events I was interested in with this bit of bash:
>
> for event in $(/usr/libexec/qemu-kvm -trace help 2>&1|grep virtio|grep
> -v "gpu\|console\|serial\|rng\|balloon\|ccw"); do virsh
> qemu-monitor-command master --hmp trace-event ${event} on; done
>
> At this point, the QEMU trace events are automatically inserted into the
> ftrace buffers and the methodology outlined by Luiz gets the guest
> kernel, host kernel, and QEMU events properly interleaved.


Thank you. I will try it.


Best,
Weiwei Jia

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

[libvirt] [PATCH] tests: fix QED disk test in xlconfigtest

[Jim Fehlig]

When LIBXL_HAVE_QED is defined, xlconfigtest fails

 9) Xen XL-2-XML Format disk-qed  ... command line: config parsing error
 in disk specification: no vdev specified in
 
`target=/var/lib/libvirt/images/XenGuest2,format=qed,backendtype=qdisk,vdev=hda,access=rw'
FAILED

As per the xl-disk-configuration(5) man page, target= must come
last in the disk specification when specified by name:

When this parameter is specified by name, ie with the target=
syntax in the configuration file, it consumes the whole rest of the
DISKSPEC including trailing whitespaces.  Therefore in that case
it must come last.

Change tests/xlconfigdata/test-disk-qed.cfg to adhere to this
restriction.

Signed-off-by: Jim Fehlig 
---
 tests/xlconfigdata/test-disk-qed.cfg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/xlconfigdata/test-disk-qed.cfg 
b/tests/xlconfigdata/test-disk-qed.cfg
index ee904e0..7842176 100644
--- a/tests/xlconfigdata/test-disk-qed.cfg
+++ b/tests/xlconfigdata/test-disk-qed.cfg
@@ -22,4 +22,4 @@ parallel = "none"
 serial = "none"
 builder = "hvm"
 boot = "d"
-disk = [ 
"target=/var/lib/libvirt/images/XenGuest2,format=qed,backendtype=qdisk,vdev=hda,access=rw"
 ]
+disk = [ 
"format=qed,backendtype=qdisk,vdev=hda,access=rw,target=/var/lib/libvirt/images/XenGuest2"
 ]
-- 
2.9.2

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

Re: [libvirt] Performance about x-data-plane

[Karl Rister]

On 01/16/2017 07:15 AM, Stefan Hajnoczi wrote:
> On Tue, Jan 03, 2017 at 12:02:14PM -0500, Weiwei Jia wrote:
>>> The expensive part is the virtqueue kick.  Recently we tried polling the
>>> virtqueue instead of waiting for the ioeventfd file descriptor and got
>>> double-digit performance improvements:
>>> https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00148.html
>>>
>>> If you want to understand the performance of your benchmark you'll have
>>> compare host/guest disk stats (e.g. request lifetime, disk utilization,
>>> queue depth, average request size) to check that the bare metal and
>>> guest workloads are really sending comparable I/O patterns to the
>>> physical disk.
>>>
>>> Then you using Linux and/or QEMU tracing to analyze the request latency
>>> by looking at interesting points in the request lifecycle like virtqueue
>>> kick, host Linux AIO io_submit(2), etc.
>>>
>>
>> Thank you. I will look into "polling the virtqueue" as you said above.
>> Currently, I just use blktrace to see disk stats and add logs in the
>> I/O workload to see the time latency for each request. What kind of
>> tools are you using to analyze request lifecycle like virtqueue kick,
>> host Linux AIO iosubmit, etc.
>>
>> Do you trace the lifecycle like this
>> (http://www.linux-kvm.org/page/Virtio/Block/Latency#Performance_data)
>> but it seems to be out of date. Does it
>> (http://repo.or.cz/qemu-kvm/stefanha.git/shortlog/refs/heads/tracing-dev-0.12.4)
>> still work on QEMU 2.4.1?
> 
> The details are out of date but the general approach to tracing the I/O
> request lifecycle still apply.
> 
> There are multiple tracing tools that can do what you need.  I've CCed
> Karl Rister who did the latest virtio-blk dataplane tracing.

I roughly followed this guide by Luiz Capitulino:

https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg00887.html

I tweaked his trace-host-and-guest script to avoid doing IO while
tracing is enabled, my version is available here:

http://people.redhat.com/~krister/tracing/trace-host-and-guest

I built QEMU with --enable-trace-backends=ftrace and then turned on the
QEMU trace events I was interested in with this bit of bash:

for event in $(/usr/libexec/qemu-kvm -trace help 2>&1|grep virtio|grep
-v "gpu\|console\|serial\|rng\|balloon\|ccw"); do virsh
qemu-monitor-command master --hmp trace-event ${event} on; done

At this point, the QEMU trace events are automatically inserted into the
ftrace buffers and the methodology outlined by Luiz gets the guest
kernel, host kernel, and QEMU events properly interleaved.

> 
> "perf record -a -e kvm:\*" is a good start.  You can use "perf probe" to
> trace QEMU's trace events (recent versions have sdt support, which means
> SystemTap tracepoints work) and also trace any function in QEMU:
> http://blog.vmsplice.net/2011/03/how-to-use-perf-probe.html
> 
> Stefan
> 


-- 
Karl Rister 

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

[libvirt] [PATCH] tests: fix compilation of shunloadtest

[Jim Fehlig]

While local builds succeed fine, a build worker building in a
chroot environment is encountering the following error with
libvirt 3.0.0 release candidates

[  162s] shunloadtest.o: In function `main':
[  162s] /home/abuild/rpmbuild/BUILD/libvirt-3.0.0/tests/shunloadtest.c:110: 
undefined reference to `dlopen'
[  162s] /home/abuild/rpmbuild/BUILD/libvirt-3.0.0/tests/shunloadtest.c:114: 
undefined reference to `dlsym'
[  162s] /home/abuild/rpmbuild/BUILD/libvirt-3.0.0/tests/shunloadtest.c:133: 
undefined reference to `dlclose'
[  162s] /home/abuild/rpmbuild/BUILD/libvirt-3.0.0/tests/shunloadtest.c:111: 
undefined reference to `dlerror'
[  162s] /home/abuild/rpmbuild/BUILD/libvirt-3.0.0/tests/shunloadtest.c:115: 
undefined reference to `dlerror'
[  162s] /home/abuild/rpmbuild/BUILD/libvirt-3.0.0/tests/shunloadtest.c:116: 
undefined reference to `dlclose'

Fix by appending LDADDS to shunloadtest_LDADD.

Signed-off-by: Jim Fehlig 
---
 tests/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c7d4748..112396f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1348,7 +1348,7 @@ libshunload_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
 
 shunloadtest_SOURCES = \
shunloadtest.c
-shunloadtest_LDADD = $(LIB_PTHREAD)
+shunloadtest_LDADD = $(LIB_PTHREAD) $(LDADDS)
 shunloadtest_DEPENDENCIES = libshunload.la
 
 sysinfotest_SOURCES = \
-- 
2.9.2

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

Re: [libvirt] [PATCH for 3.0.0] Revert "perf: Add cache_l1d perf event support"

[Daniel P. Berrange]

On Mon, Jan 16, 2017 at 12:07:39PM -0500, John Ferlan wrote:
> 
> 
> On 01/16/2017 11:57 AM, Daniel P. Berrange wrote:
> > This reverts commit ae16c95f1bb5591c27676c5de8d383e5612c3568.
> > 
> > The data was calculated incorrectly and the event name needs
> > to be changed.
> > 
> > ---
> >  docs/formatdomain.html.in   |  7 ---
> >  docs/schemas/domaincommon.rng   |  1 -
> >  include/libvirt/libvirt-domain.h| 11 ---
> >  src/libvirt-domain.c|  2 --
> >  src/qemu/qemu_driver.c  |  1 -
> >  src/util/virperf.c  |  6 +-
> >  src/util/virperf.h  |  1 -
> >  tests/genericxml2xmlindata/generic-perf.xml |  1 -
> >  tools/virsh.pod |  5 +
> >  9 files changed, 2 insertions(+), 33 deletions(-)
> > 
> 
> ACK - looks like it's a bonus that I forgot update news.xml too... There
> of course will be perl and go patches...  

Thanks, and yes, i've just pushed reverts for perl & go too.

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

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

Re: [libvirt] [PATCH for 3.0.0] Revert "perf: Add cache_l1d perf event support"

[John Ferlan]

On 01/16/2017 11:57 AM, Daniel P. Berrange wrote:
> This reverts commit ae16c95f1bb5591c27676c5de8d383e5612c3568.
> 
> The data was calculated incorrectly and the event name needs
> to be changed.
> 
> ---
>  docs/formatdomain.html.in   |  7 ---
>  docs/schemas/domaincommon.rng   |  1 -
>  include/libvirt/libvirt-domain.h| 11 ---
>  src/libvirt-domain.c|  2 --
>  src/qemu/qemu_driver.c  |  1 -
>  src/util/virperf.c  |  6 +-
>  src/util/virperf.h  |  1 -
>  tests/genericxml2xmlindata/generic-perf.xml |  1 -
>  tools/virsh.pod |  5 +
>  9 files changed, 2 insertions(+), 33 deletions(-)
> 

ACK - looks like it's a bonus that I forgot update news.xml too... There
of course will be perl and go patches...  

John

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

[libvirt] [PATCH for 3.0.0] Revert "perf: Add cache_l1d perf event support"

[Daniel P. Berrange]

This reverts commit ae16c95f1bb5591c27676c5de8d383e5612c3568.

The data was calculated incorrectly and the event name needs
to be changed.

---
 docs/formatdomain.html.in   |  7 ---
 docs/schemas/domaincommon.rng   |  1 -
 include/libvirt/libvirt-domain.h| 11 ---
 src/libvirt-domain.c|  2 --
 src/qemu/qemu_driver.c  |  1 -
 src/util/virperf.c  |  6 +-
 src/util/virperf.h  |  1 -
 tests/genericxml2xmlindata/generic-perf.xml |  1 -
 tools/virsh.pod |  5 +
 9 files changed, 2 insertions(+), 33 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 30cb196..3f7f875 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1937,7 +1937,6 @@
   event name='stalled_cycles_frontend' enabled='no'/
   event name='stalled_cycles_backend' enabled='no'/
   event name='ref_cpu_cycles' enabled='no'/
-  event name='cache_l1d' enabled='no'/
 /perf
 ...
 
@@ -2016,12 +2015,6 @@
  by applications running on the platform
   perf.ref_cpu_cycles
 
-
-  cache_l1d
-  the count of total level 1 data cache by applications running on
-   the platform
-  perf.cache_l1d
-
   
 
 Devices
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index be0a609..4d76315 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -433,7 +433,6 @@
   stalled_cycles_frontend
   stalled_cycles_backend
   ref_cpu_cycles
-  cache_l1d
 
   
   
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 1e0e74c..e303140 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2188,17 +2188,6 @@ void 
virDomainStatsRecordListFree(virDomainStatsRecordPtr *stats);
  */
 # define VIR_PERF_PARAM_REF_CPU_CYCLES "ref_cpu_cycles"
 
-/**
- * VIR_PERF_PARAM_CACHE_L1D:
- *
- * Macro for typed parameter name that represents cache_l1d
- * perf event which can be used to measure the count of total
- * level 1 data cache by applications running on the platform.
- * It corresponds to the "perf.cache_l1d" field in the
- * *Stats APIs.
- */
-# define VIR_PERF_PARAM_CACHE_L1D "cache_l1d"
-
 int virDomainGetPerfEvents(virDomainPtr dom,
virTypedParameterPtr *params,
int *nparams,
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 3023f30..5b3e842 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11250,8 +11250,6 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * CPU frequency scaling by applications running
  * as unsigned long long. It is produced by the
  * ref_cpu_cycles perf event.
- * "perf.cache_l1d" - The count of total level 1 data cache as unsigned
- *long long. It is produced by cache_l1d perf event.
  *
  * Note that entire stats groups or individual stat fields may be missing from
  * the output in case they are not supported by the given hypervisor, are not
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 42f9889..d4422f3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9877,7 +9877,6 @@ qemuDomainSetPerfEvents(virDomainPtr dom,
VIR_PERF_PARAM_STALLED_CYCLES_FRONTEND, 
VIR_TYPED_PARAM_BOOLEAN,
VIR_PERF_PARAM_STALLED_CYCLES_BACKEND, 
VIR_TYPED_PARAM_BOOLEAN,
VIR_PERF_PARAM_REF_CPU_CYCLES, 
VIR_TYPED_PARAM_BOOLEAN,
-   VIR_PERF_PARAM_CACHE_L1D, 
VIR_TYPED_PARAM_BOOLEAN,
NULL) < 0)
 return -1;
 
diff --git a/src/util/virperf.c b/src/util/virperf.c
index 8554723..f64692b 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -43,8 +43,7 @@ VIR_ENUM_IMPL(virPerfEvent, VIR_PERF_EVENT_LAST,
   "cache_references", "cache_misses",
   "branch_instructions", "branch_misses",
   "bus_cycles", "stalled_cycles_frontend",
-  "stalled_cycles_backend", "ref_cpu_cycles",
-  "cache_l1d");
+  "stalled_cycles_backend", "ref_cpu_cycles");
 
 struct virPerfEvent {
 int type;
@@ -113,9 +112,6 @@ static struct virPerfEventAttr attrs[] = {
  .attrConfig = 0,
 # endif
 },
-{.type = VIR_PERF_EVENT_CACHE_L1D,
- .attrType = PERF_TYPE_HW_CACHE,
- .attrConfig = PERF_COUNT_HW_CACHE_L1D},
 };
 typedef struct virPerfEventAttr *virPerfEventAttrPtr;
 
diff --git a/src/util/virperf.h b/src/util/virperf.h
index 4c562af..1f43c92 100644
--- a/src/util/virperf.h
+++ b/src/util/virperf.h
@@ -47,7 +47,6 @@ typedef enum {
 

Re: [libvirt] Availability of libvirt-3.0.0 release candidate 2

[Daniel P. Berrange]

On Sat, Jan 14, 2017 at 11:02:37PM +0100, Daniel Veillard wrote:
>   I missed the Friday deadline ot push it, so did it today. It's now tagged
> in git, signed tarball and rpms are at the usual place:
>ftp://libvirt.org/libvirt/
> 
>  As a result I think GA should happen on Tuesday to give at least one
> working day to raise critical issues. Nothing seems to have been reported
> on rc1 neither positive nor negative, so please give it some testing.

This series wants to change the public API for a constant just
added:

  https://www.redhat.com/archives/libvir-list/2017-January/msg00678.html

IMHO it is too late to change this, so instead I'm suggesting we revert
that public API addition entirely

  https://www.redhat.com/archives/libvir-list/2017-January/msg00702.html

and revisit it in 3.1.0

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

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

[libvirt] vms hung

[Umar Draz]

Hello,

I am running libvirt-2.0.0-10.el7_3.2.x86_64 on CentOS 7.3, I have few vms
of CentOS and Windows 2012 servers.

Randomly they are hung, but the status of virtual machine keep running, vnc
and network not responding.

I have checked all /var/log/messages and /var/log/libvirt/qemu/vm.log but
nothing found anything wrong.

Would you please help what should I do more so I can find the issue.

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

[libvirt] [RFE] Enable/allow MAC spoofing for SR-IOV virtual functions.

[Leon Goldberg]

Hey,

We've recently encountered difficulties using a virtual function as a slave
in a bond where the virtual function's MAC address differs from the bond's.

I've opened an RFE[1] suggesting MAC spoofing to be enabled/allowed in
virtual functions.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1413596

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

Re: [libvirt] [PATCH] qemu: Copy SELinux labels for namespace too

[Martin Kletzander]

On Fri, Jan 13, 2017 at 03:47:16PM +0100, Michal Privoznik wrote:

On 01/13/2017 12:43 PM, Martin Kletzander wrote:

On Fri, Jan 13, 2017 at 11:12:43AM +0100, Michal Privoznik wrote:

When creating new /dev/* for qemu, we do chown() and copy ACLs to
create the exact copy from the original /dev. I though that
copying SELinux labels is not necessary as SELinux will chose the
sane defaults. Surprisingly, it does not leaving namespace with
the following labels:

crw-rw-rw-. root root system_u:object_r:tmpfs_t:s0 random
crw---. root root system_u:object_r:tmpfs_t:s0 rtc0
drwxrwxrwt. root root system_u:object_r:tmpfs_t:s0 shm
crw-rw-rw-. root root system_u:object_r:tmpfs_t:s0 urandom

As a result, domain is unable to start:

error: internal error: process exited while connecting to monitor:
Error in GnuTLS initialization: Failed to acquire random data.
qemu-kvm: cannot initialize crypto: Unable to initialize GNUTLS
library: Failed to acquire random data.

The solution is to copy the SELinux labels as well.

Reported-by: Andrea Bolognani 
Signed-off-by: Michal Privoznik 
---
src/qemu/qemu_domain.c | 61
++
1 file changed, 61 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1399dee0d..a29866673 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -63,6 +63,9 @@
#if defined(HAVE_SYS_MOUNT_H)
# include 
#endif
+#ifdef WITH_SELINUX
+# include 
+#endif

#include 

@@ -6958,6 +6961,9 @@ qemuDomainCreateDevice(const char *device,
char *canonDevicePath = NULL;
struct stat sb;
int ret = -1;
+#ifdef WITH_SELINUX
+char *tcon = NULL;
+#endif

if (virFileResolveAllLinks(device, ) < 0) {
if (errno == ENOENT && allow_noent) {
@@ -7023,10 +7029,34 @@ qemuDomainCreateDevice(const char *device,
goto cleanup;
}

+#ifdef WITH_SELINUX
+if (getfilecon_raw(canonDevicePath, ) < 0 &&
+(errno != ENOTSUP && errno != ENODATA)) {
+virReportSystemError(errno,
+ _("Unable to get SELinux label on %s"),
canonDevicePath);
+goto cleanup;
+}
+
+if (tcon &&
+setfilecon_raw(devicePath, (VIR_SELINUX_CTX_CONST char *)
tcon) < 0) {
+VIR_WARNINGS_NO_WLOGICALOP_EQUAL_EXPR
+if (errno != EOPNOTSUPP && errno != ENOTSUP) {
+VIR_WARNINGS_RESET
+virReportSystemError(errno,
+ _("Unable to set SELinux label on %s"),
+ devicePath);
+goto cleanup;
+}
+}
+#endif
+


I think, instead of all the ifdefs, this should be a security driver API
instead of being hardcoded in places.  That way it will be processed
properly by all the security drivers.


I don't think I see what you mean. Firstly, we want to set seclabels for
some paths that are not touched by secdrivers at all (e.g. /dev/*random,
/dev/kvm). Secondly, secdrivers should honour norelabel flag and be a
no-op if that one is set. This would clash with sysadmins handling
seclabels themselves. Thirdly, no secdriver of ours deals with ACLs. We
have to in here.



It might sound a bit weird to have a security API that does not honour
relabel=no and does label an arbritrary file, but SELinux stuff should
go into SELinux security driver.  Otherwise this will be a mess.  The
only problem I see here is when the driver is disabled in the
configuration file, it will not be triggered.  But we might get around
that.

Just so you know, I'm not special-casing SELinux here.  I think
DAC-related stuff should go to DAC driver and so on.

Just my 2 cents, though.


Michal

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


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

Re: [libvirt] [PATCH v2] Add support for Veritas HyperScale (VxHS) block device protocol

[John Ferlan]

On 01/16/2017 04:07 AM, Peter Krempa wrote:
> On Sat, Jan 14, 2017 at 10:31:05 -0500, John Ferlan wrote:
>> [...]
>>
 diff --git 
 a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args 
 b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
 new file mode 100644
 index 000..f6e3e37
 --- /dev/null
 +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
> 
> [...]
> 
 +-drive file=vxhs://192.168.0.1:/eb90327c-8302-4725-9e1b-4e85ed4dc251,\
 +format=raw,if=none,id=drive-virtio-disk0,cache=none \
>>>
>>> Please use the modern syntax for disk specification, not the
>>> legacy URI syntax  ie
>>>
>>>   -drive  driver=vxhs,vdisk-id=eb90327c-8302-4725-4e85ed4dc251,\
>>>   server.host=example.com,server.port=12345,format=raw,
>>>   if=none,id=driver-virtio-disk0,cache=0
>>>
>>
>> FWIW: qemuBuildDriveStr doesn't support that yet for any driver -
>> although it probably should be updated... Timing wise whether that work
>> could get done before this work is ready - I'm not sure. While I agree
>> the code "should" use the new syntax - I do think we'd have to make
>> adjustments to all drive strings. That would probably mean knowing
>> whether the new syntax was "in place" before the current libvirt qemu
>> minimum version...
> 
> That's not true. The new syntax is used for multi-host gluster disks
> (qemuGetDriveSourceProps). Also we have QMP introspection code in place
> which can detect whether given configuration option is supported by the
> given qemu (virQEMUCapsQMPSchemaQueries[]).

hmm.. missed that path - it's not exactly well described how to get
there though. Suffice to say, well obfuscated.

I see that the syntax built for the gluster is "-drive
file.driver=gluster,..." though rather than just plain "-drive
driver=gluster,...". Whether vxhs would need to follow that model or the
one Daniel pointed out - I'm not quite sure.

In any case, to help "find" that for the Veritas team - see commit id
'7b7da9e28'... If you use gitk - the commits just before that are where
Peter has modified qemuBuildDriveSourceStr in order to accept a json
object to parse through in order to generate the newer syntax. In those
changes there is also the adjustment to the gluster backend storage code
to support the newer model - something that could be a model once/if a
vxhs storage backend is created.

> 
> Also we should not fully update the command line generator in cases
> where it's not necessary as it would just make the code more complex
> without any real benefit. The new protocols can use the new syntax
> though.
> 

Right - otherwise it'd already be done...

Still would be nice for a non json object path that could generate the
newer syntax especially since there's nothing else within the vxhs
command line syntax that would be "unique" such that the command line
generator had to be changed in order to support it (unlike the
multi-host gluster disks syntax).

I do know the syntax difference has come into play for the rbd driver
decision point to add the password-secret and I'm guessing once secret
support is added for vxhs it'll have a similar issue.

John

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

Re: [libvirt] How to generate better API documentation?

[Martin Kletzander]

On Mon, Jan 16, 2017 at 09:17:03AM +, Daniel P. Berrange wrote:

On Mon, Jan 16, 2017 at 08:24:25AM +0100, Michal Privoznik wrote:

Dear list,

now that we more or less agreed to use some new features (i.e. automatic free() 
when a variable goes out of the scope [1]), and with our NEWS efforts, do you 
think it is finally the right time to fix generation of our API docs too?

What I mean? Look here: [2] People use '@variable' notation hoping that the 
generator will put a link there. Or that our generator would allow us to:

typedef enum {
 A = X,
 B = Y,
 C = Z,
# ifdef VIR_ENUM_SENTINELS
 VIR_ENUM_LAST
# endif
} virEnum;

where X, Y, Z come from another enum.

Long story short, our documentation generator has a lot of bugs. What if
instead of putting our effort in fixing it we would switch to something
that is already available on the market? gtk-doc, doxygen, etc. Personally,
I don't have any preference. Just want to know what your opinion is.


To me the key factor is what the end result looks like, how easy it is to
navigate & find stuff. From that POV doxygen is ruled out as the HTML it
generates is just awful - I despair every time i find a project which has
use doxygen for its API docs :-(

gtk-doc is pretty good in this respect, but the limitation is that they do
not try to support every possible C style - they expect you to write code
in a gobject like style. I can't tell offhand if libvirt is close enough
to work with gtk-doc or not - I do know that QEMU failed. It is something
you'd just have to try and see if it works, and also see if the result is
better than what we have of course.


Again, if I get a green light I can put it onto the list of GSoC projects [3].


I'm not sure this is  a good idea, because I think there's a high chance
that you'd start the effort only to determine we should stick with what
we have already, which would not make for a hugely successful GSoC project.



Actually, comparison of documentation tools can be successful even if we
don't use any at the end.  If there was a value for us (e.g. we know
what we want to use), then the project us successful.  And cleaning up
some parts of the code/comments so that it is usable by the tools is
only part of the job.  Automatic generation of docs is something they
would need to set up for each of the tools, so it would be a value for
us as well.


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

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


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

[libvirt] [PATCH v3 2/2] nodedev: Fabric name must not be required for fc_host capability

[Boris Fiuczynski]

fabric_name is one of many fc_host attributes in Linux that is optional
and left to the low-level driver to decide if it is implemented.
The zfcp device driver does not provide a fabric name for an fcp host.

This patch removes the requirement for a fabric name by making it optional.

Signed-off-by: Boris Fiuczynski 
---
 docs/formatnode.html.in   |  2 +-
 docs/news.xml | 12 ++
 docs/schemas/nodedev.rng  |  8 ---
 src/node_device/node_device_linux_sysfs.c |  9 +++
 tests/fchostdata/fc_host/host6/node_name  |  1 +
 tests/fchostdata/fc_host/host6/port_name  |  1 +
 tests/fchostdata/fc_host/host6/port_state |  1 +
 tests/fchosttest.c| 40 ++-
 8 files changed, 63 insertions(+), 11 deletions(-)
 create mode 100644 tests/fchostdata/fc_host/host6/node_name
 create mode 100644 tests/fchostdata/fc_host/host6/port_name
 create mode 100644 tests/fchostdata/fc_host/host6/port_state

diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index e7b1140..f8d0e12 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -254,7 +254,7 @@
 number of vport in use. max_vports shows the
 maximum vports the HBA supports. "fc_host" implies following
 sub-elements: wwnn, wwpn, and
-fabric_wwn.
+optionally fabric_wwn.
   
 
   
diff --git a/docs/news.xml b/docs/news.xml
index baafcff..002227f 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -158,6 +158,18 @@
 
   
 
+  nodedev: Fabric name must not be required for fc_host capability
+
+
+  fabric_name is one of many fc_host attributes in Linux that is
+  optional and left to the low-level driver to decide if it is
+  implemented. For example the zfcp device driver does not provide a
+  fabric name for an fcp host. The requirement for the existence of
+  a fabric name has been removed by making it optional.
+
+  
+  
+
   qemu: Correct GetBlockInfo values
 
 
diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 9c98402..b100a6e 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -367,9 +367,11 @@
   
 
 
-
-  
-
+
+  
+
+  
+
   
 
   
diff --git a/src/node_device/node_device_linux_sysfs.c 
b/src/node_device/node_device_linux_sysfs.c
index be99c41..13520cd 100644
--- a/src/node_device/node_device_linux_sysfs.c
+++ b/src/node_device/node_device_linux_sysfs.c
@@ -72,13 +72,10 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d)
 VIR_FREE(d->scsi_host.wwnn);
 VIR_STEAL_PTR(d->scsi_host.wwnn, tmp);
 
-if (!(tmp = virReadFCHost(NULL, d->scsi_host.host, "fabric_name"))) {
-VIR_WARN("Failed to read fabric WWN for host%d",
- d->scsi_host.host);
-goto cleanup;
+if ((tmp = virReadFCHost(NULL, d->scsi_host.host, "fabric_name"))) {
+VIR_FREE(d->scsi_host.fabric_wwn);
+VIR_STEAL_PTR(d->scsi_host.fabric_wwn, tmp);
 }
-VIR_FREE(d->scsi_host.fabric_wwn);
-VIR_STEAL_PTR(d->scsi_host.fabric_wwn, tmp);
 }
 
 if (virIsCapableVport(NULL, d->scsi_host.host)) {
diff --git a/tests/fchostdata/fc_host/host6/node_name 
b/tests/fchostdata/fc_host/host6/node_name
new file mode 100644
index 000..73a91bd
--- /dev/null
+++ b/tests/fchostdata/fc_host/host6/node_name
@@ -0,0 +1 @@
+0x2002001b32a9da4e
diff --git a/tests/fchostdata/fc_host/host6/port_name 
b/tests/fchostdata/fc_host/host6/port_name
new file mode 100644
index 000..f25abeb
--- /dev/null
+++ b/tests/fchostdata/fc_host/host6/port_name
@@ -0,0 +1 @@
+0x2102001b32a9da4e
diff --git a/tests/fchostdata/fc_host/host6/port_state 
b/tests/fchostdata/fc_host/host6/port_state
new file mode 100644
index 000..b73dd46
--- /dev/null
+++ b/tests/fchostdata/fc_host/host6/port_state
@@ -0,0 +1 @@
+Online
diff --git a/tests/fchosttest.c b/tests/fchosttest.c
index a08a2e8..bb35b88 100644
--- a/tests/fchosttest.c
+++ b/tests/fchosttest.c
@@ -29,13 +29,16 @@ static char *fchost_prefix;
 
 #define TEST_FC_HOST_PREFIX fchost_prefix
 #define TEST_FC_HOST_NUM 5
+#define TEST_FC_HOST_NUM_NO_FAB 6
 
 /* Test virIsCapableFCHost */
 static int
 test1(const void *data ATTRIBUTE_UNUSED)
 {
 if (virIsCapableFCHost(TEST_FC_HOST_PREFIX,
-   TEST_FC_HOST_NUM))
+   TEST_FC_HOST_NUM) &&
+virIsCapableFCHost(TEST_FC_HOST_PREFIX,
+   TEST_FC_HOST_NUM_NO_FAB))
 return 0;
 
 return -1;
@@ -148,6 +151,39 @@ test5(const void *data ATTRIBUTE_UNUSED)
 return ret;
 }
 
+/* Test virReadFCHost fabric name optional */
+static int
+test6(const void *data 

[libvirt] [PATCH v3 1/2] util: add file exists check in virReadFCHost

[Boris Fiuczynski]

File open errors are prevented by a file exists check before
virFileReadAll is called since all callers of the virReadFCHost
method handle errors themselves based on the NULL return anyway.
Also included is a minor spelling correction in a comment.

Signed-off-by: Boris Fiuczynski 
---
 src/util/virutil.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/util/virutil.c b/src/util/virutil.c
index aeaa7f9..91178d1 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2011,7 +2011,7 @@ virGetSCSIHostNameByParentaddr(unsigned int domain,
  *
  * Read the value of sysfs "fc_host" entry.
  *
- * Returns result as a stringon success, caller must free @result after
+ * Returns result as a string on success, caller must free @result after
  * Otherwise returns NULL.
  */
 char *
@@ -2029,6 +2029,9 @@ virReadFCHost(const char *sysfs_prefix,
 host, entry) < 0)
 goto cleanup;
 
+if (!virFileExists(sysfs_path))
+goto cleanup;
+
 if (virFileReadAll(sysfs_path, 1024, ) < 0)
 goto cleanup;
 
-- 
2.5.5

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

[libvirt] [PATCH v3 0/2] Fabric name must not be required for fc_host capability

[Boris Fiuczynski]

fabric_name is one of many fc_host attributes in Linux that is optional
and left to the low-level driver to decide if it is implemented.

---
Changes sinve v2:
 - removed virReadFCHostOption method
 - changed additional file check to be used permanently

Changes since v1:
 - split of minor correction in documenting comment
 - added news entry...
 - added new virReadFCHostOption method
 - added check for file existence when optional is set
 - rearranged fchost tests to fit the numbering

---
Boris Fiuczynski (2):
  util: add file exists check in virReadFCHost
  nodedev: Fabric name must not be required for fc_host capability

 docs/formatnode.html.in   |  2 +-
 docs/news.xml | 12 ++
 docs/schemas/nodedev.rng  |  8 ---
 src/node_device/node_device_linux_sysfs.c |  9 +++
 src/util/virutil.c|  5 +++-
 tests/fchostdata/fc_host/host6/node_name  |  1 +
 tests/fchostdata/fc_host/host6/port_name  |  1 +
 tests/fchostdata/fc_host/host6/port_state |  1 +
 tests/fchosttest.c| 40 ++-
 9 files changed, 67 insertions(+), 12 deletions(-)
 create mode 100644 tests/fchostdata/fc_host/host6/node_name
 create mode 100644 tests/fchostdata/fc_host/host6/port_name
 create mode 100644 tests/fchostdata/fc_host/host6/port_state

-- 
2.5.5

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

Re: [libvirt] Performance about x-data-plane

[Stefan Hajnoczi]

On Tue, Jan 03, 2017 at 12:02:14PM -0500, Weiwei Jia wrote:
> > The expensive part is the virtqueue kick.  Recently we tried polling the
> > virtqueue instead of waiting for the ioeventfd file descriptor and got
> > double-digit performance improvements:
> > https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00148.html
> >
> > If you want to understand the performance of your benchmark you'll have
> > compare host/guest disk stats (e.g. request lifetime, disk utilization,
> > queue depth, average request size) to check that the bare metal and
> > guest workloads are really sending comparable I/O patterns to the
> > physical disk.
> >
> > Then you using Linux and/or QEMU tracing to analyze the request latency
> > by looking at interesting points in the request lifecycle like virtqueue
> > kick, host Linux AIO io_submit(2), etc.
> >
> 
> Thank you. I will look into "polling the virtqueue" as you said above.
> Currently, I just use blktrace to see disk stats and add logs in the
> I/O workload to see the time latency for each request. What kind of
> tools are you using to analyze request lifecycle like virtqueue kick,
> host Linux AIO iosubmit, etc.
> 
> Do you trace the lifecycle like this
> (http://www.linux-kvm.org/page/Virtio/Block/Latency#Performance_data)
> but it seems to be out of date. Does it
> (http://repo.or.cz/qemu-kvm/stefanha.git/shortlog/refs/heads/tracing-dev-0.12.4)
> still work on QEMU 2.4.1?

The details are out of date but the general approach to tracing the I/O
request lifecycle still apply.

There are multiple tracing tools that can do what you need.  I've CCed
Karl Rister who did the latest virtio-blk dataplane tracing.

"perf record -a -e kvm:\*" is a good start.  You can use "perf probe" to
trace QEMU's trace events (recent versions have sdt support, which means
SystemTap tracepoints work) and also trace any function in QEMU:
http://blog.vmsplice.net/2011/03/how-to-use-perf-probe.html

Stefan


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

[libvirt] [PATCH] qemu_domain: add timestamp in tainting of guests log

[Chen Hanxiao]

From: Chen Hanxiao 

We lacked of timestamp in tainting of guests log,
which bring troubles for finding guest issues:
such as whether a guest powerdown caused by qemu-monitor-command
or others issues inside guests.
If we had timestamp in tainting of guests log,
it would be helpful when checking guest's logs(eg. /var/log/messages).

Signed-off-by: Chen Hanxiao 
---
 src/qemu/qemu_domain.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 35baffb..21c90a9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4001,6 +4001,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver,
 bool closeLog = false;
 
 if (virDomainObjTaint(obj, taint)) {
+char *timestamp = NULL;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 virUUIDFormat(obj->def->uuid, uuidstr);
 
@@ -4018,27 +4019,31 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver,
 logCtxt = qemuDomainLogContextNew(driver, obj,
   
QEMU_DOMAIN_LOG_CONTEXT_MODE_ATTACH);
 if (!logCtxt) {
-if (orig_err) {
-virSetError(orig_err);
-virFreeError(orig_err);
-}
 VIR_WARN("Unable to open domainlog");
-return;
+goto cleanup;
 }
 closeLog = true;
 }
 
+if ((timestamp = virTimeStringNow()) == NULL)
+goto cleanup;
+
 if (qemuDomainLogContextWrite(logCtxt,
-  "Domain id=%d is tainted: %s\n",
+  "%s: Domain id=%d is tainted: %s\n",
+  timestamp,
   obj->def->id,
   virDomainTaintTypeToString(taint)) < 0)
 virResetLastError();
+
+ cleanup:
 if (closeLog)
 qemuDomainLogContextFree(logCtxt);
 if (orig_err) {
 virSetError(orig_err);
 virFreeError(orig_err);
 }
+
+VIR_FREE(timestamp);
 }
 }
 
-- 
2.7.4


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

Re: [libvirt] [PATCH 1/2] perf: Compute cache_l1d config value correctly

[Daniel P. Berrange]

On Sat, Jan 14, 2017 at 01:49:59PM +0530, Nitesh Konkar wrote:
> This patch computes the .attrConfig value for
> cache_l1d correctly and updates the documentation.
> The cache_l1d perf event now is renamed as
> cache_l1dra perf event for measuring read accesses
> for level 1 data cache
> 
> Signed-off-by: Nitesh Konkar 
> ---
>  docs/formatdomain.html.in   | 12 ++--
>  docs/news.xml   |  5 +++--
>  docs/schemas/domaincommon.rng   |  2 +-
>  include/libvirt/libvirt-domain.h| 12 ++--
>  src/libvirt-domain.c|  5 +++--
>  src/qemu/qemu_driver.c  |  2 +-
>  src/util/virperf.c  |  8 +---
>  src/util/virperf.h  |  2 +-
>  tests/genericxml2xmlindata/generic-perf.xml |  2 +-
>  tools/virsh.pod |  6 +++---
>  10 files changed, 30 insertions(+), 26 deletions(-)

I'm not really comfortable with us renaming the public API at this
late stage of the release, as it will have a ripple effect on all
the language bindings too.

IMHO, we need to just stick with this event name now.

> diff --git a/src/util/virperf.c b/src/util/virperf.c
> index 8554723..11e64df 100644
> --- a/src/util/virperf.c
> +++ b/src/util/virperf.c
> @@ -113,9 +113,11 @@ static struct virPerfEventAttr attrs[] = {
>   .attrConfig = 0,
>  # endif
>  },
>   .attrType = PERF_TYPE_HW_CACHE,
> - .attrConfig = PERF_COUNT_HW_CACHE_L1D},
> + .attrConfig = (PERF_COUNT_HW_CACHE_L1D) |
> +   (PERF_COUNT_HW_CACHE_OP_READ << 8) |
> +   (PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16)},
>  };

So this looks like the only part of the patch that we need to
take.

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

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

Re: [libvirt] How to generate better API documentation?

[Daniel P. Berrange]

On Mon, Jan 16, 2017 at 08:24:25AM +0100, Michal Privoznik wrote:
> Dear list,
> 
> now that we more or less agreed to use some new features (i.e. automatic 
> free() when a variable goes out of the scope [1]), and with our NEWS efforts, 
> do you think it is finally the right time to fix generation of our API docs 
> too?
> 
> What I mean? Look here: [2] People use '@variable' notation hoping that the 
> generator will put a link there. Or that our generator would allow us to:
> 
> typedef enum {
>  A = X,
>  B = Y,
>  C = Z,
> # ifdef VIR_ENUM_SENTINELS
>  VIR_ENUM_LAST
> # endif
> } virEnum;
> 
> where X, Y, Z come from another enum.
> 
> Long story short, our documentation generator has a lot of bugs. What if
> instead of putting our effort in fixing it we would switch to something
> that is already available on the market? gtk-doc, doxygen, etc. Personally,
> I don't have any preference. Just want to know what your opinion is.

To me the key factor is what the end result looks like, how easy it is to
navigate & find stuff. From that POV doxygen is ruled out as the HTML it
generates is just awful - I despair every time i find a project which has
use doxygen for its API docs :-(

gtk-doc is pretty good in this respect, but the limitation is that they do
not try to support every possible C style - they expect you to write code
in a gobject like style. I can't tell offhand if libvirt is close enough
to work with gtk-doc or not - I do know that QEMU failed. It is something
you'd just have to try and see if it works, and also see if the result is
better than what we have of course.

> Again, if I get a green light I can put it onto the list of GSoC projects [3].

I'm not sure this is  a good idea, because I think there's a high chance
that you'd start the effort only to determine we should stick with what
we have already, which would not make for a hugely successful GSoC project.

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

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

Re: [libvirt] [PATCH v2] Add support for Veritas HyperScale (VxHS) block device protocol

[Peter Krempa]

On Sat, Jan 14, 2017 at 10:31:05 -0500, John Ferlan wrote:
> [...]
> 
> >> diff --git 
> >> a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args 
> >> b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
> >> new file mode 100644
> >> index 000..f6e3e37
> >> --- /dev/null
> >> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args

[...]

> >> +-drive file=vxhs://192.168.0.1:/eb90327c-8302-4725-9e1b-4e85ed4dc251,\
> >> +format=raw,if=none,id=drive-virtio-disk0,cache=none \
> > 
> > Please use the modern syntax for disk specification, not the
> > legacy URI syntax  ie
> > 
> >   -drive  driver=vxhs,vdisk-id=eb90327c-8302-4725-4e85ed4dc251,\
> >   server.host=example.com,server.port=12345,format=raw,
> >   if=none,id=driver-virtio-disk0,cache=0
> > 
> 
> FWIW: qemuBuildDriveStr doesn't support that yet for any driver -
> although it probably should be updated... Timing wise whether that work
> could get done before this work is ready - I'm not sure. While I agree
> the code "should" use the new syntax - I do think we'd have to make
> adjustments to all drive strings. That would probably mean knowing
> whether the new syntax was "in place" before the current libvirt qemu
> minimum version...

That's not true. The new syntax is used for multi-host gluster disks
(qemuGetDriveSourceProps). Also we have QMP introspection code in place
which can detect whether given configuration option is supported by the
given qemu (virQEMUCapsQMPSchemaQueries[]).

Also we should not fully update the command line generator in cases
where it's not necessary as it would just make the code more complex
without any real benefit. The new protocols can use the new syntax
though.

Peter


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

Re: [libvirt] [PATCH v2] Add support for Veritas HyperScale (VxHS) block device protocol

[Daniel P. Berrange]

On Sat, Jan 14, 2017 at 10:31:05AM -0500, John Ferlan wrote:
> [...]
> 
> >> diff --git 
> >> a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args 
> >> b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
> >> new file mode 100644
> >> index 000..f6e3e37
> >> --- /dev/null
> >> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-vxhs.args
> >> @@ -0,0 +1,24 @@
> >> +LC_ALL=C \
> >> +PATH=/bin \
> >> +HOME=/home/test \
> >> +USER=test \
> >> +LOGNAME=test \
> >> +QEMU_AUDIO_DRV=none \
> >> +/usr/libexec/qemu-kvm \
> >> +-name QEMUGuest1 \
> >> +-S \
> >> +-M pc \
> >> +-cpu qemu32 \
> >> +-m 214 \
> >> +-smp 1,sockets=1,cores=1,threads=1 \
> >> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> >> +-nographic \
> >> +-nodefaults \
> >> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> >> +-no-acpi \
> >> +-boot c \
> >> +-usb \
> >> +-drive file=vxhs://192.168.0.1:/eb90327c-8302-4725-9e1b-4e85ed4dc251,\
> >> +format=raw,if=none,id=drive-virtio-disk0,cache=none \
> > 
> > Please use the modern syntax for disk specification, not the
> > legacy URI syntax  ie
> > 
> >   -drive  driver=vxhs,vdisk-id=eb90327c-8302-4725-4e85ed4dc251,\
> >   server.host=example.com,server.port=12345,format=raw,
> >   if=none,id=driver-virtio-disk0,cache=0
> > 
> 
> FWIW: qemuBuildDriveStr doesn't support that yet for any driver -
> although it probably should be updated... Timing wise whether that work
> could get done before this work is ready - I'm not sure. While I agree
> the code "should" use the new syntax - I do think we'd have to make
> adjustments to all drive strings. That would probably mean knowing
> whether the new syntax was "in place" before the current libvirt qemu
> minimum version...

The pre-existing drivers in libvirt need to support both new & old syntax,
in order that we have compat with older QEMU versions. As a brand new driver,
IMHO, VXHS must support the new syntax exclusively - there's no reason to
use the old syntax. We don't need to convert all existing drivers to the
new syntax at the same time, but the VHXS patch can start the trend.

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

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

Re: [libvirt] How to generate better API documentation?

[Michal Privoznik]

On 16.01.2017 08:24, Michal Privoznik wrote:
> Dear list,
> 
> now that we more or less agreed to use some new features (i.e. automatic 
> free() when a variable goes out of the scope [1]), and with our NEWS efforts, 
> do you think it is finally the right time to fix generation of our API docs 
> too?
> 
> What I mean? Look here: [2] People use '@variable' notation hoping that the 
> generator will put a link there. Or that our generator would allow us to:
> 
> typedef enum {
>  A = X,
>  B = Y,
>  C = Z,
> # ifdef VIR_ENUM_SENTINELS
>  VIR_ENUM_LAST
> # endif
> } virEnum;
> 
> where X, Y, Z come from another enum.

Also, there is no link validation. For instance, I'm going through our
network docs [3] reading about trustGuestRxFilters. And the paragraph
links non-existent anchor #elementSNICS in domain XML page.
While this one can be fixed easily, checking for others would be a waste
of time.

Michal

3: http://libvirt.org/formatnetwork.html#elementsMetadata

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