[libvirt] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features()

[Eduardo Habkost]

x86_cpu_filter_features() will be reused by code that shouldn't
print any warning. Move the warning code to a new
x86_cpu_report_filtered_features() function, and call it from
x86_cpu_realizefn().

Signed-off-by: Eduardo Habkost 
---
Changes v5 -> v6:
* Recovered v3 of patch, because x86_cpu_load_features()
  won't call x86_cpu_filter_features() anymore and we can keep
  the previous logic in x86_cpu_realizefn() that checked
  x86_cpu_filter_features() return value

Changes v4 -> v5:
* (none)

Changes v3 -> v4:
* Made x86_cpu_filter_features() void, make
  x86_cpu_report_filtered_features() return true if
  some features were filtered
---
 target-i386/cpu.c | 28 +++-
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 61240dd..1e8127b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2177,9 +2177,6 @@ static int x86_cpu_filter_features(X86CPU *cpu)
 env->features[w] &= host_feat;
 cpu->filtered_features[w] = requested_features & ~env->features[w];
 if (cpu->filtered_features[w]) {
-if (cpu->check_cpuid || cpu->enforce_cpuid) {
-report_unavailable_features(w, cpu->filtered_features[w]);
-}
 rv = 1;
 }
 }
@@ -2187,6 +2184,15 @@ static int x86_cpu_filter_features(X86CPU *cpu)
 return rv;
 }
 
+static void x86_cpu_report_filtered_features(X86CPU *cpu)
+{
+FeatureWord w;
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+report_unavailable_features(w, cpu->filtered_features[w]);
+}
+}
+
 static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
 {
 PropValue *pv;
@@ -3080,12 +3086,16 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
 }
 
-if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
-error_setg(_err,
-   kvm_enabled() ?
-   "Host doesn't support requested features" :
-   "TCG doesn't support requested features");
-goto out;
+if (x86_cpu_filter_features(cpu) &&
+(cpu->check_cpuid || cpu->enforce_cpuid)) {
+x86_cpu_report_filtered_features(cpu);
+if (cpu->enforce_cpuid) {
+error_setg(_err,
+   kvm_enabled() ?
+   "Host doesn't support requested features" :
+   "TCG doesn't support requested features");
+goto out;
+}
 }
 
 /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
-- 
2.7.4

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

[libvirt] [PATCH v6 2/3] target-i386: x86_cpu_load_features() function

[Eduardo Habkost]

When probing for CPU model information, we need to reuse the code
that initializes CPUID fields, but not the remaining side-effects
of x86_cpu_realizefn(). Move that code to a separate function
that can be reused later.

Signed-off-by: Eduardo Habkost 
---
Changes v5 -> v6:
* Move x86_cpu_filter_features() outside x86_cpu_load_features(),
  as the CPU model querying API won't run
  x86_cpu_filter_features() on most cases

Changes v4 -> v5:
* Fix typo on x86_cpu_load_features() comment
  Reported-by: Paolo Bonzini 

Changes series v3 -> v4:
* New patch added to series
---
 target-i386/cpu.c | 67 +++
 1 file changed, 43 insertions(+), 24 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1e8127b..23cc19b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2993,34 +2993,13 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu)
 env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
 }
 
