[libvirt] [PATCH v6 1/3] qmp: rename query_option_descs() to get_param_info()

[Amos Kong]

Signed-off-by: Amos Kong 
Reviewed-by: Eric Blake 
---
 util/qemu-config.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/qemu-config.c b/util/qemu-config.c
index f610101..508adbc 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -39,7 +39,7 @@ QemuOptsList *qemu_find_opts(const char *group)
 return ret;
 }
 
-static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc 
*desc)
+static CommandLineParameterInfoList *get_param_info(const QemuOptDesc *desc)
 {
 CommandLineParameterInfoList *param_list = NULL, *entry;
 CommandLineParameterInfo *info;
@@ -120,9 +120,9 @@ static CommandLineParameterInfoList 
*get_drive_infolist(void)
 
 for (i = 0; drive_config_groups[i] != NULL; i++) {
 if (!head) {
-head = query_option_descs(drive_config_groups[i]->desc);
+head = get_param_info(drive_config_groups[i]->desc);
 } else {
-cur = query_option_descs(drive_config_groups[i]->desc);
+cur = get_param_info(drive_config_groups[i]->desc);
 connect_infolist(head, cur);
 }
 }
@@ -147,7 +147,7 @@ CommandLineOptionInfoList 
*qmp_query_command_line_options(bool has_option,
 info->parameters = get_drive_infolist();
 } else {
 info->parameters =
-query_option_descs(vm_config_groups[i]->desc);
+get_param_info(vm_config_groups[i]->desc);
 }
 entry = g_malloc0(sizeof(*entry));
 entry->value = info;
-- 
1.8.5.3

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

[libvirt] [PATCH v6 0/3] fix query-command-line-options

[Amos Kong]

This patchset fixed some issues of query-command-line-options:
 * some new options that haven't argument can't be queried. (eg: -enable-fips)
 * some legacy options that have argument can't be queried. (eg: -vnc display)

More discussion:
 http://marc.info/?l=qemu-devel&m=139081830416684&w=2
 https://www.redhat.com/archives/libvir-list/2014-March/msg00318.html

V2: remove duplicate option tables, update schema (eric)
V3: fix typo in commitlog and export qemu_options talbe (eric)
V4: avoid the duplicate static table (eric)
V5: rename new field, other fix (markus)
V6: add implied-name (eric, markus)

Thanks for your review!

Amos Kong (3):
  qmp: rename query_option_descs() to get_param_info()
  query-command-line-options: expose implicit parameter name
  query-command-line-options: query all the options in qemu-options.hx

 qapi-schema.json   | 16 +---
 qemu-options.h | 12 
 util/qemu-config.c | 52 ++--
 vl.c   | 19 ++-
 4 files changed, 65 insertions(+), 34 deletions(-)

-- 
1.8.5.3

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

Re: [libvirt] [PATCH] Fix Memory Leak in testGetCaps()

[Ján Tomko]

On 03/26/2014 11:37 PM, Nehal J Wani wrote:
> While running qemucaps2xmltest, it was found that valgrind pointed out
> the following memory leaks:
> 
> ==27045== 160 (112 direct, 48 indirect) bytes in 1 blocks are definitely lost 
> in loss record 51 of 65
> ==27045==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
> ==27045==by 0x4C6BACD: virAllocVar (viralloc.c:560)
> ==27045==by 0x4CAF095: virObjectNew (virobject.c:193)
> ==27045==by 0x421453: virQEMUCapsNew (qemu_capabilities.c:1805)
> ==27045==by 0x41F04F: testQemuCapsXML (qemucaps2xmltest.c:72)
> ==27045==by 0x41FFD1: virtTestRun (testutils.c:201)
> ==27045==by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
> ==27045==by 0x42074D: virtTestMain (testutils.c:789)
> ==27045==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
> ==27045== 
> ==27045== 160 (112 direct, 48 indirect) bytes in 1 blocks are definitely lost 
> in loss record 52 of 65
> ==27045==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
> ==27045==by 0x4C6BACD: virAllocVar (viralloc.c:560)
> ==27045==by 0x4CAF095: virObjectNew (virobject.c:193)
> ==27045==by 0x421453: virQEMUCapsNew (qemu_capabilities.c:1805)
> ==27045==by 0x41F04F: testQemuCapsXML (qemucaps2xmltest.c:72)
> ==27045==by 0x41FFD1: virtTestRun (testutils.c:201)
> ==27045==by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
> ==27045==by 0x42074D: virtTestMain (testutils.c:789)
> ==27045==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
> 
> ---
>  tests/qemucaps2xmltest.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

ACK and pushed.

Jan



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

[libvirt] [PATCH] Modify help information of virsh list command

[Li Yang]

Use 'virsh list domain --title' option can get domain's title,
not description, the original help information 'show short
domain description' will confuse users, so modify it to
'show domain title'

Signed-off-by: Li Yang 
---
 tools/virsh-domain-monitor.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index de4afbb..5d19388 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1753,7 +1753,7 @@ static const vshCmdOptDef opts_list[] = {
 },
 {.name = "title",
  .type = VSH_OT_BOOL,
- .help = N_("show short domain description")
+ .help = N_("show domain title")
 },
 {.name = NULL}
 };
-- 
1.7.1

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

[libvirt] [PATCH] util: fix a typo in virprocess.c

[Hongwei Bi]

s/forcably/forcibly

Signed-off-by: Hongwei Bi 
---
 src/util/virprocess.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 405ad06..9179d73 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -283,7 +283,7 @@ int virProcessKill(pid_t pid, int sig)
  * Try to kill the process and verify it has exited
  *
  * Returns 0 if it was killed gracefully, 1 if it
- * was killed forcably, -1 if it is still alive,
+ * was killed forcibly, -1 if it is still alive,
  * or another error occurred.
  */
 int
-- 
1.7.9.5

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

Re: [libvirt] [PATCH v3 1/2] Move virBhyveTapGetRealDeviceName to virnetdevtap

[Roman Bogorodskiy]

  Ján Tomko wrote:

> ACK

Fixed headers grouping and spurious blank line and pushed, thanks! 

Roman Bogorodskiy

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

Re: [libvirt] [PATCH v3 2/2] bhyve: add xml2args unittest

[Roman Bogorodskiy]

  Ján Tomko wrote:

> > +static virCapsPtr
> > +testBhyveBuildCapabilities(void)
> > +{
> > +virCapsPtr caps;
> > +virCapsGuestPtr guest;
> > +
> > +if ((caps = virCapabilitiesNew(virArchFromHost(),
> 
> Getting the arch from the host seems wrong in a test, but it seems the bhyve
> driver doesn't care about archs.

That's correct, bhyve driver doesn't care about archs currently.

> > +if (!(conn = virGetConnect()))
> 
> Does this have any side-effects? It doesn't seem to be used anywhere.

No, it doesn't, I dropped it.

> caps and xmlopt should be unref'd here.

Fixed.

> ACK with the leak fixed and virGetConnect removed (or explained).

Thanks! Pushed with these fixes.

Roman Bogorodskiy

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

Re: [libvirt] [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx

[Amos Kong]

On Thu, Mar 20, 2014 at 10:12:43PM +0800, Amos Kong wrote:
> On Tue, Mar 11, 2014 at 10:04:56AM +0100, Markus Armbruster wrote:
> > Eric Blake  writes:
> > 
> > > On 03/07/2014 02:54 AM, Markus Armbruster wrote:
> > >> Eric Blake  writes:
> > >> 
> > >>> On 03/05/2014 07:36 PM, Amos Kong wrote:
> >  vm_config_groups[] only contains part of the options which have
> >  argument, and all options which have no argument aren't added
> >  to vm_config_groups[]. Current query-command-line-options only
> >  checks options from vm_config_groups[], so some options will
> >  be lost.
> > 
> > >
> > >>Example: -device takes unspecified parameters.  -cdrom doesn't take
> > >>parameters, it takes a file name.  Yet, the command reports the same
> > >>for both: "parameters": [], "argument": true.
> > >> 
> > >>Looks like we need a tri-state: option takes no argument, QemuOpts
> > >>argument, or other argument.
> > >
> > > I don't buy that.  '-cdrom filename' could easily be re-written [in a
> > > future qemu version] to use QemuOpts with an implied parameter name
> > > (we've done that elsewhere, such as for '-machine').  In other words, I
> > > think we could make it become shorthand for '-cdrom file=filename', at
> > > which point the QemuOpts spelling is available and would now show up as
> > > "parameters":[{"name":"file"...}].  Thus, in converting -cdrom to
> > > QemuOpts, we can still maintain command-line back-compat, while making
> > > the query-command-line-options output more featureful.  In other words,
> > > _for now_ it takes unspecified parameters, and the fact that it is only
> > > a single parameter in the form 'filename' rather than a more typical
> > > parameter 'file=filename' is not a show-stopper.
> > 
> > Incompatible change for funny filenames: -cdrom you,break=me.
> > 
> > Besides breaking funny filenames, we'd also buy ourselves some stupid
> > -readconfig / -writeconfig trouble.  Let me explain.
> > 
> > -cdrom F is effectively sugar for "-drive media=cdrom,index=2,file=FF",
> > where FF is F with comma doubled.
> > 
> > -writeconfig writes out desugared QemuOpts.  Therefore, "-cdrom r7.iso"
> > gets written as
> > 
> > [drive]
> >   media = "cdrom"
> >   index = "2"
> >   file = "r7.iso"
> > 
> > which -readconfig can read.
> > 
> > If we convert -cdrom to QemuOpts, it gets written out like this:
> > 
> > [cdrom]
> >file = "r7.iso"
> > 
> > If we continue to desugar it, it'll *also* get written out as before.
> > Either we *delete* the sugared QemuOpts to avoid duplication, or we
> > *stop* desugaring.  The latter breaks -readconfig of existing
> > configuration files, and complicates the code reading configuration from
> > QemuOpts.
> > 
> > I don't think any of the old non-QemuOpts options that have become sugar
> > for newer, more flexible QemuOpts options should be converted to
> > QemuOpts.
> > 
> > > So your idea of a tri-state (QemuOpts, no argument, or other argument)
> > > doesn't add anything - any option that takes "other argument" could be
> > > converted to take QemuOpts, and from the command line, we can't tell the
> > > difference from whether something was implemented by QemuOpts, only by
> > > whether we have introspection on what the argument consists of.
> > 
> > I doubt we can convert all existing options to QemuOpts without breaking
> > backward compatibility and complicating the code.
> > 
> > > Meanwhile, it DOES point out that our use of implicit argument in
> > > QemuOpts ought to be exposed to the introspection mechanism, for
> > > introspection to be fully descriptive.  That is, maybe we should modify
> > > our introspection to add a new 'implied-name':
> > >
> > > ##
> > > # @CommandLineParameterInfo:
> > > #
> > > ...
> > > # @implied-name: #optional, if present and true, the parameter can be
> > > #specified as '-option value' instead of the preferred
> > > #spelling of '-option name=value' (since 2.0)
> > > # Since 1.5
> > > { 'type': 'CommandLineParameterInfo',
> > >   'data': { 'name': 'str',
> > > 'type': 'CommandLineParameterType',
> > > '*help': 'str', '*implied-name': 'bool' } }
  
reply-myself

> How can we get this information? it's not good to rely on the help message.
> 
> And the parameters [] only have content when the option have a non-NULL desc
> table, so we always just return a NULL parameters list, the 'implied-name'
> information will be lost.

I'm wrong, 'implied-name' is attribute of parameter, it's stored in
QemuOptsList.implied_opt_name.

I will add this filed in V6.
 
> I thinks Markus's suggestion is fine, we can use tri-state (no-arg,
> unsuecified-para-arg, no-para-arg).

Thanks, Amos
 
> > The only use for implied-name I can think of is interpreting a user's
> > command line.  Is that a real use case?
> 
>  
> > >> 
> > >>parameters is [] unless it's a QemuOpts argument.  Then it lists the
> > >>recognize

Re: [libvirt] [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx

[Amos Kong]

On Wed, Mar 26, 2014 at 02:15:18PM +0100, Markus Armbruster wrote:
> Amos Kong  writes:
> 
> > On Fri, Mar 07, 2014 at 10:54:09AM +0100, Markus Armbruster wrote:
> >> Eric Blake  writes:
> >> 
> >> > On 03/05/2014 07:36 PM, Amos Kong wrote:
> >> >> vm_config_groups[] only contains part of the options which have
> >> >> argument, and all options which have no argument aren't added
> >> >> to vm_config_groups[]. Current query-command-line-options only
> >> >> checks options from vm_config_groups[], so some options will
> >> >> be lost.
> >> >> 
> >> >> We have macro in qemu-options.hx to generate a table that
> >> >> contains all the options. This patch tries to query options
> >> >> from the table.
> >> >> 
> >> >> Then we won't lose the legacy options that weren't added to
> >> >> vm_config_groups[] (eg: -vnc, -smbios). The options that have
> >> >> no argument will also be returned (eg: -enable-fips)
> >> >> 
> >> >> Some options that have argument have a NULL desc list, some
> >> >> options don't have argument, and "parameters" is mandatory
> >> >> in the past. So we add a new field "argument" to present
> >> >> if the option takes unspecified arguments.
> >> >
> >> > I like Markus' suggestion of naming the new field
> >> > 'unspecified-parameters' rather than 'argument'.
> >  
> > Hi Markus,
> >
> >> Looking again, there are more problems.
> >> 
> >> 1. Non-parameter argument vs. parameter argument taking unspecified
> >>parameters
> >> 
> >>Example: -device takes unspecified parameters.  -cdrom doesn't take
> >>parameters, it takes a file name.  Yet, the command reports the same
> >>for both: "parameters": [], "argument": true.
> >> 
> >>Looks like we need a tri-state: option takes no argument, QemuOpts
> >>argument, or other argument.
> >
> > '-cdrom' is the 'other argument' == 'Non-parameter argument'?
> 
> Let me clarify my terminology:
> 
> * A "parameter argument" is an option argument of the form KEY=VALUE,...
>   Since parameter arguments should always be parsed with QemuOpts[*], I
>   use the term "QemuOpts argument" interchangeably.
> 
> * A "non-parameter argument" or "other argument" is an option argument
>   that doesn't use this form.
> 
> Does that answer your question?

Got it, thanks.
 
> > We can use a enum state.
> 
> I'm not sure I got that.
> 
> > |  { 'enum': 'ArgumentStateType',
> > |'data': ['no-argument', 'unspecified-parameters-argument',
> > | 'non-parameter-argument']
> > |  }


{'enum': 'ArgumentStateType',
 'data': ['no-argument', 'qemuopts-argument', 'non-param-argument']
}

 no-argument: -enable-kvm
 qemuopts-argument:   -boot order=c,menu=on
 non-param-argument:  -cdrom file


 I don't know if it's the tri-state you suggested in previous reply.

> > |  
> > |  { 'type': 'CommandLineOptionInfo',
> > |'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'],
> > |  '*argument-state': 'ArgumentStateType' } }
> >  
> >>parameters is [] unless it's a QemuOpts argument.  Then it lists the
> >>recognized parameters.
> >
> > How about balloon? we should treat as 'taking unspecified parameters'?
> >
> > "-balloon none   disable balloon device\n"
> > "-balloon virtio[,addr=str]\n"
> >
> > I think we can only check this from help message in qemu-options.hx,
> > is it a stable/acceptable method?
> 
> -balloon is yet another sugar option:
> 
> * "-balloon none" does nothing.  It could suppress a default balloon
>   device, if such a thing existed.
> 
> * "-balloon virtio,KEY=VALUE..." is sugar for "-device
>   virtio-balloon,KEY=VALUE...".  Keys other than "addr" are
>   undocumented.
> 
> The actual option argument parsing is ad hoc, not QemuOpts.
> 
> I sure hope something like this would not pass review today.
> 
> My advice to tools introspecting the command line is to avoid sugared
> options, unless the desugaring encapsulates something they don't want to
> know.
> 
> > We introduce query-command-line-options command to avoid libvirt to
> > check qemu commandline information by scanning qemu's help message,
> > it's not strict & stable.
> >  
> >> 2. Our dear friend -drive is more complicated than you might think
> >> 
> >>We special-case it to report the union of drive_config_groups[],
> >>which contains qemu_legacy_drive_opts, qemu_common_drive_opts and
> >>qemu_drive_opts.  The latter accepts unspecified parameters.
> >
> > I'm confused here. But there is only one option (-drive), we should
> > return merged desc table (legacy + common).
> >
> > Desc table of qemu_drive_opts is NULL, then -drive can also take
> > unspecified parameters?
> 
> Yes: driver-specific parameters.
> 
> -drive takes currently takes unspecified parameters (the driver-specific
> parameters) in addition to a number of specified parameters (the common
> and legacy parameters).
> 
> >>I believe qemu_drive_opts is actually not used by the (complex!) code
> >> 

Re: [libvirt] [Qemu-devel] [PATCH v3 for 2.0] update names in option tables to match with actual command-line spelling

[Amos Kong]

On Thu, Mar 27, 2014 at 10:16:44AM +0800, Amos Kong wrote:
> On Wed, Mar 26, 2014 at 05:12:08PM +0100, Markus Armbruster wrote:
> > Eric Blake  writes:

...
> > > Reviewed-by: Eric Blake 
> > 
> > I'm not thrilled about the ABI break, but avoiding it would probably
> > take too much code for too little gain.
 
Hi Markus, Eric

> We can add an 'alias_index' field to QemuOptsList, and init it to -1
> in qemu_add_opts().
> 
> And we only re-set 'alias_index' to 'popt->index' in vl.c(option parsing part)
> then we can find opts by qemu_options[i].index.
> 
> We also need to give a warning () if it's group name of added QemuOptsList
> doesn't match with defined option name.
> 
> If someone add a new QemuOpts and the group name doesn't match, he/she will
> get a warning, and call a help function to re-set 'alias_index'.
>  
> I can send a RFC patch for this.

* Option 1
Attached two RFC patches, it added a new field ('alias_index') to
QemuOptsList, and record QEMU_OPTION enum-index to it when group
name of option table doesn't match option name.

And try to find list by index before find list by option name in
qmp_query_command_line_options().

This can avoid the ABI breaking, it also introduced some trouble.
We also need to check if alias_index of unmatched option is set
to a positive number, and report error (option name doesn't match, and
alias_index isn't set) in error state.

* Option 2
OR pass enum-index to qemu_add_opts(), and set enum-index for all the options.
   -void qemu_add_opts(QemuOptsList *list)
   +void qemu_add_opts(QemuOptsList *list, int index)

* Option 3 break ABI

Let's make a decision.

> > How can we prevent future violations of the convention "QemuOptsList
> > member name matches the name of the (primary) option using it for its
> > argument"?  Right now, all we have is /* option name */ tacked to the
> > member.  Which is at best a reminder for those who already know.
> 
> It's absolutely necessary!
>  
> > I'd ask for a test catching violations if I could think of an easy way
> > to code it.
> 
> Currently I prefer this option, so I will send a V3 with strict checking.


-- 
Amos.
>From 7e5f08331fdcc074027de0767bcbbc4d3af9a546 Mon Sep 17 00:00:00 2001
From: Amos Kong 
Date: Thu, 27 Mar 2014 07:37:24 +0800
Subject: [PATCH RFC 1/2] add alias_index to QemuOptsList

If group name of option table doesn't match with option name,
we will save the index of QEMU_OPTION index to alias_index.
Then we can try to find QemuOptsList by index for unmatched
options.

alias_index of matched options will be set to -1.

Signed-off-by: Amos Kong 
---
 hw/acpi/core.c |  2 ++
 include/qapi/qmp/qerror.h  |  3 +++
 include/qemu/config-file.h |  1 +
 include/qemu/option.h  |  1 +
 util/qemu-config.c | 16 
 vl.c   |  2 ++
 6 files changed, 25 insertions(+)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 79414b4..1f459fc 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -27,6 +27,7 @@
 #include "qapi/opts-visitor.h"
 #include "qapi/dealloc-visitor.h"
 #include "qapi-visit.h"
+#include "qemu-options.h"
 
 struct acpi_table_header {
 uint16_t _length; /* our length, not actual part of the hdr */
@@ -64,6 +65,7 @@ static QemuOptsList qemu_acpi_opts = {
 static void acpi_register_config(void)
 {
 qemu_add_opts(&qemu_acpi_opts);
+qemu_set_opt_index(qemu_acpi_opts.name, QEMU_OPTION_acpitable, NULL);
 }
 
 machine_init(acpi_register_config);
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index da75abf..cfbd29d 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -134,6 +134,9 @@ void qerror_report_err(Error *err);
 #define QERR_INVALID_OPTION_GROUP \
 ERROR_CLASS_GENERIC_ERROR, "There is no option group '%s'"
 
+#define QERR_INVALID_OPTION_INDEX \
+ERROR_CLASS_GENERIC_ERROR, "There is no option index '%d'"
+
 #define QERR_INVALID_PARAMETER \
 ERROR_CLASS_GENERIC_ERROR, "Invalid parameter '%s'"
 
diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h
index dbd97c4..1884313 100644
--- a/include/qemu/config-file.h
+++ b/include/qemu/config-file.h
@@ -8,6 +8,7 @@
 
 QemuOptsList *qemu_find_opts(const char *group);
 QemuOptsList *qemu_find_opts_err(const char *group, Error **errp);
+void qemu_set_opt_index(const char *group, int index, Error **errp);
 void qemu_add_opts(QemuOptsList *list);
 void qemu_add_drive_opts(QemuOptsList *list);
 int qemu_set_option(const char *str);
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 8c0ac34..0d63ec9 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -103,6 +103,7 @@ typedef struct QemuOptDesc {
 
 struct QemuOptsList {
 const char *name;
+int alias_index;
 const char *implied_opt_name;
 bool merge_lists;  /* Merge multiple uses of option into a single list? */
 QTAILQ_HEAD(, QemuOpts) head;
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 508adbc..87

Re: [libvirt] is there a way to convert vm's filter into comandline

[Eric Blake]

On 03/26/2014 08:43 PM, Eric Blake wrote:
> On 03/26/2014 07:20 PM, bigclouds wrote:
>> hi,all
>>  
>> is there a way to convert vm's filter into comandline, i think it is useful.
> 
> You mean, as in
>   virsh domxml-to-native qemu-argv $(virsh dumpxml $dom)

Correction:

virsh dumpxml $dom > file
virsh domxml-to-native qemu-argv file


-- 
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] is there a way to convert vm's filter into comandline

[Eric Blake]

On 03/26/2014 07:20 PM, bigclouds wrote:
> hi,all
>  
> is there a way to convert vm's filter into comandline, i think it is useful.

You mean, as in
  virsh domxml-to-native qemu-argv $(virsh dumpxml $dom)

or are you asking about the nwfilter settings applied on behalf of a guest?

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

[libvirt] [PATCH 1/3] only add qemu_tpmdev_opts when CONFIG_TPM is defined

[Amos Kong]

Signed-off-by: Amos Kong 
---
 vl.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/vl.c b/vl.c
index 2355227..596ecfa 100644
--- a/vl.c
+++ b/vl.c
@@ -449,6 +449,7 @@ static QemuOptsList qemu_object_opts = {
 },
 };
 
+#ifdef CONFIG_TPM
 static QemuOptsList qemu_tpmdev_opts = {
 .name = "tpmdev",
 .implied_opt_name = "type",
@@ -458,6 +459,7 @@ static QemuOptsList qemu_tpmdev_opts = {
 { /* end of list */ }
 },
 };
+#endif
 
 static QemuOptsList qemu_realtime_opts = {
 .name = "realtime",
@@ -2992,7 +2994,9 @@ int main(int argc, char **argv, char **envp)
 qemu_add_opts(&qemu_sandbox_opts);
 qemu_add_opts(&qemu_add_fd_opts);
 qemu_add_opts(&qemu_object_opts);
+#ifdef CONFIG_TPM
 qemu_add_opts(&qemu_tpmdev_opts);
+#endif
 qemu_add_opts(&qemu_realtime_opts);
 qemu_add_opts(&qemu_msg_opts);
 qemu_add_opts(&qemu_name_opts);
-- 
1.8.5.3

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

[libvirt] [PATCH 2/3] abort QEMU if group name in option table doesn't match with defined option name

[Amos Kong]

All the options are defined in qemu-options.hx. If we can't find a
matched option definition by group name of option table, then the
group name doesn't match with defined option name, it's not allowed
from 2.0

Signed-off-by: Amos Kong 
---
 qemu-options.h | 12 
 util/qemu-config.c | 28 
 vl.c   | 19 ++-
 3 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/qemu-options.h b/qemu-options.h
index 89a009e..4024487 100644
--- a/qemu-options.h
+++ b/qemu-options.h
@@ -28,9 +28,21 @@
 #ifndef _QEMU_OPTIONS_H_
 #define _QEMU_OPTIONS_H_
 
+#include "sysemu/arch_init.h"
+
 enum {
 #define QEMU_OPTIONS_GENERATE_ENUM
 #include "qemu-options-wrapper.h"
 };
 
+#define HAS_ARG 0x0001
+
+typedef struct QEMUOption {
+const char *name;
+int flags;
+int index;
+uint32_t arch_mask;
+} QEMUOption;
+
+extern const QEMUOption qemu_options[];
 #endif
diff --git a/util/qemu-config.c b/util/qemu-config.c
index f610101..eba5428 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -6,6 +6,14 @@
 #include "hw/qdev.h"
 #include "qapi/error.h"
 #include "qmp-commands.h"
+#include "qemu-options.h"
+
+const QEMUOption qemu_options[] = {
+{ "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
+#define QEMU_OPTIONS_GENERATE_OPTIONS
+#include "qemu-options-wrapper.h"
+{ NULL },
+};
 
 static QemuOptsList *vm_config_groups[32];
 static QemuOptsList *drive_config_groups[4];
@@ -184,6 +192,20 @@ void qemu_add_drive_opts(QemuOptsList *list)
 abort();
 }
 
+/* check if the option is defined in qemu-options.hx */
+static bool opt_is_defined(const char *name)
+{
+int i;
+
+for (i = 0; qemu_options[i].name; i++) {
+if (!strcmp(qemu_options[i].name, name)) {
+return true;
+}
+}
+
+return false;
+}
+
 void qemu_add_opts(QemuOptsList *list)
 {
 int entries, i;
@@ -193,6 +215,12 @@ void qemu_add_opts(QemuOptsList *list)
 for (i = 0; i < entries; i++) {
 if (vm_config_groups[i] == NULL) {
 vm_config_groups[i] = list;
+if (!opt_is_defined(list->name)) {
+error_report("Didn't find a matched option definition, "
+ "group name (%s) of option table must match with "
+ "defined option name (Since 2.0)", list->name);
+abort();
+}
 return;
 }
 }
diff --git a/vl.c b/vl.c
index 596ecfa..580bd22 100644
--- a/vl.c
+++ b/vl.c
@@ -111,7 +111,6 @@ int main(int argc, char **argv)
 #include "trace/control.h"
 #include "qemu/queue.h"
 #include "sysemu/cpus.h"
-#include "sysemu/arch_init.h"
 #include "qemu/osdep.h"
 
 #include "ui/qemu-spice.h"
@@ -2082,22 +2081,6 @@ static void help(int exitcode)
 exit(exitcode);
 }
 
-#define HAS_ARG 0x0001
-
-typedef struct QEMUOption {
-const char *name;
-int flags;
-int index;
-uint32_t arch_mask;
-} QEMUOption;
-
-static const QEMUOption qemu_options[] = {
-{ "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
-#define QEMU_OPTIONS_GENERATE_OPTIONS
-#include "qemu-options-wrapper.h"
-{ NULL },
-};
-
 static bool vga_available(void)
 {
 return object_class_by_name("VGA") || object_class_by_name("isa-vga");
@@ -2842,6 +2825,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
 return popt;
 }
 
+#undef HAS_ARG
+
 static gpointer malloc_and_trace(gsize n_bytes)
 {
 void *ptr = malloc(n_bytes);
-- 
1.8.5.3

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

[libvirt] [PATCH 3/3] update names in option tables to match with actual command-line spelling

[Amos Kong]

We want to establish a mapping between option name and option table,
then we can search related option table by option name.

This patch makes all the member name of QemuOptsList to match with
actual command-line spelling(option name).

[ Important Note ]

The QemuOptsList member name values are ABI, changing them can break
existing -readconfig configuration files.

This patch changes:

fromto  introduced in
acpiacpitable   0c764a9 v1.5.0
boot-opts   boot3d3b830 v1.0
smp-optssmp 12b7f57 v1.6.0

All three have calcified into ABI already.

I have updated the release note of 2.0
http://wiki.qemu.org/ChangeLog/2.0#ABI_breaking

Signed-off-by: Amos Kong 
Reviewed-by: Eric Blake 
---
 hw/acpi/core.c|  8 
 hw/nvram/fw_cfg.c |  4 ++--
 include/qemu/option.h |  2 +-
 vl.c  | 14 +++---
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 79414b4..12e9ae8 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -54,16 +54,16 @@ static const char unsigned dfl_hdr[ACPI_TABLE_HDR_SIZE - 
ACPI_TABLE_PFX_SIZE] =
 char unsigned *acpi_tables;
 size_t acpi_tables_len;
 
-static QemuOptsList qemu_acpi_opts = {
-.name = "acpi",
+static QemuOptsList qemu_acpitable_opts = {
+.name = "acpitable",
 .implied_opt_name = "data",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_acpi_opts.head),
+.head = QTAILQ_HEAD_INITIALIZER(qemu_acpitable_opts.head),
 .desc = { { 0 } } /* validated with OptsVisitor */
 };
 
 static void acpi_register_config(void)
 {
-qemu_add_opts(&qemu_acpi_opts);
+qemu_add_opts(&qemu_acpitable_opts);
 }
 
 machine_init(acpi_register_config);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 282341a..3e6b048 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -125,7 +125,7 @@ static void fw_cfg_bootsplash(FWCfgState *s)
 const char *temp;
 
 /* get user configuration */
-QemuOptsList *plist = qemu_find_opts("boot-opts");
+QemuOptsList *plist = qemu_find_opts("boot");
 QemuOpts *opts = QTAILQ_FIRST(&plist->head);
 if (opts != NULL) {
 temp = qemu_opt_get(opts, "splash");
@@ -191,7 +191,7 @@ static void fw_cfg_reboot(FWCfgState *s)
 const char *temp;
 
 /* get user configuration */
-QemuOptsList *plist = qemu_find_opts("boot-opts");
+QemuOptsList *plist = qemu_find_opts("boot");
 QemuOpts *opts = QTAILQ_FIRST(&plist->head);
 if (opts != NULL) {
 temp = qemu_opt_get(opts, "reboot-timeout");
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 8c0ac34..96b7c29 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -102,7 +102,7 @@ typedef struct QemuOptDesc {
 } QemuOptDesc;
 
 struct QemuOptsList {
-const char *name;
+const char *name;  /* option name */
 const char *implied_opt_name;
 bool merge_lists;  /* Merge multiple uses of option into a single list? */
 QTAILQ_HEAD(, QemuOpts) head;
diff --git a/vl.c b/vl.c
index 580bd22..bd44c52 100644
--- a/vl.c
+++ b/vl.c
@@ -387,7 +387,7 @@ static QemuOptsList qemu_machine_opts = {
 };
 
 static QemuOptsList qemu_boot_opts = {
-.name = "boot-opts",
+.name = "boot",
 .implied_opt_name = "order",
 .merge_lists = true,
 .head = QTAILQ_HEAD_INITIALIZER(qemu_boot_opts.head),
@@ -1358,7 +1358,7 @@ static void numa_add(const char *optarg)
 }
 
 static QemuOptsList qemu_smp_opts = {
-.name = "smp-opts",
+.name = "smp",
 .implied_opt_name = "cpus",
 .merge_lists = true,
 .head = QTAILQ_HEAD_INITIALIZER(qemu_smp_opts.head),
@@ -3226,7 +3226,7 @@ int main(int argc, char **argv, char **envp)
 drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
 break;
 case QEMU_OPTION_boot:
-opts = qemu_opts_parse(qemu_find_opts("boot-opts"), optarg, 1);
+opts = qemu_opts_parse(qemu_find_opts("boot"), optarg, 1);
 if (!opts) {
 exit(1);
 }
@@ -3595,7 +3595,7 @@ int main(int argc, char **argv, char **envp)
 break;
 }
 case QEMU_OPTION_acpitable:
-opts = qemu_opts_parse(qemu_find_opts("acpi"), optarg, 1);
+opts = qemu_opts_parse(qemu_find_opts("acpitable"), optarg, 1);
 if (!opts) {
 exit(1);
 }
@@ -3662,7 +3662,7 @@ int main(int argc, char **argv, char **envp)
 }
 break;
 case QEMU_OPTION_smp:
-if (!qemu_opts_parse(qemu_find_opts("smp-opts"), optarg, 1)) {
+if (!qemu_opts_parse(qemu_find_opts("smp"), optarg, 1)) {
 exit(1);
 }
 break;
@@ -3987,7 +3987,7 @@ int main(int argc, char **argv, char **envp)
 data_dir[data_dir_idx++] = CONFIG_QEMU_DATADIR;
 }
 
-smp_parse(qemu_opts_find(qemu_find_opts(

[libvirt] [PATCH 0/3] ABI change: change group name of option table to match with option name

[Amos Kong]

This patchset changes group names of option tables to match with option name,
this breakes ABI, release note was updated.

Amos Kong (3):
  only add qemu_tpmdev_opts when CONFIG_TPM is defined
  abort QEMU if group name in option table doesn't match with defined
option name
  update names in option tables to match with actual command-line
spelling

 hw/acpi/core.c|  8 
 hw/nvram/fw_cfg.c |  4 ++--
 include/qemu/option.h |  2 +-
 qemu-options.h| 12 
 util/qemu-config.c| 28 
 vl.c  | 37 +
 6 files changed, 60 insertions(+), 31 deletions(-)

-- 
1.8.5.3

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

Re: [libvirt] how libvirt know the hostname of vm

[Eric Blake]

On 03/26/2014 07:16 PM, bigclouds wrote:
> hi,all
> i recently notice that virt-manager show the hostname of vm, i am curious 
> about that, that is the implementation?

That's a question better asked on the virt-manager list
(http://www.redhat.com/mailman/listinfo/virt-tools-list), as libvirt is
not directly doing that.  I think it involves the use of libguestfs to
snoop (a read-only snapshot of) the disk image of the guest.

-- 
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 v3 for 2.0] update names in option tables to match with actual command-line spelling

[Amos Kong]

On Wed, Mar 26, 2014 at 05:12:08PM +0100, Markus Armbruster wrote:
> Eric Blake  writes:
> 
> > On 03/20/2014 07:07 AM, Amos Kong wrote:
> >> We want to establish a mapping between option name and option table,
> >> then we can search related option table by option name.
> >> 
> >> This patch makes all the member name of QemuOptsList to match with
> >> actual command-line spelling(option name).
> >> 
> >> [ Important Note ]
> >> 
> >> The QemuOptsList member name values are ABI, changing them can break
> >> existing -readconfig configuration files.
> >> 
> >> This patch changes:
> >> 
> >> fromto  introduced in
> >> acpiacpitable   0c764a9 v1.5.0
> >> boot-opts   boot3d3b830 v1.0
> >> smp-optssmp 12b7f57 v1.6.0
> >> 
> >> All three have calcified into ABI already.
> >> 
> >> I have updated the release note of 2.0
> >> http://wiki.qemu.org/ChangeLog/2.0#ABI_breaking
> >> 
> >> Signed-off-by: Amos Kong 
> >> ---
> >
> > The benefit of this patch is that 'query-command-line-options' gains a
> > fix where three bogus entries are replaced by their actual command line
> > spelling.  The drawback is that anyone that doesn't pay attention to the
> > ABI break announcement, and expects -readconfig and friends to work
> > while using the old spelling, is in for a surprise.  But since we have
> > prominently documented the change, and since consistency makes life
> > nicer, I'm in favor of this patch.
> >
> > Reviewed-by: Eric Blake 
> 
> I'm not thrilled about the ABI break, but avoiding it would probably
> take too much code for too little gain.

We can add an 'alias_index' field to QemuOptsList, and init it to -1
in qemu_add_opts().

And we only re-set 'alias_index' to 'popt->index' in vl.c(option parsing part)
then we can find opts by qemu_options[i].index.

We also need to give a warning () if it's group name of added QemuOptsList
doesn't match with defined option name.

If someone add a new QemuOpts and the group name doesn't match, he/she will
get a warning, and call a help function to re-set 'alias_index'.
 
I can send a RFC patch for this.


> How can we prevent future violations of the convention "QemuOptsList
> member name matches the name of the (primary) option using it for its
> argument"?  Right now, all we have is /* option name */ tacked to the
> member.  Which is at best a reminder for those who already know.

It's absolutely necessary!
 
> I'd ask for a test catching violations if I could think of an easy way
> to code it.

Currently I prefer this option, so I will send a V3 with strict checking.

-- 
Amos.

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

[libvirt] is there a way to convert vm's filter into comandline

[bigclouds]

hi,all
 
is there a way to convert vm's filter into comandline, i think it is useful.
if there is the functionality, so you think it is  worthy to be done.
 
thanks--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] how libvirt know the hostname of vm

[bigclouds]

hi,all
i recently notice that virt-manager show the hostname of vm, i am curious about 
that, that is the implementation?
 
thanks--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/n] conf: split network host structs to util/

[Eric Blake]

Continuing the refactoring of host-side storage descriptions out
of conf/domain_conf and into util/virstoragefile, this patch
focuses on details about a host name/port/transport as used by
a network storage volume.

* src/conf/domain_conf.h (virDomainDiskProtocolTransport)
(virDomainDiskHostDef, virDomainDiskHostDefClear)
(virDomainDiskHostDefFree, virDomainDiskHostDefCopy): Move...
* src/util/virstoragefile.h (virStorageNetHostTransport)
(virStorageNetHostDef, virStorageNetHostDefClear)
(virStorageNetHostDefFree, virStorageNetHostDefCopy): ...here,
with better names.
* src/util/virstoragefile.c (virStorageNetHostDefClear)
(virStorageNetHostDefFree, virStorageNetHostDefCopy): Moved from...
* src/conf/domain_conf.c (virDomainDiskHostDefClear)
(virDomainDiskHostDefFree, virDomainDiskHostDefCopy): ...here.
(virDomainDiskSourceDefClear, virDomainDiskSourceDefParse)
(virDomainDiskSourceDefFormatInternal): Adjust callers.
* src/conf/snapshot_conf.h (_virDomainSnapshotDiskDef): Likewise.
* src/conf/snapshot_conf.c (virDomainSnapshotDiskDefClear):
Likewise.
* src/qemu/qemu_command.c (qemuAddRBDHost)
(qemuParseDriveURIString, qemuParseNBDString)
(qemuBuildNetworkDriveURI, qemuParseCommandLineDisk)
(qemuParseCommandLine, qemuGetDriveSourceString): Likewise.
* src/qemu/qemu_command.h: Likewise.
* src/qemu/qemu_conf.c (qemuAddISCSIPoolSourceHost)
(qemuTranslateDiskSourcePool): Likewise.
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive)
(qemuDomainSnapshotUndoSingleDiskActive): Likewise.
* src/storage/storage_backend_gluster.c
(virStorageFileBackendGlusterInit): Likewise.
* src/storage/storage_driver.c (virStorageFileFree)
(virStorageFileInitInternal): Likewise.
* src/storage/storage_driver.h (_virStorageFile): Likewise.
* src/libvirt_private.syms (domain_conf.h): Move symbols...
(virstoragefile.h): ...as appropriate.

Signed-off-by: Eric Blake 
---
 src/conf/domain_conf.c| 88 +--
 src/conf/domain_conf.h| 28 ++-
 src/conf/snapshot_conf.c  |  5 +-
 src/conf/snapshot_conf.h  |  4 +-
 src/libvirt_private.syms  | 10 ++--
 src/qemu/qemu_command.c   | 38 +++
 src/qemu/qemu_command.h   |  6 +--
 src/qemu/qemu_conf.c  |  4 +-
 src/qemu/qemu_driver.c| 24 +-
 src/storage/storage_backend_gluster.c |  8 ++--
 src/storage/storage_driver.c  |  8 ++--
 src/storage/storage_driver.h  |  2 +-
 src/util/virstoragefile.c | 71 +++-
 src/util/virstoragefile.h | 30 +++-
 14 files changed, 167 insertions(+), 159 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 66eeaa9..159a9c3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -276,11 +276,6 @@ VIR_ENUM_IMPL(virDomainDiskProtocol, 
VIR_DOMAIN_DISK_PROTOCOL_LAST,
   "ftps",
   "tftp")

-VIR_ENUM_IMPL(virDomainDiskProtocolTransport, VIR_DOMAIN_DISK_PROTO_TRANS_LAST,
-  "tcp",
-  "unix",
-  "rdma")
-
 VIR_ENUM_IMPL(virDomainDiskSecretType, VIR_DOMAIN_DISK_SECRET_TYPE_LAST,
   "none",
   "uuid",
@@ -1245,7 +1240,7 @@ virDomainDiskSourceDefClear(virDomainDiskSourceDefPtr def)
 VIR_FREE(def->seclabels);
 }

-virDomainDiskHostDefFree(def->nhosts, def->hosts);
+virStorageNetHostDefFree(def->nhosts, def->hosts);
 virDomainDiskAuthClear(def);
 }

@@ -1282,67 +1277,6 @@ virDomainDiskAuthClear(virDomainDiskSourceDefPtr def)
 }


-void virDomainDiskHostDefClear(virDomainDiskHostDefPtr def)
-{
-if (!def)
-return;
-
-VIR_FREE(def->name);
-VIR_FREE(def->port);
-VIR_FREE(def->socket);
-}
-
-
-void
-virDomainDiskHostDefFree(size_t nhosts,
- virDomainDiskHostDefPtr hosts)
-{
-size_t i;
-
-if (!hosts)
-return;
-
-for (i = 0; i < nhosts; i++)
-virDomainDiskHostDefClear(&hosts[i]);
-
-VIR_FREE(hosts);
-}
-
-
-virDomainDiskHostDefPtr
-virDomainDiskHostDefCopy(size_t nhosts,
- virDomainDiskHostDefPtr hosts)
-{
-virDomainDiskHostDefPtr ret = NULL;
-size_t i;
-
-if (VIR_ALLOC_N(ret, nhosts) < 0)
-goto error;
-
-for (i = 0; i < nhosts; i++) {
-virDomainDiskHostDefPtr src = &hosts[i];
-virDomainDiskHostDefPtr dst = &ret[i];
-
-dst->transport = src->transport;
-
-if (VIR_STRDUP(dst->name, src->name) < 0)
-goto error;
-
-if (VIR_STRDUP(dst->port, src->port) < 0)
-goto error;
-
-if (VIR_STRDUP(dst->socket, src->socket) < 0)
-goto error;
-}
-
-return ret;
-
- error:
-virDomainDiskHostDefFree(nhosts, ret);
-return NULL;
-}
-
-
 int
 virDomainDiskGetType(virDomainDiskDefPtr def)
 {
@@ -5102,12 +5036,12 @@ virDomainDiskSourceDefParse(xmlNodePtr node,
 c

[libvirt] [PATCH 1/n] conf: split security label structs to util/

[Eric Blake]

In order to reuse the newly-created host-side disk struct in
the virstoragefile backing chain code, I first have to move
it to util/.  This starts the process, by first moving the
security label structures.

* src/conf/domain_conf.h (virDomainDefGenSecurityLabelDef)
(virDomainDiskDefGenSecurityLabelDef, virSecurityLabelDefFree)
(virSecurityDeviceLabelDefFree, virSecurityLabelDef)
(virSecurityDeviceLabelDef): Move...
* src/util/virseclabel.h: ...to new file.
(virSecurityLabelDefNew, virSecurityDeviceLabelDefNew): Rename the
GenSecurity functions.
* src/qemu/qemu_process.c (qemuProcessAttach): Adjust callers.
* src/security/security_manager.c (virSecurityManagerGenLabel):
Likewise.
* src/security/security_selinux.c
(virSecuritySELinuxSetSecurityFileLabel): Likewise.
* src/util/virseclabel.c: New file.
* src/conf/domain_conf.c: Move security code, and fix fallout.
* src/Makefile.am (UTIL_SOURCES): Build new file.
* src/libvirt_private.syms (domain_conf.h): Move symbols...
(virseclabel.h): ...to new section.

Signed-off-by: Eric Blake 
---
 src/Makefile.am |  1 +
 src/conf/domain_conf.c  | 51 -
 src/conf/domain_conf.h  | 43 +
 src/libvirt_private.syms| 11 --
 src/qemu/qemu_process.c |  2 +-
 src/security/security_manager.c |  2 +-
 src/security/security_selinux.c |  2 +-
 src/util/virseclabel.c  | 82 +
 src/util/virseclabel.h  | 67 +
 9 files changed, 161 insertions(+), 100 deletions(-)
 create mode 100644 src/util/virseclabel.c
 create mode 100644 src/util/virseclabel.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 55427ed..54206e4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -142,6 +142,7 @@ UTIL_SOURCES =  
\
util/virprocess.c util/virprocess.h \
util/virrandom.h util/virrandom.c   \
util/virscsi.c util/virscsi.h   \
+   util/virseclabel.c util/virseclabel.h   \
util/virsexpr.c util/virsexpr.h \
util/virsocketaddr.h util/virsocketaddr.c   \
util/virstatslinux.c util/virstatslinux.h   \
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6fb216e..66eeaa9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1149,29 +1149,6 @@ 
virDomainGraphicsListenDefClear(virDomainGraphicsListenDefPtr def)
 return;
 }

-void
-virSecurityLabelDefFree(virSecurityLabelDefPtr def)
-{
-if (!def)
-return;
-VIR_FREE(def->model);
-VIR_FREE(def->label);
-VIR_FREE(def->imagelabel);
-VIR_FREE(def->baselabel);
-VIR_FREE(def);
-}
-
-
-void
-virSecurityDeviceLabelDefFree(virSecurityDeviceLabelDefPtr def)
-{
-if (!def)
-return;
-VIR_FREE(def->model);
-VIR_FREE(def->label);
-VIR_FREE(def);
-}
-

 void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
 {
@@ -19422,34 +19399,6 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr 
def, const char *model)
 return NULL;
 }

-virSecurityLabelDefPtr
-virDomainDefGenSecurityLabelDef(const char *model)
-{
-virSecurityLabelDefPtr seclabel = NULL;
-
-if (VIR_ALLOC(seclabel) < 0 ||
-VIR_STRDUP(seclabel->model, model) < 0) {
-virSecurityLabelDefFree(seclabel);
-seclabel = NULL;
-}
-
-return seclabel;
-}
-
-virSecurityDeviceLabelDefPtr
-virDomainDiskDefGenSecurityLabelDef(const char *model)
-{
-virSecurityDeviceLabelDefPtr seclabel = NULL;
-
-if (VIR_ALLOC(seclabel) < 0 ||
-VIR_STRDUP(seclabel->model, model) < 0) {
-virSecurityDeviceLabelDefFree(seclabel);
-seclabel = NULL;
-}
-
-return seclabel;
-}
-

 typedef struct {
 const char *devAlias;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f3f24c4..a249208 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -47,6 +47,7 @@
 # include "virbitmap.h"
 # include "virstoragefile.h"
 # include "virnuma.h"
+# include "virseclabel.h"

 /* forward declarations of all device types, required by
  * virDomainDeviceDef
@@ -328,39 +329,6 @@ struct _virDomainDeviceInfo {
 int bootIndex;
 };

-enum virDomainSeclabelType {
-VIR_DOMAIN_SECLABEL_DEFAULT,
-VIR_DOMAIN_SECLABEL_NONE,
-VIR_DOMAIN_SECLABEL_DYNAMIC,
-VIR_DOMAIN_SECLABEL_STATIC,
-
-VIR_DOMAIN_SECLABEL_LAST
-};
-
-/* Security configuration for domain */
-typedef struct _virSecurityLabelDef virSecurityLabelDef;
-typedef virSecurityLabelDef *virSecurityLabelDefPtr;
-struct _virSecurityLabelDef {
-char *model;/* name of security model */
-char *label;/* security label string */
-char *imagelabel;   /* security image label string */
-char *baselabel;/* base name of label string */
-int type;   /* virDoma

[libvirt] [PATCH 0/n] round 2 of storage chain refactoring

[Eric Blake]

In round 1, I split out a new struct in domain_conf.h.  This starts
round 2: moving the struct into util/virstoragefile.h, so it can be
shared by domain, snapshot, and existing virstoragefile backing
chain operations.  It's turned out to be a bigger process than I
first thought, so I've tried splitting it into smaller patches
to ease review.  I also want to get review started on the parts
I have compiling, while still working on the rest of the series,
since I know that at least Peter will be heavily impacted by
some of the changes in this series.

Eric Blake (2):
  conf: split security label structs to util/
  conf: split network host structs to util/

 src/Makefile.am   |   1 +
 src/conf/domain_conf.c| 139 +++---
 src/conf/domain_conf.h|  71 +
 src/conf/snapshot_conf.c  |   5 +-
 src/conf/snapshot_conf.h  |   4 +-
 src/libvirt_private.syms  |  21 ++---
 src/qemu/qemu_command.c   |  38 +-
 src/qemu/qemu_command.h   |   6 +-
 src/qemu/qemu_conf.c  |   4 +-
 src/qemu/qemu_driver.c|  24 +++---
 src/qemu/qemu_process.c   |   2 +-
 src/security/security_manager.c   |   2 +-
 src/security/security_selinux.c   |   2 +-
 src/storage/storage_backend_gluster.c |   8 +-
 src/storage/storage_driver.c  |   8 +-
 src/storage/storage_driver.h  |   2 +-
 src/util/virseclabel.c|  82 
 src/util/virseclabel.h|  67 
 src/util/virstoragefile.c |  71 -
 src/util/virstoragefile.h |  30 +++-
 20 files changed, 328 insertions(+), 259 deletions(-)
 create mode 100644 src/util/virseclabel.c
 create mode 100644 src/util/virseclabel.h

-- 
1.8.5.3

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

[libvirt] [PATCH] Fix Memory Leak in testGetCaps()

[Nehal J Wani]

While running qemucaps2xmltest, it was found that valgrind pointed out
the following memory leaks:

==27045== 160 (112 direct, 48 indirect) bytes in 1 blocks are definitely lost 
in loss record 51 of 65
==27045==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==27045==by 0x4C6BACD: virAllocVar (viralloc.c:560)
==27045==by 0x4CAF095: virObjectNew (virobject.c:193)
==27045==by 0x421453: virQEMUCapsNew (qemu_capabilities.c:1805)
==27045==by 0x41F04F: testQemuCapsXML (qemucaps2xmltest.c:72)
==27045==by 0x41FFD1: virtTestRun (testutils.c:201)
==27045==by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
==27045==by 0x42074D: virtTestMain (testutils.c:789)
==27045==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==27045== 
==27045== 160 (112 direct, 48 indirect) bytes in 1 blocks are definitely lost 
in loss record 52 of 65
==27045==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==27045==by 0x4C6BACD: virAllocVar (viralloc.c:560)
==27045==by 0x4CAF095: virObjectNew (virobject.c:193)
==27045==by 0x421453: virQEMUCapsNew (qemu_capabilities.c:1805)
==27045==by 0x41F04F: testQemuCapsXML (qemucaps2xmltest.c:72)
==27045==by 0x41FFD1: virtTestRun (testutils.c:201)
==27045==by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
==27045==by 0x42074D: virtTestMain (testutils.c:789)
==27045==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)

---
 tests/qemucaps2xmltest.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c
index 8bf6738..cef5735 100644
--- a/tests/qemucaps2xmltest.c
+++ b/tests/qemucaps2xmltest.c
@@ -125,6 +125,7 @@ testGetCaps(char *capsData, const testQemuData *data)
 goto error;
 }
 
+virObjectUnref(qemuCaps);
 return caps;
 
  error:
-- 
1.7.1

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

[libvirt] [PATCH v4 5/5] virsh: Expose new virDomainFSFreeze and virDomainFSThaw API

[Tomoki Sekiyama]

These are exposed under domfsfreeze command and domfsthaw command.

Signed-off-by: Tomoki Sekiyama 
---
 tools/virsh-domain.c |   92 ++
 tools/virsh.pod  |   15 
 2 files changed, 107 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d9aa4fa..b0cf68d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11391,6 +11391,86 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
 return ret;
 }
 
+static const vshCmdInfo info_domfsfreeze[] = {
+{.name = "help",
+ .data = N_("Freeze domain's mounted filesystems.")
+},
+{.name = "desc",
+ .data = N_("Freeze domain's mounted filesystems.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_domfsfreeze[] = {
+{.name = "domain",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("domain name, id or uuid")
+},
+{.name = NULL}
+};
+static bool
+cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+bool ret = false;
+unsigned int flags = 0;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return ret;
+
+if (virDomainFSFreeze(dom, flags) < 0) {
+vshError(ctl, _("Unable to freeze filesystems"));
+goto cleanup;
+}
+
+ret = true;
+
+ cleanup:
+virDomainFree(dom);
+return ret;
+}
+
+static const vshCmdInfo info_domfsthaw[] = {
+{.name = "help",
+ .data = N_("Thaw domain's mounted filesystems.")
+},
+{.name = "desc",
+ .data = N_("Thaw domain's mounted filesystems.")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_domfsthaw[] = {
+{.name = "domain",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("domain name, id or uuid")
+},
+{.name = NULL}
+};
+static bool
+cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom = NULL;
+bool ret = false;
+unsigned int flags = 0;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return ret;
+
+if (virDomainFSThaw(dom, flags) < 0) {
+vshError(ctl, _("Unable to thaw filesystems"));
+goto cleanup;
+}
+
+ret = true;
+
+ cleanup:
+virDomainFree(dom);
+return ret;
+}
+
 const vshCmdDef domManagementCmds[] = {
 {.name = "attach-device",
  .handler = cmdAttachDevice,
@@ -11538,6 +11618,18 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_domdisplay,
  .flags = 0
 },
+{.name = "domfsfreeze",
+ .handler = cmdDomFSFreeze,
+ .opts = opts_domfsfreeze,
+ .info = info_domfsfreeze,
+ .flags = 0
+},
+{.name = "domfsthaw",
+ .handler = cmdDomFSThaw,
+ .opts = opts_domfsthaw,
+ .info = info_domfsthaw,
+ .flags = 0
+},
 {.name = "domfstrim",
  .handler = cmdDomFSTrim,
  .opts = opts_domfstrim,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 20352cb..3f41f77 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -941,6 +941,21 @@ Output a URI which can be used to connect to the graphical 
display of the
 domain via VNC, SPICE or RDP. If I<--include-password> is specified, the
 SPICE channel password will be included in the URI.
 
+=item B I
+
+Freeze all mounted filesystems within a running domain to prepare for
+consistent snapshots.
+
+Note that B command has a I<--quiesce> option to freeze
+and thaw the filesystems automatically to keep snapshots consistent.
+B command is only needed when a user wants to utilize the
+native snapshot features of storage devices not supported by libvirt yet.
+
+=item B I
+
+Thaw all mounted filesystems within a running domain, which are frozen
+by domfsfreeze command.
+
 =item B I [I<--minimum> B]
 [I<--mountpoint mountPoint>]
 

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

[libvirt] [PATCH v4 1/5] Introduce virDomainFSFreeze() and virDomainFSThaw() public API

[Tomoki Sekiyama]

These will freeze and thaw filesystems within guest. The APIs take @flags
arguments which are currently not used, for future extensions.

Signed-off-by: Tomoki Sekiyama 
---
 include/libvirt/libvirt.h.in |6 
 src/driver.h |   10 ++
 src/libvirt.c|   70 ++
 src/libvirt_public.syms  |2 +
 4 files changed, 88 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 91e3e3b..afe92c6 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5277,6 +5277,12 @@ int virDomainFSTrim(virDomainPtr dom,
 unsigned long long minimum,
 unsigned int flags);
 
+int virDomainFSFreeze(virDomainPtr dom,
+  unsigned int flags);
+
+int virDomainFSThaw(virDomainPtr dom,
+unsigned int flags);
+
 /**
  * virSchedParameterType:
  *
diff --git a/src/driver.h b/src/driver.h
index e66fc7a..50258e2 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -1149,6 +1149,14 @@ typedef int
  unsigned int flags,
  int cancelled);
 
+typedef int
+(*virDrvDomainFSFreeze)(virDomainPtr dom,
+unsigned int flags);
+
+typedef int
+(*virDrvDomainFSThaw)(virDomainPtr dom,
+  unsigned int flags);
+
 typedef struct _virDriver virDriver;
 typedef virDriver *virDriverPtr;
 
@@ -1363,6 +1371,8 @@ struct _virDriver {
 virDrvDomainMigrateFinish3Params domainMigrateFinish3Params;
 virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params;
 virDrvConnectGetCPUModelNames connectGetCPUModelNames;
+virDrvDomainFSFreeze domainFSFreeze;
+virDrvDomainFSThaw domainFSThaw;
 };
 
 
diff --git a/src/libvirt.c b/src/libvirt.c
index 4454829..48b9902 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -20658,3 +20658,73 @@ virDomainFSTrim(virDomainPtr dom,
 virDispatchError(dom->conn);
 return -1;
 }
+
+/**
+ * virDomainFSFreeze:
+ * @dom: a domain object
+ * @flags: extra flags, not used yet, so callers should always pass 0
+ *
+ * Freeze filesystems within the guest (hence guest agent may be
+ * required depending on hypervisor used).
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+int
+virDomainFSFreeze(virDomainPtr dom,
+  unsigned int flags)
+{
+VIR_DOMAIN_DEBUG(dom, "flags=%x", flags);
+
+virResetLastError();
+
+virCheckDomainReturn(dom, -1);
+virCheckReadOnlyGoto(dom->conn->flags, error);
+
+if (dom->conn->driver->domainFSFreeze) {
+int ret = dom->conn->driver->domainFSFreeze(dom, flags);
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(dom->conn);
+return -1;
+}
+
+/**
+ * virDomainFSThaw:
+ * @dom: a domain object
+ * @flags: extra flags, not used yet, so callers should always pass 0
+ *
+ * Thaw the frozen filesystems within the guest (hence guest agent
+ * may be required depending on hypervisor used).
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+int
+virDomainFSThaw(virDomainPtr dom,
+unsigned int flags)
+{
+VIR_DOMAIN_DEBUG(dom, "flags=%x", flags);
+
+virResetLastError();
+
+virCheckDomainReturn(dom, -1);
+virCheckReadOnlyGoto(dom->conn->flags, error);
+
+if (dom->conn->driver->domainFSThaw) {
+int ret = dom->conn->driver->domainFSThaw(dom, flags);
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(dom->conn);
+return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 9ab0c92..42793c9 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -648,6 +648,8 @@ LIBVIRT_1.2.1 {
 LIBVIRT_1.2.3 {
 global:
 virDomainCoreDumpWithFormat;
+virDomainFSFreeze;
+virDomainFSThaw;
 } LIBVIRT_1.2.1;
 
 

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

[libvirt] [PATCH v4 0/5] Expose FSFreeze/FSThaw within the guest as API

[Tomoki Sekiyama]

Hello,

This is patchset v4 to add FSFreeze/FSThaw API for custom disk snapshotting.

Changes to v3:
 * fix typp and label spacing
 * rebased to recent tree
 (v3: http://www.redhat.com/archives/libvir-list/2014-March/msg01358.html )

=== Description ===

Currently FSFreeze and FSThaw are supported by qemu guest agent and they are
used internally in snapshot-create command with --quiesce option.
However, when users want to utilize the native snapshot feature of storage
devices (such as LVM over iSCSI, enterprise storage appliances, etc.),
they need to issue fsfreeze command separately from libvirt-driven snapshots.
(OpenStack cinder provides these storages' snapshot feature, but it cannot
 quiesce the guest filesystems automatically for now.)

Although virDomainQemuGuestAgent() API could be used for this purpose, it
is only for debugging and is not supported officially.

This patchset adds virDomainFSFreeze()/virDomainFSThaw() APIs and virsh
domfsfreeze/domfsthaw commands to enable the users to freeze and thaw
domain's filesystems cleanly.

The APIs have flags option currently unsupported for future extension.
---

Tomoki Sekiyama (5):
  Introduce virDomainFSFreeze() and virDomainFSThaw() public API
  remote: Implement virDomainFSFreeze and virDomainFSThaw
  qemu: Track domain quiesced status
  qemu: Implement virDomainFSFreeze and virDomainFSThaw
  virsh: Expose new virDomainFSFreeze and virDomainFSThaw API


 include/libvirt/libvirt.h.in |6 ++
 src/access/viraccessperm.c   |2 -
 src/access/viraccessperm.h   |6 ++
 src/driver.h |   10 +++
 src/libvirt.c|   70 
 src/libvirt_public.syms  |2 +
 src/qemu/qemu_domain.c   |5 +
 src/qemu/qemu_domain.h   |2 +
 src/qemu/qemu_driver.c   |  144 ++
 src/remote/remote_driver.c   |2 +
 src/remote/remote_protocol.x |   25 +++
 src/remote_protocol-structs  |9 +++
 src/rpc/gendispatch.pl   |2 +
 tools/virsh-domain.c |   92 +++
 tools/virsh.pod  |   15 
 15 files changed, 376 insertions(+), 16 deletions(-)

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

[libvirt] [PATCH v4 4/5] qemu: Implement virDomainFSFreeze and virDomainFSThaw

[Tomoki Sekiyama]

Use qemuDomainSnapshotFSFreeze() and qemuDomainSnapshotFSFreeze() which are
already implemented for snapshot quiescing.

Signed-off-by: Tomoki Sekiyama 
---
 src/qemu/qemu_driver.c |   78 
 1 file changed, 78 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7fd8b6d..ebd27dd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16701,6 +16701,82 @@ qemuConnectGetCPUModelNames(virConnectPtr conn,
 }
 
 
+static int
+qemuDomainFSFreeze(virDomainPtr dom,
+   unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainFSFreezeEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("domain is not running"));
+goto endjob;
+}
+
+ret = qemuDomainSnapshotFSFreeze(driver, vm);
+
+ endjob:
+if (!qemuDomainObjEndJob(driver, vm))
+vm = NULL;
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
+
+static int
+qemuDomainFSThaw(virDomainPtr dom,
+ unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainFSThawEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("domain is not running"));
+goto endjob;
+}
+
+ret = qemuDomainSnapshotFSThaw(driver, vm, true);
+
+ endjob:
+if (!qemuDomainObjEndJob(driver, vm))
+vm = NULL;
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+return ret;
+}
+
+
 static virDriver qemuDriver = {
 .no = VIR_DRV_QEMU,
 .name = QEMU_DRIVER_NAME,
@@ -16891,6 +16967,8 @@ static virDriver qemuDriver = {
 .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */
 .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */
 .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */
+.domainFSFreeze = qemuDomainFSFreeze, /* 1.2.3 */
+.domainFSThaw = qemuDomainFSThaw, /* 1.2.3 */
 };
 
 

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

[libvirt] [PATCH v4 3/5] qemu: Track domain quiesced status

[Tomoki Sekiyama]

Adds an quiesced flag into qemuDomainObjPrivate that tracks whether guest
filesystems of the domain is quiesced or not.

It also modify error code from qemuDomainSnapshotFSFreeze and
qemuDomainSnapshotFSThaw, so that a caller can know whether the command is
actually sent to the guest agent. If the error is caused before sending a
freeze command, a counterpart thaw command shouldn't be sent either, not to
thaw the guest unexpectedly by error handling code.

Signed-off-by: Tomoki Sekiyama 
---
 src/qemu/qemu_domain.c |5 
 src/qemu/qemu_domain.h |2 +
 src/qemu/qemu_driver.c |   66 +++-
 3 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 36cb2c6..52aa09d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -357,6 +357,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
 virBufferAddLit(buf, "\n");
 }
 
+if (priv->quiesced)
+virBufferAddLit(buf, "\n");
+
 return 0;
 }
 
@@ -518,6 +521,8 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void 
*data)
 }
 VIR_FREE(nodes);
 
+priv->quiesced = virXPathBoolean("boolean(./quiesced)", ctxt) == 1;
+
 return 0;
 
  error:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index b2830c4..5fb1665 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -176,6 +176,8 @@ struct _qemuDomainObjPrivate {
 char **qemuDevices; /* NULL-terminated list of devices aliases known to 
QEMU */
 
 bool hookRun;  /* true if there was a hook run over this domain */
+
+bool quiesced; /* true if the domain filesystems are quiesced */
 };
 
 typedef enum {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b032441..7fd8b6d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12100,32 +12100,61 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr 
driver,
 }
 
 
+/* Return -1 if request is not sent to agent due to misconfig, -2 if request
+ * is sent but failed, and number of frozen filesystems on success. */
 static int
-qemuDomainSnapshotFSFreeze(virDomainObjPtr vm)
+qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver, virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverConfigPtr cfg;
 int freezed;
 
 if (!qemuDomainAgentAvailable(priv, true))
 return -1;
 
+if (priv->quiesced) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("domain is already quiesced"));
+return -1;
+}
+
 qemuDomainObjEnterAgent(vm);
 freezed = qemuAgentFSFreeze(priv->agent);
 qemuDomainObjExitAgent(vm);
 
-return freezed;
+if (freezed >= 0)
+priv->quiesced = true;
+
+cfg = virQEMUDriverGetConfig(driver);
+if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) {
+virObjectUnref(cfg);
+return -2;
+}
+virObjectUnref(cfg);
+
+return freezed < 0 ? -2 : freezed;
 }
 
+/* Return -1 if request is not sent to agent due to misconfig, -2 if request
+ * is send but failed, and number of thawed filesystems on success. */
 static int
-qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report)
+qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver,
+ virDomainObjPtr vm, bool report)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
+virQEMUDriverConfigPtr cfg;
 int thawed;
 virErrorPtr err = NULL;
 
 if (!qemuDomainAgentAvailable(priv, report))
 return -1;
 