-#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
-   (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \
-   (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
-#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
- (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
- (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
-static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
+/* Load CPUID data based on configured features */
+static void x86_cpu_load_features(X86CPU *cpu, Error **errp)
 {
-CPUState *cs = CPU(dev);
-X86CPU *cpu = X86_CPU(dev);
-X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
 CPUX86State *env = >env;
-Error *local_err = NULL;
-static bool ht_warned;
 FeatureWord w;
 GList *l;
-
-if (xcc->kvm_required && !kvm_enabled()) {
-char *name = x86_cpu_class_get_model_name(xcc);
-error_setg(_err, "CPU model '%s' requires KVM", name);
-g_free(name);
-goto out;
-}
-
-if (cpu->apic_id == UNASSIGNED_APIC_ID) {
-error_setg(errp, "apic-id property was not initialized properly");
-return;
-}
+Error *local_err = NULL;
 
 /*TODO: cpu->host_features incorrectly overwrites features
  * set using "feat=on|off". Once we fix this, we can convert
@@ -3086,6 +3065,46 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
 }
 
+out:
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+}
+}
+
+#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \
+   (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \
+   (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
+#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
+ (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
+ (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
+static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
+{
+CPUState *cs = CPU(dev);
+X86CPU *cpu = X86_CPU(dev);
+X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
+CPUX86State *env = >env;
+Error *local_err = NULL;
+static bool ht_warned;
+
+if (xcc->kvm_required && !kvm_enabled()) {
+char *name = x86_cpu_class_get_model_name(xcc);
+error_setg(_err, "CPU model '%s' requires KVM", name);
+g_free(name);
+goto out;
+}
+
+if (cpu->apic_id == UNASSIGNED_APIC_ID) {
+error_setg(errp, "apic-id property was not initialized properly");
+return;
+}
+
+x86_cpu_load_features(cpu, _err);
+if (local_err) {
+goto out;
+}
+
+x86_cpu_filter_features(cpu);
+
 if (x86_cpu_filter_features(cpu) &&
 (cpu->check_cpuid || cpu->enforce_cpuid)) {
 x86_cpu_report_filtered_features(cpu);
-- 
2.7.4

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

[libvirt] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions

[Eduardo Habkost]

Fill the "unavailable-features" field on the x86 implementation
of query-cpu-definitions.

Cc: Jiri Denemark 
Cc: libvir-list@redhat.com
Signed-off-by: Eduardo Habkost 
---
Changes v5 -> v6:
* Call x86_cpu_filter_features(), now that x86_cpu_load_features()
  won't run it automatically

Changes v4 -> v5:
* (none)

Changes v3 -> v4:
* Handle missing XSAVE components cleanly, but looking up
  the original feature that required it
* Use x86_cpu_load_features() function

Changes v2 -> v3:
* Create a x86_cpu_feature_name() function, to
  isolate the code that returns the property name

Changes v1 -> v2:
* Updated to the new schema: no @runnable field, and
  always report @unavailable-features as present
---
 target-i386/cpu.c | 76 +++
 1 file changed, 76 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 23cc19b..63330ce 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s)
 }
 }
 
+/* Return the feature property name for a feature flag bit */
+static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
+{
+/* XSAVE components are automatically enabled by other features,
+ * so return the original feature name instead
+ */
+if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) {
+int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr;
+
+if (comp < ARRAY_SIZE(x86_ext_save_areas) &&
+x86_ext_save_areas[comp].bits) {
+w = x86_ext_save_areas[comp].feature;
+bitnr = ctz32(x86_ext_save_areas[comp].bits);
+}
+}
+
+assert(bitnr < 32);
+assert(w < FEATURE_WORDS);
+return feature_word_info[w].feat_names[bitnr];
+}
+
 /* Compatibily hack to maintain legacy +-feat semantic,
  * where +-feat overwrites any feature set by
  * feat=on|feat even if the later is parsed after +-feat
@@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char 
*typename, char *features,
 }
 }
 
+static void x86_cpu_load_features(X86CPU *cpu, Error **errp);
+static int x86_cpu_filter_features(X86CPU *cpu);
+
+/* Check for missing features that may prevent the CPU class from
+ * running using the current machine and accelerator.
+ */
+static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
+ strList **missing_feats)
+{
+X86CPU *xc;
+FeatureWord w;
+Error *err = NULL;
+strList **next = missing_feats;
+
+if (xcc->kvm_required && !kvm_enabled()) {
+strList *new = g_new0(strList, 1);
+new->value = g_strdup("kvm");;
+*missing_feats = new;
+return;
+}
+
+xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc;
+
+x86_cpu_load_features(xc, );
+if (err) {
+/* Errors at x86_cpu_load_features should never happen,
+ * but in case it does, just report the model as not
+ * runnable at all using the "type" property.
+ */
+strList *new = g_new0(strList, 1);
+new->value = g_strdup("type");
+*next = new;
+next = >next;
+}
+
+x86_cpu_filter_features(xc);
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+uint32_t filtered = xc->filtered_features[w];
+int i;
+for (i = 0; i < 32; i++) {
+if (filtered & (1UL << i)) {
+strList *new = g_new0(strList, 1);
+new->value = g_strdup(x86_cpu_feature_name(w, i));
+*next = new;
+next = >next;
+}
+}
+}
+
+object_unref(OBJECT(xc));
+}
+
 /* Print all cpuid feature names in featureset
  */
 static void listflags(FILE *f, fprintf_function print, const char **featureset)
@@ -2122,6 +2196,8 @@ static void x86_cpu_definition_entry(gpointer data, 
gpointer user_data)
 
 info = g_malloc0(sizeof(*info));
 info->name = x86_cpu_class_get_model_name(cc);
+x86_cpu_class_check_missing_features(cc, >unavailable_features);
+info->has_unavailable_features = true;
 
 entry = g_malloc0(sizeof(*entry));
 entry->value = info;
-- 
2.7.4

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

[libvirt] [PATCH v6 0/3] Add runnability info to query-cpu-definitions

[Eduardo Habkost]

This series extends query-cpu-definitions to include an extra
field: "unavailable-features". The new field can be used to find
out reasons that prevent the CPU model from running in the
current host.

This will return information based on the current machine and
accelerator only. In the future we may extend these mechanisms to
allow querying other machines and other accelerators without
restarting QEMU, but it will require some reorganization of
QEMU's main code.

To be able to implement this more cleanly, the series rework some
of the feature/property name code.

This series can be seen in the git branch at:
  https://github.com/ehabkost/qemu-hacks.git 
work/query-cpu-definitions-runnable-info

The series is based on my x86-next branch:
   https://github.com/ehabkost/qemu.git x86-next

Changes v5 -> v6:
* Rebased to x86-next, that already has 8 of the previous patches
  from v5 applied
* Removed x86_cpu_filter_features() from x86_cpu_load_features(),
  because some of the commands in the CPU model query API need
  info about CPU models before filtering
* Recovered v3 of
  "target-i386: Move warning code outside x86_cpu_filter_features()"
  because now we can keep the simpler logic that checked
  the return value of x86_cpu_filter_features()

Diff v5 -> v6:

  diff --git a/target-i386/cpu.c b/target-i386/cpu.c
  index d53e711..63330ce 100644
  --- a/target-i386/cpu.c
  +++ b/target-i386/cpu.c
  @@ -2052,6 +2052,7 @@ static void x86_cpu_parse_featurestr(const char 
*typename, char *features,
   }

   static void x86_cpu_load_features(X86CPU *cpu, Error **errp);
  +static int x86_cpu_filter_features(X86CPU *cpu);

   /* Check for missing features that may prevent the CPU class from
* running using the current machine and accelerator.
  @@ -2085,6 +2086,8 @@ static void 
x86_cpu_class_check_missing_features(X86CPUClass *xcc,
   next = >next;
   }

  +x86_cpu_filter_features(xc);
  +
   for (w = 0; w < FEATURE_WORDS; w++) {
   uint32_t filtered = xc->filtered_features[w];
   int i;
  @@ -2234,11 +2237,14 @@ static uint32_t 
x86_cpu_get_supported_feature_word(FeatureWord w,

   /*
* Filters CPU feature words based on host availability of each feature.
  + *
  + * Returns: 0 if all flags are supported by the host, non-zero otherwise.
*/
  -static void x86_cpu_filter_features(X86CPU *cpu)
  +static int x86_cpu_filter_features(X86CPU *cpu)
   {
   CPUX86State *env = >env;
   FeatureWord w;
  +int rv = 0;

   for (w = 0; w < FEATURE_WORDS; w++) {
   uint32_t host_feat =
  @@ -2246,22 +2252,21 @@ static void x86_cpu_filter_features(X86CPU *cpu)
   uint32_t requested_features = env->features[w];
   env->features[w] &= host_feat;
   cpu->filtered_features[w] = requested_features & ~env->features[w];
  +if (cpu->filtered_features[w]) {
  +rv = 1;
  +}
   }
  +
  +return rv;
   }

  -/* Report list of filtered features to stderr.
  - * Returns true if some features were found to be filtered, false otherwise
  - */
  -static bool x86_cpu_report_filtered_features(X86CPU *cpu)
  +static void x86_cpu_report_filtered_features(X86CPU *cpu)
   {
   FeatureWord w;
  -uint32_t filtered = 0;

   for (w = 0; w < FEATURE_WORDS; w++) {
  -filtered |= cpu->filtered_features[w];
   report_unavailable_features(w, cpu->filtered_features[w]);
   }
  -return filtered;
   }

   static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
  @@ -3136,8 +3141,6 @@ static void x86_cpu_load_features(X86CPU *cpu, Error 
**errp)
   env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
   }

  -x86_cpu_filter_features(cpu);
  -
   out:
   if (local_err != NULL) {
   error_propagate(errp, local_err);
  @@ -3176,8 +3179,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
   goto out;
   }

  -if (cpu->check_cpuid || cpu->enforce_cpuid) {
  -if (x86_cpu_report_filtered_features(cpu) && cpu->enforce_cpuid) {
  +x86_cpu_filter_features(cpu);
  +
  +if (x86_cpu_filter_features(cpu) &&
  +(cpu->check_cpuid || cpu->enforce_cpuid)) {
  +x86_cpu_report_filtered_features(cpu);
  +if (cpu->enforce_cpuid) {
   error_setg(_err,
  kvm_enabled() ?
  "Host doesn't support requested features" :

Changes v4 -> v5:
* New patch: "target-i386: Register aliases for feature names with underscores"
* On patch: "tests: Add test case for x86 feature parsing compatibility":
  * Fix typo on commit message
Reported-by: Jonathan Neuschäfer 
  * Add comment noting that the "[+-]feature" compatibility mode
will be removed soon
* On patch: "target-i386: Make plus_features/minus_features QOM-based":
  * Removed feat2prop() call on , as we now have property aliases
for the old names containing underscores
* On patch: 

Re: [libvirt] [Qemu-devel] [PATCH v5 11/12] qmp: Add runnability information to query-cpu-definitions

[Eduardo Habkost]

On Fri, Sep 30, 2016 at 03:49:45PM -0300, Eduardo Habkost wrote:
> Add a new optional field to query-cpu-definitions schema:
> "unavailable-features". It will contain a list of QOM properties
> that prevent the CPU model from running in the current host.
> 
> Cc: David Hildenbrand 
> Cc: Michael Mueller 
> Cc: Christian Borntraeger 
> Cc: Cornelia Huck 
> Cc: Jiri Denemark 
> Cc: libvir-list@redhat.com
> Reviewed-by: Eric Blake 
> Signed-off-by: Eduardo Habkost 

Applied this to x86-next.

-- 
Eduardo

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

Re: [libvirt] [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Alex Williamson]

On Fri, 7 Oct 2016 10:46:22 +0530
Kirti Wankhede  wrote:

> Ping..
> 
> Pulling the questions at the top.
> 
> >> Will libvirt report 'description' RO attribute, its output would be
> >> string, so that user could be able to see the configuration of that
> >> profile?
> >>  
> >
> > Daniel,
> > Waiting for your input on this.
> >  
> >> We can have 'class' as optional attribute. So Intel don't have to
> >> provide 'class' attribute and they don't have to specify mandatory
> >> attributes of that class. We would provide 'class' attribute and provide
> >> mandatory attributes.  

Hi Kirti,

I would proceed with your design and posting, this is not a
sufficiently significant issue to delay a new version.  The next
posting will likely not be the last, more feedback will come.

Personally I don't see an issue.  We should have a small number
of mandatory sysfs interfaces for mdev devices, essentially just
enough to manage the life cycle of a device, even if it requires
vendor specific knowledge to understand the exact composition of
that device.  Some sort of human readable description seems like
a reasonable optional feature for a userspace tool like libvirt
to take advantage of.  A class definition with an available set
of attributes per class would clearly enhance the ability of a
userspace tool to understand the device, but does not seem
strictly required for managing the device.  Thanks,

Alex

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

Re: [libvirt] [PATCH 2/2] tools: Exclude Xen dom0 from libvirt-guests.sh list

[Eric Blake]

On 10/07/2016 02:56 AM, Stefan Bader wrote:
> With newer versions of libvirt Domain-0 is again visible in the list of
> running guests but it should not be considered as a guest for shutdown
> or suspend.
> 
> Signed-off-by Stefan Bader 
> ---
>  tools/libvirt-guests.sh.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

ACK.

> 
> diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
> index 7380b4b..791d927 100644
> --- a/tools/libvirt-guests.sh.in
> +++ b/tools/libvirt-guests.sh.in
> @@ -121,7 +121,7 @@ list_guests() {
>  return 1
>  fi
>  
> -echo $list
> +echo "$list" | grep -v ----
>  }
>  
>  # guest_name URI UUID
> @@ -539,7 +539,7 @@ gueststatus() {
>  for uri in $URIS; do
>  set +f
>  echo "* $uri URI:"
> -retval run_virsh "$uri" list || echo
> +retval run_virsh "$uri" list | grep -v "Domain-0" || echo
>  done
>  set +f
>  }
> 

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



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

Re: [libvirt] [PATCH 1/2] tools: Ignore newlines in libvirt-guests.sh guest list

[Eric Blake]

On 10/07/2016 02:56 AM, Stefan Bader wrote:
> The list file expects all guest UUIDs on the same line as the URI
> which the guests run on. This does not happen when the list is
> echo'ed in quotes. When stripping the quotes, newlines get transformed
> into spaces. Without this, only the first guest on the list is actually
> handled.
> 
> Based on a fix by Omar Siam 
> 
> Bug-Ubuntu: http://bugs.launchpad.net/bugs/1591695
> 
> Signed-off-by: Stefan Bader 
> ---
>  tools/libvirt-guests.sh.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

ACK.

> 
> diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
> index 7f74b85..7380b4b 100644
> --- a/tools/libvirt-guests.sh.in
> +++ b/tools/libvirt-guests.sh.in
> @@ -499,7 +499,7 @@ stop() {
>  fi
>  
>  if [ -n "$list" ]; then
> -echo "$uri" "$list" >>"$LISTFILE"
> +echo "$uri" $list >>"$LISTFILE"

This is one case where "" is harmful - we WANT the shell to perform
word-splitting, and the content of $list should be safe for splitting
(ie. since all elements of the list are UUIDs, none of them should
contain any other whitespace or shell metacharacters that would
misbehave by losing the "").


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



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

Re: [libvirt] RFC: Exposing "ready" bool (of `query-block-jobs`) or QMP BLOCK_JOB_READY event

[Eric Blake]

On 10/07/2016 02:09 AM, Peter Krempa wrote:

>> The existing virDomainGetBlockJobInfo() can't be extended, but it can be
>> fixed to quit reporting cur==end when ready:false.
> 
> Yes, I agree about this one (although I don't really like it [1]), but
> this one will actually fix software not listening for events without any
> change.
> 
> Any new API would not help since the apps would need to change anyways
> thus can use the current correct approach right away even with older
> libvirt versions.
> 
> Peter
> 
> [1]: I'm expecting users to start complaining: "Why is this last byte of
> my image taking so long to copy after the rest copied pretty quickly".

And our response is "We never promised that cur and end are bytes, only
rough status indicators. And we don't know why qemu is taking so long -
move the bug report to them" - if the user can even see this state long
enough for it to bother them.  (Nova is hitting it, because it is a
software-triggered reaction time, not a human in the mix; my
understanding is that it is still at most a fraction of a second where
we'd even have to do the fudging).

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



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

Re: [libvirt] [Qemu-devel] [PATCH v5 00/12] Add runnability info to query-cpu-definitions

[Eduardo Habkost]

On Fri, Sep 30, 2016 at 03:49:34PM -0300, Eduardo Habkost wrote:
[...]
> Eduardo Habkost (12):
>   tests: Add test case for x86 feature parsing compatibility
>   target-i386: List CPU models using subclass list
>   target-i386: Disable VME by default with TCG
>   target-i386: Register aliases for feature names with underscores
>   target-i386: Make plus_features/minus_features QOM-based
>   target-i386: Remove underscores from feat_names arrays
>   target-i386: Register properties for feature aliases manually
>   target-i386: xsave: Add FP and SSE bits to x86_ext_save_areas

Patches 1-8 (above) were applied to x86-next. I will send v6 of
9-12 (below), with a small change.

>   target-i386: Move warning code outside x86_cpu_filter_features()
>   target-i386: x86_cpu_load_features() function
>   qmp: Add runnability information to query-cpu-definitions
>   target-i386: Return runnability information on query-cpu-definitions
> 
>  qapi-schema.json  |  23 +-
>  target-i386/cpu-qom.h |   4 +
>  target-i386/cpu.c | 510 
> ++
>  tests/test-x86-cpuid-compat.c |  44 
>  4 files changed, 388 insertions(+), 193 deletions(-)
> 
-- 
Eduardo

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

Re: [libvirt] [PATCH] vz: fix vnc host address

[John Ferlan]

On 10/07/2016 12:05 PM, Mikhail Feoktistov wrote:
> Empty string means 127.0.0.1 in terms of vzSDK
> But in this case we should set 0.0.0.0
> ---
>  .gnulib | 2 +-

^^^
>  src/vz/vz_sdk.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 


You need to update your branch before having this pushed...

git submodule update --init --recursive

John

> diff --git a/.gnulib b/.gnulib
> index e89b4a7..a2a3943 16
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit e89b4a7aefce9cb02963920712ba7cdd13641644
> +Subproject commit a2a39436b65f329630df4a93ec4e30aeae403c54
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index f2a5c96..7011cbf 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -2967,7 +2967,7 @@ static int prlsdkApplyGraphicsParams(PRL_HANDLE sdkdom,
>  
>  glisten = virDomainGraphicsGetListen(gr, 0);
>  pret = PrlVmCfg_SetVNCHostName(sdkdom, glisten && glisten->address ?
> -   glisten->address : "");
> +   glisten->address : "0.0.0.0");
>  prlsdkCheckRetGoto(pret, cleanup);
>  
>  ret = 0;
> 

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

Re: [libvirt] [PATCH rfc v2 0/8] fspool: backend directory

[Olga Krishtal]

On 24/09/16 00:12, John Ferlan wrote:


On 09/23/2016 11:56 AM, Olga Krishtal wrote:

On 21/09/16 19:17, Maxim Nestratov wrote:

20 сент. 2016 г., в 23:52, John Ferlan  написал(а):




On 09/15/2016 03:32 AM, Olga Krishtal wrote:
Hi everyone, we would like to propose the first implementation of fspool
with directory backend.

Filesystem pools is a facility to manage filesystems resources similar
to how storage pools manages volume resources. Furthermore new API follows
storage API closely where it makes sense. Uploading/downloading operations
are not defined yet as it is not obvious how to make it properly. I guess
we can use some kind of tar to make a stream from a filesystem. Please share
you thoughts on this particular issue.

So how do you differentiate between with the existing 

Pool type=fs still provides volumes, i. e. block devices rather than 
filesystem, though this storage pool can mount file systems resided on a source 
block device.


http://libvirt.org/storage.html#StorageBackendFS

Sure the existing fs pool requires/uses a source block device as the
source path and this new variant doesn't require that source but seems
to use some item in order to dictate how to "define" the source on the
fly. Currently only a "DIR" is created - so how does that differ from a
"dir" pool.


Same here, storage "dir" provides files, which are in fact block devices for guests. 
While filesystem pool "dir" provides guests with file systems.



I think it'll be confusing to have and differentiate fspool and pool
commands.

Some time ago, we wrote the proposal description and asked for
everyone's advice and opinion.
The aim of fspool is to provide filesystems, not volumes. The simplest
type of fspool is directory pool and
it do has a lot in common with storage_backend_fs. However,  in the
proposal description we said that
the plan is to use other backends:  eg, storage volumes from storage
pool as the source of fs, zfs, etc.
  The final api for fspool will be significantly different, because of
the other backends needs.

Can you please try to create an extra line after the paragraph you're
responding to and the start of your paragraph and then one after.


Thanks for noticing. It looks better.


Anyway, as I pointed out - that description wasn't in my (short term)
memory. Keeping a trail of pointers to previous stuff helps those that
want to refresh their memory on the history.


I will hold this links through the next versions.
https://www.redhat.com/archives/libvir-list/2016-April/msg01941.html
https://www.redhat.com/archives/libvir-list/2016-May/msg00208.html



If you're going to "reuse" things, then using the 'src/util/*' is the
way to go rather than trying to drag in storage_{driver|backend*} APIs.
Crossing driver boundaries is something IIRC we try to avoid.


As I have written before at the moment we have only one backend for 
fspool -

directory. It is the simplest backend and only the starting point.
I think that it is too early to decide which parts should be moved to 
src/util/*.
Moreover,  as fspool items and storage pool volumes are pretty 
different, it
could be possible that they have very little in common. That said, I 
would leave

things as they are, but if you insist I can try.


I didn't dig through all the patches, but from the few I did look at it
seems as though all that's done is to rip out the guts of stuff not
desired from the storage pool driver and replace it with this new code
attributing all the work to the new author/copyright. IOW: Lots of
places where StoragePool appears to be exactly the same as the FSPool.


I have written this lines as a part of  GPLv2+ boilerplate:
https://www.redhat.com/archives/libvir-list/2016-August/msg01160.html, 
which I took from
other libvirt parts. And I guess it was naturally to change name and 
company, don't you?
And again, if you insist I can leave out the author/copyright as it 
wasn't the aim of this series.




I think you need to find a different means to do what you want. It's not
100% what the end goal is.I did download/git am the patches and scan a few 
patches...
  * In patch 2 you've totally missed how to modify libvirt_public.syms
  * In patch 3, the build breaks in "conf/fs_conf" since the "if { if {}
}" aren't done properly in virFSPoolDefFormatBuf.
  * In patch 5 the remote_protocol_structs fails check/syntax-check... I
stopped there in my build each patch test.

According to the guide I have to do:
|make check| and |make syntax-check for every patch|

Always a good plan!


And it was done.

And yet as we find out *all the time* some compilers complain more than
others. Watch the list - we have a CI environment in which we find all
sorts of oddities.  In any case, the code in question is:

+if (def->target.perms.mode != (mode_t) -1 ||
+def->target.perms.uid != (uid_t) -1 ||
+def->target.perms.gid != (gid_t) -1 ||
+def->target.perms.label) {
+virBufferAddLit(buf, "\n");
+

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

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> This is needed in order to migrate a domain with shmem devices as that
> is not allowed to migrate.

Sure, but how would anyone know since the migration fails... Not sure
where could/should describe it, but perhaps something nice to be
described somewhere...

> 
> 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

Jeez someday I should try to learn how to use these hotplug tests ;-)

[...]

> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 72dd93bbe467..03d75be5e3d7 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2242,6 +2242,131 @@ qemuDomainAttachHostDevice(virConnectPtr conn,
>  return -1;
>  }
> 
> +
> +int
> +qemuDomainAttachShmemDevice(virQEMUDriverPtr driver,
> +virDomainObjPtr vm,
> +virDomainShmemDefPtr shmem)
> +{
> +int ret = -1;
> +char *shmstr = NULL;
> +char *charAlias = NULL;
> +char *memAlias = NULL;
> +bool release_backing = false;
> +bool release_address = true;
> +virErrorPtr orig_err = NULL;
> +virJSONValuePtr props = NULL;
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +switch ((virDomainShmemModel)shmem->model) {
> +case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
> +case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:


This is where I'd expect the capabilities checks to be


> +break;
> +
> +case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +   _("live attach of shmem model '%s' is not supported"),
> +   virDomainShmemModelTypeToString(shmem->model));
> +/* fall-through */
> +case VIR_DOMAIN_SHMEM_MODEL_LAST:
> +return -1;
> +}
> +
> +if (qemuAssignDeviceShmemAlias(vm->def, shmem, -1) < 0)
> +return -1;
> +
> +if (qemuDomainPrepareShmemChardev(shmem) < 0)
> +return -1;
> +
> +if (VIR_REALLOC_N(vm->def->shmems, vm->def->nshmems + 1) < 0)
> +return -1;
> +
> +if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
> + shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
> + (virDomainPCIAddressEnsureAddr(priv->pciaddrs, >info) < 0))
> +return -1;
> +
> +if (!(shmstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))
> +goto cleanup;
> +
> +if (shmem->server.enabled) {
> +if (virAsprintf(, "char%s", shmem->info.alias) < 0)
> +goto cleanup;
> +} else {
> +if (!(props = qemuBuildShmemBackendMemProps(shmem)))
> +goto cleanup;
> +
> +if (virAsprintf(, "shmmem-%s", shmem->info.alias) < 0)
> +goto cleanup;
> +}
> +
> +qemuDomainObjEnterMonitor(driver, vm);
> +
> +if (shmem->server.enabled) {
> +if (qemuMonitorAttachCharDev(priv->mon, charAlias,
> + >server.chr) < 0)
> +goto exit_monitor;
> +} else {
> +if (qemuMonitorAddObject(priv->mon, "memory-backend-file",
> + memAlias, props) < 0) {

Some day we should change the AddObject to be  

> +props = NULL;
> +goto exit_monitor;
> +}
> +props = NULL;
> +}
> +
> +release_backing = true;
> +
> +if (qemuMonitorAddDevice(priv->mon, 

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

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> 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.
> 

It seems less "added support for" and more "forced libvirt to choose"
between one or the other as of qemu v2.6.

> 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)

Still would need to FreeAndReset - I'd be OK if it were an || to the
previous if, although I know that causes agita for others.

I suppose you could to the error: label path/option too.

> +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);

Hmm... perhaps not so much of an issue as previously thought... This
will only be called for the -plain anyway and well shmem->size had
better not be zero (since you're using "U:")..

So still leaves me wondering if we should fail if a size is provided for
-doorbell

> +
> +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;
> 

Should there be caps checks for :

QEMU_CAPS_DEVICE_IVSHMEM_PLAIN
QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL

You added the caps in the xml2argvtest, so I'd expect them...  Obviously
they wouldn't work on qemu 2.5 or earlier.

As long as the memory leak is handled and there's an answer for the caps
checks, this is ACK-able from my POV.

John
>  case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
> +

Re: [libvirt] Analysis of the effect of adding PCIe root ports

[Laine Stump]

On 10/07/2016 11:16 AM, Andrea Bolognani wrote:

On Fri, 2016-10-07 at 10:17 -0400, Laine Stump wrote:

So here's a rewording of your description (with a couple additional
conditions) to see if I understand everything correctly:
  
1) during initial domain definition:
  
A) If there are *no pci controllers at all* (not even a pci-root or

pcie-root) *and there are any unaddressed devices that need a PCI slot*
then auto-add enough controllers for the requested devices, *and* make
sure there are enough empty slots for "N" (do we stick with 4? or make
it 3?) devices to be added later without needing more controllers. (So,
if the domain has no PCI devices, we don't add anything extra, and also
if it only has PCI devices that already have addresses, then we also
don't add anything extra).
  
B) if there is at least one pci controller specified in the XML, and

there are any unused slots in the pci controllers in the provided XML,
then use them for the unaddressed devices. If there are more devices
that need an address at this time, also add controllers for them, but no
*extra* controllers.
  
(Note to Rich: libguestfs could avoid the extra controllers either by

adding a pci-root/pcie-root to the XML, or by manually addressing the
devices. The latter would actually be better, since it would avoid the
need for any pcie-root-ports).
  
2) When adding a device to the persistent config (i.e. offline): if

there is an empty slot on a controller, use it. If not, add a controller
for that device *but no extra controllers*
  
3) when adding a device to the guest machine (i.e. hotplug / online), if

there is an empty slot on a controller, use it. If not, then fail.
  
The differences I see from what (I think) you suggested are:
  
* if there aren't  any unaddressed pci devices (even if there are no

controllers in the config), then we also don't add any extra controllers
(although we will of course add the pci-root or pcie-root, to
acknowledge it is there).
  
* if another controller is needed for adding a device offline, it's okay

to add it.

So instead of guaranteeing that there will always be an empty
slot available for hotplug during a single start/destroy
cycle of the guest, we would be guaranteeing that there will
be 3/4 empty slots available for either hotplug or coldplug
throughout the entire life of the guest.


A better way to put it is that we guarantee there will be "N" (3 or 4 or 
whatever) slots available when the domain is originally defined. Once 
any change has been made, all bets are off.




Sounds like a pretty good compromise to me.

The only problem I can think of is that there might be
management applications that add eg. a pcie-root in the XML
when first defining a guest, and after the change such guests
would get zero hotpluggable ports. Then again it's probably
okay to expect such management applications to add the
necessary number of pcie-root-ports themselves.


Yeah, if they know enough to be adding a root-port, then they know 
enough to add extras.




Maybe we could relax the wording on A) and ignore any
pci{,e}-root? Even though there is really no reason for
either a user or a management application to add them
explicitly when defining a guest, I feel like they might be
special enough to deserve an exception.


I thought about that; I'm not sure. In the case of libguestfs, even if 
Rich added a pcie-root, I guess he would still be manually addressing 
his pci devices, so that would be clue enough that he knew what he was 
doing and didn't want any extra.


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

[libvirt] [PATCH] vz: fix vnc host address

[Mikhail Feoktistov]

Empty string means 127.0.0.1 in terms of vzSDK
But in this case we should set 0.0.0.0
---
 .gnulib | 2 +-
 src/vz/vz_sdk.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gnulib b/.gnulib
index e89b4a7..a2a3943 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit e89b4a7aefce9cb02963920712ba7cdd13641644
+Subproject commit a2a39436b65f329630df4a93ec4e30aeae403c54
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..7011cbf 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2967,7 +2967,7 @@ static int prlsdkApplyGraphicsParams(PRL_HANDLE sdkdom,
 
 glisten = virDomainGraphicsGetListen(gr, 0);
 pret = PrlVmCfg_SetVNCHostName(sdkdom, glisten && glisten->address ?
-   glisten->address : "");
+   glisten->address : "0.0.0.0");
 prlsdkCheckRetGoto(pret, cleanup);
 
 ret = 0;
-- 
1.8.3.1

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

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

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> 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

intentionally

s/then having/in order to avoid 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);
>  }
> -

Rogue editor change or OCD?

>  virObjectUnref(cfg);
>  }
> 
> 

Although I'm sure the qemu commit referenced explains the defaults,
maybe a bit of details here would help...  To paraphrase from my quick
read... The 'size' is only relevant for IVSHMEM, it's not used for plain
and doorbell.  The usage of plain or doorbell is based upon whether
someone chooses  or . One cannot choose both in this new
world. Selection of one or the other in the XML infers the type of shmem
device.

>  static int
> +qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm)
> +{
> +if (!shm->size)
> +shm->size = 4 << 20;

I guess since we're not saving/migrating blindly setting won't cause
strange failures for virDomainShmemDefFind/virDomainShmemDefEquals, but
will it cause a problem for virDomainShmemDefCheckABIStability? There's
a call to the ABI code from qemuDomainRevertToSnapshot which has
comments for the transitions that would fall into this code being a
migration, but without trying to page *that* back into my short term
memory - well I figured I'd just note it and see what the consensus was!

This also seem to have an effect on the default for -plain, but gets
reset for -doorbell. [1]

I think it'd be 'cleaner' if each model was part of a switch statement
rather than the opposite world here where "server" assumes "-plain" and
"msi" assumes "-doorbell".  In the long run it'll be cleaner to read the
code I think if we know what each has for a default...

> +
> +/* 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;

[1]
Is setting this to 0 the "correct" thing to do or should it be an error
if a "-doorbell" shmem device uses the 'size' field? Blindly setting it
to zero just doesn't seem right.

>From the qemu checkin note...

Changes from ivshmem to ivshmem-plain:
...

* Properties "shm" and "size" are gone.  Use property "memdev"
  instead.
...

Changes from ivshmem to ivshmem-doorbell:
...
* Property "size" is gone.  The new device can only map all the shared
  memory received from the server.
...


There's an implication in the following patch too...

I think it'd be good to post a v2 of this patch at least before an ACK.


John



> +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,
>  }
>  }
> 
> 

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

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> 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(-)
> 
Doh... As I'm reading patch 7, I realized...

This doesn't alter qemuxml2xmltest.c and it needs to as well as of
course creating XML for each type (which can be stolen from 7/8)

John

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

Re: [libvirt] Analysis of the effect of adding PCIe root ports

[Andrea Bolognani]

On Fri, 2016-10-07 at 10:17 -0400, Laine Stump wrote:
> So here's a rewording of your description (with a couple additional 
> conditions) to see if I understand everything correctly:
> 
> 1) during initial domain definition:
> 
> A) If there are *no pci controllers at all* (not even a pci-root or 
> pcie-root) *and there are any unaddressed devices that need a PCI slot* 
> then auto-add enough controllers for the requested devices, *and* make 
> sure there are enough empty slots for "N" (do we stick with 4? or make 
> it 3?) devices to be added later without needing more controllers. (So, 
> if the domain has no PCI devices, we don't add anything extra, and also 
> if it only has PCI devices that already have addresses, then we also 
> don't add anything extra).
> 
> B) if there is at least one pci controller specified in the XML, and 
> there are any unused slots in the pci controllers in the provided XML, 
> then use them for the unaddressed devices. If there are more devices 
> that need an address at this time, also add controllers for them, but no 
> *extra* controllers.
> 
> (Note to Rich: libguestfs could avoid the extra controllers either by 
> adding a pci-root/pcie-root to the XML, or by manually addressing the 
> devices. The latter would actually be better, since it would avoid the 
> need for any pcie-root-ports).
> 
> 2) When adding a device to the persistent config (i.e. offline): if 
> there is an empty slot on a controller, use it. If not, add a controller 
> for that device *but no extra controllers*
> 
> 3) when adding a device to the guest machine (i.e. hotplug / online), if 
> there is an empty slot on a controller, use it. If not, then fail.
> 
> The differences I see from what (I think) you suggested are:
> 
> * if there aren't  any unaddressed pci devices (even if there are no 
> controllers in the config), then we also don't add any extra controllers 
> (although we will of course add the pci-root or pcie-root, to 
> acknowledge it is there).
> 
> * if another controller is needed for adding a device offline, it's okay 
> to add it.

So instead of guaranteeing that there will always be an empty
slot available for hotplug during a single start/destroy
cycle of the guest, we would be guaranteeing that there will
be 3/4 empty slots available for either hotplug or coldplug
throughout the entire life of the guest.

Sounds like a pretty good compromise to me.

The only problem I can think of is that there might be
management applications that add eg. a pcie-root in the XML
when first defining a guest, and after the change such guests
would get zero hotpluggable ports. Then again it's probably
okay to expect such management applications to add the
necessary number of pcie-root-ports themselves.

Maybe we could relax the wording on A) and ignore any
pci{,e}-root? Even though there is really no reason for
either a user or a management application to add them
explicitly when defining a guest, I feel like they might be
special enough to deserve an exception.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

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

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> 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(+)
> 

re-ACK (this is ACK'd previously somewhere IIRC)

John

The question becomes who gets to push first and who gets to merge their
capabilities changes ;-)


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

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

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> The old ivshmem is deprecated in QEMU, so let's use the better
> ivshmem-{plain,doorbell} variants instead.
> 

Perhaps explained a bit differently (my understanding ;-))... Older
versions of qemu had one type (ivshmem - the current default), but newer
versions of qemu will force selection of a specific type using "plain"
or "doorbell" as the model type depending on which "features" are
desired (??) [fill in the details]

I guess the nagging question is - what happens if someone "chooses" to
have  in this newer world?  Does the deprecation
from qemu kick in and the domain cannot start?  We should note that
somehow/somewhere.

> 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(-)
> 

docs/formatdomain.html.in needs to change here too

In particular what constitutes each setting - that is are there XML
elements that would or wouldn't make sense for the type of shmem device
(I'm thinking of Daniel's table here).

BTW: When using your "Since" tags, it'll need to be clear which version
of qemu no longer supports "type='ivshmem'" and forces someone to choose
-plain or -doorbell.

ACK w/ the doc change


John


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

Re: [libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Andrea Bolognani]

On Fri, 2016-10-07 at 11:42 +0200, Andrea Bolognani wrote:
> On Fri, 2016-10-07 at 11:19 +0200, Boris Fiuczynski wrote:
> > Andrea,
> > there is another "side effect" of the Requires directive.
> > libvirt-guests gets automatically started when libvirt is updated to 
> > v2.3.0. This has some rather nasty implications for the end users.
> 
> Mh, I don't see why that would happen.
> 
> The Requires relationship goes in the opposite direction,
> so if libvirt-guests was not running before the upgrade
> there should be no reason for it to be started, whether
> that relationship is there or not.
> 
> I'll try to reproduce this on my machine and get back to
> you in a while.

I tried upgrading from 2.2.0 to 2.3.0 a bunch of times, but
I haven't been able to reproduce the failure you're
reporting: libvirt-guests is never started automatically.

Can you provide more information? What distribution are
you using? Are you building from source, or using your
distribution's packages? Are you sure libvirt-guests was
not running even before upgrade?

-- 
Andrea Bolognani / Red Hat / Virtualization

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

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

[Daniel P. Berrange]

On Fri, Oct 07, 2016 at 10:53:48AM -0400, John Ferlan wrote:
> 
> 
> On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> > 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(-)
> > 
> 
> docs/formatdomain.html.in ??
> 
> Just so I'm sure I understand ;-)... This is the existing 'model type'
> for 'shmem' being implemented as the "default" (e.g. 0 entry) because we
> know at some short time in the future we're going to be adding a new
> type (or two).
> 
> Since the  is now going to be the default on
> output, we should explain in some way... and encourage choosing
> something else because "at some point in the future" ;-) we'll deprecate
> this one (whenever that dire time exists who knows).
> 
> Making sure #2 - we don't have to care about save files, true?  Since
> the default will be to now to ShmemDefFormat the  - a save file
> read on an older libvirt will have a failure, but that'd be true for any
> XML change I suppose.

IIRC ivshmem is non-migratable, so its impossible to have any existing
save files.


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 3/8] conf, qemu: Add support for shmem model

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> 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(-)
> 

docs/formatdomain.html.in ??

Just so I'm sure I understand ;-)... This is the existing 'model type'
for 'shmem' being implemented as the "default" (e.g. 0 entry) because we
know at some short time in the future we're going to be adding a new
type (or two).

Since the  is now going to be the default on
output, we should explain in some way... and encourage choosing
something else because "at some point in the future" ;-) we'll deprecate
this one (whenever that dire time exists who knows).

Making sure #2 - we don't have to care about save files, true?  Since
the default will be to now to ShmemDefFormat the  - a save file
read on an older libvirt will have a failure, but that'd be true for any
XML change I suppose.

I understand it does matter that the model is printed just in case
someone decided that's the model they wanted... If we really wanted
overkill, some sort of bool could be created to keep track of whether
model was read or not and then to not print out if not read in...

So I don't forget, once you get to implementing 'plain' and 'doorbell',
then perhaps some details from the v2 reply to 6/7 would be helpful to
the less than informed user trying to figure out which to use ;-)

ACK - in principle your call on the ShmemDefFormat bool or not...

John

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

Re: [libvirt] Analysis of the effect of adding PCIe root ports

[Laine Stump]

On 10/06/2016 12:10 PM, Daniel P. Berrange wrote:

On Thu, Oct 06, 2016 at 11:57:17AM -0400, Laine Stump wrote:

On 10/06/2016 11:31 AM, Daniel P. Berrange wrote:

On Thu, Oct 06, 2016 at 12:58:51PM +0200, Andrea Bolognani wrote:

On Wed, 2016-10-05 at 18:36 +0100, Richard W.M. Jones wrote:

(b) It would be nice to turn the whole thing off for people who don't
care about / need hotplugging.

I had contemplated having an "availablePCIeSlots" (or something like
that) that was either an attribute of the config, or an option in
qemu.conf or libvirtd.conf. If we had such a setting, it could be
set to "0".

I remember some pushback when this was proposed. Maybe we
should just give up on the idea of providing spare
hotpluggable PCIe slots by default and ask the user to add
them explicitly after all.


Note that changes to libvirt conf files are not usable by libguestfs.
The setting would need to go into the XML, and please also make it
possible to determine if $random version of libvirt supports the
setting, either by a version check or something in capabilities.

Note that you can avoid using any PCIe root port at all by
assigning PCI addresses manually. It looks like the overhead
for the small (I'm assuming) number of devices a libguestfs
appliance will use is low enough that you will probably not
want to open that can of worm, though.

For most apps the performance impact of the PCI enumeration
is not a big deal. So having libvirt ensure there's enough
available hotpluggable PCIe slots is reasonable, as long as
we leave a get-out clause for libguestfs.

This could be as simple as declaring that *if* we see one
or more  in the input XML, then libvirt
will honour those and not try to add new controllers to the
guest.

That way, by default libvirt will just "do the right thing"
and auto-create a suitable number of controllers needed to
boot the guest.

Apps that want strict control though, can specify the
 elements themselves.  Libvirt can still
auto-assign device addresses onto these controllers.
It simply wouldn't add any further controllers itself
at that point.


Even if it was adding offline, and there wasn't any place to put a new 
device? (i.e., the operation would fail without adding a controller, and 
libvirt was able to add it). Or am I taking your statement beyond its 
intent (I'm good at that :-)




  NB I'm talking cold-boot here. So libguestfs
would specify  itself to the minimal set it wants
to optimize its boot performance.

That works for the initial definition of the domain, but as soon as you've
saved it once, there will be controllers explicitly in the config, and since
we don't have any way of differentiating between auto-added controllers and
those specifically requested by the user, we have to assume they were
explicitly added, so such a check is then meaningless because you will
*always* have PCI controllers.

Ok, so coldplug was probably the wrong word to use. What I actually
meant was "at time of initial define", since that's when libvirt
actually does its controller auto-creation. If you later add more
devices to the guest, whether it is online or offline, that libvirt
would still be auto-adding more controllers if required (and if
possible) . I was not expecting libvirt to remember whether we
were auto-adding controllers the first time or not.


Say you create a domain definition with no controllers, you would get enough
for the devices in the initial config, plus "N" more empty root ports. Let's
say you then add 4 more devices (either hotplug or coldplug, doesn't
matter). Those devices are placed on the existing unused pcie-root-ports.
But now all your ports are full, and since you have PCI controllers in the
config, libvirt is going to say "Ah, this user knows what they want to do,
so I'm not going to add any extras! I'm so smart!". This would be especially
maddening in the case of "coldplug", where libvirt could have easily added a
new controller to accomodate the new device, but didn't.

Unless we don't care what happens after the initial definition (and then
adding of "N" new devices), trying to behave properly purely based on
whether or not there are any PCI controllers present in the config isn't
going to work.

I think that's fine.

Lets stop talking about coldplug since that's very misleading.


Do you mean use of the term "coldplug" at all, or talking about what 
happens when you add a device to the persistent config of the domain but 
not to the currently running guest itself?




What I mean is that...

1. When initially defining a guest

If no controllers are present, auto-add controllers implied
by the machine type, sufficient to deal with all currently
listed devices, plus "N" extra spare ports.

Else, simply assign devices to the controllers listed in
the XML config. If there are no extra spare ports after
doing this, so be it. It was the application's choice
to have not listed enough controllers to allow later
addition of more 

Re: [libvirt] [PATCH 1/3] docs: Alter descriptions of perf cpu_cycles

[Pavel Hrdina]

On Fri, Oct 07, 2016 at 09:48:24AM -0400, John Ferlan wrote:
> 
> 
> On 10/07/2016 09:32 AM, Pavel Hrdina wrote:
> > On Fri, Oct 07, 2016 at 08:35:35AM -0400, John Ferlan wrote:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1381714
> >>
> >> Alter the descriptions to match what the cpu_cycles actually is
> >>
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  docs/formatdomain.html.in | 2 +-
> >>  src/libvirt-domain.c  | 2 +-
> >>  src/util/virperf.h| 2 +-
> >>  tools/virsh.pod   | 9 +
> >>  4 files changed, 8 insertions(+), 7 deletions(-)
> > 
> > Update also doc text in include/libvirt/libvirt-domain.h.
> > 
> 
> Thanks ...
> 
> I thought about this one too - it was a bit more tricky since the
> existing text doesn't say it's representing "cpu cycles one instruction
> needs" rather it's indicating the cpu cycles "which can be used"...
> 
> Anyway, how about if I change from :
> 
>  * Macro for typed parameter name that represents cpu_cycles perf event
>  * which can be used to measure how many cpu cycles one instruction needs.
>  * It corresponds to the "perf.cpu_cycles" field in the *Stats APIs.
> 
> To:
> 
>  * Macro for typed parameter name that represents cpu_cycles perf event
>  * describing the total/elapsed cpu cycles. This can be used to measure
>  * how many cpu cycles one instruction needs.
>  * It corresponds to the "perf.cpu_cycles" field in the *Stats APIs.
> 
> Or I could just remove the "This can be... " sentence altogether. Since
> it's not describing what the data is but how it can be used in
> conjunction with the instructions value.

I'm OK with both versions, I guess you don't have to remove that part,
it's nice to have that example how it can be used.

Pavel


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

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

[Martin Kletzander]

On Fri, Oct 07, 2016 at 09:52:12AM -0400, John Ferlan wrote:



On 09/27/2016 08:24 AM, Martin Kletzander wrote:

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(+)



Seems like a reasonable thing to disallow...

Never quite sure what the "norm" is - start message w/ capital letter or
not... I think others in the code go with lowercase...  Your call.



All of them in qemuMigrationIsAllowed() start with lowercase, so I
changed that, but I think rest of the code is uppercase and since it's a
sentence after a colon, it should be so, but that's not what this series
is about, so... =D


ACK,

John

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;



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

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

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> 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(+)
> 

Seems like a reasonable thing to disallow...

Never quite sure what the "norm" is - start message w/ capital letter or
not... I think others in the code go with lowercase...  Your call.

ACK,

John
> 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;
> 

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

Re: [libvirt] [PATCH 1/3] docs: Alter descriptions of perf cpu_cycles

[John Ferlan]

On 10/07/2016 09:32 AM, Pavel Hrdina wrote:
> On Fri, Oct 07, 2016 at 08:35:35AM -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1381714
>>
>> Alter the descriptions to match what the cpu_cycles actually is
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  docs/formatdomain.html.in | 2 +-
>>  src/libvirt-domain.c  | 2 +-
>>  src/util/virperf.h| 2 +-
>>  tools/virsh.pod   | 9 +
>>  4 files changed, 8 insertions(+), 7 deletions(-)
> 
> Update also doc text in include/libvirt/libvirt-domain.h.
> 

Thanks ...

I thought about this one too - it was a bit more tricky since the
existing text doesn't say it's representing "cpu cycles one instruction
needs" rather it's indicating the cpu cycles "which can be used"...

Anyway, how about if I change from :

 * Macro for typed parameter name that represents cpu_cycles perf event
 * which can be used to measure how many cpu cycles one instruction needs.
 * It corresponds to the "perf.cpu_cycles" field in the *Stats APIs.

To:

 * Macro for typed parameter name that represents cpu_cycles perf event
 * describing the total/elapsed cpu cycles. This can be used to measure
 * how many cpu cycles one instruction needs.
 * It corresponds to the "perf.cpu_cycles" field in the *Stats APIs.

Or I could just remove the "This can be... " sentence altogether. Since
it's not describing what the data is but how it can be used in
conjunction with the instructions value.


John

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

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

[John Ferlan]

On 09/27/2016 08:24 AM, Martin Kletzander wrote:
> 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(-)
> 

ACK - while you're here... instead of 'break;', just return i; and don't
forget to remove the if (i < def->nshmems) return i;

You can make it a separate patch and assume the ACK...

John
> 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)
> 

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

Re: [libvirt] [PATCH 3/3] util: Check/ignore already disabled event

[Pavel Hrdina]

On Fri, Oct 07, 2016 at 08:35:37AM -0400, John Ferlan wrote:
> If the event is already disabled, then don't bother with setting it
> disabled again.  Causes unnecessary error on systems that don't support
> the feature anyway.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virperf.c | 3 +++
>  1 file changed, 3 insertions(+)

ACK

Pavel


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

Re: [libvirt] [PATCH 2/3] util: Clear up some perf error messages

[Pavel Hrdina]

On Fri, Oct 07, 2016 at 08:35:36AM -0400, John Ferlan wrote:
> Make it clearer that the perf event is based/for the host cpu and
> use the virPerfEventTypeToString to convert the type to a string
> 
> Signed-off-by: John Ferlan 
> ---
>  src/util/virperf.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

ACK

Pavel


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

Re: [libvirt] [PATCH 1/3] docs: Alter descriptions of perf cpu_cycles

[Pavel Hrdina]

On Fri, Oct 07, 2016 at 08:35:35AM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1381714
> 
> Alter the descriptions to match what the cpu_cycles actually is
> 
> Signed-off-by: John Ferlan 
> ---
>  docs/formatdomain.html.in | 2 +-
>  src/libvirt-domain.c  | 2 +-
>  src/util/virperf.h| 2 +-
>  tools/virsh.pod   | 9 +
>  4 files changed, 8 insertions(+), 7 deletions(-)

Update also doc text in include/libvirt/libvirt-domain.h.

ACK with the updated doc text.

Pavel


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

[libvirt] [PATCH 0/3] Clean up some perf event issues

[John Ferlan]

Resolve a couple of recently filed issues regarding perf events.
Details in each patch.

John Ferlan (3):
  docs: Alter descriptions of perf cpu_cycles
  util: Clear up some perf error messages
  util: Check/ignore already disabled event

 docs/formatdomain.html.in |  2 +-
 src/libvirt-domain.c  |  2 +-
 src/util/virperf.c| 13 -
 src/util/virperf.h|  2 +-
 tools/virsh.pod   |  9 +
 5 files changed, 16 insertions(+), 12 deletions(-)

-- 
2.7.4

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

[libvirt] [PATCH 3/3] util: Check/ignore already disabled event

[John Ferlan]

If the event is already disabled, then don't bother with setting it
disabled again.  Causes unnecessary error on systems that don't support
the feature anyway.

Signed-off-by: John Ferlan 
---
 src/util/virperf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/util/virperf.c b/src/util/virperf.c
index 6387430..5d57962 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -234,6 +234,9 @@ virPerfEventDisable(virPerfPtr perf,
 if (event == NULL)
 return -1;
 
+if (!event->enabled)
+return 0;
+
 if (ioctl(event->fd, PERF_EVENT_IOC_DISABLE) < 0) {
 virReportSystemError(errno,
  _("unable to disable host cpu perf event for %s"),
-- 
2.7.4

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

[libvirt] [PATCH 2/3] util: Clear up some perf error messages

[John Ferlan]

Make it clearer that the perf event is based/for the host cpu and
use the virPerfEventTypeToString to convert the type to a string

Signed-off-by: John Ferlan 
---
 src/util/virperf.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/util/virperf.c b/src/util/virperf.c
index a65a4bf..6387430 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -175,7 +175,7 @@ virPerfEventEnable(virPerfPtr perf,
type == VIR_PERF_EVENT_MBMT ||
type == VIR_PERF_EVENT_MBML)) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
-   _("unable to enable perf event for %s"),
+   _("unable to enable host cpu perf event for %s"),
virPerfEventTypeToString(event->type));
 return -1;
 }
@@ -205,14 +205,14 @@ virPerfEventEnable(virPerfPtr perf,
 event->fd = syscall(__NR_perf_event_open, , pid, -1, -1, 0);
 if (event->fd < 0) {
 virReportSystemError(errno,
- _("Unable to open perf event for %s"),
+ _("unable to open host cpu perf event for %s"),
  virPerfEventTypeToString(event->type));
 goto error;
 }
 
 if (ioctl(event->fd, PERF_EVENT_IOC_ENABLE) < 0) {
 virReportSystemError(errno,
- _("Unable to enable perf event for %s"),
+ _("unable to enable host cpu perf event for %s"),
  virPerfEventTypeToString(event->type));
 goto error;
 }
@@ -236,8 +236,8 @@ virPerfEventDisable(virPerfPtr perf,
 
 if (ioctl(event->fd, PERF_EVENT_IOC_DISABLE) < 0) {
 virReportSystemError(errno,
- _("Unable to disable perf event type=%d"),
- event->type);
+ _("unable to disable host cpu perf event for %s"),
+ virPerfEventTypeToString(event->type));
 return -1;
 }
 
-- 
2.7.4

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

[libvirt] [PATCH 1/3] docs: Alter descriptions of perf cpu_cycles

[John Ferlan]

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

Alter the descriptions to match what the cpu_cycles actually is

Signed-off-by: John Ferlan 
---
 docs/formatdomain.html.in | 2 +-
 src/libvirt-domain.c  | 2 +-
 src/util/virperf.h| 2 +-
 tools/virsh.pod   | 9 +
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1266e9d..b989c8f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1954,7 +1954,7 @@
 
 
   cpu_cycles
-  the number of cpu cycles one instruction needs
+  the count of cpu cycles (total/elapsed)
   perf.cpu_cycles
 
 
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 9ba9f49..f9f5a46 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11482,7 +11482,7 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  *   It is produced by cache_references perf event.
  * "perf.instructions" - The count of instructions as unsigned long long.
  *   It is produced by instructions perf event.
- * "perf.cpu_cycles"   - The number of cpu cycles one instruction needs as
+ * "perf.cpu_cycles"   - The count of cpu cycles (total/elapsed) as an
  *   unsigned long long. It is produced by cpu_cycles
  *   perf event.
  *
diff --git a/src/util/virperf.h b/src/util/virperf.h
index 41be878..3fca2d0 100644
--- a/src/util/virperf.h
+++ b/src/util/virperf.h
@@ -32,7 +32,7 @@ typedef enum {
 VIR_PERF_EVENT_MBMT,   /* Memory Bandwidth Monitoring Total */
 VIR_PERF_EVENT_MBML,   /* Memory Bandwidth Monitor Limit for controller */
 
-VIR_PERF_EVENT_CPU_CYCLES,   /* CPU Cycles per instruction */
+VIR_PERF_EVENT_CPU_CYCLES,   /* Count of CPU Cycles (total/elapsed) */
 VIR_PERF_EVENT_INSTRUCTIONS, /* Count of instructions for application 
*/
 VIR_PERF_EVENT_CACHE_REFERENCES, /* Cache hits by applications */
 VIR_PERF_EVENT_CACHE_MISSES, /* Cache misses by applications */
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 227c9b2..85992de 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -934,7 +934,7 @@ I<--perf> returns the statistics of all enabled perf events:
 "perf.cmt" - the cache usage in Byte currently used
 "perf.mbmt" - total system bandwidth from one level of cache
 "perf.mbml" - bandwidth of memory traffic for a memory controller
-"perf.cpu_cycles" - the number of cpu cycles one instruction needs
+"perf.cpu_cycles" - the count of cpu cycles (total/elapsed)
 "perf.instructions" - the count of instructions
 "perf.cache_references" - the count of cache hits
 "perf.cache_misses" - the count of caches misses
@@ -2247,9 +2247,10 @@ B
  applications running on th e platform.
   instructions - Provides the count of instructions executed
  by applications running on the platform.
-  cpu_cycles   - Provides the number of cpu_cycles for one
- instruction. May be used with instructions
- in order to get a cycles per instruction.
+  cpu_cycles   - Provides the count of cpu cycles
+ (total/elapsed). May be used with
+ instructions in order to get a cycles
+ per instruction.
 
 B: The statistics can be retrieved using the B command using
 the I<--perf> flag.
-- 
2.7.4

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

[libvirt] [PATCH 0/2] Maybe fix a crash and some unnecessary logging

[Martin Kletzander]

First patch fixes some unnecessary loging I've found out thanks to
Vincent's crash reproducer.  The second one is an RFC, I would like to
know what others think about and can find (and maybe help).  More info
in that commit message.


Martin Kletzander (2):
  Don't update timer if there's none.
  Purge marked callbacks before dispatching events

 src/conf/object_event.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

--
2.10.1

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

[libvirt] [PATCH 2/2 RFC] Purge marked callbacks before dispatching events

[Martin Kletzander]

I don't have any justification for this except an empirical one: with
this patch the code from Bug 1363628 doesn't crash after "leaking".

I currently don't have properly working valgrind, but I'm working on it
and it might shed some light into this (although it might also not
happen due to the slowdown).

However in the meantime I attempted some analysis and I got even more
confused than before, I guess.  With this patch the code doesn't crash,
even though virObjectEventStateQueueDispatch() properly skips callbacks
marked as deleted.  What I feel is even more weird, that if I duplicate
the purgatory function (instead of moving it), it crashes again, and I
feel like even more often.

Weirdly-fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1363628

Signed-off-by: Martin Kletzander 
---
 src/conf/object_event.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index e5af4be68a7e..4066b4673b42 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -821,13 +821,13 @@ virObjectEventStateFlush(virObjectEventStatePtr state)
 if (state->timer != -1)
 virEventUpdateTimeout(state->timer, -1);

+/* Purge any deleted callbacks */
+virObjectEventCallbackListPurgeMarked(state->callbacks);
+
 virObjectEventStateQueueDispatch(state,
  ,
  state->callbacks);

-/* Purge any deleted callbacks */
-virObjectEventCallbackListPurgeMarked(state->callbacks);
-
 state->isDispatching = false;
 virObjectEventStateUnlock(state);
 }