+if (!priv->quiesced && report) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("domain is not quiesced"));
+return -1;
+}
+
 qemuDomainObjEnterAgent(vm);
 if (!report)
 err = virSaveLastError();
@@ -12134,8 +12163,18 @@ qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool 
report)
 virSetError(err);
 qemuDomainObjExitAgent(vm);
 
+if (thawed >= 0)
+priv->quiesced = false;
+
+cfg = virQEMUDriverGetConfig(driver);
+if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) {
+virObjectUnref(cfg);
+return -2;
+}
+virObjectUnref(cfg);
+
 virFreeError(err);
-return thawed;
+return thawed < 0 ? -2 : thawed;
 }
 
 /* The domain is expected to be locked and inactive. */
@@ -13114,17 +13153,18 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr 
conn,
 goto cleanup;
 
 /* If quiesce was requested, then issue a freeze command, and a
- * counterpart thaw command, no matter what.  The command will
- * fail if the guest is paused or the guest agent is not
- * running.  */
+ * counterpart thaw command when it is actually sent to agent.
+ * The command will fail if the guest is paused or the guest agent
+ * is not running, or is already quiesced.  */
 if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) {
-if (qemuDomainSna

[libvirt] [PATCH v4 2/5] remote: Implement virDomainFSFreeze and virDomainFSThaw

[Tomoki Sekiyama]

New rules are added in fixup_name in gendispatch.pl to keep the name
FSFreeze and FSThaw. This adds a new ACL permission 'fs_freeze',
which is also applied to VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag.

Signed-off-by: Tomoki Sekiyama 
---
 src/access/viraccessperm.c   |2 +-
 src/access/viraccessperm.h   |6 ++
 src/remote/remote_driver.c   |2 ++
 src/remote/remote_protocol.x |   25 +++--
 src/remote_protocol-structs  |9 +
 src/rpc/gendispatch.pl   |2 ++
 6 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c
index d517c66..462f46c 100644
--- a/src/access/viraccessperm.c
+++ b/src/access/viraccessperm.c
@@ -39,7 +39,7 @@ VIR_ENUM_IMPL(virAccessPermDomain,
   "start", "stop", "reset",
   "save", "delete",
   "migrate", "snapshot", "suspend", "hibernate", "core_dump", 
"pm_control",
-  "init_control", "inject_nmi", "send_input", "send_signal", 
"fs_trim",
+  "init_control", "inject_nmi", "send_input", "send_signal", 
"fs_trim", "fs_freeze",
   "block_read", "block_write", "mem_read",
   "open_graphics", "open_device", "screenshot",
   "open_namespace");
diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
index 6d14f05..ac48d70 100644
--- a/src/access/viraccessperm.h
+++ b/src/access/viraccessperm.h
@@ -242,6 +242,12 @@ typedef enum {
  */
 VIR_ACCESS_PERM_DOMAIN_FS_TRIM,  /* Issue TRIM to guest filesystems */
 
+/**
+ * @desc: Freeze and thaw domain filesystems
+ * @message: Freezing and thawing domain filesystems require authorization
+ */
+VIR_ACCESS_PERM_DOMAIN_FS_FREEZE,/* Freeze and thaw guest filesystems 
*/
+
 /* Peeking at guest */
 
 /**
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index ed7dde6..f26a2e4 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7800,6 +7800,8 @@ static virDriver remote_driver = {
 .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */
 .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 
*/
 .connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.3 */
+.domainFSFreeze = remoteDomainFSFreeze, /* 1.2.3 */
+.domainFSThaw = remoteDomainFSThaw, /* 1.2.3 */
 };
 
 static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 6c445cc..f8ce568 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -2959,6 +2959,15 @@ struct remote_network_event_lifecycle_msg {
 int detail;
 };
 
+struct remote_domain_fsfreeze_args {
+remote_nonnull_domain dom;
+unsigned int flags;
+};
+
+struct remote_domain_fsthaw_args {
+remote_nonnull_domain dom;
+unsigned int flags;
+};
 
 
 /*- Protocol. -*/
@@ -4289,7 +4298,7 @@ enum remote_procedure {
 /**
  * @generate: both
  * @acl: domain:snapshot
- * @acl: domain:write:VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE
+ * @acl: domain:fs_freeze:VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE
  */
 REMOTE_PROC_DOMAIN_SNAPSHOT_CREATE_XML = 185,
 
@@ -5275,5 +5284,17 @@ enum remote_procedure {
  * @generate: both
  * @acl: domain:core_dump
  */
-REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334
+REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334,
+
+/**
+ * @generate: both
+ * @acl: domain:fs_freeze
+ */
+REMOTE_PROC_DOMAIN_FSFREEZE = 335,
+
+/**
+ * @generate: both
+ * @acl: domain:fs_freeze
+ */
+REMOTE_PROC_DOMAIN_FSTHAW = 336
 };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 456d0da..b5b21e3 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -2426,6 +2426,13 @@ struct remote_network_event_lifecycle_msg {
 remote_nonnull_network net;
 intevent;
 intdetail;
+struct remote_domain_fsfreeze_args {
+remote_nonnull_domain  dom;
+u_int  flags;
+};
+struct remote_domain_fsthaw_args {
+remote_nonnull_domain  dom;
+u_int  flags;
 };
 enum remote_procedure {
 REMOTE_PROC_CONNECT_OPEN = 1,
@@ -2762,4 +2769,6 @@ enum remote_procedure {
 REMOTE_PROC_DOMAIN_EVENT_CALLBACK_PMSUSPEND_DISK = 332,
 REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333,
 REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334,
+REMOTE_PROC_DOMAIN_FSFREEZE = 335,
+REMOTE_PROC_DOMAIN_FSTHAW = 336,
 };
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index b76bbac..9cd620e 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -64,6 +64,8 @@ sub fixup_name {
 $name =~ s/Nmi$/NMI/;
 $name =~ s/Pm/PM/;
 $name =~ s/Fstrim$/FSTrim/;
+$name =~ s/Fsfreeze$

[libvirt] [TCK] nwfilter tests and libvirt commit 4f209434

[Mike Latimer]

Hi,

As I've been looking through libvirt-tck tests, I found that commit 4f209434 
(in libvirt) changes a condition that the nwfilter/050-apply-verify-host.t 
relies on.

Specifically, the 050-apply-verify-host.t test creates a number of filters with 
invalid values (such as dscp='64', and protocol='256'). Without commit 
4f209434, these invalid values are silently dropped off the end of the rule, as 
in the following example:

#virsh nwfilter-define  

  5c6d49af-b071-6127-b4ec-6f8ed4b55335
  
 
  

#virsh nwfilter-dumpxml tck-testcase
  

  

With commit 4f209434 in place, the entire filter is rejected with the following 
error:  'internal error: dscp has illegal value 64'. As the filter is not 
created, the testing of that filter by the 050-apply-verify-host test fails.

I agree that the change is the right thing to do, but I'm wondering how best 
to handle the now failing tests. This test (050-apply-verify-host.t) runs 
~1200 subtests, and about 10% of those tests fail due to the change. I'm 
thinking that the illegal options (and only those illegal options) should be 
removed from the now broken tests, and a few new tests added to specifically 
test for the failure to add the entire rule when illegal data is passed. 
Thoughts?

BTW - Is anyone else running the full libvirt-tck suite against recent 
versions of libvirt? There are quite a few issues (such as this one) which 
should be easily seen...

Thanks,
Mike

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

[libvirt] [PATCH v2] Fix Memory Leak in virQEMUCapsInitGuestFromBinary()

[Nehal J Wani]

While running qemucaps2xmltest, it was found that valgrind pointed out
the following memory leaks:

==29896== 0 bytes in 1 blocks are definitely lost in loss record 1 of 65
==29896==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==29896==by 0x4C6B45E: virAllocN (viralloc.c:191)
==29896==by 0x4232A9: virQEMUCapsGetMachineTypesCaps 
(qemu_capabilities.c:1999)
==29896==by 0x4234E7: virQEMUCapsInitGuestFromBinary 
(qemu_capabilities.c:789)
==29896==by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
==29896==by 0x41FFD1: virtTestRun (testutils.c:201)
==29896==by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
==29896==by 0x42074D: virtTestMain (testutils.c:789)
==29896==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==29896== 
==29896== 0 bytes in 1 blocks are definitely lost in loss record 2 of 65
==29896==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==29896==by 0x4C6B45E: virAllocN (viralloc.c:191)
==29896==by 0x4232A9: virQEMUCapsGetMachineTypesCaps 
(qemu_capabilities.c:1999)
==29896==by 0x4234E7: virQEMUCapsInitGuestFromBinary 
(qemu_capabilities.c:789)
==29896==by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
==29896==by 0x41FFD1: virtTestRun (testutils.c:201)
==29896==by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
==29896==by 0x42074D: virtTestMain (testutils.c:789)
==29896==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)

---
 src/qemu/qemu_capabilities.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7673592..aef8bc1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -789,6 +789,10 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
 if (virQEMUCapsGetMachineTypesCaps(qemubinCaps, &nmachines, &machines) < 0)
 goto cleanup;
 
+/* Free unneeded memory given by malloc(0) */
+if (!nmachines)
+VIR_FREE(machines);
+
 /* We register kvm as the base emulator too, since we can
  * just give -no-kvm to disable acceleration if required */
 if ((guest = virCapabilitiesAddGuest(caps,
-- 
1.7.1

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

Re: [libvirt] Crash when using python-libvirt setSchedulerParameters

[Brian Rak]

On 3/25/2014 6:50 AM, Michal Privoznik wrote:

On 24.03.2014 21:45, Brian Rak wrote:

I'm seeing a very weird (and somewhat reproducable) crash in
setSchedulerParameters.  The backtrace looks like this:

*** glibc detected *** python2.7: free(): invalid pointer:
0x0152bc48 ***
=== Backtrace: =
/lib64/libc.so.6(+0x76166)[0x7faa9e991166]
/usr/lib64/python2.7/site-packages/libvirtmod.so(virFree+0x29)[0x7faa9887bfe9] 



/lib/libvirt.so.0(virTypedParamsClear+0x54)[0x7faa98342fe4]
/lib/libvirt.so.0(virTypedParamsFree+0x1e)[0x7faa9834302e]
/usr/lib64/python2.7/site-packages/libvirtmod.so(+0x1b4dc)[0x7faa9886c4dc] 

/usr/lib64/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x5629)[0x7faa9f63a129] 

/usr/lib64/libpython2.7.so.1.0(PyEval_EvalFrameEx+0x68e8)[0x7faa9f63b3e8] 


/usr/lib64/libpython2.7.so.1.0(PyEval_EvalCodeEx+0x8ae)[0x7faa9f63bd5e]
/usr/lib64/libpython2.7.so.1.0(PyEval_EvalCode+0x32)[0x7faa9f63be72]
/usr/lib64/libpython2.7.so.1.0(+0xff25c)[0x7faa9f65625c]
/usr/lib64/libpython2.7.so.1.0(PyRun_FileExFlags+0x90)[0x7faa9f656330]
/usr/lib64/libpython2.7.so.1.0(PyRun_SimpleFileExFlags+0xef)[0x7faa9f6578cf] 



/usr/lib64/libpython2.7.so.1.0(Py_Main+0xc56)[0x7faa9f6693f6]
/lib64/libc.so.6(__libc_start_main+0xfd)[0x7faa9e939d1d]
python2.7[0x400649]


I am using:
Python2.7
Python-Libvirt 1.2.2 with the "[PATCH libvirt-python 1/2]
setPyVirTypedParameter: Copy full field name" patch.
Libvirt 1.2.1


Is the following patch applied as well?

commit 69c4600d61fa74c4977d2471a29fb73f0fe5edb0
Author: Michal Privoznik 
AuthorDate: Tue Mar 18 09:20:00 2014 +0100
Commit: Michal Privoznik 
CommitDate: Tue Mar 18 14:43:10 2014 +0100

setPyVirTypedParameter: free whole return variable on error

The @ret value is built in a loop. However, if in one iteration
there's an error, we should free all the fields built so far. For
instance, if there's an error and the previous item was
type of VIR_TYPED_PARAM_STRING we definitely must free it.

Signed-off-by: Michal Privoznik 


Can you install debuginfo so we see the full stack trace?

Michal

I don't have that patch applied.  I will apply it and give it a shot.

These are all custom builds of python-libvirt and libvirt, so the 
debuginfo package is not immediately available.


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

Re: [libvirt] [PATCH v3 2/2] bhyve: add xml2args unittest

[Ján Tomko]

On 03/26/2014 05:53 PM, Roman Bogorodskiy wrote:
> At this point unittest covers 4 basic cases:
> 
>  - minimal working XML for bhyve
>  - same as above, but with virtio disk
>  - ACPI and APIC args test
>  - MAC address test
> ---
>  tests/Makefile.am  |  26 
>  .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args  |   3 +
>  tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml |  24 
>  tests/bhyvexml2argvdata/bhyvexml2argv-base.args|   3 +
>  tests/bhyvexml2argvdata/bhyvexml2argv-base.xml |  20 +++
>  .../bhyvexml2argv-disk-virtio.args |   3 +
>  .../bhyvexml2argv-disk-virtio.xml  |  20 +++
>  tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args |   3 +
>  tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml  |  21 +++
>  tests/bhyvexml2argvmock.c  |  49 +++
>  tests/bhyvexml2argvtest.c  | 153 
> +
>  11 files changed, 325 insertions(+)
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml
>  create mode 100644 tests/bhyvexml2argvmock.c
>  create mode 100644 tests/bhyvexml2argvtest.c
> 


> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
> new file mode 100644
> index 000..463feab
> --- /dev/null
> +++ b/tests/bhyvexml2argvtest.c
> @@ -0,0 +1,153 @@
> +#include 
> +
> +#include "testutils.h"
> +
> +#ifdef WITH_BHYVE
> +
> +# include "datatypes.h"
> +
> +# include "bhyve/bhyve_utils.h"
> +# include "bhyve/bhyve_command.h"
> +
> +# define VIR_FROM_THIS VIR_FROM_BHYVE
> +
> +static bhyveConn driver;
> +
> +static virCapsPtr
> +testBhyveBuildCapabilities(void)
> +{
> +virCapsPtr caps;
> +virCapsGuestPtr guest;
> +
> +if ((caps = virCapabilitiesNew(virArchFromHost(),

Getting the arch from the host seems wrong in a test, but it seems the bhyve
driver doesn't care about archs.

> +   0, 0)) == NULL)
> +return NULL;
> +
> +if ((guest = virCapabilitiesAddGuest(caps, "hvm",
> + VIR_ARCH_X86_64,
> + "bhyve",
> + NULL, 0, NULL)) == NULL)
> +goto error;
> +
> +if (virCapabilitiesAddGuestDomain(guest,
> +  "bhyve", NULL, NULL, 0, NULL) == NULL)
> +goto error;
> +
> +return caps;
> +
> + error:
> +virObjectUnref(caps);
> +return NULL;
> +}
> +
> +static int testCompareXMLToArgvFiles(const char *xml,
> + const char *cmdline)
> +{
> +char *expectargv = NULL;
> +int len;
> +char *actualargv = NULL;
> +virConnectPtr conn;
> +virDomainDefPtr vmdef = NULL;
> +virDomainObj vm;
> +virCommandPtr cmd = NULL;
> +int ret = -1;
> +
> +
> +if (!(conn = virGetConnect()))

Does this have any side-effects? It doesn't seem to be used anywhere.

> +goto out;
> +
> +if (!(vmdef = virDomainDefParseFile(xml, driver.caps, driver.xmlopt,
> +1 << VIR_DOMAIN_VIRT_BHYVE,
> +VIR_DOMAIN_XML_INACTIVE)))
> +goto out;
> +
> +vm.def = vmdef;
> +
> +if (!(cmd = virBhyveProcessBuildBhyveCmd(&driver, &vm)))
> +goto out;
> +
> +if (!(actualargv = virCommandToString(cmd)))
> +goto out;
> +
> +len = virtTestLoadFile(cmdline, &expectargv);
> +if (len < 0)
> +goto out;
> +
> +if (len && expectargv[len - 1] == '\n')
> +expectargv[len - 1] = '\0';
> +
> +if (STRNEQ(expectargv, actualargv)) {
> +virtTestDifference(stderr, expectargv, actualargv);
> +goto out;
> +}
> +
> +ret = 0;
> +
> + out:
> +VIR_FREE(expectargv);
> +VIR_FREE(actualargv);
> +virCommandFree(cmd);
> +virDomainDefFree(vmdef);
> +virObjectUnref(conn);
> +return ret;
> +}
> +
> +static int
> +testCompareXMLToArgvHelper(const void *data)
> +{
> +int ret = -1;
> +const char *name = data;
> +char *xml = NULL;
> +char *args = NULL;
> +
> +if (virAsprintf(&xml, "%s/bhyvexml2argvdata/bhyvexml2argv-%s.xml",
> +abs_srcdir, name) < 0 ||
> +virAsprintf(&args, "%s/bhyvexml2argvdata/bhyvexml2argv-%s.args",
> +abs_srcdir, name) < 0)
> +goto cleanup;
> +
> +ret = testCompareXMLToArgvFiles(xml, args);
> +
> + cleanup:
> +VIR_FREE(xm

Re: [libvirt] [PATCH v3 1/2] Move virBhyveTapGetRealDeviceName to virnetdevtap

[Ján Tomko]

On 03/26/2014 05:53 PM, Roman Bogorodskiy wrote:
> To ease mocking for bhyve unit tests move virBhyveTapGetRealDeviceName()
> out of bhyve_command.c to virnetdevtap and rename it to
> virNetDevTapGetRealDeviceName().
> ---
>  src/bhyve/bhyve_command.c | 74 +---
>  src/libvirt_private.syms  |  1 +
>  src/util/virnetdevtap.c   | 87 
> +++
>  src/util/virnetdevtap.h   |  3 ++
>  4 files changed, 92 insertions(+), 73 deletions(-)
> 

> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> index 32ad406..3072146 100644
> --- a/src/util/virnetdevtap.c
> +++ b/src/util/virnetdevtap.c
> @@ -22,6 +22,9 @@
>  
>  #include 
>  
> +#include 
> +#include 
> +

I'd rather group these together with the other system headers below, but
that's not really an issue.

>  #include "virmacaddr.h"
>  #include "virnetdevtap.h"
>  #include "virnetdev.h"
> @@ -38,8 +41,11 @@
>  #include 
>  #ifdef __linux__
>  # include /* IFF_TUN, IFF_NO_PI */
> +#elif defined(__FreeBSD__)
> +# include 
>  #endif
>  
> +

Spurious blank line addition.

>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
>  VIR_LOG_INIT("util.netdevtap");

ACK

Jan



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] Fix Memory Leak in virQEMUCapsInitGuestFromBinary()

[Daniel P. Berrange]

On Thu, Mar 27, 2014 at 12:00:49AM +0530, Nehal J Wani wrote:
> While running qemucaps2xmltest, it was found that valgrind pointed out
> the following memory leaks:
> 
> ==29896== 0 bytes in 1 blocks are definitely lost in loss record 1 of 65
> ==29896==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
> ==29896==by 0x4C6B45E: virAllocN (viralloc.c:191)
> ==29896==by 0x4232A9: virQEMUCapsGetMachineTypesCaps 
> (qemu_capabilities.c:1999)
> ==29896==by 0x4234E7: virQEMUCapsInitGuestFromBinary 
> (qemu_capabilities.c:789)
> ==29896==by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
> ==29896==by 0x41FFD1: virtTestRun (testutils.c:201)
> ==29896==by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
> ==29896==by 0x42074D: virtTestMain (testutils.c:789)
> ==29896==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
> ==29896== 
> ==29896== 0 bytes in 1 blocks are definitely lost in loss record 2 of 65
> ==29896==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
> ==29896==by 0x4C6B45E: virAllocN (viralloc.c:191)
> ==29896==by 0x4232A9: virQEMUCapsGetMachineTypesCaps 
> (qemu_capabilities.c:1999)
> ==29896==by 0x4234E7: virQEMUCapsInitGuestFromBinary 
> (qemu_capabilities.c:789)
> ==29896==by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
> ==29896==by 0x41FFD1: virtTestRun (testutils.c:201)
> ==29896==by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
> ==29896==by 0x42074D: virtTestMain (testutils.c:789)
> ==29896==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
> 
> ---
>  src/qemu/qemu_capabilities.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 7673592..a28816d 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -800,6 +800,7 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
>   machines)) == NULL)
>  goto cleanup;
>  
> +virCapabilitiesFreeMachines(machines, nmachines);
>  machines = NULL;
>  nmachines = 0;
>  
> @@ -852,6 +853,7 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
>  goto cleanup;
>  }
>  
> +virCapabilitiesFreeMachines(machines, nmachines);
>  machines = NULL;
>  nmachines = 0;

This is wrong. virCapabilitiesAddGuest owns the machines pointer after
it completes successfully


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

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

[libvirt] [PATCH] Fix Memory Leak in virQEMUCapsInitGuestFromBinary()

[Nehal J Wani]

While running qemucaps2xmltest, it was found that valgrind pointed out
the following memory leaks:

==29896== 0 bytes in 1 blocks are definitely lost in loss record 1 of 65
==29896==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==29896==by 0x4C6B45E: virAllocN (viralloc.c:191)
==29896==by 0x4232A9: virQEMUCapsGetMachineTypesCaps 
(qemu_capabilities.c:1999)
==29896==by 0x4234E7: virQEMUCapsInitGuestFromBinary 
(qemu_capabilities.c:789)
==29896==by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
==29896==by 0x41FFD1: virtTestRun (testutils.c:201)
==29896==by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
==29896==by 0x42074D: virtTestMain (testutils.c:789)
==29896==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==29896== 
==29896== 0 bytes in 1 blocks are definitely lost in loss record 2 of 65
==29896==at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==29896==by 0x4C6B45E: virAllocN (viralloc.c:191)
==29896==by 0x4232A9: virQEMUCapsGetMachineTypesCaps 
(qemu_capabilities.c:1999)
==29896==by 0x4234E7: virQEMUCapsInitGuestFromBinary 
(qemu_capabilities.c:789)
==29896==by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
==29896==by 0x41FFD1: virtTestRun (testutils.c:201)
==29896==by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
==29896==by 0x42074D: virtTestMain (testutils.c:789)
==29896==by 0x3E6CE1ED1C: (below main) (libc-start.c:226)

---
 src/qemu/qemu_capabilities.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7673592..a28816d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -800,6 +800,7 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
  machines)) == NULL)
 goto cleanup;
 
+virCapabilitiesFreeMachines(machines, nmachines);
 machines = NULL;
 nmachines = 0;
 
@@ -852,6 +853,7 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
 goto cleanup;
 }
 