--
2.10.1

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

[libvirt] [PATCH 1/2] Don't update timer if there's none.

[Martin Kletzander]

Sometimes virObjectEventStateFlush can be called without timer (if the
last event was unregistered right when the timer fired).  There is a
check for timer == -1, but that triggers warning and other log messages,
which is unnecessary.

Signed-off-by: Martin Kletzander 
---
 src/conf/object_event.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index b5a6a81a3a04..e5af4be68a7e 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -818,7 +818,8 @@ virObjectEventStateFlush(virObjectEventStatePtr state)
 tempQueue.events = state->queue->events;
 state->queue->count = 0;
 state->queue->events = NULL;
-virEventUpdateTimeout(state->timer, -1);
+if (state->timer != -1)
+virEventUpdateTimeout(state->timer, -1);

 virObjectEventStateQueueDispatch(state,
  ,
-- 
2.10.1

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

Re: [libvirt] [PATCH] util: Fix build on s390

[Christian Borntraeger]

On 10/07/2016 12:43 PM, Jiri Denemark wrote:
> GCC on s390 complains
> 
> util/virconf.c: In function 'virConfGetValueSizeT':
> util/virconf.c:1220:9: error: format '%zu' expects argument of type
> 'size_t', but argument 9 has type 'unsigned int' [-Werror=format=]

Interesting, we have never seen this. is this 31bit (s390) or 64bit(s390x)?
What gcc? Can you maybe provide the virconf.i file to see how the SIZE_MAX 
define
was resolved?

Christian

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

Re: [libvirt] [libvirt-perl PATCH 1/2] Add blkdeviotune length parameters

[Daniel P. Berrange]

On Fri, Oct 07, 2016 at 07:18:01AM -0400, John Ferlan wrote:
> 
> 
> On 10/07/2016 06:58 AM, Daniel P. Berrange wrote:
> > On Fri, Oct 07, 2016 at 06:47:04AM -0400, John Ferlan wrote:
> >> Add VIR_DOMAIN_BLOCK_IOTUNE*LENGTH and VIR_DOMAIN_TUNABLE_BLKDEV*LENGTH
> >> parameters and their descriptions.
> >>
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  Changes|  2 +-
> >>  Virt.xs| 12 
> >>  lib/Sys/Virt/Domain.pm | 25 +
> >>  3 files changed, 38 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Changes b/Changes
> >> index 4985100..8d5683a 100644
> >> --- a/Changes
> >> +++ b/Changes
> >> @@ -2,7 +2,7 @@ Revision history for perl module Sys::Virt
> >>  
> >>  2.4.0 2016-00-00
> >>  
> >> - - XXX
> >> + - Add constants for the new block iotune parameters for length
> >>  
> >>  2.3.0 2016-10-06
> >>  
> >> diff --git a/Virt.xs b/Virt.xs
> >> index b4ca049..88ed722 100644
> >> --- a/Virt.xs
> >> +++ b/Virt.xs
> >> @@ -8275,6 +8275,12 @@ BOOT:
> >>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, 
> >> BLOCK_IOTUNE_READ_IOPS_SEC_MAX);
> >>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, 
> >> BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX);
> >>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, 
> >> BLOCK_IOTUNE_SIZE_IOPS_SEC);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH, 
> >> BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH, 
> >> BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH, 
> >> BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH, 
> >> BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH, 
> >> BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH, 
> >> BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH);
> >>  
> >>  
> >>REGISTER_CONSTANT(VIR_DOMAIN_BLOCK_RESIZE_BYTES, 
> >> BLOCK_RESIZE_BYTES);
> >> @@ -8484,6 +8490,12 @@ BOOT:
> >>
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX, 
> >> TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX);
> >>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX, 
> >> TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX);
> >>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC, 
> >> TUNABLE_BLKDEV_SIZE_IOPS_SEC);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH, 
> >> TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH, 
> >> TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH,
> >>  TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH, 
> >> TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH,
> >>  TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH);
> >> +  
> >> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH, 
> >> TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH);
> >>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN, 
> >> TUNABLE_IOTHREADSPIN);
> > 
> > You missed docs these this set of constants, so i'm suprised
> > make test didn't complain 
> > 
> 
> Oh right - forgot the second hunk...  They're just a copy of the BLOCK
> tunable anyway, so I copied and renamed the params to match the other
> TUNABLE_BLKDEV defs in lib/Sys/Virt/Domain.pm:

Ok, ACK with that added


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] [libvirt-perl PATCH 1/2] Add blkdeviotune length parameters

[John Ferlan]

On 10/07/2016 06:58 AM, Daniel P. Berrange wrote:
> On Fri, Oct 07, 2016 at 06:47:04AM -0400, John Ferlan wrote:
>> Add VIR_DOMAIN_BLOCK_IOTUNE*LENGTH and VIR_DOMAIN_TUNABLE_BLKDEV*LENGTH
>> parameters and their descriptions.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  Changes|  2 +-
>>  Virt.xs| 12 
>>  lib/Sys/Virt/Domain.pm | 25 +
>>  3 files changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/Changes b/Changes
>> index 4985100..8d5683a 100644
>> --- a/Changes
>> +++ b/Changes
>> @@ -2,7 +2,7 @@ Revision history for perl module Sys::Virt
>>  
>>  2.4.0 2016-00-00
>>  
>> - - XXX
>> + - Add constants for the new block iotune parameters for length
>>  
>>  2.3.0 2016-10-06
>>  
>> diff --git a/Virt.xs b/Virt.xs
>> index b4ca049..88ed722 100644
>> --- a/Virt.xs
>> +++ b/Virt.xs
>> @@ -8275,6 +8275,12 @@ BOOT:
>>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, 
>> BLOCK_IOTUNE_READ_IOPS_SEC_MAX);
>>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, 
>> BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX);
>>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, 
>> BLOCK_IOTUNE_SIZE_IOPS_SEC);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH, 
>> BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH, 
>> BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH, 
>> BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH, 
>> BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH, 
>> BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH, 
>> BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH);
>>  
>>  
>>REGISTER_CONSTANT(VIR_DOMAIN_BLOCK_RESIZE_BYTES, BLOCK_RESIZE_BYTES);
>> @@ -8484,6 +8490,12 @@ BOOT:
>>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX, 
>> TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX);
>>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX, 
>> TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX);
>>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC, 
>> TUNABLE_BLKDEV_SIZE_IOPS_SEC);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH, 
>> TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH, 
>> TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH, 
>> TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH, 
>> TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH, 
>> TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH);
>> +  
>> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH, 
>> TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH);
>>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN, 
>> TUNABLE_IOTHREADSPIN);
> 
> You missed docs these this set of constants, so i'm suprised
> make test didn't complain 
> 

Oh right - forgot the second hunk...  They're just a copy of the BLOCK
tunable anyway, so I copied and renamed the params to match the other
TUNABLE_BLKDEV defs in lib/Sys/Virt/Domain.pm:

diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm
index 8bf4115..b2488a6 100644
--- a/lib/Sys/Virt/Domain.pm
+++ b/lib/Sys/Virt/Domain.pm
@@ -4297,6 +4297,31 @@ Maximum write throughput in I/O operations per sec

 The maximum I/O operations per second

+=item Sys::Virt::Domain::TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum total bytes processed per
second.
+
+=item Sys::Virt::Domain::TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum bytes read per second.
+
+=item Sys::Virt::Domain::TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum bytes written per second.
+
+=item Sys::Virt::Domain::TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum total I/O operations processed
+per second.
+
+=item Sys::Virt::Domain::TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum I/O operations read per second.
+
+=item Sys::Virt::Domain::TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum I/O operations written per
second.
+


John

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

Re: [libvirt] PCP libvirt Plugin

[Marko Myllynen]

Hi,

On 2016-10-07 12:55, Michal Privoznik wrote:
> On 07.10.2016 09:22, Marko Myllynen wrote:
>> On 2016-10-06 15:01, Martin Kletzander wrote:
>>> On Wed, Oct 05, 2016 at 07:28:00PM +0300, Marko Myllynen wrote:

 FYI, I've written a PCP plugin (PMDA in PCP parlance) to support most
 hypervisor / domain information and metrics available over the libvirt
 Python API, it's up to date as of libvirt 2.3 (so it already supports
 the recently added perf event metrics).

 The libvirt metrics available with PCP libvirt PMDA and their
 descriptions are listed below, they include all the recently added perf
 event metrics as well as combined and per-device metrics for each VM.

 I wonder could this be mentioned at https://libvirt.org/apps.html ?
>>>
>>> Great to hear!  Sure!  Would you mind sending a patch against libvirt's
>>> docs/apps.html.in with what you'd like to have mentioned there?  If you
>>> don't like doing that I can do that for you, just let me know.
>>
>> Sure, how about this?
> 
> Almost :-)
> 
> s,,,

Ha, my HTML coding skillz are a bit rusty, it seems! :-)

> I've fixed all of that, came up with a commit message and pushed.
> 
> Congratulations on your first libvirt contribution!

Thanks!

-- 
Marko Myllynen

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

[libvirt] [PATCH v8 3/5] qemu: Introduce qemuDomainChardevPrivatePtr

[John Ferlan]

Modeled after the qemuDomainHostdevPrivatePtr (commit id '27726d8c'),
create a privateData pointer in the _virDomainChardevDef to allow storage
of private data for a hypervisor in order to at least temporarily store
secret data for usage during qemuBuildCommandLine.

NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the secret data, there's no need to add code
code to handle this new structure there.

Signed-off-by: John Ferlan 
---
 src/conf/domain_conf.c| 29 
 src/conf/domain_conf.h|  4 +++-
 src/libxl/libxl_domain.c  |  2 +-
 src/lxc/lxc_native.c  |  2 +-
 src/qemu/qemu_domain.c| 44 +++
 src/qemu/qemu_domain.h| 14 ++
 src/qemu/qemu_parse_command.c |  4 ++--
 src/vz/vz_sdk.c   |  2 +-
 src/xenconfig/xen_sxpr.c  |  2 +-
 src/xenconfig/xen_xl.c|  2 +-
 10 files changed, 89 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1f7c43f..c52144f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2091,6 +2091,8 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
 VIR_FREE(def->seclabels);
 }
 
+virObjectUnref(def->privateData);
+
 VIR_FREE(def);
 }
 
@@ -10294,7 +10296,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
  * default port.
  */
 virDomainChrDefPtr
-virDomainChrDefNew(void)
+virDomainChrDefNew(virDomainXMLOptionPtr xmlopt)
 {
 virDomainChrDefPtr def = NULL;
 
@@ -10302,6 +10304,11 @@ virDomainChrDefNew(void)
 return NULL;
 
 def->target.port = -1;
+
+if (xmlopt && xmlopt->privateData.chardevNew &&
+!(def->privateData = xmlopt->privateData.chardevNew()))
+VIR_FREE(def);
+
 return def;
 }
 
@@ -10349,7 +10356,8 @@ virDomainChrDefNew(void)
  *
  */
 static virDomainChrDefPtr