+virCapabilitiesFreeMachines(machines, nmachines);
 machines = NULL;
 nmachines = 0;
 
-- 
1.7.1

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

[libvirt] [PATCH v3 2/2] bhyve: add xml2args unittest

[Roman Bogorodskiy]

At this point unittest covers 4 basic cases:

 - minimal working XML for bhyve
 - same as above, but with virtio disk
 - ACPI and APIC args test
 - MAC address test
---
 tests/Makefile.am  |  26 
 .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args  |   3 +
 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml |  24 
 tests/bhyvexml2argvdata/bhyvexml2argv-base.args|   3 +
 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml |  20 +++
 .../bhyvexml2argv-disk-virtio.args |   3 +
 .../bhyvexml2argv-disk-virtio.xml  |  20 +++
 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args |   3 +
 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml  |  21 +++
 tests/bhyvexml2argvmock.c  |  49 +++
 tests/bhyvexml2argvtest.c  | 153 +
 11 files changed, 325 insertions(+)
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml
 create mode 100644 tests/bhyvexml2argvmock.c
 create mode 100644 tests/bhyvexml2argvtest.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index dc6a676..6e15af8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -64,6 +64,7 @@ LDADDS = \
../src/libvirt.la
 
 EXTRA_DIST =   \
+   bhyvexml2argvdata \
capabilityschemadata \
capabilityschematest \
commanddata \
@@ -233,6 +234,10 @@ if WITH_VMWARE
 test_programs += vmwarevertest
 endif WITH_VMWARE
 
+if WITH_BHYVE
+test_programs += bhyvexml2argvtest
+endif WITH_BHYVE
+
 if WITH_CIL
 test_programs += objectlocking
 endif WITH_CIL
@@ -354,6 +359,10 @@ test_libraries += libqemumonitortestutils.la \
$(NULL)
 endif WITH_QEMU
 
+if WITH_BHYVE
+test_libraries += bhyvexml2argvmock.la
+endif WITH_BHYVE
+
 if WITH_DBUS
 test_libraries += virsystemdmock.la
 endif WITH_DBUS
@@ -610,6 +619,23 @@ else ! WITH_VMWARE
 EXTRA_DIST += vmwarevertest.c
 endif ! WITH_VMWARE
 
+if WITH_BHYVE
+bhyvexml2argvmock_la_SOURCES = \
+   bhyvexml2argvmock.c
+bhyvexml2argvmock_la_CFLAGS = $(AM_CFLAGS)
+bhyvexml2argvmock_la_LDFLAGS = -module -avoid-version \
+   -rpath /evil/libtool/hack/to/force/shared/lib/creation
+
+bhyve_LDADDS = ../src/libvirt_driver_bhyve_impl.la
+bhyve_LDADDS += $(LDADDS)
+bhyvexml2argvtest_SOURCES = \
+   bhyvexml2argvtest.c \
+   testutils.c testutils.h
+bhyvexml2argvtest_LDADD = $(bhyve_LDADDS)
+else ! WITH_BHYVE
+EXTRA_DIST += bhyvexml2argvtest.c bhyvexml2argvmock.c
+endif ! WITH_BHYVE
+
 networkxml2xmltest_SOURCES = \
networkxml2xmltest.c \
testutils.c testutils.h
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args
new file mode 100644
index 000..60a56b9
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args
@@ -0,0 +1,3 @@
+/usr/sbin/bhyve -c 1 -m 214 -A -I -H -P -s 0:0,hostbridge \
+-s 1:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \
+-s 2:0,ahci-hd,/tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml 
b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml
new file mode 100644
index 000..b429fef
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml
@@ -0,0 +1,24 @@
+
+  bhyve
+  df3be7e7-a104-11e3-aeb0-50e5492bd3dc
+  219136
+  1
+  
+hvm
+  
+  
+
+
+  
+  
+
+  
+  
+  
+
+
+  
+  
+
+  
+
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-base.args
new file mode 100644
index 000..9d4faa5
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.args
@@ -0,0 +1,3 @@
+/usr/sbin/bhyve -c 1 -m 214 -H -P -s 0:0,hostbridge \
+-s 1:0,virtio-net,faketapdev,mac=52:54:00:00:00:00 \
+-s 2:0,ahci-hd,/tmp/freebsd.img bhyve
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml 
b/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml
new file mode 100644
index 000..8c96f77
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-base.xml
@@ -0,0 +1,20 @@
+
+  bhyve
+  df3be7e7-a104-11e3-aeb0-50e5492bd3dc
+  219136
+  1
+  
+hvm
+  
+  
+
+  
+  
+  
+
+
+  
+  
+
+  
+
diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args 
b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args
new file mode 100644
index 000..54ad2b8
--- /dev/null
+++ b/tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args
@@ -0,0 +1,3 @@

[libvirt] [PATCH v3 1/2] Move virBhyveTapGetRealDeviceName to virnetdevtap

[Roman Bogorodskiy]

To ease mocking for bhyve unit tests move virBhyveTapGetRealDeviceName()
out of bhyve_command.c to virnetdevtap and rename it to
virNetDevTapGetRealDeviceName().
---
 src/bhyve/bhyve_command.c | 74 +---
 src/libvirt_private.syms  |  1 +
 src/util/virnetdevtap.c   | 87 +++
 src/util/virnetdevtap.h   |  3 ++
 4 files changed, 92 insertions(+), 73 deletions(-)

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 42b71fb..3373cfc 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -21,10 +21,7 @@
 
 #include 
 
-#include 
 #include 
-#include 
-#include 
 #include 
 #include 
 
@@ -41,75 +38,6 @@
 
 VIR_LOG_INIT("bhyve.bhyve_command");
 
-static char*
-virBhyveTapGetRealDeviceName(char *name)
-{
-/* This is an ugly hack, because if we rename
- * tap device to vnet%d, its device name will be
- * still /dev/tap%d, and bhyve tries to open /dev/tap%d,
- * so we have to find the real name
- */
-char *ret = NULL;
-struct dirent *dp;
-char *devpath = NULL;
-int fd;
-
-DIR *dirp = opendir("/dev");
-if (dirp == NULL) {
-virReportSystemError(errno,
- _("Failed to opendir path '%s'"),
- "/dev");
-return NULL;
-}
-
-while ((dp = readdir(dirp)) != NULL) {
-if (STRPREFIX(dp->d_name, "tap")) {
-struct ifreq ifr;
-if (virAsprintf(&devpath, "/dev/%s", dp->d_name) < 0) {
-goto cleanup;
-}
-if ((fd = open(devpath, O_RDWR)) < 0) {
-if (errno == EBUSY) {
-VIR_FREE(devpath);
-continue;
-}
-virReportSystemError(errno, _("Unable to open '%s'"), devpath);
-goto cleanup;
-}
-
-if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) {
-virReportSystemError(errno, "%s",
- _("Unable to query tap interface name"));
-goto cleanup;
-}
-
-if (STREQ(name, ifr.ifr_name)) {
-/* we can ignore the return value
- * because we still have nothing
- * to do but return;
- */
-ignore_value(VIR_STRDUP(ret, dp->d_name));
-goto cleanup;
-}
-
-VIR_FREE(devpath);
-VIR_FORCE_CLOSE(fd);
-}
-
-errno = 0;
-}
-
-if (errno != 0)
-virReportSystemError(errno, "%s",
- _("Unable to iterate over TAP devices"));
-
- cleanup:
-VIR_FREE(devpath);
-VIR_FORCE_CLOSE(fd);
-closedir(dirp);
-return ret;
-}
-
 static int
 bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr cmd)
 {
@@ -161,7 +89,7 @@ bhyveBuildNetArgStr(const virDomainDef *def, virCommandPtr 
cmd)
 }
 }
 
-realifname = virBhyveTapGetRealDeviceName(net->ifname);
+realifname = virNetDevTapGetRealDeviceName(net->ifname);
 
 if (realifname == NULL) {
 VIR_FREE(net->ifname);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2357f95..38fbf63 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1584,6 +1584,7 @@ virNetDevTapCreate;
 virNetDevTapCreateInBridgePort;
 virNetDevTapDelete;
 virNetDevTapGetName;
+virNetDevTapGetRealDeviceName;
 
 
 # util/virnetdevveth.h
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 32ad406..3072146 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -22,6 +22,9 @@
 
 #include 
 
+#include 
+#include 
+
 #include "virmacaddr.h"
 #include "virnetdevtap.h"
 #include "virnetdev.h"
@@ -38,8 +41,11 @@
 #include 
 #ifdef __linux__
 # include /* IFF_TUN, IFF_NO_PI */
+#elif defined(__FreeBSD__)
+# include 
 #endif
 
+
 #define VIR_FROM_THIS VIR_FROM_NONE
 
 VIR_LOG_INIT("util.netdevtap");
@@ -72,6 +78,87 @@ virNetDevTapGetName(int tapfd ATTRIBUTE_UNUSED, char 
**ifname ATTRIBUTE_UNUSED)
 #endif
 }
 
+/**
+ * virNetDevTapGetRealDeviceName:
+ * @ifname: the interface name
+ *
+ * Lookup real interface name (i.e. name of the device entry in /dev),
+ * because e.g. on FreeBSD if we rename tap device to vnetN its device
+ * entry still remains unchanged (/dev/tapX), but bhyve needs a name
+ * that matches /dev entry.
+ *
+ * Returns the proper interface name or NULL if no corresponding interface
+ * found.
+ */
+char*
+virNetDevTapGetRealDeviceName(char *ifname ATTRIBUTE_UNUSED)
+{
+#ifdef TAPGIFNAME
+char *ret = NULL;
+struct dirent *dp;
+char *devpath = NULL;
+int fd;
+
+DIR *dirp = opendir("/dev");
+if (dirp == NULL) {
+virReportSystemError(errno,
+ _("Failed to opendir path '%s'"),
+ "/dev");
+return NULL;
+}
+
+while ((dp = readdir(dirp)) != NULL) {
+  

[libvirt] [PATCH v3 0/2] bhyve: add xml2args unittest

[Roman Bogorodskiy]

Changes from v2:
 - Make virBhyveTapGetRealDeviceName a stub on non-FreeBSD
 - Add bhyvexml2argvdata and bhyvexml2argvmock.c to EXTRA_DIST
 - Include bhyve headers in tests only if WITH_BHYVE is defined
 - Don't use 'util/' for #include when not needed

Changes from v1:
 - Chase MAC address support by adding virMacAddrGenerate() mock, so
   we can get a constant MAC address
 - Add a test for the case when MAC address is specified in the
   domain xml

Roman Bogorodskiy (2):
  Move virBhyveTapGetRealDeviceName to virnetdevtap
  bhyve: add xml2args unittest

 src/bhyve/bhyve_command.c  |  74 +-
 src/libvirt_private.syms   |   1 +
 src/util/virnetdevtap.c|  87 
 src/util/virnetdevtap.h|   3 +
 tests/Makefile.am  |  26 
 .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args  |   3 +
 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml |  24 
 tests/bhyvexml2argvdata/bhyvexml2argv-base.args|   3 +
 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml |  20 +++
 .../bhyvexml2argv-disk-virtio.args |   3 +
 .../bhyvexml2argv-disk-virtio.xml  |  20 +++
 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args |   3 +
 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml  |  21 +++
 tests/bhyvexml2argvmock.c  |  49 +++
 tests/bhyvexml2argvtest.c  | 153 +
 15 files changed, 417 insertions(+), 73 deletions(-)
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args
 create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml
 create mode 100644 tests/bhyvexml2argvmock.c
 create mode 100644 tests/bhyvexml2argvtest.c

-- 
1.8.4.2

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

Re: [libvirt] [Qemu-devel] [PATCH v3 for 2.0] update names in option tables to match with actual command-line spelling

[Markus Armbruster]

Eric Blake  writes:

> On 03/20/2014 07:07 AM, Amos Kong wrote:
>> We want to establish a mapping between option name and option table,
>> then we can search related option table by option name.
>> 
>> This patch makes all the member name of QemuOptsList to match with
>> actual command-line spelling(option name).
>> 
>> [ Important Note ]
>> 
>> The QemuOptsList member name values are ABI, changing them can break
>> existing -readconfig configuration files.
>> 
>> This patch changes:
>> 
>> fromto  introduced in
>> acpiacpitable   0c764a9 v1.5.0
>> boot-opts   boot3d3b830 v1.0
>> smp-optssmp 12b7f57 v1.6.0
>> 
>> All three have calcified into ABI already.
>> 
>> I have updated the release note of 2.0
>> http://wiki.qemu.org/ChangeLog/2.0#ABI_breaking
>> 
>> Signed-off-by: Amos Kong 
>> ---
>
> The benefit of this patch is that 'query-command-line-options' gains a
> fix where three bogus entries are replaced by their actual command line
> spelling.  The drawback is that anyone that doesn't pay attention to the
> ABI break announcement, and expects -readconfig and friends to work
> while using the old spelling, is in for a surprise.  But since we have
> prominently documented the change, and since consistency makes life
> nicer, I'm in favor of this patch.
>
> Reviewed-by: Eric Blake 

I'm not thrilled about the ABI break, but avoiding it would probably
take too much code for too little gain.

How can we prevent future violations of the convention "QemuOptsList
member name matches the name of the (primary) option using it for its
argument"?  Right now, all we have is /* option name */ tacked to the
member.  Which is at best a reminder for those who already know.

I'd ask for a test catching violations if I could think of an easy way
to code it.

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

Re: [libvirt] [PATCH] security_dac: Honor norelabel attribute

[Jiri Denemark]

On Mon, Mar 24, 2014 at 17:37:07 +0100, Michal Privoznik wrote:
> The inspiration for this patch comes from a question on the list
> asking if there's a way to not label some disks. Well, in DAC driver
> there's not. Even if user have requested norelabel:
> 
> 
>   
>   
> 
>   
>   
>   
>function='0x0'/>
> 
> 
> the DAC driver ignores this completely. When adjusting the code, I
> realized it's a ragbag with plenty of things that we try to avoid.
> >From the variety just a few things: callback data were passed as:
> 
> void params[2];
> params[0] = mgr;
> params[1] = def;
> 
> Or my favorite - checking for passed pointer being non NULL on each
> level of the stack, in each callee. As a pattern of readable code the
> selinux driver was used.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/security/security_dac.c | 244 
> 
>  1 file changed, 131 insertions(+), 113 deletions(-)
> 
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 9f45063..3b8fe04 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -53,6 +53,15 @@ struct _virSecurityDACData {
>  char *baselabel;
>  };
>  
> +typedef struct _virSecurityDACCallbackData virSecurityDACCallbackData;
> +typedef virSecurityDACCallbackData *virSecurityDACCallbackDataPtr;
> +
> +struct _virSecurityDACCallbackData {
> +virSecurityManagerPtr manager;
> +virSecurityLabelDefPtr secdef;
> +};
> +
> +
>  /* returns -1 on error, 0 on success */
>  int
>  virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
> @@ -81,65 +90,42 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr 
> mgr,
>  
>  /* returns 1 if label isn't found, 0 on success, -1 on error */
>  static int
> -virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr)
> +virSecurityDACParseIds(virSecurityLabelDefPtr seclabel,
> +   uid_t *uidPtr, gid_t *gidPtr)
>  {
> -uid_t uid;
> -gid_t gid;
> -virSecurityLabelDefPtr seclabel;
> -
> -if (def == NULL)
> -return 1;
> -
> -seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
> -if (seclabel == NULL || seclabel->label == NULL) {
> -VIR_DEBUG("DAC seclabel for domain '%s' wasn't found", def->name);
> +if (!seclabel || !seclabel->label)
>  return 1;
> -}
>  
> -if (virParseOwnershipIds(seclabel->label, &uid, &gid) < 0)
> +if (virParseOwnershipIds(seclabel->label, uidPtr, gidPtr) < 0)
>  return -1;
>  
> -if (uidPtr)
> -*uidPtr = uid;
> -if (gidPtr)
> -*gidPtr = gid;
> -
>  return 0;
>  }
>  
>  static int
> -virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
> +virSecurityDACGetIds(virSecurityLabelDefPtr seclabel,
> + virSecurityDACDataPtr priv,
>   uid_t *uidPtr, gid_t *gidPtr,
>   gid_t **groups, int *ngroups)
>  {
>  int ret;
>  
> -if (!def && !priv) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("Failed to determine default DAC seclabel "
> - "for an unknown object"));
> -return -1;
> -}
> -
>  if (groups)
>  *groups = priv ? priv->groups : NULL;
>  if (ngroups)
>  *ngroups = priv ? priv->ngroups : 0;
>  
> -if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) <= 0)
> +if ((ret = virSecurityDACParseIds(seclabel, uidPtr, gidPtr)) <= 0)
>  return ret;
>  
>  if (!priv) {
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> -   _("DAC seclabel couldn't be determined "
> - "for domain '%s'"), def->name);
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("DAC seclabel couldn't be determined"));
>  return -1;
>  }
>  
> -if (uidPtr)
> -*uidPtr = priv->user;
> -if (gidPtr)
> -*gidPtr = priv->group;
> +*uidPtr = priv->user;
> +*gidPtr = priv->group;
>  
>  return 0;
>  }
> @@ -147,60 +133,36 @@ virSecurityDACGetIds(virDomainDefPtr def, 
> virSecurityDACDataPtr priv,
>  
>  /* returns 1 if label isn't found, 0 on success, -1 on error */
>  static int
> -virSecurityDACParseImageIds(virDomainDefPtr def,
> +virSecurityDACParseImageIds(virSecurityLabelDefPtr seclabel,
>  uid_t *uidPtr, gid_t *gidPtr)
>  {
> -uid_t uid;
> -gid_t gid;
> -virSecurityLabelDefPtr seclabel;
> -
> -if (def == NULL)
> -return 1;
> -
> -seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
> -if (seclabel == NULL || seclabel->imagelabel == NULL) {
> -VIR_DEBUG("DAC imagelabel for domain '%s' wasn't found", def->name);
> +if (!seclabel || !seclabel->imagelabel)
>  return 1;
> -}
>  
> -if (virParseOwnershipIds(seclabel->imagelabel, &uid, &gid)

[libvirt] [PATCH] cpu: Properly check input parameters

[Jiri Denemark]

Most of the APIs in CPU driver do not expect to get NULL for input
parameters. Let's mark them with ATTRIBUTE_NONNULL and also check for
some members of virCPUDef when the APIs expect them have some specific
values.

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c | 48 +---
 src/cpu/cpu.h | 36 
 2 files changed, 61 insertions(+), 23 deletions(-)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 9cd2300..528e1a7 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -108,12 +108,6 @@ cpuCompareXML(virCPUDefPtr host,
 if (cpu == NULL)
 goto cleanup;
 
-if (!cpu->model) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("no CPU model specified"));
-goto cleanup;
-}
-
 ret = cpuCompare(host, cpu);
 
  cleanup:
@@ -146,6 +140,12 @@ cpuCompare(virCPUDefPtr host,
 
 VIR_DEBUG("host=%p, cpu=%p", host, cpu);
 
+if (!cpu->model) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("no guest CPU model specified"));
+return VIR_CPU_COMPARE_ERROR;
+}
+
 if ((driver = cpuGetSubDriver(host->arch)) == NULL)
 return VIR_CPU_COMPARE_ERROR;
 
@@ -203,14 +203,15 @@ cpuDecode(virCPUDefPtr cpu,
 }
 
 if (models == NULL && nmodels != 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("nonzero nmodels doesn't match with NULL 
models"));
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("nonzero nmodels doesn't match with NULL models"));
 return -1;
 }
 
-if (cpu == NULL) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("invalid CPU definition"));
+if (cpu->type > VIR_CPU_TYPE_GUEST ||
+cpu->mode != VIR_CPU_MODE_CUSTOM) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("invalid CPU definition stub"));
 return -1;
 }
 
@@ -264,6 +265,12 @@ cpuEncode(virArch arch,
   virArchToString(arch), cpu, forced, required,
   optional, disabled, forbidden, vendor);
 
+if (!cpu->model) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("no guest CPU model specified"));
+return -1;
+}
+
 if ((driver = cpuGetSubDriver(arch)) == NULL)
 return -1;
 
@@ -367,6 +374,12 @@ cpuGuestData(virCPUDefPtr host,
 
 VIR_DEBUG("host=%p, guest=%p, data=%p, msg=%p", host, guest, data, msg);
 
+if (!guest->model) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("no guest CPU model specified"));
+return VIR_CPU_COMPARE_ERROR;
+}
+
 if ((driver = cpuGetSubDriver(host->arch)) == NULL)
 return VIR_CPU_COMPARE_ERROR;
 
@@ -529,6 +542,19 @@ cpuBaseline(virCPUDefPtr *cpus,
 return NULL;
 }
 
+for (i = 0; i < ncpus; i++) {
+if (!cpus[i]) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("invalid CPU definition at index %zu"), i);
+return NULL;
+}
+if (!cpus[i]->model) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("no CPU model specified at index %zu"), i);
+return NULL;
+}
+}
+
 if (models == NULL && nmodels != 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("nonzero nmodels doesn't match with NULL 
models"));
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 27169fe..e9f2713 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -119,18 +119,21 @@ struct cpuArchDriver {
 
 extern virCPUCompareResult
 cpuCompareXML(virCPUDefPtr host,
-  const char *xml);
+  const char *xml)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 extern virCPUCompareResult
 cpuCompare  (virCPUDefPtr host,
- virCPUDefPtr cpu);
+ virCPUDefPtr cpu)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 extern int
 cpuDecode   (virCPUDefPtr cpu,
  const virCPUData *data,
  const char **models,
  unsigned int nmodels,
- const char *preferred);
+ const char *preferred)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 extern int
 cpuEncode   (virArch arch,
@@ -140,7 +143,8 @@ cpuEncode   (virArch arch,
  virCPUDataPtr *optional,
  virCPUDataPtr *disabled,
  virCPUDataPtr *forbidden,
- virCPUDataPtr *vendor);
+ virCPUDataPtr *vendor)
+ATTRIBUTE_NONNULL(2);
 
 extern void
 cpuDataFree (virCPUDataPtr data);