-virDomainChrDefParseXML(xmlXPathContextPtr ctxt,
+virDomainChrDefParseXML(virDomainXMLOptionPtr xmlopt,
+xmlXPathContextPtr ctxt,
 xmlNodePtr node,
 virSecurityLabelDefPtr* vmSeclabels,
 int nvmSeclabels,
@@ -10361,7 +10369,7 @@ virDomainChrDefParseXML(xmlXPathContextPtr ctxt,
 virDomainChrDefPtr def;
 bool seenTarget = false;
 
-if (!(def = virDomainChrDefNew()))
+if (!(def = virDomainChrDefNew(xmlopt)))
 return NULL;
 
 type = virXMLPropString(node, "type");
@@ -13538,7 +13546,8 @@ virDomainDeviceDefParse(const char *xmlStr,
 goto error;
 break;
 case VIR_DOMAIN_DEVICE_CHR:
-if (!(dev->data.chr = virDomainChrDefParseXML(ctxt,
+if (!(dev->data.chr = virDomainChrDefParseXML(xmlopt,
+  ctxt,
   node,
   def->seclabels,
   def->nseclabels,
@@ -17160,7 +17169,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 
 for (i = 0; i < n; i++) {
-virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
+virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
  nodes[i],
  def->seclabels,
  def->nseclabels,
@@ -17187,7 +17197,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 
 for (i = 0; i < n; i++) {
-virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
+virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
  nodes[i],
  def->seclabels,
  def->nseclabels,
@@ -17216,7 +17227,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 
 for (i = 0; i < n; i++) {
-virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
+virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
  nodes[i],
  def->seclabels,
  def->nseclabels,
@@ -17235,7 +17247,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 
 for (i = 0; i < n; i++) {
-virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
+virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
  

[libvirt] [PATCH v8 5/5] qemu: Add the ability to hotplug a secret object for TCP chardev TLS

[John Ferlan]

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

Complete the implementation of support for TLS encryption on
chardev TCP transports by adding the hotplug ability of a secret
to generate the passwordid for the TLS object

Likewise, add the ability to hot unplug that secret object as well

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_driver.c  |  2 +-
 src/qemu/qemu_hotplug.c | 62 +
 src/qemu/qemu_hotplug.h |  3 ++-
 tests/qemuhotplugtest.c |  2 +-
 4 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 29a7e3f..aa2a974 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7569,7 +7569,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 break;
 
 case VIR_DOMAIN_DEVICE_CHR:
-ret = qemuDomainAttachChrDevice(driver, vm,
+ret = qemuDomainAttachChrDevice(conn, driver, vm,
 dev->data.chr);
 if (!ret) {
 alias = dev->data.chr->info.alias;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8e4057c..31a822d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1638,7 +1638,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
 return ret;
 }
 
-int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
+int qemuDomainAttachChrDevice(virConnectPtr conn,
+  virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainChrDefPtr chr)
 {
@@ -1652,8 +1653,11 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 char *charAlias = NULL;
 bool chardevAttached = false;
 bool tlsobjAdded = false;
+bool secobjAdded = false;
 virJSONValuePtr tlsProps = NULL;
 char *tlsAlias = NULL;
+virJSONValuePtr secProps = NULL;
+char *secAlias = NULL;
 bool need_release = false;
 
 if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
@@ -1677,12 +1681,30 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 if (qemuDomainChrPreInsert(vmdef, chr) < 0)
 goto cleanup;
 
+if (qemuDomainSecretChardevPrepare(conn, driver, priv, chr) < 0)
+goto cleanup;
+
 if (cfg->chardevTLS &&
 dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) {
+qemuDomainChardevPrivatePtr chardevPriv =
+QEMU_DOMAIN_CHARDEV_PRIVATE(chr);
+qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
+
+/* Add a secret object in order to access the TLS environment.
+ * The secinfo will only be created for serial TCP device. */
+if (secinfo) {
+if (qemuBuildSecretInfoProps(secinfo, ) < 0)
+goto cleanup;
+
+if (!(secAlias = qemuDomainGetSecretAESAlias(chr->info.alias,
+ false)))
+goto cleanup;
+}
+
 if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
  dev->data.tcp.listen,
  cfg->chardevTLSx509verify,
- NULL,
+ secAlias,
  priv->qemuCaps,
  ) < 0)
 goto cleanup;
@@ -1693,6 +1715,15 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 }
 
 qemuDomainObjEnterMonitor(driver, vm);
+if (secAlias) {
+rc = qemuMonitorAddObject(priv->mon, "secret",
+  secAlias, secProps);
+secProps = NULL;
+if (rc < 0)
+goto exit_monitor;
+secobjAdded = true;
+}
+
 if (tlsAlias) {
 rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
   tlsAlias, tlsProps);
@@ -1723,6 +1754,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 qemuDomainReleaseDeviceAddress(vm, >info, NULL);
 VIR_FREE(tlsAlias);
 virJSONValueFree(tlsProps);
+VIR_FREE(secAlias);
+virJSONValueFree(secProps);
 VIR_FREE(charAlias);
 VIR_FREE(devstr);
 virObjectUnref(cfg);
@@ -1730,6 +1763,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 
  exit_monitor:
 orig_err = virSaveLastError();
+if (secobjAdded)
+ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
 if (tlsobjAdded)
 ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
 /* detach associated chardev on error */
@@ -4319,6 +4354,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
 virDomainDefPtr vmdef = vm->def;
 virDomainChrDefPtr tmpChr;
 char *objAlias = NULL;
+char *secAlias = NULL;
 char *devstr = NULL;
 
 if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
@@ -4332,9 +4368,21 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
 
 

[libvirt] [PATCH v8 1/5] domain: Add optional 'tls' attribute for TCP chardev

[John Ferlan]

Add an optional "tls='yes'" option for a TCP chardev for the
express purpose to enable setting up TLS for the chardev. This
will assume that the qemu.conf settings have been adjusted as
well as the environment configured properly.

Signed-off-by: John Ferlan 
---
 docs/formatdomain.html.in  | 21 +
 docs/schemas/domaincommon.rng  |  5 +++
 src/conf/domain_conf.c | 22 +-
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_command.c|  3 +-
 src/qemu/qemu_hotplug.c|  3 +-
 ...emuxml2argv-serial-tcp-tlsx509-chardev-tls.args | 30 +
 ...qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml | 50 ++
 .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml|  2 +-
 tests/qemuxml2argvtest.c   |  3 ++
 ...muxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml |  1 +
 .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml  |  2 +-
 tests/qemuxml2xmltest.c|  1 +
 13 files changed, 139 insertions(+), 5 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml
 create mode 12 
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1266e9d..010530e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6203,6 +6203,27 @@ qemu-kvm -net nic,model=? /dev/null
   /devices
   ...
 
+
+  Since 2.4.0, some hypervisors support using
+  a TLS X.509 certificate environment in order to encrypt all serial TCP
+  connections via a hypervisor configuration option. In order to enable
+  TLS for the domain an optional attribute tls can be set to
+  "yes". Combined with the hypervisor's capability to utilize the TLS
+  environment allows for the character device to use the encrypted
+  communication. If the attribute is not present, then the default
+  setting is "no".
+
+
+  ...
+  devices
+serial type="tcp"
+  source mode='connect' host="127.0.0.1" service="" tls="yes"/
+  protocol type="raw"/
+  target port="0"/
+/serial
+  /devices
+  ...
+
 UDP network console
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6eeb4e9..362b90d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3453,6 +3453,11 @@
 
   
 
+
+  
+
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f562323..1f7c43f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1961,6 +1961,8 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
 
 if (VIR_STRDUP(dest->data.tcp.service, src->data.tcp.service) < 0)
 return -1;
+
+dest->data.tcp.haveTLS = src->data.tcp.haveTLS;
 break;
 
 case VIR_DOMAIN_CHR_TYPE_UNIX:
@@ -,6 +10001,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 char *master = NULL;
 char *slave = NULL;
 char *append = NULL;
+char *haveTLS = NULL;
 int remaining = 0;
 
 while (cur != NULL) {
@@ -10006,6 +10009,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 if (xmlStrEqual(cur->name, BAD_CAST "source")) {
 if (!mode)
 mode = virXMLPropString(cur, "mode");
+if (!haveTLS)
+haveTLS = virXMLPropString(cur, "tls");
 
 switch ((virDomainChrType) def->type) {
 case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -10182,6 +10187,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 def->data.tcp.listen = true;
 }
 
+if (haveTLS &&
+(def->data.tcp.haveTLS =
+ virTristateBoolTypeFromString(haveTLS)) <= 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("unknown chardev 'tls' setting '%s'"),
+   haveTLS);
+goto error;
+}
+
 if (!protocol)
 def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW;
 else if ((def->data.tcp.protocol =
@@ -10266,6 +10280,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 VIR_FREE(append);
 VIR_FREE(logappend);
 VIR_FREE(logfile);
+VIR_FREE(haveTLS);
 
 return remaining;
 
@@ -21417,7 +21432,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, "data.tcp.listen ? "bind" : "connect");
 virBufferEscapeString(buf, "host='%s' ", def->data.tcp.host);
-virBufferEscapeString(buf, "service='%s'/>\n", def->data.tcp.service);
+

[libvirt] [PATCH v8 4/5] qemu: Add a secret object to/for a chardev tcp with secret

[John Ferlan]

Add the secret object prior to the chardev tcp so the 'passwordid=' can
be added if the domain XML has a  for the chardev TLS.

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_command.c| 32 ++-
 src/qemu/qemu_command.h|  1 +
 src/qemu/qemu_domain.c | 99 +-
 src/qemu/qemu_domain.h | 16 +++-
 src/qemu/qemu_hotplug.c|  1 +
 src/qemu/qemu_process.c|  6 +-
 ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 +
 ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++
 tests/qemuxml2argvtest.c   | 19 +
 9 files changed, 253 insertions(+), 9 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b290ade..a6e1762 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -683,6 +683,7 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
  * @tlspath: path to the TLS credentials
  * @listen: boolen listen for client or server setting
  * @verifypeer: boolean to enable peer verification (form of authorization)
+ * @secalias: if one exists, the alias of the security object for passwordid
  * @qemuCaps: capabilities
  * @propsret: json properties to return
  *
@@ -694,6 +695,7 @@ int
 qemuBuildTLSx509BackendProps(const char *tlspath,
  bool isListen,
  bool verifypeer,
+ const char *secalias,
  virQEMUCapsPtr qemuCaps,
  virJSONValuePtr *propsret)
 {
@@ -719,6 +721,10 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
  NULL) < 0)
 goto cleanup;
 
+if (secalias &&
+virJSONValueObjectAdd(*propsret, "s:passwordid", secalias, NULL) < 0)
+goto cleanup;
+
 ret = 0;
 
  cleanup:
@@ -733,6 +739,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
  * @tlspath: path to the TLS credentials
  * @listen: boolen listen for client or server setting
  * @verifypeer: boolean to enable peer verification (form of authorization)
+ * @addpasswordid: boolean to handle adding passwordid to object
  * @inalias: Alias for the parent to generate object alias
  * @qemuCaps: capabilities
  *
@@ -745,6 +752,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
 const char *tlspath,
 bool isListen,
 bool verifypeer,
+bool addpasswordid,
 const char *inalias,
 virQEMUCapsPtr qemuCaps)
 {
@@ -752,11 +760,16 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
 char *objalias = NULL;
 virJSONValuePtr props = NULL;
 char *tmp = NULL;
+char *secalias = NULL;
 
-if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer,
- qemuCaps, ) < 0)
+if (addpasswordid &&
+!(secalias = qemuDomainGetSecretAESAlias(inalias, false)))
 return -1;
 
+if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, secalias,
+ qemuCaps, ) < 0)
+goto cleanup;
+
 if (!(objalias = qemuAliasTLSObjFromChardevAlias(inalias)))
 goto cleanup;
 
@@ -772,6 +785,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
 virJSONValueFree(props);
 VIR_FREE(objalias);
 VIR_FREE(tmp);
+VIR_FREE(secalias);
 return ret;
 }
 
@@ -5009,6 +5023,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
 if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
 dev->data.tcp.listen,
 cfg->chardevTLSx509verify,
+cfg->chardevTLSx509haveUUID,
 alias, qemuCaps) < 0)
 goto error;
 
@@ -8534,6 +8549,19 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
 
 /* Use -chardev with -device if they are available */
 if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) {
+qemuDomainChardevPrivatePtr chardevPriv =
+QEMU_DOMAIN_CHARDEV_PRIVATE(serial);
+qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
+
+/* Add the secret object first if necessary. The
+ * secinfo is added only to a TCP serial device during
+ * qemuDomainSecretChardevPrepare. Subsequent called
+ * functions can just check the config fields */
+if (cfg->chardevTLS && cfg->chardevTLSx509haveUUID &&
+

[libvirt] [PATCH v8 2/5] conf: Introduce {default|chardev}_tls_x509_secret_uuid

[John Ferlan]

Add a new qemu.conf variables to store the UUID for the secret that could
be used to present credentials to access the TLS chardev.  Since this will
be a server level and it's possible to use some sort of default, introduce
both the default and chardev logic at the same time making the setting of
the chardev check for it's own value, then if not present checking whether
the default value had been set.

The chardevTLSx509haveUUID bool will be used as the marker for whether
the chardevTLSx509secretUUID was successfully read. In the future this
is how it'd determined whether to add the secret object for a TLS object.

Signed-off-by: John Ferlan 
---
 src/qemu/libvirtd_qemu.aug |  2 ++
 src/qemu/qemu.conf | 24 
 src/qemu/qemu_conf.c   | 22 ++
 src/qemu/qemu_conf.h   |  3 +++
 src/qemu/test_libvirtd_qemu.aug.in |  2 ++
 5 files changed, 53 insertions(+)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 988201e..73ebeda 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -29,6 +29,7 @@ module Libvirtd_qemu =
(* Config entry grouped by function - same order as example config *)
let default_tls_entry = str_entry "default_tls_x509_cert_dir"
  | bool_entry "default_tls_x509_verify"
+ | str_entry "default_tls_x509_secret_uuid"
 
let vnc_entry = str_entry "vnc_listen"
  | bool_entry "vnc_auto_unix_socket"
@@ -51,6 +52,7 @@ module Libvirtd_qemu =
let chardev_entry = bool_entry "chardev_tls"
  | str_entry "chardev_tls_x509_cert_dir"
  | bool_entry "chardev_tls_x509_verify"
+ | str_entry "chardev_tls_x509_secret_uuid"
 
let nogfx_entry = bool_entry "nographics_allow_host_audio"
 
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index e4c2aae..f136fa9 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -28,6 +28,20 @@
 #
 #default_tls_x509_verify = 1
 
+#
+# Libvirt assumes the server-key.pem file is unencrypted by default.
+# To use an encrypted server-key.pem file, the password to decrypt the
+# the PEM file is requird. This can be provided by creating a secret
+# object in libvirt and then uncommenting this setting to set the UUID
+# UUID of the secret.
+#
+# NB This default all-zeros UUID will not work. Replace it with the
+# output from the UUID for the TLS secret from a 'virsh secret-list'
+# command and then uncomment the entry
+#
+#default_tls_x509_secret_uuid = "----"
+
+
 # VNC is configured to listen on 127.0.0.1 by default.
 # To make it listen on all public interfaces, uncomment
 # this next option.
@@ -214,6 +228,16 @@
 #chardev_tls_x509_verify = 1
 
 
+# Uncomment and use the following option to override the default secret
+# uuid provided in the default_tls_x509_secret_uuid parameter.
+#
+# NB This default all-zeros UUID will not work. Replace it with the
+# output from the UUID for the TLS secret from a 'virsh secret-list'
+# command and then uncomment the entry
+#
+#chardev_tls_x509_secret_uuid = "----"
+
+
 # By default, if no graphical front end is configured, libvirt will disable
 # QEMU audio output since directly talking to alsa/pulseaudio may not work
 # with various security settings. If you know what you're doing, enable
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 635fa27..c650961 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -365,6 +365,7 @@ static void virQEMUDriverConfigDispose(void *obj)
 VIR_FREE(cfg->nvramDir);
 
 VIR_FREE(cfg->defaultTLSx509certdir);
+VIR_FREE(cfg->defaultTLSx509secretUUID);
 
 VIR_FREE(cfg->vncTLSx509certdir);
 VIR_FREE(cfg->vncListen);
@@ -377,6 +378,7 @@ static void virQEMUDriverConfigDispose(void *obj)
 VIR_FREE(cfg->spiceSASLdir);
 
 VIR_FREE(cfg->chardevTLSx509certdir);
+VIR_FREE(cfg->chardevTLSx509secretUUID);
 
 while (cfg->nhugetlbfs) {
 cfg->nhugetlbfs--;
@@ -424,6 +426,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 int ret = -1;
 int rv;
 size_t i, j;
+bool defaultTLSx509haveUUID;
 char *stdioHandler = NULL;
 char *user = NULL, *group = NULL;
 char **controllers = NULL;
@@ -446,6 +449,12 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 goto cleanup;
 if (virConfGetValueBool(conf, "default_tls_x509_verify", 
>defaultTLSx509verify) < 0)
 goto cleanup;
+if ((rv = virConfGetValueString(conf, "default_tls_x509_secret_uuid",
+>defaultTLSx509secretUUID)) < 0)
+goto cleanup;
+if (rv == 1)
+defaultTLSx509haveUUID = true;
+
 if (virConfGetValueBool(conf, "vnc_auto_unix_socket", 
>vncAutoUnixSocket) < 0)
 goto cleanup;
 if (virConfGetValueBool(conf, "vnc_tls", >vncTLS) < 0)
@@ -513,6 

[libvirt] [PATCH v8 0/5] Add native TLS encrypted chardev TCP support

[John Ferlan]

v7: http://www.redhat.com/archives/libvir-list/2016-September/msg00729.html

Differences in v8

1. Update to use 2.4.0 instead of 2.3.0
2. Alter text in qemu.conf as requested in review of v7
3. After update to top of branch found one more virDomainChrDefNew in 
   a recent commit that needed a NULL parameter

John Ferlan (5):
  domain: Add optional 'tls' attribute for TCP chardev
  conf: Introduce {default|chardev}_tls_x509_secret_uuid
  qemu: Introduce qemuDomainChardevPrivatePtr
  qemu: Add a secret object to/for a chardev tcp with secret
  qemu: Add the ability to hotplug a secret object for TCP chardev TLS

 docs/formatdomain.html.in  |  21 +++
 docs/schemas/domaincommon.rng  |   5 +
 src/conf/domain_conf.c |  51 ++--
 src/conf/domain_conf.h |   5 +-
 src/libxl/libxl_domain.c   |   2 +-
 src/lxc/lxc_native.c   |   2 +-
 src/qemu/libvirtd_qemu.aug |   2 +
 src/qemu/qemu.conf |  24 
 src/qemu/qemu_command.c|  35 -
 src/qemu/qemu_command.h|   1 +
 src/qemu/qemu_conf.c   |  22 
 src/qemu/qemu_conf.h   |   3 +
 src/qemu/qemu_domain.c | 143 -
 src/qemu/qemu_domain.h |  30 -
 src/qemu/qemu_driver.c |   2 +-
 src/qemu/qemu_hotplug.c|  64 -
 src/qemu/qemu_hotplug.h|   3 +-
 src/qemu/qemu_parse_command.c  |   4 +-
 src/qemu/qemu_process.c|   6 +-
 src/qemu/test_libvirtd_qemu.aug.in |   2 +
 src/vz/vz_sdk.c|   2 +-
 src/xenconfig/xen_sxpr.c   |   2 +-
 src/xenconfig/xen_xl.c |   2 +-
 tests/qemuhotplugtest.c|   2 +-
 ...emuxml2argv-serial-tcp-tlsx509-chardev-tls.args |  30 +
 ...qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml |  50 +++
 .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml|   2 +-
 ...xml2argv-serial-tcp-tlsx509-secret-chardev.args |  38 ++
 ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml |  50 +++
 tests/qemuxml2argvtest.c   |  22 
 ...muxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml |   1 +
 .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml  |   2 +-
 tests/qemuxml2xmltest.c|   1 +
 33 files changed, 594 insertions(+), 37 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
 create mode 12 
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml

-- 
2.7.4

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

Re: [libvirt] [libvirt-perl PATCH 1/2] Add blkdeviotune length parameters

[Daniel P. Berrange]

On Fri, Oct 07, 2016 at 06:47:04AM -0400, John Ferlan wrote:
> Add VIR_DOMAIN_BLOCK_IOTUNE*LENGTH and VIR_DOMAIN_TUNABLE_BLKDEV*LENGTH
> parameters and their descriptions.
> 
> Signed-off-by: John Ferlan 
> ---
>  Changes|  2 +-
>  Virt.xs| 12 
>  lib/Sys/Virt/Domain.pm | 25 +
>  3 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/Changes b/Changes
> index 4985100..8d5683a 100644
> --- a/Changes
> +++ b/Changes
> @@ -2,7 +2,7 @@ Revision history for perl module Sys::Virt
>  
>  2.4.0 2016-00-00
>  
> - - XXX
> + - Add constants for the new block iotune parameters for length
>  
>  2.3.0 2016-10-06
>  
> diff --git a/Virt.xs b/Virt.xs
> index b4ca049..88ed722 100644
> --- a/Virt.xs
> +++ b/Virt.xs
> @@ -8275,6 +8275,12 @@ BOOT:
>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, 
> BLOCK_IOTUNE_READ_IOPS_SEC_MAX);
>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, 
> BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX);
>REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, 
> BLOCK_IOTUNE_SIZE_IOPS_SEC);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH, 
> BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH, 
> BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH, 
> BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH, 
> BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH, 
> BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH, 
> BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH);
>  
>  
>REGISTER_CONSTANT(VIR_DOMAIN_BLOCK_RESIZE_BYTES, BLOCK_RESIZE_BYTES);
> @@ -8484,6 +8490,12 @@ BOOT:
>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX, 
> TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX);
>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX, 
> TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX);
>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC, 
> TUNABLE_BLKDEV_SIZE_IOPS_SEC);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH, 
> TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH, 
> TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH, 
> TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH, 
> TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH, 
> TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH);
> +  
> REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH, 
> TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH);
>REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN, 
> TUNABLE_IOTHREADSPIN);

You missed docs these this set of constants, so i'm suprised
make test didn't complain 


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] [libvirt-perl PATCH 2/2] Add VIR_DOMAIN_VCPU_HOTPLUGGABLE

[John Ferlan]

Signed-off-by: John Ferlan 
---
 Changes| 1 +
 Virt.xs| 1 +
 lib/Sys/Virt/Domain.pm | 4 
 3 files changed, 6 insertions(+)

diff --git a/Changes b/Changes
index 8d5683a..897040b 100644
--- a/Changes
+++ b/Changes
@@ -3,6 +3,7 @@ Revision history for perl module Sys::Virt
 2.4.0 2016-00-00
 
  - Add constants for the new block iotune parameters for length
+ - Add VIR_DOMAIN_VCPU_HOTPLUGGABLE
 
 2.3.0 2016-10-06
 
diff --git a/Virt.xs b/Virt.xs
index 88ed722..92f300b 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -8315,6 +8315,7 @@ BOOT:
   REGISTER_CONSTANT(VIR_DOMAIN_VCPU_CONFIG, VCPU_CONFIG);
   REGISTER_CONSTANT(VIR_DOMAIN_VCPU_MAXIMUM, VCPU_MAXIMUM);
   REGISTER_CONSTANT(VIR_DOMAIN_VCPU_GUEST, VCPU_GUEST);
+  REGISTER_CONSTANT(VIR_DOMAIN_VCPU_HOTPLUGGABLE, VCPU_HOTPLUGGABLE);
 
 
   REGISTER_CONSTANT(VIR_DOMAIN_SHUTDOWN_DEFAULT, SHUTDOWN_DEFAULT);
diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm
index 8bf4115..5e6669c 100644
--- a/lib/Sys/Virt/Domain.pm
+++ b/lib/Sys/Virt/Domain.pm
@@ -2793,6 +2793,10 @@ Flag to request adjustment of the maximum vCPU value
 
 Flag to request the guest VCPU mask
 
+=item Sys::Virt::Domain::VCPU_GUEST_HOTPLUGGABLE
+
+Flag to request to make vcpus added hot(un)pluggable
+
 =back
 
 =head2 STATE CHANGE EVENTS
-- 
2.7.4

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

[libvirt] [libvirt-perl PATCH 0/2] Add new flags

[John Ferlan]

Patch 1 is related to the on list iotune length/duration parameter

Patch 2 is related to the already pushed vcpu hotpluggable flag


These would be pushed once I push the blkdeviotune changes - the order
was only because I made the blkdeviotune changes, then built and found
that the vcpu hotpluggable flag cause test failure.


John Ferlan (2):
  Add blkdeviotune length parameters
  Add VIR_DOMAIN_VCPU_HOTPLUGGABLE

 Changes|  3 ++-
 Virt.xs| 13 +
 lib/Sys/Virt/Domain.pm | 29 +
 3 files changed, 44 insertions(+), 1 deletion(-)

-- 
2.7.4

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

[libvirt] [libvirt-perl PATCH 1/2] Add blkdeviotune length parameters

[John Ferlan]

Add VIR_DOMAIN_BLOCK_IOTUNE*LENGTH and VIR_DOMAIN_TUNABLE_BLKDEV*LENGTH
parameters and their descriptions.

Signed-off-by: John Ferlan 
---
 Changes|  2 +-
 Virt.xs| 12 
 lib/Sys/Virt/Domain.pm | 25 +
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/Changes b/Changes
index 4985100..8d5683a 100644
--- a/Changes
+++ b/Changes
@@ -2,7 +2,7 @@ Revision history for perl module Sys::Virt
 
 2.4.0 2016-00-00
 
- - XXX
+ - Add constants for the new block iotune parameters for length
 
 2.3.0 2016-10-06
 
diff --git a/Virt.xs b/Virt.xs
index b4ca049..88ed722 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -8275,6 +8275,12 @@ BOOT:
   REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, 
BLOCK_IOTUNE_READ_IOPS_SEC_MAX);
   REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, 
BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX);
   REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, 
BLOCK_IOTUNE_SIZE_IOPS_SEC);
+  
REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH, 
BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH);
+  REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH, 
BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH);
+  
REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH, 
BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH);
+  REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH, 
BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH);
+  REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH, 
BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH);
+  REGISTER_CONSTANT_STR(VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH, 
BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH);
 
 
   REGISTER_CONSTANT(VIR_DOMAIN_BLOCK_RESIZE_BYTES, BLOCK_RESIZE_BYTES);
@@ -8484,6 +8490,12 @@ BOOT:
   REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX, 
TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX);
   REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX, 
TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX);
   REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC, 
TUNABLE_BLKDEV_SIZE_IOPS_SEC);
+  
REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH, 
TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH);
+  
REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH, 
TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH);
+  
REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH, 
TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH);
+  
REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH, 
TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH);
+  
REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH, 
TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH);
+  
REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH, 
TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH);
   REGISTER_CONSTANT_STR(VIR_DOMAIN_TUNABLE_CPU_IOTHREADSPIN, 
TUNABLE_IOTHREADSPIN);
 
 
diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm
index e37b078..8bf4115 100644
--- a/lib/Sys/Virt/Domain.pm
+++ b/lib/Sys/Virt/Domain.pm
@@ -2566,6 +2566,31 @@ The maximum I/O operations written per second.
 
 The maximum I/O operations per second
 
+=item Sys::Virt::Domain::BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum total bytes processed per second.
+
+=item Sys::Virt::Domain::BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum bytes read per second.
+
+=item Sys::Virt::Domain::BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum bytes written per second.
+
+=item Sys::Virt::Domain::BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum total I/O operations processed
+per second.
+
+=item Sys::Virt::Domain::BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum I/O operations read per second.
+
+=item Sys::Virt::Domain::BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH
+
+The duration in seconds allowed for maximum I/O operations written per second.
+
 =back
 
 =head2 SCHEDULER CONSTANTS
-- 
2.7.4

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

[libvirt] [PATCH] util: Fix build on s390

[Jiri Denemark]

GCC on s390 complains

util/virconf.c: In function 'virConfGetValueSizeT':
util/virconf.c:1220:9: error: format '%zu' expects argument of type
'size_t', but argument 9 has type 'unsigned int' [-Werror=format=]

Signed-off-by: Jiri Denemark 
---
 src/util/virconf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virconf.c b/src/util/virconf.c
index 3e49f41..1372389 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1219,7 +1219,7 @@ int virConfGetValueSizeT(virConfPtr conf,
 if (((unsigned long long)cval->l) > SIZE_MAX) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("%s: value for '%s' parameter must be in range 
0:%zu"),
-   conf->filename, setting, SIZE_MAX);
+   conf->filename, setting, (size_t) SIZE_MAX);
 return -1;
 }
 #endif
-- 
2.10.1

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

Re: [libvirt] PCP libvirt Plugin

[Michal Privoznik]

On 07.10.2016 09:22, Marko Myllynen wrote:
> Hi,
> 
> On 2016-10-06 15:01, Martin Kletzander wrote:
>> On Wed, Oct 05, 2016 at 07:28:00PM +0300, Marko Myllynen wrote:
>>>
>>> FYI, I've written a PCP plugin (PMDA in PCP parlance) to support most
>>> hypervisor / domain information and metrics available over the libvirt
>>> Python API, it's up to date as of libvirt 2.3 (so it already supports
>>> the recently added perf event metrics).
>>>
>>> The libvirt metrics available with PCP libvirt PMDA and their
>>> descriptions are listed below, they include all the recently added perf
>>> event metrics as well as combined and per-device metrics for each VM.
>>>
>>> I wonder could this be mentioned at https://libvirt.org/apps.html ?
>>
>> Great to hear!  Sure!  Would you mind sending a patch against libvirt's
>> docs/apps.html.in with what you'd like to have mentioned there?  If you
>> don't like doing that I can do that for you, just let me know.
> 
> Sure, how about this?

Almost :-)

> 
> --- a/apps.html
> +++ b/apps.html

this file is generated from the apps.html.in.

> @@ -372,6 +372,12 @@
>  You can use this tool to either set up a new Nagios installation for
>  your Xen or QEMU/KVM guests, or to integrate with your existing 
> Nagios
>  installation.
> +  http://www.pcp.io/man/man1/pmdalibvirt.1.html; 
> shape="rect">PCP
> +The PCP libvirt PMDA (plugin) is part of the
> +http://pcp.io/; shape="rect">PCP toolkit and provides

s,,,

> +hypervisor and guest information and complete set of guest 
> performance
> +metrics. It supports pCPU, vCPU, memory, block device, network 
> interface,
> +and performance event metrics for each virtual guest.
>http://community.zenoss.org/docs/DOC-4687; 
> shape="rect">Zenoss
>  The Zenoss libvirt Zenpack adds support for monitoring virtualization
>  servers.  It has been tested with KVM, QEMU, VMware ESX, and VMware
> 
> Thanks,
> 

I've fixed all of that, came up with a commit message and pushed.

Congratulations on your first libvirt contribution!

Michal

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

Re: [libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Andrea Bolognani]

On Fri, 2016-10-07 at 11:19 +0200, Boris Fiuczynski wrote:
> Andrea,
> there is another "side effect" of the Requires directive.
> libvirt-guests gets automatically started when libvirt is updated to 
> v2.3.0. This has some rather nasty implications for the end users.

Mh, I don't see why that would happen.

The Requires relationship goes in the opposite direction,
so if libvirt-guests was not running before the upgrade
there should be no reason for it to be started, whether
that relationship is there or not.

I'll try to reproduce this on my machine and get back to
you in a while.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] src: Threat pid as signed

[Michal Privoznik]

This initially started as a fix of some debug printing in
virCgroupDetect. However it turned out that other places suffer
from the similar problem. While dealing with pids, esp. in cases
where we cannot use pid_t for ABI stability reasons, we often
chose an unsigned integer type. This makes no sense as pid_t is
signed.
Also, new syntax-check rule is introduced so we won't repeat this
mistake.

Signed-off-by: Michal Privoznik 
---

Technically, this is v2 of [1], but like 99% of the patch is
different, so I'm sending this as a completely new patch.

1: https://www.redhat.com/archives/libvir-list/2016-October/msg00254.html

 cfg.mk|  8 
 src/locking/lock_driver_sanlock.c |  4 ++--
 src/lxc/lxc_controller.c  |  4 ++--
 src/lxc/lxc_domain.c  |  8 
 src/lxc/lxc_driver.c  |  4 ++--
 src/lxc/lxc_monitor.c |  3 +--
 src/lxc/lxc_process.c |  8 
 src/qemu/qemu_process.c   | 12 ++--
 src/util/vircgroup.c  | 24 
 src/util/viridentity.c|  2 +-
 src/util/virpolkit.c  |  1 +
 src/util/virprocess.c | 14 ++
 src/util/virsystemd.c |  9 +
 src/util/virutil.c|  4 ++--
 14 files changed, 56 insertions(+), 49 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 9f5949c..b33b1e2 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -581,6 +581,11 @@ sc_prohibit_int_assign_bool:
halt='use bool type for boolean values' \
  $(_sc_search_regexp)
 
+sc_prohibit_unsigned_pid:
+   @prohibit='\ [^,=]+pid'  
\
+   halt='use signed type for pid values'   \
+ $(_sc_search_regexp)
+
 # Many of the function names below came from this filter:
 # git grep -B2 '\<_('|grep -E '\.c- *[[:alpha:]_][[:alnum:]_]* ?\(.*[,;]$' \
 # |sed 's/.*\.c-  *//'|perl -pe 's/ ?\(.*//'|sort -u \
@@ -1214,6 +1219,9 @@ 
exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets = \
 exclude_file_name_regexp--sc_prohibit_int_ijk = \
   
^(src/remote_protocol-structs|src/remote/remote_protocol\.x|cfg\.mk|include/libvirt/libvirt.+|src/admin_protocol-structs|src/admin/admin_protocol\.x)$$
 
+exclude_file_name_regexp--sc_prohibit_unsigned_pid = \
+  
^(include/libvirt/libvirt-qemu.h|src/(driver-hypervisor.h|libvirt-qemu.c|locking/lock_protocol.x|lxc/lxc_monitor_protocol.x|qemu/qemu_driver.c|remote/qemu_protocol.x|util/virpolkit.c|util/virsystemd.c)|tests/virpolkittest.c|tools/virsh-domain.c)$$
+
 exclude_file_name_regexp--sc_prohibit_getenv = \
   ^tests/.*\.[ch]$$
 