@@ -152,7 +156,8 @@ extern virCPUCompareResult
 cpuGuestData(virCPUDefPtr host,
  virCPUDefPtr guest,
  virCPUDataPtr *data,
- char **msg);
+ char **msg)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 extern char *
 cpuBaselineXML(const char **xmlCPUs,
@@ -166,30 +171,37 @@ cpuBaseline (virCPUDefPtr *cpus,

Re: [libvirt] [PATCH v2] dnsmasq: allowing RFC 2782 compliant SRV records

[Laine Stump]

On 03/08/2014 01:13 AM, Steven Malin wrote:
> This patch allows RFC 2782 compliant SRV records in the network
> config and corrects the documentation.

Thanks for bringing this bug to light in such a proactive way (with a
patch rather than just a complaint :-)). Your patch pointed out that the
SRV records produced by libvirt were totally unusable (since any
application will be looking for _service._protocol.example.com, but
libvirt was unable to produce any such records).

Rather than allowing libvirt to optionally create usable SRV records
though, I made a different patch that *forces* it to create usable
records, while fixing some other bugs that I found along the way:

  https://www.redhat.com/archives/libvir-list/2014-March/msg01206.html

I just pushed that patch upstream (commit
6612d1adb7943daf05feec002d7a025263c84a4f). Please give it a try and see
if it solves your problems.

Note that once this patch is applied, you will *not* want to put the
leading "_" in either the service or protocol name in the configuration.
(This would have created a backward compatibility problem, except that
the old code couldn't have been usable by anyone anyway).

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

[libvirt] [PATCH] cpu: Add documentation for CPU driver APIs

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---
 src/cpu/cpu.c | 202 ++
 1 file changed, 202 insertions(+)

diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index e91f5bb..9cd2300 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -77,6 +77,19 @@ cpuGetSubDriver(virArch arch)
 }
 
 
+/**
+ * cpuCompareXML:
+ *
+ * @host: host CPU definition
+ * @xml: XML description of either guest or host CPU to be compared with @host
+ *
+ * Compares the CPU described by @xml with @host CPU.
+ *
+ * Returns VIR_CPU_COMPARE_ERROR on error, VIR_CPU_COMPARE_INCOMPATIBLE when
+ * the two CPUs are incompatible, VIR_CPU_COMPARE_IDENTICAL when the two CPUs
+ * are identical, VIR_CPU_COMPARE_SUPERSET when the @xml CPU is a superset of
+ * the @host CPU.
+ */
 virCPUCompareResult
 cpuCompareXML(virCPUDefPtr host,
   const char *xml)
@@ -112,6 +125,19 @@ cpuCompareXML(virCPUDefPtr host,
 }
 
 
+/**
+ * cpuCompare:
+ *
+ * @host: host CPU definition
+ * @cpu: either guest or host CPU to be compared with @host
+ *
+ * Compares the CPU described by @cpu with @host CPU.
+ *
+ * Returns VIR_CPU_COMPARE_ERROR on error, VIR_CPU_COMPARE_INCOMPATIBLE when
+ * the two CPUs are incompatible, VIR_CPU_COMPARE_IDENTICAL when the two CPUs
+ * are identical, VIR_CPU_COMPARE_SUPERSET when the @cpu CPU is a superset of
+ * the @host CPU.
+ */
 virCPUCompareResult
 cpuCompare(virCPUDefPtr host,
virCPUDefPtr cpu)
@@ -134,6 +160,31 @@ cpuCompare(virCPUDefPtr host,
 }
 
 
+/**
+ * cpuDecode:
+ *
+ * @cpu: CPU definition stub to be filled in
+ * @data: internal CPU data to be decoded into @cpu definition
+ * @models: list of CPU models that can be considered when decoding @data
+ * @nmodels: number of CPU models in @models
+ * @preferred: CPU models that should be used if possible
+ *
+ * Decodes internal CPU data into a CPU definition consisting of a CPU model
+ * and a list of CPU features. The @cpu model stub is supposed to have arch,
+ * type, match and fallback members set, this function will add the rest. If
+ * @models list is NULL, all models supported by libvirt will be considered
+ * when decoding the data. In general, this function will select the model
+ * closest to the CPU specified by @data unless @preferred is non-NULL, in
+ * which case the @preferred model will be used as long as it is compatible
+ * with @data.
+ *
+ * For VIR_ARCH_I686 and VIR_ARCH_X86_64 architectures this means the computed
+ * CPU definition will have the shortest possible list of additional features.
+ * When @preferred is non-NULL, the @preferred model will be used even if
+ * other models would result in a shorter list of additional features.
+ *
+ * Returns 0 on success, -1 on error.
+ */
 int
 cpuDecode(virCPUDefPtr cpu,
   const virCPUData *data,
@@ -177,6 +228,25 @@ cpuDecode(virCPUDefPtr cpu,
 }
 
 
+/**
+ * cpuEncode:
+ *
+ * @arch: CPU architecture
+ * @cpu: CPU definition to be encoded into internal CPU driver representation
+ * @forced: where to store CPU data corresponding to forced features
+ * @required: where to store CPU data corresponding to required features
+ * @optional: where to store CPU data corresponding to optional features
+ * @disabled: where to store CPU data corresponding to disabled features
+ * @forbidden: where to store CPU data corresponding to forbidden features
+ * @vendor: where to store CPU data corresponding to CPU vendor
+ *
+ * Encode CPU definition from @cpu into internal CPU driver representation.
+ * Any of @forced, @required, @optional, @disabled, @forbidden and @vendor
+ * arguments can be NULL in case the caller is not interested in the
+ * corresponding data.
+ *
+ * Returns 0 on success, -1 on error.
+ */
 int
 cpuEncode(virArch arch,
   const virCPUDef *cpu,
@@ -209,6 +279,15 @@ cpuEncode(virArch arch,
 }
 
 
+/**
+ * cpuDataFree:
+ *
+ * @data: CPU data structure to be freed
+ *
+ * Free internal CPU data.
+ *
+ * Returns nothing.
+ */
 void
 cpuDataFree(virCPUDataPtr data)
 {
@@ -233,6 +312,13 @@ cpuDataFree(virCPUDataPtr data)
 }
 
 
+/**
+ * cpuNodeData:
+ *
+ * @arch: CPU architecture
+ *
+ * Returns CPU data for host CPU or NULL on error.
+ */
 virCPUDataPtr
 cpuNodeData(virArch arch)
 {
@@ -254,6 +340,23 @@ cpuNodeData(virArch arch)
 }
 
 
+/**
+ * cpuGuestData:
+ *
+ * @host: host CPU definition
+ * @guest: guest CPU definition
+ * @data: computed guest CPU data
+ * @msg: error message describing why the @guest and @host CPUs are considered
+ *   incompatible
+ *
+ * Computes guest CPU data for the @guest CPU definition when run on the @host
+ * CPU.
+ *
+ * Returns VIR_CPU_COMPARE_ERROR on error, VIR_CPU_COMPARE_INCOMPATIBLE when
+ * the two CPUs are incompatible (@msg will describe the incompatibility),
+ * VIR_CPU_COMPARE_IDENTICAL when the two CPUs are identical,
+ * VIR_CPU_COMPARE_SUPERSET when the @guest CPU is a superset of the @host CPU.
+ */
 virCPUCompareResult
 cpuGuestData(virCPUDefPtr host,
  virCPUDefPt

Re: [libvirt] [libvirt-java] [PATCH 35/65] Fix memleak in StorageVol.getXMLDesc

[Claudio Bley]

At Fri, 21 Feb 2014 10:59:44 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:43PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/StorageVol.java  |8 +++-
> >  src/main/java/org/libvirt/jna/Libvirt.java |2 +-
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [PATCH 0/5] virCommandRunRegex cleanups

[Ján Tomko]

On 03/26/2014 02:20 PM, Michal Privoznik wrote:
> On 25.03.2014 08:17, Ján Tomko wrote:
>> Simplify the code. This should have no functional change
>> (except for the impossible leak fix in 3/5). Most of this
>> functionality is tested in viriscsitest.
>>
>> Ján Tomko (5):
>>Remove useless 'maxReg' variable
>>Simplify the loop in virCommandRunRegex
>>Free groups in case of a partial match
>>Use VIR_STRNDUP instead of modifying the matched string
>>Shift the for loop over matched vars by one
>>
>>   src/util/vircommand.c | 46 +++---
>>   1 file changed, 19 insertions(+), 27 deletions(-)
>>
> 
> ACK series.
> 

Thanks; pushed now.

Jan




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 0/5] virCommandRunRegex cleanups

[Michal Privoznik]

On 25.03.2014 08:17, Ján Tomko wrote:

Simplify the code. This should have no functional change
(except for the impossible leak fix in 3/5). Most of this
functionality is tested in viriscsitest.

Ján Tomko (5):
   Remove useless 'maxReg' variable
   Simplify the loop in virCommandRunRegex
   Free groups in case of a partial match
   Use VIR_STRNDUP instead of modifying the matched string
   Shift the for loop over matched vars by one

  src/util/vircommand.c | 46 +++---
  1 file changed, 19 insertions(+), 27 deletions(-)



ACK series.

Michal

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

Re: [libvirt] [libvirt-java] [PATCH 33/65] Fix memleak in DomainSnapshot.getXMLDesc

[Claudio Bley]

At Fri, 21 Feb 2014 10:59:02 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:41PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/DomainSnapshot.java |   10 +-
> >  src/main/java/org/libvirt/jna/Libvirt.java|2 +-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 00/65]

[Claudio Bley]

At Fri, 21 Feb 2014 11:21:17 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:08PM +0100, Claudio Bley wrote:
> > Hi.
> > 
> > Here are a few patches that piled up in my local branch. Some of them
> > I already submitted to this list, but there has been no reponse to
> > them.
> > 
> > Included are a few trivial fixes as well as memory leak fixes and
> > additions to the public API.
> > 
> > There had been some minor interest in my first version of domain event
> > support in the Java wrapper[1], which I have reworked almost entirely.
> > 
> > If nobody objects, say, within the next two weeks or so, I'll go ahead
> > and push the whole series as was suggested to me by Daniel P. Berrange
> > on this list[2].
> 
> BTW, when you push these patches, you should also update AUTHORS
> to list yourself as the primary maintainer of the binding, similar
> to how we list maintainers in the main libvirt AUTHORS file.

Sure thing. I've pushed this a couple of minutes ago under the
trivial rule:

--
commit c04d0aef148e9f89b08a4ccbfc17b17e2e4c8caf
Author: Claudio Bley 
Date:   Wed Mar 26 10:50:41 2014 +0100

Take over libvirt-java as the primary maintainer

diff --git a/AUTHORS b/AUTHORS
index 8404152..07809bf 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -1,6 +1,10 @@
 Packaging Daniel Veillard 
 Initial code base from Tóth István 

+Primary maintainer:
+
+Claudio Bley 
+
 Patches provided by:

 Daniel Schwager 
@@ -11,5 +15,4 @@ Luis Carlos Moreira da Costa 
 Andrea Sansottera 
 Stefan Majer 
 Wido den Hollander 
-Claudio Bley 
 Eric Blake 
--

Regards,
Claudio

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

Re: [libvirt] [libvirt-java] [PATCH 34/65] Fix memleak in StorageVol.getPath

[Claudio Bley]

At Fri, 21 Feb 2014 10:59:25 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:42PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/StorageVol.java  |   10 +-
> >  src/main/java/org/libvirt/jna/Libvirt.java |2 +-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 08/65] Depend on JNA versions 3.4.1 to 4.0.0

[Claudio Bley]

At Fri, 21 Feb 2014 10:36:10 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:16PM +0100, Claudio Bley wrote:
> > Specify a version range for the net.java.dev.jna / jna artifact
> > in order to accept any version we tested the libvirt Java bindings
> > against.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> > As discussed previously[1], with a little delay, here's the patch.
> > 
> > [1]: http://www.redhat.com/archives/libvir-list/2013-September/msg00929.html
> > 
> >  pom.xml.in |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 32/65] Fix memleak in StoragePool.listVolumes

[Claudio Bley]

At Fri, 21 Feb 2014 10:58:35 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:40PM +0100, Claudio Bley wrote:
> > We need to free the char* entries of the result array returned
> > ourselves.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/StoragePool.java |   13 ++---
> >  src/main/java/org/libvirt/jna/Libvirt.java |2 +-
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [RFC] VM which uses macvtap will not respond ping request when being migrated

[Viktor Mihajlovski]

On 03/25/2014 07:15 AM, Wangrui (K) wrote:
> macvtap0 is belong to vm at the dest side. When migrate begins macvtap0 will 
> send the packet like this:
> 
> 07:57:59.233270 IP6 :: > ff02::16: HBH ICMP6, multicast listener report v2, 1 
> group record(s), lengt 28
> 07:58:00.096088 IP6 :: > ff02::1:ff6d:4665: ICMP6, neighbor solicitation, who 
> has fe80::5054:ff:fe6d4665, length 24
> 07:58:00.123900 IP6 fe80::2a6e:d4ff:fe50:8d7 > ipv6-allrouters: ICMP6, router 
> solicitation, length 1
> 07:58:00.175891 IP6 fe80::2a6e:d4ff:fe50:8d7 > ff02::16: HBH ICMP6, multicast 
> listener report v2, 1 roup record(s), length 28
> 07:58:01.096089 IP6 fe80::5054:ff:fe6d:4665 > ipv6-allrouters: ICMP6, router 
> solicitation, length 16
> 
> the virNetDevSetOnline in the libvirt makes macvtap0 up on the dest side, so 
> macvtap0 sends the packets.
> 

well, what happens here is that the newly created interface is subject
to IPv6 autoconfiguration (you can see that it has obtained an IPv6
link local address). The resulting ICMP6 traffic will update your
switch's MAC address tables as you have correctly observed.

A workaround (but no more) *could be* to disable the IPv6
autoconfiguration for the interface in question (awkward/racy).

A better approach would probably be to not online the macvtap interface
until the domain is really running (here: after the migration was
completed). Would need a rewrite of the network interface code in
the qemu driver.

-- 

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

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] [libvirt-java] [PATCH 31/65] Fix memleak in Domain.snapshotListNames

[Claudio Bley]

eAt Fri, 21 Feb 2014 10:57:44 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:39PM +0100, Claudio Bley wrote:
> > We need to free the char* entries of the result array returned ourselves.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Domain.java  |   14 +++---
> >  src/main/java/org/libvirt/jna/Libvirt.java |2 +-
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 30/65] Fix Domain.getSchedulerParameters / getSchedulerType

[Claudio Bley]

At Fri, 21 Feb 2014 10:57:10 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:38PM +0100, Claudio Bley wrote:
> > The getSchedulerType method returns a String (the name of the
> > scheduler), not a String array containing a single element.
> > 
> > It's OK to just pass null for the second argument to
> > virDomainGetSchedulerType.
> > 
> > Ensure to free the returned string.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Domain.java |   43 
> > +++--
> >  1 file changed, 25 insertions(+), 18 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 23/65] Remove processError method from StoragePool class

[Claudio Bley]

At Fri, 21 Feb 2014 10:49:15 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:31PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to
> > ErrorHandler.processError(..).
> > 
> > Adjust the doc comment for storageVolLookupByName to indicate that
> > it might return null.
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error occurs.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/StoragePool.java |   95 
> > +---
> >  1 file changed, 30 insertions(+), 65 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 28/65] Remove ErrorHandler.processError(Libvirt) method

[Claudio Bley]

At Fri, 21 Feb 2014 10:51:17 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:36PM +0100, Claudio Bley wrote:
> > It was deprecated and is no longer used.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/ErrorHandler.java |   12 
> >  1 file changed, 12 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 26/65] Remove processError method from Connect class

[Claudio Bley]

At Fri, 21 Feb 2014 10:50:22 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:34PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to
> > ErrorHandler.processError(..) and remove calls to the deprecated
> > ErrorHandler.processError(Libvirt) method.
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error occurs.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Connect.java |   69 
> > +---
> >  1 file changed, 11 insertions(+), 58 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 20/65] Remove processError method from Network class

[Claudio Bley]

At Fri, 21 Feb 2014 10:45:55 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:28PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to 
> > ErrorHandler.processError(..).
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error occurs.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Network.java |   61 
> > +---
> >  1 file changed, 17 insertions(+), 44 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 27/65] Call processError only when virInitialize signalled an error

[Claudio Bley]

At Fri, 21 Feb 2014 10:50:57 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:35PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Library.java |4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 25/65] Remove processError method from Stream class

[Claudio Bley]

At Fri, 21 Feb 2014 10:49:52 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:33PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to
> > ErrorHandler.processError(..).
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error occurs.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Stream.java |   64 
> > +++--
> >  1 file changed, 20 insertions(+), 44 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 24/65] Remove processError method from StorageVol class

[Claudio Bley]

At Fri, 21 Feb 2014 10:49:33 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:32PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to
> > ErrorHandler.processError(..).
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error occurs.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/StorageVol.java |   56 
> > +
> >  1 file changed, 16 insertions(+), 40 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 22/65] Remove processError method from Secret class

[Claudio Bley]

At Fri, 21 Feb 2014 10:48:44 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:30PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to
> > ErrorHandler.processError(..).
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error occurs.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Secret.java |   61 
> > +
> >  1 file changed, 16 insertions(+), 45 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 19/65] Remove processError from Interface class

[Claudio Bley]

At Fri, 21 Feb 2014 10:45:22 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:27PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to 
> > ErrorHandler.processError(..).
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error occurs.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Interface.java |   43 
> > --
> >  1 file changed, 11 insertions(+), 32 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 21/65] Remove processError method from NetworkFilter class

[Claudio Bley]

At Fri, 21 Feb 2014 10:46:19 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:29PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to
> > ErrorHandler.processError(..).
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error occurs.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/NetworkFilter.java |   43 
> > ++
> >  1 file changed, 10 insertions(+), 33 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 16/65] Remove processError from Device class

[Claudio Bley]

At Fri, 21 Feb 2014 10:43:43 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:24PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to 
> > ErrorHandler.processError(..).
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Device.java |   49 
> > ++---
> >  1 file changed, 14 insertions(+), 35 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 15/65] Start refactoring of error handling

[Claudio Bley]

At Fri, 21 Feb 2014 10:43:15 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:23PM +0100, Claudio Bley wrote:
> > Almost every class contains a processError() method with an identical
> > definition, just forwarding the call to ErrorHandler.processError(Libvirt).
> > 
> > This function is always called after a libvirt function call (as per its
> > javadoc comment).
> > 
> > But, actually, there's no use in always calling processError when there was
> > no error signalled by the libvirt function having been called. This is just
> > a waste of CPU cycles.
> > 
> > Furthermore, it's more than ugly that the error handling is littered all
> > over the place in every class.
> > 
> > This patch lays ground for generalizing the error handling in a common
> > place and removing those functions from the individual classes.
> > 
> > Basically, this copies the processError(int) and processError(T)
> > methods from the Connect class to the ErrorHandler class as static
> > methods.
> > 
> > It deprecates the processError(Libvirt) method, which will be removed
> > eventually in a later patch.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/ErrorHandler.java |   47 
> > ++-
> >  1 file changed, 46 insertions(+), 1 deletion(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 18/65] Remove processError from DomainSnapshot class

[Claudio Bley]

At Fri, 21 Feb 2014 10:45:01 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:26PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to 
> > ErrorHandler.processError(..).
> > 
> > Also correct wrong javadoc comments stating that methods would return
> > a value in case an error happens.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/DomainSnapshot.java |   23 
> > ++-
> >  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 17/65] Remove processError from Domain class

[Claudio Bley]

At Fri, 21 Feb 2014 10:44:42 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:25PM +0100, Claudio Bley wrote:
> > Wrap any fallible libvirt function in a call to 
> > ErrorHandler.processError(..).
> > 
> > Also update erroneous javadoc comments stating that methods would return a
> > value in case an error occurs.  In case of a libvirt error, a
> > LibvirtException is thrown.
> > 
> > Add processErrorIfZero(long) to ErrorHandler class to handle special
> > libvirt return codes, such as for virDomainGetMaxMemory. Use it in
> > Domain.getMaxMemory().
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Domain.java   |  328 
> > +--
> >  src/main/java/org/libvirt/ErrorHandler.java |5 +
> >  2 files changed, 113 insertions(+), 220 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 14/65] test: ensure the Device.listCapabilities method works

[Claudio Bley]

At Fri, 21 Feb 2014 10:42:22 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:22PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/test/java/org/libvirt/TestJavaBindings.java |   14 ++
> >  1 file changed, 14 insertions(+)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 13/65] Make Device.listCapabilities return only valid array elements

[Claudio Bley]

At Fri, 21 Feb 2014 10:41:35 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:21PM +0100, Claudio Bley wrote:
> > The libvirt function virNodeDeviceListCaps might return fewer elements
> > than requested.  Take this into account and properly decode the UTF-8
> > strings returned.
> > 
> > Additionally, the given strings are freed before returning the resulting
> > string array.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Device.java  |   10 +++---
> >  src/main/java/org/libvirt/jna/Libvirt.java |2 +-
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 10/65] Fix wrapping of native size_t data type

[Claudio Bley]