diff --git a/src/locking/lock_driver_sanlock.c 
b/src/locking/lock_driver_sanlock.c
index 332c2de..280219f 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -87,7 +87,7 @@ struct _virLockManagerSanlockPrivate {
 char *vm_name;
 unsigned char vm_uuid[VIR_UUID_BUFLEN];
 unsigned int vm_id;
-unsigned int vm_pid;
+int vm_pid;
 unsigned int flags;
 bool hasRWDisks;
 int res_count;
@@ -494,7 +494,7 @@ static int virLockManagerSanlockNew(virLockManagerPtr lock,
 if (VIR_STRDUP(priv->vm_name, param->value.str) < 0)
 goto error;
 } else if (STREQ(param->key, "pid")) {
-priv->vm_pid = param->value.ui;
+priv->vm_pid = param->value.iv;
 } else if (STREQ(param->key, "id")) {
 priv->vm_id = param->value.ui;
 } else if (STREQ(param->key, "uri")) {
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 8c581df..508bc3e 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1022,7 +1022,7 @@ static void virLXCControllerSignalChildIO(virNetDaemonPtr 
dmn,
 int status;
 
 ret = waitpid(-1, , WNOHANG);
-VIR_DEBUG("Got sig child %d vs %lld", ret, (unsigned long 
long)ctrl->initpid);
+VIR_DEBUG("Got sig child %d vs %lld", ret, (long long) ctrl->initpid);
 if (ret == ctrl->initpid) {
 virNetDaemonQuit(dmn);
 virMutexLock();
@@ -2328,7 +2328,7 @@ virLXCControllerEventSendInit(virLXCControllerPtr ctrl,
 {
 virLXCMonitorInitEventMsg msg;
 
-VIR_DEBUG("Init pid %llu", (unsigned long long)initpid);
+VIR_DEBUG("Init pid %lld", (long long) initpid);
 memset(, 0, sizeof(msg));
 msg.initpid = initpid;
 
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index 9027c25..3a7404f 100644
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -328,8 +328,8 @@ virLXCDomainObjPrivateXMLFormat(virBufferPtr buf,
 {
 virLXCDomainObjPrivatePtr priv = vm->privateData;
 
-virBufferAsprintf(buf, "\n",
-  (unsigned long long)priv->initpid);
+virBufferAsprintf(buf, "\n",
+  (long long) priv->initpid);
 
 return 0;
 }
@@ -340,9 +340,9 @@ 

Re: [libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Boris Fiuczynski]

On 10/07/2016 09:31 AM, Andrea Bolognani wrote:

On Fri, 2016-10-07 at 09:12 +0200, Boris Fiuczynski wrote:

Daniel, Andrea,
have you looked into the change in behaviour caused by patch 4 of this
series? When libvirtd is stopped I get all my running domains suspended.
Restarting libvirtd now also causes a disruption since all domains are
suspended and than resumed.
Before the commit of this patch the domains used to remain running
without these disruptions and as a user this is what I expect to happen.
I propose to revert commit fb2025ede9ecde1aa04ba6e01ce0c82c9e42dc66.


Oh, you're right! Thank you for reporting this issue.

We don't want to revert the change though, merely to
downgrade the Requires to a Wants.

That way, libvirtd stopping (or restarting) will not
cause libvirt-guests to stop, but when starting
libvirt-guests an attempt will still be made to start
up libvirtd if it's not running already.

I'll have patches on the list shortly.

--
Andrea Bolognani / Red Hat / Virtualization



Andrea,
there is another "side effect" of the Requires directive.
libvirt-guests gets automatically started when libvirt is updated to 
v2.3.0. This has some rather nasty implications for the end users.


--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] [PATCH] libvirt-guests: Weaken dependency on libvirtd

[Boris Fiuczynski]

On 10/07/2016 10:19 AM, Andrea Bolognani wrote:

The Requires relationship is very strong, in that it prevents
a unit from running unless all the units it Requires are
running as well.

This turns out to be a problem because we want to be able to
restart libvirtd at any time without having libvirt-guests
suspend or shutdown running guests.

Turn the Requires relationship into a Wants relationship:
this way starting libvirt-guests will cause systemd to (attempt
to) start libvirtd as well, but stopping or restarting libvirtd
will not alter libvirt-guests' running state.
---
 tools/libvirt-guests.service.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
index b4f54f2..02e6747 100644
--- a/tools/libvirt-guests.service.in
+++ b/tools/libvirt-guests.service.in
@@ -1,6 +1,6 @@
 [Unit]
 Description=Suspend/Resume Running libvirt Guests
-Requires=libvirtd.service
+Wants=libvirtd.service
 After=network.target
 After=time-sync.target
 After=libvirtd.service



Reviewed-by: Boris Fiuczynski 

--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

[libvirt] [PATCH] libvirt-guests: Weaken dependency on libvirtd

[Andrea Bolognani]

The Requires relationship is very strong, in that it prevents
a unit from running unless all the units it Requires are
running as well.

This turns out to be a problem because we want to be able to
restart libvirtd at any time without having libvirt-guests
suspend or shutdown running guests.

Turn the Requires relationship into a Wants relationship:
this way starting libvirt-guests will cause systemd to (attempt
to) start libvirtd as well, but stopping or restarting libvirtd
will not alter libvirt-guests' running state.
---
 tools/libvirt-guests.service.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
index b4f54f2..02e6747 100644
--- a/tools/libvirt-guests.service.in
+++ b/tools/libvirt-guests.service.in
@@ -1,6 +1,6 @@
 [Unit]
 Description=Suspend/Resume Running libvirt Guests
-Requires=libvirtd.service
+Wants=libvirtd.service
 After=network.target
 After=time-sync.target
 After=libvirtd.service
-- 
2.7.4

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

Re: [libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Boris Fiuczynski]

On 10/07/2016 09:31 AM, Andrea Bolognani wrote:

On Fri, 2016-10-07 at 09:12 +0200, Boris Fiuczynski wrote:

Daniel, Andrea,
have you looked into the change in behaviour caused by patch 4 of this
series? When libvirtd is stopped I get all my running domains suspended.
Restarting libvirtd now also causes a disruption since all domains are
suspended and than resumed.
Before the commit of this patch the domains used to remain running
without these disruptions and as a user this is what I expect to happen.
I propose to revert commit fb2025ede9ecde1aa04ba6e01ce0c82c9e42dc66.


Oh, you're right! Thank you for reporting this issue.

We don't want to revert the change though, merely to
downgrade the Requires to a Wants.

That way, libvirtd stopping (or restarting) will not
cause libvirt-guests to stop, but when starting
libvirt-guests an attempt will still be made to start
up libvirtd if it's not running already.

I'll have patches on the list shortly.

--
Andrea Bolognani / Red Hat / Virtualization



The directive Wants works for me. Just tried it out.

--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

[libvirt] [PATCH 2/2] tools: Exclude Xen dom0 from libvirt-guests.sh list

[Stefan Bader]

With newer versions of libvirt Domain-0 is again visible in the list of
running guests but it should not be considered as a guest for shutdown
or suspend.

Signed-off-by Stefan Bader 
---
 tools/libvirt-guests.sh.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
index 7380b4b..791d927 100644
--- a/tools/libvirt-guests.sh.in
+++ b/tools/libvirt-guests.sh.in
@@ -121,7 +121,7 @@ list_guests() {
 return 1
 fi
 
-echo $list
+echo "$list" | grep -v ----
 }
 
 # guest_name URI UUID
@@ -539,7 +539,7 @@ gueststatus() {
 for uri in $URIS; do
 set +f
 echo "* $uri URI:"
-retval run_virsh "$uri" list || echo
+retval run_virsh "$uri" list | grep -v "Domain-0" || echo
 done
 set +f
 }
-- 
1.9.1

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

[libvirt] [PATCH 1/2] tools: Ignore newlines in libvirt-guests.sh guest list

[Stefan Bader]

The list file expects all guest UUIDs on the same line as the URI
which the guests run on. This does not happen when the list is
echo'ed in quotes. When stripping the quotes, newlines get transformed
into spaces. Without this, only the first guest on the list is actually
handled.

Based on a fix by Omar Siam 

Bug-Ubuntu: http://bugs.launchpad.net/bugs/1591695

Signed-off-by: Stefan Bader 
---
 tools/libvirt-guests.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
index 7f74b85..7380b4b 100644
--- a/tools/libvirt-guests.sh.in
+++ b/tools/libvirt-guests.sh.in
@@ -499,7 +499,7 @@ stop() {
 fi
 
 if [ -n "$list" ]; then
-echo "$uri" "$list" >>"$LISTFILE"
+echo "$uri" $list >>"$LISTFILE"
 fi
 done
 set +f
-- 
1.9.1

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

[libvirt] libvirt-guest.sh bug fixes

[Stefan Bader]

Two small changes, before I forget about submitting them...

First one affects all environments the same. The list of UIDs which
is generated has each element on a separate line. And using quotes
in the echo preserves those newlines. However the processing assumes
one line per URI and all UIDs separated by spaces. So without dropping
the quotes only one guest will get shutdown/suspended.

The second change is for Xen environments only. Domain-0 appears in
the list of guests and it is a persistent one. So on shutdown, the
script tries to stop Domain-0 (which is not working) and then waits
the whole timeout for it to stop.

-Stefan


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

Re: [libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Andrea Bolognani]

On Fri, 2016-10-07 at 09:12 +0200, Boris Fiuczynski wrote:
> Daniel, Andrea,
> have you looked into the change in behaviour caused by patch 4 of this 
> series? When libvirtd is stopped I get all my running domains suspended.
> Restarting libvirtd now also causes a disruption since all domains are 
> suspended and than resumed.
> Before the commit of this patch the domains used to remain running 
> without these disruptions and as a user this is what I expect to happen.
> I propose to revert commit fb2025ede9ecde1aa04ba6e01ce0c82c9e42dc66.

Oh, you're right! Thank you for reporting this issue.

We don't want to revert the change though, merely to
downgrade the Requires to a Wants.

That way, libvirtd stopping (or restarting) will not
cause libvirt-guests to stop, but when starting
libvirt-guests an attempt will still be made to start
up libvirtd if it's not running already.

I'll have patches on the list shortly.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] PCP libvirt Plugin

[Marko Myllynen]

Hi,

On 2016-10-06 15:01, Martin Kletzander wrote:
> On Wed, Oct 05, 2016 at 07:28:00PM +0300, Marko Myllynen wrote:
>>
>> FYI, I've written a PCP plugin (PMDA in PCP parlance) to support most
>> hypervisor / domain information and metrics available over the libvirt
>> Python API, it's up to date as of libvirt 2.3 (so it already supports
>> the recently added perf event metrics).
>>
>> The libvirt metrics available with PCP libvirt PMDA and their
>> descriptions are listed below, they include all the recently added perf
>> event metrics as well as combined and per-device metrics for each VM.
>>
>> I wonder could this be mentioned at https://libvirt.org/apps.html ?
> 
> Great to hear!  Sure!  Would you mind sending a patch against libvirt's
> docs/apps.html.in with what you'd like to have mentioned there?  If you
> don't like doing that I can do that for you, just let me know.

Sure, how about this?

--- a/apps.html
+++ b/apps.html
@@ -372,6 +372,12 @@
 You can use this tool to either set up a new Nagios installation for
 your Xen or QEMU/KVM guests, or to integrate with your existing Nagios
 installation.
+  http://www.pcp.io/man/man1/pmdalibvirt.1.html; 
shape="rect">PCP
+The PCP libvirt PMDA (plugin) is part of the
+http://pcp.io/; shape="rect">PCP toolkit and provides
+hypervisor and guest information and complete set of guest performance
+metrics. It supports pCPU, vCPU, memory, block device, network 
interface,
+and performance event metrics for each virtual guest.
   http://community.zenoss.org/docs/DOC-4687; 
shape="rect">Zenoss
 The Zenoss libvirt Zenpack adds support for monitoring virtualization
 servers.  It has been tested with KVM, QEMU, VMware ESX, and VMware

Thanks,

-- 
Marko Myllynen

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

Re: [libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Boris Fiuczynski]

Daniel, Andrea,
have you looked into the change in behaviour caused by patch 4 of this 
series? When libvirtd is stopped I get all my running domains suspended.
Restarting libvirtd now also causes a disruption since all domains are 
suspended and than resumed.
Before the commit of this patch the domains used to remain running 
without these disruptions and as a user this is what I expect to happen.

I propose to revert commit fb2025ede9ecde1aa04ba6e01ce0c82c9e42dc66.


On 09/06/2016 03:58 PM, Daniel P. Berrange wrote:

On Tue, Sep 06, 2016 at 03:55:20PM +0200, Andrea Bolognani wrote:

Make libvirt on systemd nicer for the user, by getting rid of
some confusing behavior, and overall more solid.

More details in each specific patch.


ACK to all


Regards,
Daniel




--
Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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

Re: [libvirt] RFC: Exposing "ready" bool (of `query-block-jobs`) or QMP BLOCK_JOB_READY event

[Peter Krempa]

On Thu, Oct 06, 2016 at 15:51:38 -0500, Eric Blake wrote:
> On 10/06/2016 10:06 AM, Kashyap Chamarthy wrote:
> > On Thu, Oct 06, 2016 at 09:25:26AM -0500, Eric Blake wrote:
> >> On 10/06/2016 06:34 AM, Peter Krempa wrote:
> > 
> > [...]
> > 
> >>> We expose the state of the copy job in the XML and forward the READY
> >>> event from qemu to the users.
> >>
> >> I was not aware of that when I was chatting on IRC yesterday; that's
> >> useful to know, because virDomainGetBlockJobInfo() is NOT exposing that
> >> information at the moment.
> > 
> > That is what this RFC was asking to consider -- whether an [I think it
> > has to be a new one] API should report.
> 
> I think we've answered this one - we don't need a new API, because
> parsing the domain XML already provides the current 'ready':true/false
> status from qemu.
> 
> The existing virDomainGetBlockJobInfo() can't be extended, but it can be
> fixed to quit reporting cur==end when ready:false.

Yes, I agree about this one (although I don't really like it [1]), but
this one will actually fix software not listening for events without any
change.

Any new API would not help since the apps would need to change anyways
thus can use the current correct approach right away even with older
libvirt versions.

Peter

[1]: I'm expecting users to start complaining: "Why is this last byte of
my image taking so long to copy after the rest copied pretty quickly".


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

Re: [libvirt] [PATCH] virCgroupDetect: Print pid as long long

[Michal Privoznik]

On 06.10.2016 17:42, Daniel P. Berrange wrote:
> On Thu, Oct 06, 2016 at 05:18:11PM +0200, Pavel Hrdina wrote:
>> On Thu, Oct 06, 2016 at 04:58:40PM +0200, Michal Privoznik wrote:
>>> Pids are signed, report them as such.
>>>
>>> Before:
>>> Detected mount/mapping 2:cpuset at /sys/fs/cgroup/cpuset in 
>>> /machine/qemu-4-fedora.libvirt-qemu/vcpu0 for pid 18446744073709551615
>>>
>>> After:
>>> Detected mount/mapping 2:cpuset at /sys/fs/cgroup/cpuset in 
>>> /machine/qemu-4-fedora.libvirt-qemu/vcpu0 for pid -1
>>>
>>> Signed-off-by: Michal Privoznik 
>>> ---
>>>  src/util/vircgroup.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
>>> index 8b52816..0a06a8a 100644
>>> --- a/src/util/vircgroup.c
>>> +++ b/src/util/vircgroup.c
>>> @@ -732,11 +732,12 @@ virCgroupDetect(virCgroupPtr group,
>>>  return -1;
>>>  }
>>>  
>>> -VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid 
>>> %llu", i,
>>> +VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid %lld",
>>> +  i,
>>>virCgroupControllerTypeToString(i),
>>>group->controllers[i].mountPoint,
>>>group->controllers[i].placement,
>>> -  (unsigned long long)pid);
>>> +  (long long) pid);
>>>  }
>>>  
>>>  return 0;
>>
>> Printing -1 instead of long number is better, but there are lot of
>> other places where we print pids as unsigned:
>>
>>   git grep "unsigned long long.*pid"
>>
>> it would be worth to change them too.
> 
> And add a syntax-check rule to detect such casts.

Well, I don't really like the idea turning every unsigned pid to signed.
There are places where a non-negative PID can be safely assumed (e.g.
virDomainQemuAttach, locking protocol). But okay, I will fix all the
occurrences that I think suffer the same problem and just exclude the
rest from syntax-check.

Michal

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