At Fri, 21 Feb 2014 14:42:26 +0100,
Claudio Bley wrote:
> 
> At Fri, 21 Feb 2014 10:37:36 +,
> Daniel P. Berrange wrote:
> > 
> > On Thu, Feb 13, 2014 at 04:22:18PM +0100, Claudio Bley wrote:
> > > Libvirt function parameters having type (pointer to) size_t were
> > > wrapped via JNA using int, long or even NativeLong. Alas, none of
> > > these is actually correct as the size of size_t may be the same as the
> > > size of either (unsigned) int, long or even long long on different
> > > platforms.
> > > 
> > > JNA provides the size of a native size_t to us, so using this
> > > information we define and use class SizeT and class SizeTByReference for
> > > wrapping a native size_t and size_t*, respectively.
> > > 
> > > Signed-off-by: Claudio Bley 
> > > ---
> > >  src/main/java/org/libvirt/Domain.java  |5 +-
> > >  src/main/java/org/libvirt/Secret.java  |   10 ++--
> > >  src/main/java/org/libvirt/Stream.java  |7 ++-
> > >  src/main/java/org/libvirt/jna/Libvirt.java |   14 +++---
> > >  src/main/java/org/libvirt/jna/SizeT.java   |   19 
> > >  .../java/org/libvirt/jna/SizeTByReference.java |   50 
> > > 
> > >  6 files changed, 87 insertions(+), 18 deletions(-)
> > >  create mode 100644 src/main/java/org/libvirt/jna/SizeT.java
> > >  create mode 100644 src/main/java/org/libvirt/jna/SizeTByReference.java
> > 
> > ACK
> 
> Thanks. I'll squash this in before pushing, since throwing an
> IllegalArgumentException is not appropriate here.
> 
> --- >8 --- 8<  >8 - 8< 
> ---
> diff --git a/src/main/java/org/libvirt/jna/SizeTByReference.java 
> b/src/main/java/org/libvirt/jna/SizeTByReference.java
> index 474527f..24a4677 100644
> --- a/src/main/java/org/libvirt/jna/SizeTByReference.java
> +++ b/src/main/java/org/libvirt/jna/SizeTByReference.java
> @@ -30,7 +30,7 @@ public final class SizeTByReference extends ByReference {
>  p.setLong(0, value);
>  break;
>  default:
> -throw new IllegalArgumentException("Unsupported size: " + 
> Native.SIZE_T_SIZE);
> +throw new RuntimeException("Unsupported size: " + 
> Native.SIZE_T_SIZE);
>  }
>  }
> 
> @@ -44,7 +44,7 @@ public final class SizeTByReference extends ByReference {
>  case 8:
>  return p.getLong(0);
>  default:
> -throw new IllegalArgumentException("Unsupported size: " + 
> Native.SIZE_T_SIZE);
> +throw new RuntimeException("Unsupported size: " + 
> Native.SIZE_T_SIZE);
>  }
>  }
>  }

Pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 12/65] tests: remove obsolete test driver

[Claudio Bley]

At Fri, 21 Feb 2014 10:40:46 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:20PM +0100, Claudio Bley wrote:
> > JUnit is used for quite some time now, which supercedes the tests
> > defined in the old "test" class.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/test/java/test.java |  280 
> > ---
> >  1 file changed, 280 deletions(-)
> >  delete mode 100644 src/test/java/test.java
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 09/65] jna: load virt-0 or virt library depending on the platform

[Claudio Bley]

At Fri, 21 Feb 2014 10:36:33 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:17PM +0100, Claudio Bley wrote:
> > On Windows, the libvirt DLL is called libvirt-0.dll. Trying to load
> > the "virt" library hence fails to find the file. Branch on the platform
> > and load "virt-0" if we're running on this OS, use "virt" otherwise.
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/jna/Libvirt.java |3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 07/65] Ignore editor backup files

[Claudio Bley]

At Fri, 21 Feb 2014 10:35:42 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:15PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  .gitignore |2 ++
> >  1 file changed, 2 insertions(+)
> 
> ACK. Counts as a trivial patch that can be pushed without asking.

OK, thanks anyway :-). Pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 06/65] Make comments proper javadoc comments for enum constants

[Claudio Bley]

At Fri, 21 Feb 2014 10:35:17 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:14PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Error.java |  397 
> > ++
> >  1 file changed, 262 insertions(+), 135 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 05/65] test: ensure that exceptions are thrown when expected

[Claudio Bley]

At Fri, 21 Feb 2014 10:34:52 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:13PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/test/java/org/libvirt/TestJavaBindings.java |2 ++
> >  1 file changed, 2 insertions(+)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 04/65] test: fix typo in testConnection()

[Claudio Bley]

At Fri, 21 Feb 2014 10:34:24 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:12PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/test/java/org/libvirt/TestJavaBindings.java |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 03/65] Fix typos in Error.java

[Claudio Bley]

At Fri, 21 Feb 2014 10:34:05 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:11PM +0100, Claudio Bley wrote:
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Error.java |6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 01/65] Fix warnings about using raw types

[Claudio Bley]

At Fri, 21 Feb 2014 10:32:08 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:09PM +0100, Claudio Bley wrote:
> > Eclipse generates this kind of warning:
> > 
> > org/libvirt/jna/virConnectCredential.java:20:
> > List is a raw type. References to generic type List should be 
> > parameterized
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/jna/virConnectAuth.java  |4 ++--
> >  .../java/org/libvirt/jna/virConnectCredential.java |4 ++--
> >  .../java/org/libvirt/jna/virDomainBlockInfo.java   |4 ++--
> >  .../java/org/libvirt/jna/virDomainBlockStats.java  |4 ++--
> >  src/main/java/org/libvirt/jna/virDomainInfo.java   |4 ++--
> >  .../org/libvirt/jna/virDomainInterfaceStats.java   |4 ++--
> >  .../java/org/libvirt/jna/virDomainJobInfo.java |4 ++--
> >  .../java/org/libvirt/jna/virDomainMemoryStats.java |4 ++--
> >  src/main/java/org/libvirt/jna/virError.java|4 ++--
> >  src/main/java/org/libvirt/jna/virNodeInfo.java |4 ++--
> >  .../java/org/libvirt/jna/virSchedParameter.java|4 ++--
> >  .../java/org/libvirt/jna/virStoragePoolInfo.java   |4 ++--
> >  .../java/org/libvirt/jna/virStorageVolInfo.java|4 ++--
> >  src/main/java/org/libvirt/jna/virVcpuInfo.java |4 ++--
> >  14 files changed, 28 insertions(+), 28 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [libvirt-java] [PATCH 02/65] Fix warnings about accessing static methods

[Claudio Bley]

At Fri, 21 Feb 2014 10:33:44 +,
Daniel P. Berrange wrote:
> 
> On Thu, Feb 13, 2014 at 04:22:10PM +0100, Claudio Bley wrote:
> > java/org/libvirt/Error.java:217:
> > The static method wrap(int) from the type Error.ErrorDomain should be
> > accessed in a static way
> > 
> > Signed-off-by: Claudio Bley 
> > ---
> >  src/main/java/org/libvirt/Error.java |6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> ACK

Thanks, pushed.

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

Re: [libvirt] [Qemu-devel] [PATCH v4 2/2] query-command-line-options: query all the options in qemu-options.hx

[Markus Armbruster]

Amos Kong  writes:

> On Fri, Mar 07, 2014 at 10:54:09AM +0100, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>> > On 03/05/2014 07:36 PM, Amos Kong wrote:
>> >> vm_config_groups[] only contains part of the options which have
>> >> argument, and all options which have no argument aren't added
>> >> to vm_config_groups[]. Current query-command-line-options only
>> >> checks options from vm_config_groups[], so some options will
>> >> be lost.
>> >> 
>> >> We have macro in qemu-options.hx to generate a table that
>> >> contains all the options. This patch tries to query options
>> >> from the table.
>> >> 
>> >> Then we won't lose the legacy options that weren't added to
>> >> vm_config_groups[] (eg: -vnc, -smbios). The options that have
>> >> no argument will also be returned (eg: -enable-fips)
>> >> 
>> >> Some options that have argument have a NULL desc list, some
>> >> options don't have argument, and "parameters" is mandatory
>> >> in the past. So we add a new field "argument" to present
>> >> if the option takes unspecified arguments.
>> >
>> > I like Markus' suggestion of naming the new field
>> > 'unspecified-parameters' rather than 'argument'.
>  
> Hi Markus,
>
>> Looking again, there are more problems.
>> 
>> 1. Non-parameter argument vs. parameter argument taking unspecified
>>parameters
>> 
>>Example: -device takes unspecified parameters.  -cdrom doesn't take
>>parameters, it takes a file name.  Yet, the command reports the same
>>for both: "parameters": [], "argument": true.
>> 
>>Looks like we need a tri-state: option takes no argument, QemuOpts
>>argument, or other argument.
>
> '-cdrom' is the 'other argument' == 'Non-parameter argument'?

Let me clarify my terminology:

* A "parameter argument" is an option argument of the form KEY=VALUE,...
  Since parameter arguments should always be parsed with QemuOpts[*], I
  use the term "QemuOpts argument" interchangeably.

* A "non-parameter argument" or "other argument" is an option argument
  that doesn't use this form.

Does that answer your question?

> We can use a enum state.

I'm not sure I got that.

> |  { 'enum': 'ArgumentStateType',
> |'data': ['no-argument', 'unspecified-parameters-argument',
> | 'non-parameter-argument']
> |  }
> |  
> |  { 'type': 'CommandLineOptionInfo',
> |'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'],
> |  '*argument-state': 'ArgumentStateType' } }
>  
>>parameters is [] unless it's a QemuOpts argument.  Then it lists the
>>recognized parameters.
>
> How about balloon? we should treat as 'taking unspecified parameters'?
>
> "-balloon none   disable balloon device\n"
> "-balloon virtio[,addr=str]\n"
>
> I think we can only check this from help message in qemu-options.hx,
> is it a stable/acceptable method?

-balloon is yet another sugar option:

* "-balloon none" does nothing.  It could suppress a default balloon
  device, if such a thing existed.

* "-balloon virtio,KEY=VALUE..." is sugar for "-device
  virtio-balloon,KEY=VALUE...".  Keys other than "addr" are
  undocumented.

The actual option argument parsing is ad hoc, not QemuOpts.

I sure hope something like this would not pass review today.

My advice to tools introspecting the command line is to avoid sugared
options, unless the desugaring encapsulates something they don't want to
know.

> We introduce query-command-line-options command to avoid libvirt to
> check qemu commandline information by scanning qemu's help message,
> it's not strict & stable.
>  
>> 2. Our dear friend -drive is more complicated than you might think
>> 
>>We special-case it to report the union of drive_config_groups[],
>>which contains qemu_legacy_drive_opts, qemu_common_drive_opts and
>>qemu_drive_opts.  The latter accepts unspecified parameters.
>
> I'm confused here. But there is only one option (-drive), we should
> return merged desc table (legacy + common).
>
> Desc table of qemu_drive_opts is NULL, then -drive can also take
> unspecified parameters?

Yes: driver-specific parameters.

-drive takes currently takes unspecified parameters (the driver-specific
parameters) in addition to a number of specified parameters (the common
and legacy parameters).

>>I believe qemu_drive_opts is actually not used by the (complex!) code
>>parsing the argument of -drive.
>> 
>>Nevertheless, said code accepts more than just qemu_legacy_drive_opts
>>and qemu_common_drive_opts, namely driver-specific parameters.
>> 
>>Until we define those properly in a schema, I guess the best we can
>>do is add one more case: option takes QemuOpts argument, but
>>parameters is not exhaustive.
>
>
> Thanks, Amos


[*] Leftovers still parsed by other means, if any, should be converted
to QemuOpts.

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

Re: [libvirt] [PATCH 2/3] qemu: extract guest capabilities initialization

[Michal Privoznik]

On 17.03.2014 16:19, Francesco Romani wrote:

this patch decouples the binary and the capabilities detection
from the guest initialization.

The purpose is to make testing easier.
---
  src/qemu/qemu_capabilities.c | 45 +---
  src/qemu/qemu_capabilities.h |  7 +++
  2 files changed, 37 insertions(+), 15 deletions(-)


I had to rebase this, but that's not your fault.



diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 061ddae..6faef02 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -689,18 +689,12 @@ virQEMUCapsInitGuest(virCapsPtr caps,
   virArch hostarch,
   virArch guestarch)
  {
-virCapsGuestPtr guest;
  size_t i;
-bool haskvm = false;
-bool haskqemu = false;
  char *kvmbin = NULL;
  char *binary = NULL;
-virCapsGuestMachinePtr *machines = NULL;
-size_t nmachines = 0;
  virQEMUCapsPtr qemubinCaps = NULL;
  virQEMUCapsPtr kvmbinCaps = NULL;
  int ret = -1;
-bool hasdisksnapshot = false;

  /* Check for existence of base emulator, or alternate base
   * which can be used with magic cpu choice
@@ -748,6 +742,35 @@ virQEMUCapsInitGuest(virCapsPtr caps,
  }
  }

+ret = virQEMUCapsInitGuestFromBinary(caps,
+ binary, qemubinCaps,
+ kvmbin, kvmbinCaps,
+ guestarch);
+
+VIR_FREE(binary);
+VIR_FREE(kvmbin);
+virObjectUnref(qemubinCaps);
+virObjectUnref(kvmbinCaps);
+
+return ret;
+}
+
+int
+virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
+   const char *binary,
+   virQEMUCapsPtr qemubinCaps,
+   const char *kvmbin,
+   virQEMUCapsPtr kvmbinCaps,
+   virArch guestarch)
+{
+virCapsGuestPtr guest;
+bool haskvm = false;
+bool haskqemu = false;
+virCapsGuestMachinePtr *machines = NULL;
+size_t nmachines = 0;
+int ret = -1;
+bool hasdisksnapshot = false;
+
  if (!binary)
  return 0;

@@ -845,18 +868,10 @@ virQEMUCapsInitGuest(virCapsPtr caps,

  ret = 0;

-cleanup:
-VIR_FREE(binary);
-VIR_FREE(kvmbin);
-virObjectUnref(qemubinCaps);
-virObjectUnref(kvmbinCaps);
-
-return ret;
-
  error:
  virCapabilitiesFreeMachines(machines, nmachines);

-goto cleanup;
+return ret;
  }


We tend to use cleanup in this pattern. Error path should be used only 
for error not for successful return path too. Such approach, however, 
requires the substitution of 'error' with 'cleanup' in the rest of the 
function. I'll take care of that.


Michal

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

Re: [libvirt] [PATCH 0/3] v2: qemu: export disk snapshot capability

[Michal Privoznik]

On 17.03.2014 16:19, Francesco Romani wrote:


This patch series extend the QEMU capabilities XML to report
if the underlying QEMU binary supports, or not, the live
disk snapshotting.

Without this patch series, the only way to know if QEMU
has this support is to actually request a disk snapshot and
to see what happens.

The change is split in three patches:

* patch #1
actually adds the new element in the QEMU capabilities XML.
formatcaps.html.in wasn't very detailed about the actual XML format,
so I've not updated it.
Anyone feel free to point out what should be added, and I'll comply.

The new element has the form

because I'd like to convey two informations:
- disk snapshot is supposed to be here, and it is (default='on')
- disk snapshot is supposed to be here, and is NOT (default='off')

Put in a different way, I tried to help the client as much as
possible.
This patch was alread reviewed in the first version of this changeset
and it is unchanged.

* patch #2
Extracts the actual QMEU guest capability inizialitation from the binary 
probe/capabilities
code, in order to isolate the actual initialization from the probing part.
I added a new function for the real initialization and left the interface 
unchanged.
Existing code will still call the old code which do the probing and after that 
calls
the new initialization function.
The main purpose of this patch is to
* allow to write an unit test for this change
* make the unit test more robust (with respect the first version of this change)
   and make it independent from the filesystem layout/qemu availability


* patch #3
add a new unit test, aiming to test not only this new feature
but also the whole output XML capabilities.


Francesco Romani (3):
   qemu: export disk snapshot support in capabilities
   qemu: extract guest capabilities initialization
   qemu: add unit tests for the capabilities xml

  docs/schemas/capability.rng|   6 +
  src/qemu/qemu_capabilities.c   |  50 +++--
  src/qemu/qemu_capabilities.h   |   7 +
  tests/Makefile.am  |  10 +-
  tests/qemucaps2xmldata/all_1.6.0-1.caps| 142 ++
  tests/qemucaps2xmldata/all_1.6.0-1.xml |  31 
  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++
  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml  |  31 
  tests/qemucaps2xmltest.c   | 206 +
  9 files changed, 609 insertions(+), 15 deletions(-)
  create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps
  create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.xml
  create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps
  create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
  create mode 100644 tests/qemucaps2xmltest.c



ACKed and pushed.

Michal

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

Re: [libvirt] [PATCH 3/3] qemu: add unit tests for the capabilities xml

[Michal Privoznik]

On 17.03.2014 16:19, Francesco Romani wrote:

the test is loosely inspired from qemucapabilitiestest
and qemuxml2xmltest.

Added a new test instead of extending an existing one because
the feature being tested don't really fits nicely in any
existing place.
---
  tests/Makefile.am  |  10 +-
  tests/qemucaps2xmldata/all_1.6.0-1.caps| 142 ++
  tests/qemucaps2xmldata/all_1.6.0-1.xml |  31 
  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++
  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml  |  31 
  tests/qemucaps2xmltest.c   | 206 +
  6 files changed, 560 insertions(+), 1 deletion(-)
  create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps
  create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.xml
  create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps
  create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
  create mode 100644 tests/qemucaps2xmltest.c




diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c
new file mode 100644
index 000..c447af7
--- /dev/null
+++ b/tests/qemucaps2xmltest.c
@@ -0,0 +1,206 @@
+/*
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ */


I'm adding Authors line here among with your name.


+
+#include 
+
+#include "testutils.h"
+#include "qemu/qemu_capabilities.h"
+
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+
+static int
+testCompareXMLToXML(const char *inxmldata, const char *outxmldata)
+{
+int ret = 0;
+
+if (STRNEQ(outxmldata, inxmldata)) {
+virtTestDifference(stderr, outxmldata, inxmldata);
+ret = -1;
+}
+
+return ret;
+}


While this works, I'm changing it to match the pattern used in the rest 
of the code.



+
+
+typedef struct _testQemuData testQemuData;
+typedef testQemuData *testQemuDataPtr;
+struct _testQemuData {
+const char *base;
+virArch guestarch;
+};
+
+static virQEMUCapsPtr
+testQemuGetCaps(char *caps)
+{
+virQEMUCapsPtr qemuCaps = NULL;
+xmlDocPtr xml;
+xmlXPathContextPtr ctxt = NULL;
+ssize_t i, n;
+xmlNodePtr *nodes = NULL;
+
+if (!(xml = virXMLParseStringCtxt(caps, "(test caps)", &ctxt)))
+goto error;
+
+if ((n = virXPathNodeSet("/qemuCaps/flag", ctxt, &nodes)) < 0) {
+fprintf(stderr, "failed to parse qemu capabilities flags");
+goto error;
+}
+
+if (n > 0) {


There's no need for this. I mean, if there's no /qemuCaps/flag the 
virXPathNodeSet() returns 0 which may be handy if we want to test empty 
capabilities.



+if (!(qemuCaps = virQEMUCapsNew()))
+goto error;
+
+for (i = 0; i < n; i++) {
+char *str = virXMLPropString(nodes[i], "name");
+if (str) {
+int flag = virQEMUCapsTypeFromString(str);
+if (flag < 0) {
+fprintf(stderr, "Unknown qemu capabilities flag %s", str);
+VIR_FREE(str);
+goto error;
+}
+VIR_FREE(str);
+virQEMUCapsSet(qemuCaps, flag);
+}
+}
+}
+
+VIR_FREE(nodes);
+xmlFreeDoc(xml);
+xmlXPathFreeContext(ctxt);
+return qemuCaps;
+
+error:
+VIR_FREE(nodes);
+virObjectUnref(qemuCaps);
+xmlFreeDoc(xml);
+xmlXPathFreeContext(ctxt);
+return NULL;
+}


Michal

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

Re: [libvirt] [PATCH 0/3] v2: qemu: export disk snapshot capability

[Michal Privoznik]

On 17.03.2014 16:19, Francesco Romani wrote:


This patch series extend the QEMU capabilities XML to report
if the underlying QEMU binary supports, or not, the live
disk snapshotting.

Without this patch series, the only way to know if QEMU
has this support is to actually request a disk snapshot and
to see what happens.

The change is split in three patches:

* patch #1
actually adds the new element in the QEMU capabilities XML.
formatcaps.html.in wasn't very detailed about the actual XML format,
so I've not updated it.
Anyone feel free to point out what should be added, and I'll comply.

The new element has the form

because I'd like to convey two informations:
- disk snapshot is supposed to be here, and it is (default='on')
- disk snapshot is supposed to be here, and is NOT (default='off')

Put in a different way, I tried to help the client as much as
possible.
This patch was alread reviewed in the first version of this changeset
and it is unchanged.

* patch #2
Extracts the actual QMEU guest capability inizialitation from the binary 
probe/capabilities
code, in order to isolate the actual initialization from the probing part.
I added a new function for the real initialization and left the interface 
unchanged.
Existing code will still call the old code which do the probing and after that 
calls
the new initialization function.
The main purpose of this patch is to
* allow to write an unit test for this change
* make the unit test more robust (with respect the first version of this change)
   and make it independent from the filesystem layout/qemu availability


* patch #3
add a new unit test, aiming to test not only this new feature
but also the whole output XML capabilities.


Francesco Romani (3):
   qemu: export disk snapshot support in capabilities
   qemu: extract guest capabilities initialization
   qemu: add unit tests for the capabilities xml

  docs/schemas/capability.rng|   6 +
  src/qemu/qemu_capabilities.c   |  50 +++--
  src/qemu/qemu_capabilities.h   |   7 +
  tests/Makefile.am  |  10 +-
  tests/qemucaps2xmldata/all_1.6.0-1.caps| 142 ++
  tests/qemucaps2xmldata/all_1.6.0-1.xml |  31 
  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps | 141 ++
  tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml  |  31 
  tests/qemucaps2xmltest.c   | 206 +
  9 files changed, 609 insertions(+), 15 deletions(-)
  create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.caps
  create mode 100644 tests/qemucaps2xmldata/all_1.6.0-1.xml
  create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.caps
  create mode 100644 tests/qemucaps2xmldata/nodisksnapshot_1.6.0-1.xml
  create mode 100644 tests/qemucaps2xmltest.c



ACKed and pushed.

Michal

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

Re: [libvirt] [PATCH v2 2/2] bhyve: add xml2args unittest

[Ján Tomko]

On 03/23/2014 07:17 AM, Roman Bogorodskiy wrote:
> At this point unittest covers 4 basic cases:
> 
>  - minimal working XML for bhyve
>  - same as above, but with virtio disk
>  - ACPI and APIC args test
>  - MAC address test
> ---
>  src/util/virnetdevtap.c|   1 -
>  tests/Makefile.am  |  25 
>  .../bhyvexml2argvdata/bhyvexml2argv-acpiapic.args  |   3 +
>  tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml |  24 
>  tests/bhyvexml2argvdata/bhyvexml2argv-base.args|   3 +
>  tests/bhyvexml2argvdata/bhyvexml2argv-base.xml |  20 +++
>  .../bhyvexml2argv-disk-virtio.args |   3 +
>  .../bhyvexml2argv-disk-virtio.xml  |  20 +++
>  tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args |   3 +
>  tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml  |  21 +++
>  tests/bhyvexml2argvmock.c  |  49 +++
>  tests/bhyvexml2argvtest.c  | 155 
> +
>  12 files changed, 326 insertions(+), 1 deletion(-)
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-acpiapic.xml
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-base.xml
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-disk-virtio.xml
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-macaddr.xml
>  create mode 100644 tests/bhyvexml2argvmock.c
>  create mode 100644 tests/bhyvexml2argvtest.c
> 

> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 8025e77..4da1d60 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -232,6 +232,10 @@ if WITH_VMWARE
>  test_programs += vmwarevertest
>  endif WITH_VMWARE
>  
> +if WITH_BHYVE
> +test_programs += bhyvexml2argvtest
> +endif WITH_BHYVE
> +
>  if WITH_CIL
>  test_programs += objectlocking
>  endif WITH_CIL
> @@ -353,6 +357,10 @@ test_libraries += libqemumonitortestutils.la \
>   $(NULL)
>  endif WITH_QEMU
>  
> +if WITH_BHYVE
> +test_libraries += bhyvexml2argvmock.la
> +endif WITH_BHYVE
> +
>  if WITH_DBUS
>  test_libraries += virsystemdmock.la
>  endif WITH_DBUS
> @@ -602,6 +610,23 @@ else ! WITH_VMWARE
>  EXTRA_DIST += vmwarevertest.c
>  endif ! WITH_VMWARE
>  
> +if WITH_BHYVE
> +bhyvexml2argvmock_la_SOURCES = \
> + bhyvexml2argvmock.c
> +bhyvexml2argvmock_la_CFLAGS = $(AM_CFLAGS)
> +bhyvexml2argvmock_la_LDFLAGS = -module -avoid-version \
> + -rpath /evil/libtool/hack/to/force/shared/lib/creation
> +
> +bhyve_LDADDS = ../src/libvirt_driver_bhyve_impl.la
> +bhyve_LDADDS += $(LDADDS)
> +bhyvexml2argvtest_SOURCES = \
> + bhyvexml2argvtest.c \
> + testutils.c testutils.h
> +bhyvexml2argvtest_LDADD = $(bhyve_LDADDS)
> +else ! WITH_BHYVE
> +EXTRA_DIST += bhyvexml2argvtest.c
> +endif ! WITH_BHYVE

bhyvexml2argvdata should also be in EXTRA_DIST

> +
>  networkxml2xmltest_SOURCES = \
>   networkxml2xmltest.c \
>   testutils.c testutils.h

> +++ b/tests/bhyvexml2argvmock.c
> @@ -0,0 +1,49 @@
> +#include 
> +
> +#include "util/virstring.h"
> +#include "util/virnetdev.h"
> +#include "util/virnetdevtap.h"

These should work without the 'util/' too.

> +#include "internal.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_BHYVE
> +

> diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
> new file mode 100644
> index 000..a6ad033
> --- /dev/null
> +++ b/tests/bhyvexml2argvtest.c
> @@ -0,0 +1,155 @@
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "datatypes.h"
> +#include "bhyve/bhyve_utils.h"
> +#include "bhyve/bhyve_command.h"
> +
> +#include "testutils.h"

Only config.h and testutils.h are needed without the bhyve driver.

Other than that, looks good to me.

Jan



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 v2 1/2] Move virBhyveTapGetRealDeviceName to virnetdevtap

[Ján Tomko]

On 03/23/2014 07:17 AM, Roman Bogorodskiy wrote:
> To ease mocking for bhyve unit tests move virBhyveTapGetRealDeviceName()
> out of bhyve_command.c to virnetdevtap and rename it to
> virNetDevTapGetRealDeviceName().
> ---
>  src/bhyve/bhyve_command.c | 70 +
>  src/libvirt_private.syms  |  1 +
>  src/util/virnetdevtap.c   | 79 
> +++
>  src/util/virnetdevtap.h   |  3 ++
>  4 files changed, 84 insertions(+), 69 deletions(-)
> 
>  
> @@ -72,6 +79,78 @@ virNetDevTapGetName(int tapfd ATTRIBUTE_UNUSED, char 
> **ifname ATTRIBUTE_UNUSED)
>  #endif
>  }
>  
> +/**
> + * virNetDevTapGetRealDeviceName:
> + * @ifname: the interface name
> + *
> + * Lookup real interface name (i.e. name of the device entry in /dev),
> + * because e.g. on FreeBSD if we rename tap device to vnetN its device
> + * entry still remains unchanged (/dev/tapX), but bhyve needs a name
> + * that matches /dev entry.
> + *
> + * Returns the proper interface name or NULL if no corresponding interface
> + * found.
> + */
> +char*
> +virNetDevTapGetRealDeviceName(char *ifname)
> +{
> +char *ret = NULL;
> +struct dirent *dp;
> +char *devpath = NULL;
> +int fd;
> +
> +DIR *dirp = opendir("/dev");
> +if (dirp == NULL) {
> +virReportSystemError(errno,
> + _("Failed to opendir path '%s'"),
> + "/dev");
> +return NULL;
> +}
> +
> +while ((dp = readdir(dirp)) != NULL) {
> +if (STRPREFIX(dp->d_name, "tap")) {
> +struct ifreq ifr;
> +if (virAsprintf(&devpath, "/dev/%s", dp->d_name) < 0) {
> +goto cleanup;
> +}
> +if ((fd = open(devpath, O_RDWR)) < 0) {
> +virReportSystemError(errno, _("Unable to open '%s'"), 
> devpath);
> +goto cleanup;
> +}
> +
> +if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) {

This fails to build on Linux. The whole function should be #ifdef'd and return
-1 on non-FreeBSD.

> +virReportSystemError(errno, "%s",
> + _("Unable to query tap interface 
> name"));
> +goto cleanup;
> +}
> +

Jan




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] qemuDomainAttachDeviceFlags: Parse device xml as inactive

[Ján Tomko]

On 03/19/2014 04:19 PM, Michal Privoznik wrote:
> In all other drivers we are doing so. Moreover, we don't want to parse
> runtime information in attach (even if the attach is meant as live)
> because we are generating the runtime info ourselves. We can't trust
> users they supply sane values anyway.
> 
> ==1140== 9 bytes in 1 blocks are definitely lost in loss record 72 of 1,151
> ==1140==at 0x4A06C2B: malloc (in 
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==1140==by 0x623C758: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
> ==1140==by 0x50FD763: virXMLPropString (virxml.c:483)
> ==1140==by 0x510F8B7: virDomainDeviceInfoParseXML (domain_conf.c:3685)
> ==1140==by 0x511ACFD: virDomainChrDefParseXML (domain_conf.c:7535)
> ==1140==by 0x5121D13: virDomainDeviceDefParse (domain_conf.c:9918)
> ==1140==by 0x13AE6313: qemuDomainAttachDeviceFlags (qemu_driver.c:6926)
> ==1140==by 0x13AE65FA: qemuDomainAttachDevice (qemu_driver.c:7005)
> ==1140==by 0x51C77DA: virDomainAttachDevice (libvirt.c:10231)
> ==1140==by 0x127FDD: remoteDispatchDomainAttachDevice 
> (remote_dispatch.h:2404)
> ==1140==by 0x127EC5: remoteDispatchDomainAttachDeviceHelper 
> (remote_dispatch.h:2382)
> ==1140==by 0x5241F81: virNetServerProgramDispatchCall 
> (virnetserverprogram.c:437)
> 
> When doing live attach, we are passing the inactive definition anyway
> since we are passing the result of virDomainDeviceDefCopy() which does
> inactive copy by default.
> 
> Moreover, we are doing the same mistake in qemuhotplugtest.
> 
> Just a side note - it makes perfect sense to parse the runtime info
> like alias in qemuDomainDetachDevice and qemuDomainUpdateDeviceFlags()
> as in some cases the only difference to distinguish two devices can be
> just their alias.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_driver.c  | 6 +-
>  tests/qemuhotplugtest.c | 7 ++-
>  2 files changed, 7 insertions(+), 6 deletions(-)

ACK



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 2/2] query-command-line-options: query all the options in qemu-options.hx

[Amos Kong]

On Tue, Mar 11, 2014 at 06:46:10PM -0600, Eric Blake wrote:
> On 03/06/2014 11:09 PM, Amos Kong wrote:
> > vm_config_groups[] only contains part of the options which have
> > parameters, and all options which have no parameter aren't added
> > to vm_config_groups[]. Current query-command-line-options only
> > checks options from vm_config_groups[], so some options will
> > be lost.
> > 
> > We have macro in qemu-options.hx to generate a table that
> > contains all the options. This patch tries to query options
> > from the table.
> > 
> > Then we won't lose the legacy options that weren't added to
> > vm_config_groups[] (eg: -vnc, -smbios). The options that have
> > no parameter will also be returned (eg: -enable-fips)
> > 
> > Some options that have parameters have a NULL desc list, some
> > options don't have parameters, and "parameters" is mandatory
> > in the past. So we add a new field "unspecified-parameters" to
> > present if the option takes unspecified parameters.
> > 
> > This patch also fixes options to match their actual command-line
> > spelling rather than an alternate name associated with the
> > option table in use by the command.
> > 
> > Signed-off-by: Amos Kong 
> > ---
> >  qapi-schema.json   |  9 +++--
> >  qemu-options.h | 12 
> >  util/qemu-config.c | 43 ---
> >  vl.c   | 19 ++-
> >  4 files changed, 57 insertions(+), 26 deletions(-)
 
> Based on the thread on v4, it sounds like this design is still not
> finalized, and won't make 2.0.  It sounds like once we start exposing
> all options, it would also be nice to show which options are sugar for
> other spellings.

It's not good to check if one option is sugar for other spellings from
help message. Does libvirt really need this information?

As I mentioned in V4 thread, we can add a tri-state:

+# @ArgumentStateType:
+#
+# Possible types for argument state.
+#
+# @no-argument: the option takes no argument
+#
+# @argument-with-parameters: the option takes argument with
+#unspecified parameters
+#
+# @argument-without-parameters: the option takes argument without
+#   unspecified parameters
+#
+# Since 2.1
+##
+{ 'enum': 'ArgumentStateType',
+  'data': ['no-argument', 'argument-with-parameters',
+   'argument-without-parameters']
+}

I only implement the code by checking help message

  -option_name xxx,[xx=xx]\n

However, I will send a V6 later, let discuss based on it.


Thanks, Amos

> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 193e7e4..fb7ca1b 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4070,12 +4070,17 @@
> >  #
> >  # @option: option name
> >  #
> > -# @parameters: an array of @CommandLineParameterInfo
> > +# @parameters: array of @CommandLineParameterInfo, possibly empty
> > +# @unspecified-parameters: @optional present if the @parameters array is 
> > empty.
> 
> Blank lines between the two parameters, for consistency.
> 
> > +#  If true, then the option takes unspecified
> > +#  parameters, if false, then the option takes no
> > +#  parameter (since 2.0)
> 
> So this will need to be changed to '(since 2.1)'.
> 
-- 
Amos.


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

Re: [libvirt] [PATCH 2/2] Ensure JNA callbacks cannot get GCed

[Chris Ellis]

On Wed, Mar 26, 2014 at 7:24 AM, Claudio Bley  wrote:

> At Wed, 26 Mar 2014 02:43:49 +,
> Chris Ellis wrote:
> >
> > Currently nothing prevents the JNA callback objects used when registering
> > for domain events from being garbage collected.  JNA requires that
> callback
> > objects are not GCed whilst they are in use by C code.
> >
> > To solve this we hold a reference to the callback alongside the callback
> id.
> > This ensures that the JNA callback objects will retain in memory while
> it is
> > registered with the C layer.
> >
> > We also use an IdentityHashMap rather than a HashMap to store the
> EventListener
> > objects.
> > ---
> >  src/main/java/org/libvirt/Connect.java | 37
> ++
> >  1 file changed, 24 insertions(+), 13 deletions(-)
>
> Nice catch. ACK.
>

Thanks.


>
> Should we squash this into patch 41/65 before pushing the series?
>

Yes that would make sense, or you can chain my patch at the end, I'll leave
that upto you.

I've noticed that there are some other related errors, I'll push patches
for these later today.

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

[libvirt] question for in ethernet type and multiple queues in KVM (start VM failed)

[Wangrui (K)]

Hi,
  
I use ethernet vif for VM (libvirt 1.1.0  qemu 1.5.1). xml as such









tap_mq is a tap device with multi_queue property which was created on host 
by ip command, such as follows
#ip tuntap add tap_mq mode tap multi_queue

I want to use multiple queue feature for this vif so I set queues='2' in 
xml.

When start VM , an error occurs 
"error: unsupported configuration: Multiqueue network is not supported for: 
ethernet".
This error is reported from function qemuBuildInterfaceCommandLine

/* Currently nothing besides TAP devices supports multiqueue. */
if (net->driver.virtio.queues > 0 &&
!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
  actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("Multiqueue network is not supported for: %s"),
   virDomainNetTypeToString(actualType));
return -1;
}

The comment specifies that only TAP device supports multiqueue. But 
VIR_DOMAIN_NET_TYPE_ETHERNET is also TAP device I think.

IMHO, libvirt can build qemu commandline as 
"-device virtio-net,netdev=net1,mq=on,vectors=5,mac=52:54:00:19:9e:4e"
"-netdev type=tap,ifname=tap0,id=net1,vhost=on,vhostforce=on,queues=2".
Qemu can open tapfds and vhostfds by itself other than libvirt opens and 
transfers them to qemu in this case.

Or there's other way to use multiqueue feature in ethernet tpye ?

Regards,
Rui

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

Re: [libvirt] question for in ethernet type and multiple queues in KVM (start VM failed)

[Daniel P. Berrange]

On Wed, Mar 26, 2014 at 09:31:17AM +, Gonglei (Arei) wrote:
> > >
> > >  I use ethernet vif for VM (libvirt 1.1.0  qemu 1.5.1). xml as such
> > >
> > >  
> > >  
> > >  
> > >  
> > >  
> > >  
> > >  
> > >
> > >  tap_mq is a tap device with multi_queue property which was created
> > on host by ip command, such as follows
> > >  #ip tuntap add tap_mq mode tap multi_queue
> > >
> > >  I want to use multiple queue feature for this vif so I set 
> > > queues='2' in
> > xml.
> > >
> > >  When start VM , an error occurs
> > >  "error: unsupported configuration: Multiqueue network is not
> > supported for: ethernet".
> > >  This error is reported from function qemuBuildInterfaceCommandLine
> > >
> > >  /* Currently nothing besides TAP devices supports multiqueue. */
> > >  if (net->driver.virtio.queues > 0 &&
> > >  !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
> > >actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
> > >  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > > _("Multiqueue network is not supported
> > for: %s"),
> > > virDomainNetTypeToString(actualType));
> > >  return -1;
> > >  }
> > >
> > >  The comment specifies that only TAP device supports multiqueue. But
> > VIR_DOMAIN_NET_TYPE_ETHERNET is also TAP device I think.
> > 
> > Even though the device is TAP device indeed, it's not from the libvirt
> > POV. Libvirt doesn't examine the target dev to see if it is TAP,
> > physical NIC, sit, ... whatever.
> 
> Yep, but I think libvirt should provide this ability which process
> multi_queue of ethernet type. And the result is guaranteed by the
> emulator, such as Qemu.

Really you'd be better off not using type=ethernet at all. If there is
something preventing you doing that, then it would be a better use of
time to solve that rather than add more features to type=ethernet
which we don't want anyone to have to use.

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

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

Re: [libvirt] [PATCH] maint: use $(SED) instead of sed for syntax-check

[Roman Bogorodskiy]

  Roman Bogorodskiy wrote:

> Some syntax-check rules use GNU sed specific regexps, so allow
> to override which sed would be used to fix 'syntax-check' for
> non GNU-userland systems.
> ---
>  cfg.mk | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)

Any updates on that?

Roman Bogorodskiy

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

Re: [libvirt] question for in ethernet type and multiple queues in KVM (start VM failed)

[Daniel P. Berrange]

On Wed, Mar 26, 2014 at 08:59:04AM +, Wangrui (K) wrote:
> Hi,
>   
> I use ethernet vif for VM (libvirt 1.1.0  qemu 1.5.1). xml as such
> 
> 
> 
> 
> 
> 
> 
> 
> 
> tap_mq is a tap device with multi_queue property which was created on 
> host by ip command, such as follows
> #ip tuntap add tap_mq mode tap multi_queue
> 
> I want to use multiple queue feature for this vif so I set queues='2' in 
> xml.
> 
> When start VM , an error occurs 
> "error: unsupported configuration: Multiqueue network is not supported 
> for: ethernet".
> This error is reported from function qemuBuildInterfaceCommandLine
> 
> /* Currently nothing besides TAP devices supports multiqueue. */
> if (net->driver.virtio.queues > 0 &&
> !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
>   actualType == VIR_DOMAIN_NET_TYPE_BRIDGE)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>_("Multiqueue network is not supported for: %s"),
>virDomainNetTypeToString(actualType));
> return -1;
> }
> 
> The comment specifies that only TAP device supports multiqueue. But
> VIR_DOMAIN_NET_TYPE_ETHERNET is also TAP device I think.

No, libvirt can't assume that - it just knows it is something that /looks/
like a tap device but it might not be. This is just an example of one of
the many problems with type=ethernet and why we recommend people never
to use this & why its taints any domain using it.


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

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

Re: [libvirt] [PATCH] qemu: remove redundant virQEMUDriverGetConfig

[Daniel P. Berrange]

On Tue, Mar 25, 2014 at 04:37:33PM -0400, Tomoki Sekiyama wrote:
> qemuDomainSetSchedulerParametersFlags() calls virQEMUDriverGetConfig() twice
> and makes the reference counter leak. This removes redundant call.
> ---
>  src/qemu/qemu_driver.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 76f789e..ca971f3 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9050,7 +9050,6 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
>  if (virDomainSetSchedulerParametersFlagsEnsureACL(dom->conn, vm->def, 
> flags) < 0)
>  goto cleanup;
>  
> -cfg = virQEMUDriverGetConfig(driver);
>  if (!cfg->privileged) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("CPU tuning is not available in session mode"));

IMHO should delete the other call to GetConfig. We should only take the
reference once we've checked the ACL


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

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

Re: [libvirt] [libvirt-java] [PATCH] Set Java source level in pom

[Chris Ellis]

On Wed, Mar 26, 2014 at 4:32 AM, Eric Blake  wrote:

> On 03/25/2014 10:19 PM, Eric Blake wrote:
> > On 03/25/2014 09:29 PM, Chris Ellis wrote:
> >> Hi
> >>
> >> Simple patch to set the Java source level within the pom, this makes it
> >> easier to import the project into Eclipse.  I've also added an ignore
> for
> >> a local pom file.
> >>
> >> I've added plugins to publish the sources and javadocs into Maven.  This
> >> allows Maven (and IDE integration) to automatically use the javadocs and
> >> source.  This should make life easier for users of libvirt-java.
> >>
> >> Regards,
> >> Chris
> >>
> >> Chris Ellis (1):
> >>   Update pom to include Java source level
> >>
> >>  .gitignore |  1 +
> >>  pom.xml.in | 41 +
> >>  2 files changed, 42 insertions(+)
> >>
> >
> > There was no patch accompanying this diffstat.  See what the archives
> > saw:
> https://www.redhat.com/archives/libvir-list/2014-March/msg01591.html
> >
> > You might want to try sending again :)
>
> Ah, I see it now:
> https://www.redhat.com/archives/libvir-list/2014-March/msg01593.html
>
> Usually git numbers a cover letter as 0/N to make it obvious it is a
> cover; also, a cover letter isn't necessary when sending a single patch.
>  At any rate, I'll leave the actual patch review up to someone more
> familiar with Maven.
>

Sorry for the confusion, once I saw the emails arrive I realized it looked
stupid,
I shouldn't have used --cover-letter for the single patch.

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

Re: [libvirt] [PATCH 3/3] Coverity: Resolve a RESOURCE_LEAK

[Daniel P. Berrange]

On Tue, Mar 25, 2014 at 03:00:02PM -0600, Eric Blake wrote:
> On 03/25/2014 12:00 PM, John Ferlan wrote:
> > On error the lofd would have been leaked.
> > 
> > Signed-off-by: John Ferlan 
> > ---
> >  src/lxc/lxc_controller.c | 14 ++
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> 
> > +
> > + cleanup:
> >  VIR_FREE(loname);
> > +if (ret == -1)
> 
> I might have written 'if (ret < 0)' as that is slightly faster on many
> machines, but what you have works.

As a coding style point we use 'ret < 0' in the vast majority of places,
so we should prefer that over 'ret == -1'


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

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

Re: [libvirt] [libvirt-java] [PATCH] Update pom to include Java source level

[Chris Ellis]

On Wed, Mar 26, 2014 at 7:52 AM, Claudio Bley  wrote:

> At Wed, 26 Mar 2014 03:29:34 +,
> Chris Ellis wrote:
> >
> > Update the pom to set the Java source level.  This enables Eclipse
> > to correctly configure itself.
>
> So, you let ant generate the pom.xml and use that with Eclipse, right?
>

Yes, once there is a valid pom you can just import the project into Eclipse
via the Eclipse / Maven integration.

Is there any reason we are building via Ant, rather than using Maven for
the
build?


>
> > Also include plugins to publish the source and javadocs vai Maven.
>
> s/vai/via
>
> > Update .gitignore to exclude a local pom.
> > ---
> >  .gitignore |  1 +
> >  pom.xml.in | 41 +
> >  2 files changed, 42 insertions(+)
>
> > diff --git a/pom.xml.in b/pom.xml.in
> > index 4f49a3a..fd743ad 100644
> > --- a/pom.xml.in
> > +++ b/pom.xml.in
> > @@ -30,6 +30,47 @@
> >[3.4.1,4.0.0]
> >  
> >
> > +
>^
> trailing whitespaces (git warns about this)
>
> ACK, but I did not yet push it since your patch is not against
> libvirt-java master.
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list