Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/20/18 18:37, Markus Armbruster wrote:
> Laszlo Ersek  writes:
> 
>> On 04/20/18 14:53, Markus Armbruster wrote:
>>> Laszlo Ersek  writes:
>>
>> [snip]
>>
 The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386,
 lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel,
 moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4,
 sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb.

 That is, at least the following constants in CpuInfoArch have no
 corresponding (identical) mapping -- I'll state to the right of the
 arrow the emulation targets which I know or presume to be associated
 with the CpuInfoArch constant:
 - x86 -> i386, x86_64
 - sparc -> sparc, sparc64
 - ppc -> ppc, ppc64, ppcemb
 - mips -> mips, mips64, mips64el, mipsel
 - s390 -> s390x
 - riscv -> riscv32, riscv64

 The only constant that seems to have a 1:1 mapping is "tricore", but I
 could perfectly well be thinking even that just because I have no clue
 about "tricore".

 So, I don't think CpuInfoArch can be updated so that it both remains
 compatible with current QMP clients, and serves "firmware.json". In the
 firmware schema we don't just need CPU architecture, but actual emulator
 names (and board / machine types).
>>>
>>> The completely orthodox fix for CpuInfo would be:
>>>
>>> * Keep @arch unchanged.  In particular, keep "other" for all targets
>>>   other than 'x86', 'sparc', 'ppc', 'mips', 'tricore'.
>>>
>>> * Add a new member @target of new enum type Target, whose values match
>>>   configures idea of targets exactly.
>>>
>>> * Make the new member the union tag.
>>>
>>> It's too late for complete orthodoxy; we already changed @arch.
>>>
>>> A common enum type Target makes sense anyway, I think.
>>>
>>> Using it in CpuInfo as described above may make sense, too.  Could be
>>> left to a follow-up patch.
>>>
 I grepped 'qapi/*json' for the whole word "x86_64", and the only thing
 that remotely matches is the @TargetInfo structure, in which the @arch
 field is a string, coming with the example "x86_64". The example also
 names "i386" separately.
>>>
>>> Well spotted.
>>>
>>> TargetInfo member @arch is set to TARGET_NAME, which matches configure's
>>> idea of the target.  If we add enum Target, we should change @arch's
>>> type from str to Target.
>>>
 So what might make sense is to introduce a separate enum in
 "qapi/misc.json" with all the softmmu targets I listed above, and change
 the type of @TargetInfo.@arch to that enum.
>>>
>>> I arrived at this conclusion, too.
>>>
 In parallel,
 qmp_query_target() would have to be updated to look up the enum value
 matching TARGET_NAME. (I do think we can ignore linux-user etc emulators
 for collecting the relevant arches here: @TargetInfo is only used in the
 "query-target" QMP command, and Markus said above that QMP is only used
 with system emulation.)

 Should I do this?
>>>
>>> Yes, please.
>>
>> OK, so here's my understanding:
>>
>> (1) I'll introduce a new @Target enum in misc.json. I'll inherit /
>> include it in firmware.json. This is compatible with what Dan said.
>>
>> (2) I'll change @TargetInfo.@arch to the new type. I believe this will
>> break the compilation of qmp_query_target(); I'll hack on that until it
>> builds again, with the lookup. :)
>>
>> (3) Regarding the addition of @target to CpuInfo (accompanying @arch)
>> doesn't look hard; what *does* look hard is changing the union
>> discriminator from @arch to @target. @target has many more values, and I
>> would have to map all of them to the (fewer) @arch values that currently
>> do *not* select @CpuInfoOther. Here's an example:
>>
>> - Both @i386 and @x86_64 from @target mean @x86 in @arch,
>> - because @x86 currently selects @CpuInfoX86, not @CpuInfoOther,
>>   both @i386 and @x86_64 must be assigned @CpuInfoX86.
>>
>> This depends on the knowledge that "x86" actually *means* "i386 plus
>> x86_64", and I totally don't have that knowledge for the rest of the
>> arches / targets.
> 
> Start with qmp_query_cpus()'s code to initialize @arch:
> 
> #if defined(TARGET_I386)
> info->value->arch = CPU_INFO_ARCH_X86;
> info->value->u.x86.pc = env->eip + env->segs[R_CS].base;
> #elif defined(TARGET_PPC)
> info->value->arch = CPU_INFO_ARCH_PPC;
> info->value->u.ppc.nip = env->nip;
> #elif defined(TARGET_SPARC)
> info->value->arch = CPU_INFO_ARCH_SPARC;
> info->value->u.q_sparc.pc = env->pc;
> info->value->u.q_sparc.npc = env->npc;
> #elif defined(TARGET_MIPS)
> info->value->arch = CPU_INFO_ARCH_MIPS;
> info->value->u.q_mips.PC = env->active_tc.PC;
> #elif defined(TARGET_TRICORE)
> info->value->arch = CPU_INFO_ARCH_TRICORE;
> in

[libvirt] [qemu RFC v3 2/3] qapi: change the type of TargetInfo.arch from string to enum SysEmuTarget

[Laszlo Ersek]

Now that we have @SysEmuTarget, it makes sense to restict
@TargetInfo.@arch to valid sysemu targets at the schema level.

Cc: "Daniel P. Berrange" 
Cc: David Gibson 
Cc: Eric Blake 
Cc: Gerd Hoffmann 
Cc: Kashyap Chamarthy 
Cc: Markus Armbruster 
Cc: Paolo Bonzini 
Cc: Thomas Huth 
Signed-off-by: Laszlo Ersek 
---

Notes:
RFCv3:

- The patch is new in this version. [Markus]

 qapi/misc.json |  6 --
 arch_init.c| 18 +-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/qapi/misc.json b/qapi/misc.json
index 5636f4a14997..3b4fbc534089 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -5,6 +5,8 @@
 # = Miscellanea
 ##
 
+{ 'include': 'common.json' }
+
 ##
 # @qmp_capabilities:
 #
@@ -2449,12 +2451,12 @@
 #
 # Information describing the QEMU target.
 #
-# @arch: the target architecture (eg "x86_64", "i386", etc)
+# @arch: the target architecture
 #
 # Since: 1.2.0
 ##
 { 'struct': 'TargetInfo',
-  'data': { 'arch': 'str' } }
+  'data': { 'arch': 'SysEmuTarget' } }
 
 ##
 # @query-target:
diff --git a/arch_init.c b/arch_init.c
index 6ee07478bd11..4367f30291f8 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -29,6 +29,7 @@
 #include "hw/pci/pci.h"
 #include "hw/audio/soundhw.h"
 #include "qapi/qapi-commands-misc.h"
+#include "qapi/error.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "hw/acpi/acpi.h"
@@ -111,8 +112,23 @@ int xen_available(void)
 TargetInfo *qmp_query_target(Error **errp)
 {
 TargetInfo *info = g_malloc0(sizeof(*info));
+Error *local_err = NULL;
 
-info->arch = g_strdup(TARGET_NAME);
+/*
+ * The fallback enum value is irrelevant here (TARGET_NAME is a
+ * macro and can never be NULL), so simply pass zero. Also, the
+ * lookup should never fail -- if it fails, then @SysEmuTarget needs
+ * extending. Catch that with an assertion, but also handle it
+ * gracefully, in case someone adventurous disables assertions.
+ */
+info->arch = qapi_enum_parse(&SysEmuTarget_lookup, TARGET_NAME, 0,
+ &local_err);
+g_assert(!local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+qapi_free_TargetInfo(info);
+return NULL;
+}
 
 return info;
 }
-- 
2.14.1.3.gb7cf6e02401b


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

[libvirt] [qemu RFC v3 3/3] qapi: add "firmware.json"

[Laszlo Ersek]

Add a schema that describes the different uses and properties of virtual
machine firmware.

Each firmware executable installed on a host system should come with at
least one JSON file that conforms to this schema. Each file informs the
management applications about
- the firmware's properties and one possible use case / feature set,
- configuration bits that are required to run the firmware binary.

In addition, define rules for management apps for picking the highest
priority firmware JSON file when multiple such files match the search
criteria.

Cc: "Daniel P. Berrange" 
Cc: David Gibson 
Cc: Eric Blake 
Cc: Gerd Hoffmann 
Cc: Kashyap Chamarthy 
Cc: Markus Armbruster 
Cc: Paolo Bonzini 
Cc: Thomas Huth 
Signed-off-by: Laszlo Ersek 
---

Notes:
RFCv3:

- Previous version (RFCv2) was posted at:
  <20180417224054.26363-1-lersek@redhat.com">http://mid.mail-archive.com/20180417224054.26363-1-lersek@redhat.com>

- Trim the CC list a little.

- Wrap to a 72-char margin, for an easier reading. [Markus]

- Run spell-checker. [Markus]

- Rename @FirmwareType to @FirmwareOSInterface, to better express the
  concept. Update the enum constants accordingly (incl. documentation);
  in particular replace @slof with @openfirmware. Rename the @type field
  in @Firmware to @interface-type. [David, Gerd, Paolo, Thomas]

- Better yet, rename @interface-type to @interface-types (plural), and
  turn it into a list of @FirmwareOSInterface entries, to express an
  ordered compat list with multiple firmware<->OS interface types. [Dan,
  Eric, Gerd, Paolo]

- Drop @FirmwareArchitecture, and switch the type of
  @FirmwareTarget.@architecture to @SysEmuTarget (now defined in
  "common.json"). [Dan, Gerd, Markus]

- Switch elements of @FirmwareTarget.@machines from aliases ("pc",
  "q35", "virt") to concrete machine types, with glob patterns
  understood. [Dan, Gerd]

- Rename @secure-boot-enrolled-keys to @enrolled-keys, and drop the
  dependency on @secure-boot in its documentation. [Gerd]

- Describe the firmware JSON directories and the search rules to
  management apps, under the @Firmware element. Relatedly, remove the
  language on any concrete certificates under @enrolled-keys, as these
  can be customized through the overrides. Also refresh the commit
  message. [Dan, Gerd]

- Rename @pathname fields to @filename. [Markus]

- Don't replace "@executable.@filename" with "@executable.filename"
  (that is, keep the second @ sign), because removing the 2nd @ sign
  messes up the documentation ("filename" stops being type-set in
  monospace in the HTML, for example). [Markus]

- Rename @nvram_template to @nvram-template. [Thomas]

- Keep @x-check-firmware for a while longer. [Dan, Gerd, Markus, Paolo]

RFCv2:

- previous version (RFCv1) was posted at
  <20180407000117.25640-1-lersek@redhat.com">http://mid.mail-archive.com/20180407000117.25640-1-lersek@redhat.com>

- v2 is basically a rewrite from scratch, starting out with Dan's
  definitions from
  <20180410102033.GL5155@redhat.com">http://mid.mail-archive.com/20180410102033.GL5155@redhat.com> and
  <20180410110357.GP5155@redhat.com">http://mid.mail-archive.com/20180410110357.GP5155@redhat.com>,
  hopefully addressing others' feedback as well

RFCv1:

- Folks on the CC list, please try to see if the suggested schema is
  flexible enough to describe the virtual firmware(s) that you are
  familiar with. Thanks!

 Makefile  |   9 +
 Makefile.objs |   4 +
 qapi/firmware.json| 554 ++
 qapi/qapi-schema.json |   1 +
 qmp.c |   5 +
 .gitignore|   4 +
 6 files changed, 577 insertions(+)
 create mode 100644 qapi/firmware.json

diff --git a/Makefile b/Makefile
index d71dd5bea416..32034abe1583 100644
--- a/Makefile
+++ b/Makefile
@@ -97,6 +97,7 @@ GENERATED_FILES += qapi/qapi-types-block.h 
qapi/qapi-types-block.c
 GENERATED_FILES += qapi/qapi-types-char.h qapi/qapi-types-char.c
 GENERATED_FILES += qapi/qapi-types-common.h qapi/qapi-types-common.c
 GENERATED_FILES += qapi/qapi-types-crypto.h qapi/qapi-types-crypto.c
+GENERATED_FILES += qapi/qapi-types-firmware.h qapi/qapi-types-firmware.c
 GENERATED_FILES += qapi/qapi-types-introspect.h qapi/qapi-types-introspect.c
 GENERATED_FILES += qapi/qapi-types-migration.h qapi/qapi-types-migration.c
 GENERATED_FILES += qapi/qapi-types-misc.h qapi/qapi-types-misc.c
@@ -115,6 +116,7 @@ GENERATED_FILES += qapi/qapi-visit-block.h 
qapi/qapi-visit-block.c
 GENERATED_FILES += qapi/qapi-visit-char.h qapi/qapi-visit-char.c
 GENERATED_FILES += qapi/qapi-visit-common.h qapi/qapi-visit-common.c
 GENERATED_FILES += qapi/qapi-visit-crypto.h qapi/qapi-visit-crypto.c
+GENERATED_FILES += qapi/qapi-visit-firmware.h

[libvirt] [qemu RFC v3 1/3] qapi: add SysEmuTarget to "common.json"

[Laszlo Ersek]

We'll soon need an enumeration type that lists all the softmmu targets
that QEMU (the project) supports. Introduce @SysEmuTarget to
"common.json".

Cc: "Daniel P. Berrange" 
Cc: David Gibson 
Cc: Eric Blake 
Cc: Gerd Hoffmann 
Cc: Kashyap Chamarthy 
Cc: Markus Armbruster 
Cc: Paolo Bonzini 
Cc: Thomas Huth 
Signed-off-by: Laszlo Ersek 
---

Notes:
RFCv3:

- The patch is new in this version. [Dan, Markus]

- The original idea was to call the new enum @Target; however, @Target
  generates exactly the TARGET_AARCH64, TARGET_ALPHA, TARGET_ARM, ...
  enumeration constants that conflict with the poisoned preprocessing
  macros of the same names. Hence @SysEmuTarget -- it's more accurate
  anyway, since we want it to stand for system emulation targets.

- Also, we discussed defining the new type in either "common.json" or
  "misc.json". "misc.json" turned out to be a problem: "firmware.json"
  would then include "misc.json" for the new type's sake, but that
  inclusion would become the first appearance of "misc.json" -- within
  "firmware.json". That messed up the generated documentation. By adding
  the new type to "common.json", "misc.json" (see the 2nd patch) and
  "firmware.json" (see the 3rd patch) can both consume the new type
  without problems.

 qapi/common.json | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/qapi/common.json b/qapi/common.json
index d9b14dd429f3..1fd63172754a 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -126,3 +126,22 @@
 ##
 { 'enum': 'OffAutoPCIBAR',
   'data': [ 'off', 'auto', 'bar0', 'bar1', 'bar2', 'bar3', 'bar4', 'bar5' ] }
+
+##
+# @SysEmuTarget:
+#
+# The comprehensive enumeration of QEMU system emulation ("softmmu")
+# targets. Run "./configure --help" in the project root directory, and
+# look for the *-softmmu targets near the "--target-list" option. The
+# individual target constants are not documented here, for the time
+# being.
+#
+# Since: 2.13
+##
+{ 'enum' : 'SysEmuTarget',
+  'data' : [ 'aarch64', 'alpha', 'arm', 'cris', 'hppa', 'i386', 'lm32',
+ 'm68k', 'microblaze', 'microblazeel', 'mips', 'mips64',
+ 'mips64el', 'mipsel', 'moxie', 'nios2', 'or1k', 'ppc',
+ 'ppc64', 'ppcemb', 'riscv32', 'riscv64', 's390x', 'sh4',
+ 'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
+ 'x86_64', 'xtensa', 'xtensaeb' ] }
-- 
2.14.1.3.gb7cf6e02401b


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

[libvirt] [qemu RFC v3 0/3] qapi: add "firmware.json"

[Laszlo Ersek]

This version seeks to address the RFCv2 feedback. Changes are noted per
patch.

Cc: "Daniel P. Berrange" 
Cc: David Gibson 
Cc: Eric Blake 
Cc: Gerd Hoffmann 
Cc: Kashyap Chamarthy 
Cc: Markus Armbruster 
Cc: Paolo Bonzini 
Cc: Thomas Huth 

Thanks,
Laszlo

Laszlo Ersek (3):
  qapi: add SysEmuTarget to "common.json"
  qapi: change the type of TargetInfo.arch from string to enum
SysEmuTarget
  qapi: add "firmware.json"

 Makefile  |   9 +
 Makefile.objs |   4 +
 qapi/common.json  |  19 ++
 qapi/firmware.json| 554 ++
 qapi/misc.json|   6 +-
 qapi/qapi-schema.json |   1 +
 arch_init.c   |  18 +-
 qmp.c |   5 +
 .gitignore|   4 +
 9 files changed, 617 insertions(+), 3 deletions(-)
 create mode 100644 qapi/firmware.json

-- 
2.14.1.3.gb7cf6e02401b

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

Re: [libvirt] [RFC v2] external (pull) backup API

[Eric Blake]

On 04/20/2018 01:24 PM, John Snow wrote:

>>> Why is option 3 unworkable, exactly?:
>>>
>>> (3) Checkpoints exist as structures only with libvirt. They are saved
>>> and remembered in the XML entirely.
>>>
>>> Or put another way:
>>>
>>> Can you explain to me why it's important for libvirt to be able to
>>> reconstruct checkpoint information from a qcow2 file?
>>>
>>
>> In short it take extra effort for metadata to be consistent when 
>> libvirtd crashes occurs. See for more detailed explanation 
>> in [1] starting from words "Yes it is possible".
>>
>> [1] https://www.redhat.com/archives/libvir-list/2018-April/msg01001.html

I'd argue the converse. Libvirt already knows how to do atomic updates
of XML files that it tracks.  If libvirtd crashes/restarts in the middle
of an API call, you already have indeterminate results of whether the
API worked or failed; once libvirtd is restarted, you'll have to
probably retry the command.  For all other cases, the API call
completes, and either no XML changes were made (the command failed and
reports the failure properly), or all XML changes were made (the command
created the appropriate changes to track the new checkpoint, including
whatever bitmap names have to be recorded to map the relation between
checkpoints and bitmaps).

Consider the case of internal snapshots.  Already, we have the case
where qemu itself does not track enough useful metadata about internal
snapshots (right now, just a name and timestamp of creation); so libvirt
additionally tracks further information in : the name,
timestamp, relationship to any previous snapshot (libvirt can then
reconstruct a tree relationship between all snapshots; where a parent
can have more than one child if you roll back to a snapshot and then
execute the guest differently), the set of disks participating in the
snapshot, and the  description at the time of the snapshot (if
you hotplug devices, or even the fact that creating external snapshots
changes which file is the active qcow2 in a backing chain, you'll need
to know how to roll back to the prior domain state as part of
reverting).  This is approximately the same set of information that a
 will need to track.

I'm slightly tempted to just overload  to track three
modes instead of two (internal, external, and now checkpoint); but think
that will probably be a bit too confusing, so more likely I will create
 as a new object, but copy a lot of coding paradigms
from .

So, from that point of view, libvirt tracking the relationship between
qcow2 bitmaps in order to form checkpoint information can be done ALL
with libvirt, and without NEEDING the qcow2 file to track any relations
between bitmaps.  BUT, libvirt's job can probably be made easier if
qcow2 would, at the least, allow bitmaps to track their parent, and/or
provide APIs to easily merge a parent..intermediate..child chain of
related bitmaps to be merged into a single bitmap, for easy runtime
creation of the temporary bitmap used to express the delta between two
checkpoints.

> 
> OK; I can't speak to the XML design (I'll leave that to Eric and other
> libvirt engineers) but the data consistency issues make sense.

And I'm still trying to figure out exactly what is needed, to capture
everything needed to create checkpoints and take backups (both push and
pull model).  Reverting to data from an external backup may be a bit
more manual, at least at first (after all, we STILL don't have decent
libvirt support for rolling back to external snapshots, several years
later).  In other words, my focus right now is "how can we safely track
checkpoints for capturing of point-in-time incremental backups with
minimal guest downtime", rather than "given an incremental backup
captured previously, how do we roll a guest back to that point in time".

> 
> ATM I am concerned that by shifting the snapshots into bitmap names that
> you still leave yourself open for data corruption if these bitmaps are
> modified outside of libvirt -- these third party tools can't possibly
> understand the schema that they were created under.
> 
> (Though I suppose very simply that if a bitmap is missing you'd be able
> to detect that in libvirt and signal an error, but it's not very nice.)

Well, we also have to realize that third-party tools shouldn't really be
mucking around with bitmaps they don't understand.  If you are going to
manipulate a qcow2 file that contains persistent bitmaps, you should not
delete a bitmap you did not create; and if the bitmap is autoloaded, you
must obey the rules and amend the bitmap for any guest-visible changes
you make during your data edits.  Just like a third-party tool shouldn't
really be deleting internal snapshots it didn't create.  I don't think
we have to worry as much about being robust to what a third party tool
would do behind our backs (after all, the point of the pull model
backups is so that third-party tools can track the backup in the format
THEY choose, after reading the dirty bitmap and data over N

Re: [libvirt] [PATCH v2 2/2] conf: Rework/rename virDomainObjListFindByIDRef

[Jim Fehlig]

On 04/20/2018 07:08 AM, John Ferlan wrote:

Rework the code such that virDomainObjListFindByID will always
return a locked/ref counted object so that the callers can
always do the same cleanup logic to call virDomainObjEndAPI.
Makes accessing the objects much more consistent.

NB:
There were 2 callers (lxcDomainLookupByID and qemuDomainLookupByID)
that were already using the ByID name, but not virDomainObjEndAPI -
these were changed as well in this update/patch.

Signed-off-by: John Ferlan 
---
  src/bhyve/bhyve_driver.c|  2 +-
  src/conf/virdomainobjlist.c | 35 +--
  src/conf/virdomainobjlist.h |  2 --
  src/libvirt_private.syms|  1 -
  src/libxl/libxl_domain.c|  2 +-
  src/libxl/libxl_driver.c|  2 +-
  src/lxc/lxc_driver.c|  3 +--
  src/openvz/openvz_driver.c  |  2 +-
  src/qemu/qemu_driver.c  |  5 ++---
  src/test/test_driver.c  |  2 +-
  src/uml/uml_driver.c|  2 +-
  src/vmware/vmware_driver.c  |  2 +-
  src/vz/vz_driver.c  |  2 +-
  13 files changed, 20 insertions(+), 42 deletions(-)


Reviewed-by: Jim Fehlig 

Regards,
Jim



diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index bd8c6b30ed..768578a43f 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -844,7 +844,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
  virDomainObjPtr vm;
  virDomainPtr dom = NULL;
  
-vm = virDomainObjListFindByIDRef(privconn->domains, id);

+vm = virDomainObjListFindByID(privconn->domains, id);
  
  if (!vm) {

  virReportError(VIR_ERR_NO_DOMAIN,
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 02b52465d0..d57ed10a5f 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -109,44 +109,27 @@ static int virDomainObjListSearchID(const void *payload,
  return want;
  }
  
-static virDomainObjPtr

-virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
- int id,
- bool ref)
+
+virDomainObjPtr
+virDomainObjListFindByID(virDomainObjListPtr doms,
+ int id)
  {
  virDomainObjPtr obj;
+
  virObjectRWLockRead(doms);
  obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL);
-if (ref) {
-virObjectRef(obj);
-virObjectRWUnlock(doms);
-}
+virObjectRef(obj);
+virObjectRWUnlock(doms);
  if (obj) {
  virObjectLock(obj);
  if (obj->removing) {
  virObjectUnlock(obj);
-if (ref)
-virObjectUnref(obj);
+virObjectUnref(obj);
  obj = NULL;
  }
  }
-if (!ref)
-virObjectRWUnlock(doms);
-return obj;
-}
-
-virDomainObjPtr
-virDomainObjListFindByID(virDomainObjListPtr doms,
- int id)
-{
-return virDomainObjListFindByIDInternal(doms, id, false);
-}
  
-virDomainObjPtr

-virDomainObjListFindByIDRef(virDomainObjListPtr doms,
-int id)
-{
-return virDomainObjListFindByIDInternal(doms, id, true);
+return obj;
  }
  
  
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h

index 1b77a95ba9..7e2dece3aa 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -34,8 +34,6 @@ virDomainObjListPtr virDomainObjListNew(void);
  
  virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,

   int id);
-virDomainObjPtr virDomainObjListFindByIDRef(virDomainObjListPtr doms,
-int id);
  virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
 const unsigned char *uuid);
  virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f6c76d4341..d2728749fb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -952,7 +952,6 @@ virDomainObjListCollect;
  virDomainObjListConvert;
  virDomainObjListExport;
  virDomainObjListFindByID;
-virDomainObjListFindByIDRef;
  virDomainObjListFindByName;
  virDomainObjListFindByUUID;
  virDomainObjListForEach;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index e401327df4..d4859d6707 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -450,7 +450,7 @@ libxlDomainShutdownThread(void *opaque)
  
  cfg = libxlDriverConfigGet(driver);
  
-vm = virDomainObjListFindByIDRef(driver->domains, ev->domid);

+vm = virDomainObjListFindByID(driver->domains, ev->domid);
  if (!vm) {
  VIR_INFO("Received event for unknown domain ID %d", ev->domid);
  goto cleanup;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 24553c7272..55a93a489b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1082,7 +1082,7 @@ libxlDomainLookupByID(virConnectPtr conn, in

Re: [libvirt] [PATCH v2 1/2] conf: Rework/rename virDomainObjListFindByUUIDRef

[Jim Fehlig]

On 04/20/2018 07:08 AM, John Ferlan wrote:

Now that every caller is using virDomainObjListFindByUUIDRef,
let's just remove it and keep the name as virDomainObjListFindByUUID.

Signed-off-by: John Ferlan 
---
  src/bhyve/bhyve_driver.c |  4 ++--
  src/conf/virdomainobjlist.c  | 35 +++
  src/conf/virdomainobjlist.h  |  2 --
  src/libvirt_private.syms |  1 -
  src/libxl/libxl_driver.c |  4 ++--
  src/lxc/lxc_driver.c |  4 ++--
  src/openvz/openvz_driver.c   |  2 +-
  src/qemu/qemu_driver.c   |  4 ++--
  src/test/test_driver.c   |  4 ++--
  src/uml/uml_driver.c |  4 ++--
  src/util/virclosecallbacks.c |  4 ++--
  src/vmware/vmware_driver.c   |  2 +-
  src/vz/vz_driver.c   |  4 ++--
  src/vz/vz_sdk.c  | 14 +++---
  src/vz/vz_utils.c|  2 +-
  15 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 24c4a9c80e..bd8c6b30ed 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -168,7 +168,7 @@ bhyveDomObjFromDomain(virDomainPtr domain)
  bhyveConnPtr privconn = domain->conn->privateData;
  char uuidstr[VIR_UUID_STRING_BUFLEN];
  
-vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid);

+vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
  if (!vm) {
  virUUIDFormat(domain->uuid, uuidstr);
  virReportError(VIR_ERR_NO_DOMAIN,
@@ -791,7 +791,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
  virDomainObjPtr vm;
  virDomainPtr dom = NULL;
  
-vm = virDomainObjListFindByUUIDRef(privconn->domains, uuid);

+vm = virDomainObjListFindByUUID(privconn->domains, uuid);
  
  if (!vm) {

  char uuidstr[VIR_UUID_STRING_BUFLEN];
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index c09e5e5d2f..02b52465d0 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -149,10 +149,10 @@ virDomainObjListFindByIDRef(virDomainObjListPtr doms,
  return virDomainObjListFindByIDInternal(doms, id, true);
  }
  
-static virDomainObjPtr

-virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
-   const unsigned char *uuid,
-   bool ref)
+
+virDomainObjPtr
+virDomainObjListFindByUUID(virDomainObjListPtr doms,
+   const unsigned char *uuid)
  {
  char uuidstr[VIR_UUID_STRING_BUFLEN];
  virDomainObjPtr obj;
@@ -161,41 +161,20 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr 
doms,
  virUUIDFormat(uuid, uuidstr);
  
  obj = virHashLookup(doms->objs, uuidstr);

-if (ref) {
-virObjectRef(obj);
-virObjectRWUnlock(doms);
-}
+virObjectRef(obj);
+virObjectRWUnlock(doms);
  if (obj) {
  virObjectLock(obj);
  if (obj->removing) {
  virObjectUnlock(obj);
-if (ref)
-virObjectUnref(obj);
+virObjectUnref(obj);
  obj = NULL;
  }
  }
-if (!ref)
-virObjectRWUnlock(doms);
  return obj;
  }
  
  
-virDomainObjPtr

-virDomainObjListFindByUUID(virDomainObjListPtr doms,
-   const unsigned char *uuid)
-{
-return virDomainObjListFindByUUIDInternal(doms, uuid, false);
-}
-
-
-virDomainObjPtr
-virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
-  const unsigned char *uuid)
-{
-return virDomainObjListFindByUUIDInternal(doms, uuid, true);
-}
-
-
  virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
 const char *name)
  {
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
index bb186bde30..1b77a95ba9 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -38,8 +38,6 @@ virDomainObjPtr 
virDomainObjListFindByIDRef(virDomainObjListPtr doms,
  int id);
  virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
 const unsigned char *uuid);
-virDomainObjPtr virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
-  const unsigned char *uuid);


There's one more mention of this function in the comments for 
virDomainObjEndAPI() in domain_conf.c. Otherwise


Reviewed-by: Jim Fehlig 

Regards,
Jim


  virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
 const char *name);
  
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms

index b31f599bd2..f6c76d4341 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -955,7 +955,6 @@ virDomainObjListFindByID;
  virDomainObjListFindByIDRef;
  virDomainObjListFindByName;
  virDomainObjListFindByUUID;
-virDomainObjListFindByUUIDRef;
  virDomainObjListForEach;
  v

Re: [libvirt] [PATCH] git: add config file telling git-publish how to send patches

[Peter Krempa]

On Fri, Apr 20, 2018 at 11:36:03 +0100, Daniel Berrange wrote:
> On Fri, Apr 20, 2018 at 12:22:35PM +0200, Martin Kletzander wrote:
> > On Thu, Apr 19, 2018 at 05:29:22PM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Apr 19, 2018 at 06:11:20PM +0200, Martin Kletzander wrote:
> > > > On Thu, Apr 19, 2018 at 10:06:45AM +0100, Daniel P. Berrangé wrote:
> > > > > On Thu, Apr 19, 2018 at 10:55:01AM +0200, Peter Krempa wrote:
> > > > > > On Wed, Apr 18, 2018 at 18:35:18 +0100, Daniel Berrange wrote:

[...]

> > > > > > > diff --git a/.gitpublish b/.gitpublish
> > > > > > > new file mode 100644
> > > > > > > index 00..857f0d552c
> > > > > > > --- /dev/null
> > > > > > > +++ b/.gitpublish
> > > > > > > @@ -0,0 +1,3 @@
> > > > > > > +[gitpublishprofile "default"]
> > > > > > > +base = master
> > > > > > > +to = libvir-list@redhat.com
> > > > > >
> > > > > > ACK
> > > > > >
> > > > > > As a side-question. Does git-publish have the option to send 
> > > > > > GPG-signed
> > > > > > mails? I always wanted that, but not enough to hack it into
> > > > > > git-send-email.

[...]

> > Sure, I never meant the commits should stay signed.  It doesn't really
> > make much sense in most scenarios to have individual commits signed.
> > Signed tag is an inherent sign for all parent commits, which is more
> > than enough.  What I wanted to have established was a way for the people
> > receiving the commit to be able to see that it really came from me and
> > it was not tampered with.  For that case I only care about the medium
> > (through which the commit is sent) to be covered.  That's the only part
> > of the chain where I'm missing some effort to enhance integrity and
> > accountability.
> 
> Oh so you don't really need git commit signing for that then. Essentially
> you can just git-format the patches and do normal "email" signing when
> sending them, so the receipient verifies the email, rather than the commit
> patch. That sounds like something you could integrate into git-publish.

Actually this is exactly what I'm looking for. I don't really care
whether the commits are signed in GIT, but I care whether the emails are
sent signed. It should be easy to integrate if git is able to 'am' the
signed emailed patches correctly, which I did not try yet.


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

Re: [libvirt] [RFC PATCH 22/30] tests: qemublock: Add tests for all other format without special options

[Peter Krempa]

On Fri, Apr 20, 2018 at 13:55:35 +0200, Kevin Wolf wrote:
> Am 19.04.2018 um 17:25 hat Peter Krempa geschrieben:
> > Similarly to the 'raw' case add tests for bochs, cloop, dmg, ploop, vdi
> > vhd, and vpc. Covering all supproted non-backing formats.
> > 
> > Note that the JSON name for 'ploop' maps to 'parallels' and 'vhd' maps
> > to 'vhdx'.
> 
> Your -drive lines below mention format=ploop/vhd, though. That wouldn't
> actually work.

So, is it something that we should actually forbid? I did not actually
try all of the weird formats, so I just assumed that if we'd happily
generate the -drive command line and the 'blockdev' version exists it's
equivalent.

> (Also 'vhd' as an alias for 'vhdx' is super confusing, because VHD is
> really the name of the format implemented by QEMU's 'vpc' driver - which
> is already a source of confusion on its own.)

Hmmm, maybe that is a bug in my implementation. Since libvirt has a VPC
format and also a VHD format I thohght that VHD is just another name for
'vhxd'. If they are different, we maybe should forbid it altogether. I
confess that I did a simple prefix match rather than any complex
analysis.


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

Re: [libvirt] [RFC v2] external (pull) backup API

[John Snow]

On 04/20/2018 08:22 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 19.04.2018 20:28, John Snow wrote:
>>
>>
>> On 04/16/2018 06:20 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>
>>> So my point is: if we are going to implement something complicated,
>>> let's implement entirely what we want, not a semi-solution. Otherwise,
>>> implement a minimal and simple thing, to just make it all work (my
>>> current solution).
>>
>> So basically:
>>
>> (1) Using bitmap names: It's a hack, but it works; and
>> (2) Adding parentage information to QEMU bitmaps is also a hack, but a
>> more permanent commitment to the hack.
>>
>> And further, both (1) and (2) leave the same problem that if a third
>> party utility deletes the bitmap, they are checkpoint-unaware and will
>> ruin the metadata.
>>
>> (Though QEMU could be taught to disallow the deleting of bitmaps with
>> parents/children, unless you specify --force or --mergeleft or
>> --mergeright or some such. That's not an option with the
>> name-as-metadata strategy.)
>>
>> Why is option 3 unworkable, exactly?:
>>
>> (3) Checkpoints exist as structures only with libvirt. They are saved
>> and remembered in the XML entirely.
>>
>> Or put another way:
>>
>> Can you explain to me why it's important for libvirt to be able to
>> reconstruct checkpoint information from a qcow2 file?
>>
> 
> In short it take extra effort for metadata to be consistent when 
> libvirtd crashes occurs. See for more detailed explanation 
> in [1] starting from words "Yes it is possible".
> 
> [1] https://www.redhat.com/archives/libvir-list/2018-April/msg01001.html
> 
> Nikolay
> 

OK; I can't speak to the XML design (I'll leave that to Eric and other
libvirt engineers) but the data consistency issues make sense.

ATM I am concerned that by shifting the snapshots into bitmap names that
you still leave yourself open for data corruption if these bitmaps are
modified outside of libvirt -- these third party tools can't possibly
understand the schema that they were created under.

(Though I suppose very simply that if a bitmap is missing you'd be able
to detect that in libvirt and signal an error, but it's not very nice.)

I'll pick up discussion with Eric and Vladimir in the other portion of
this thread where we're discussing a checkpoints API and we'll pick this
up on QEMU list if need be.

Thank you,
--John

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

[libvirt] [PATCH 1/4] Revert "util: virlog: Introduce wildcard to log filters"

[Daniel P . Berrangé]

This reverts commit 8daa593b0741452a78daea76075254ae7d3c6ab6.

There are two undesirable aspects to the impl

  - Only a bare wildcard is permitted
  - The wildcard match is not performed in the order listed
---
 src/remote/libvirtd.conf |  4 +---
 src/util/virlog.c| 44 +---
 src/util/virlog.h|  1 -
 3 files changed, 2 insertions(+), 47 deletions(-)

diff --git a/src/remote/libvirtd.conf b/src/remote/libvirtd.conf
index 7d823cf1ad..9c0080dc06 100644
--- a/src/remote/libvirtd.conf
+++ b/src/remote/libvirtd.conf
@@ -368,9 +368,7 @@
 # Multiple filters can be defined in a single @filters, they just need to be
 # separated by spaces. Note that libvirt performs "first" match, i.e. if
 # there are concurrent filters, the first one that matches will be applied,
-# given the order in log_filters with the exception of a wildcard filter, since
-# that's only taken into account if no other filter has matched, so
-# "4:* 1:util.pci" and "1:util.pci 4:*" are equivalent definitions.
+# given the order in log_filters.
 #
 # e.g. to only get warning or errors from the remote layer and only errors
 # from the event layer:
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 81a9dc4395..5810643e1b 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -497,38 +497,6 @@ virLogHostnameString(char **rawmsg,
 return 0;
 }
 
-/* virLogFiltersFind:
- * @filters: haystack
- * @nfilters: haystack size
- * @key1: primary string 'needle'
- * @key2: secondary integer 'needle'
- *
- * Performs "first match" search on the input set of filters, using @key1
- * (string) and  @key2 (integer) as primary and secondary keys respectively.
- * Secondary key is only considered if primary key wasn't provided.
- *
- * Returns a pointer to the matched object or NULL if no match was found.
- */
-static virLogFilterPtr
-virLogFiltersFind(virLogFilterPtr *filters,
-  size_t nfilters,
-  const char *key1,
-  unsigned int key2)
-{
-size_t i;
-
-if (!key1 && key2 == 0)
-return NULL;
-
-for (i = 0; i < nfilters; i++) {
-if ((key1 && STREQ(key1, filters[i]->match)) ||
-filters[i]->flags == key2)
-return filters[i];
-}
-
-return NULL;
-}
-
 
 static void
 virLogSourceUpdate(virLogSourcePtr source)
@@ -1441,8 +1409,7 @@ virLogFilterNew(const char *match,
 virLogFilterPtr ret = NULL;
 char *mdup = NULL;
 
-virCheckFlags(VIR_LOG_STACK_TRACE |
-  VIR_LOG_WILDCARD, NULL);
+virCheckFlags(VIR_LOG_STACK_TRACE, NULL);
 
 if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) {
 virReportError(VIR_ERR_INVALID_ARG, _("Invalid log priority %d"),
@@ -1561,8 +1528,6 @@ virLogDefineOutputs(virLogOutputPtr *outputs, size_t 
noutputs)
 int
 virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters)
 {
-virLogFilterPtr rc;
-
 if (virLogInitialize() < 0)
 return -1;
 
@@ -1570,10 +1535,6 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t 
nfilters)
 virLogResetFilters();
 virLogFilters = filters;
 virLogNbFilters = nfilters;
-
-/* if there's a wildcard filter, update default priority */
-if ((rc = virLogFiltersFind(filters, nfilters, NULL, VIR_LOG_WILDCARD)))
-virLogDefaultPriority = rc->priority;
 virLogUnlock();
 
 return 0;
@@ -1749,9 +1710,6 @@ virLogParseFilter(const char *src)
 match++;
 }
 
-if (STREQ(match, "*"))
-flags |= VIR_LOG_WILDCARD;
-
 /* match string cannot comprise just from a single '+' */
 if (!*match) {
 virReportError(VIR_ERR_INVALID_ARG,
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 07b91f0b0a..8973f83b20 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -174,7 +174,6 @@ typedef void (*virLogCloseFunc) (void *data);
 
 typedef enum {
 VIR_LOG_STACK_TRACE = (1 << 0),
-VIR_LOG_WILDCARD = (1 << 1)
 } virLogFlags;
 
 int virLogGetNbFilters(void);
-- 
2.14.3

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

[libvirt] [PATCH 2/4] log: rename virLogFlags to virLogFilterFlags to match docs

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virlog.c | 2 +-
 src/util/virlog.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 5810643e1b..5262d613f6 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -87,7 +87,7 @@ VIR_ENUM_IMPL(virLogDestination, VIR_LOG_TO_OUTPUT_LAST,
 struct _virLogFilter {
 char *match;
 virLogPriority priority;
-unsigned int flags;
+unsigned int flags; /* bitwise OR of virLogFilterFlags */
 };
 
 static int virLogFiltersSerial = 1;
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 8973f83b20..35dba16cfe 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -174,7 +174,7 @@ typedef void (*virLogCloseFunc) (void *data);
 
 typedef enum {
 VIR_LOG_STACK_TRACE = (1 << 0),
-} virLogFlags;
+} virLogFilterFlags;
 
 int virLogGetNbFilters(void);
 int virLogGetNbOutputs(void);
-- 
2.14.3

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

[libvirt] [PATCH 3/4] log: support logging using shell wildcard syntax

[Daniel P . Berrangé]

Rather than specialcasing handling of the '*' character, use fnmatch()
to get normal shell wildcard syntax, as described in 'man glob(7)'.

Signed-off-by: Daniel P. Berrangé 
---
 src/util/virlog.c | 17 +++--
 src/util/virlog.h |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 5262d613f6..1db10fcc71 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -40,6 +40,7 @@
 #if HAVE_SYS_UN_H
 # include 
 #endif
+#include 
 
 #include "virerror.h"
 #include "virlog.h"
@@ -508,7 +509,9 @@ virLogSourceUpdate(virLogSourcePtr source)
 size_t i;
 
 for (i = 0; i < virLogNbFilters; i++) {
-if (strstr(source->name, virLogFilters[i]->match)) {
+if ((virLogFilters[i]->flags & VIR_LOG_GLOB) ?
+(fnmatch(virLogFilters[i]->match, source->name, 0) == 0) :
+(strstr(source->name, virLogFilters[i]->match) != NULL)) {
 priority = virLogFilters[i]->priority;
 flags = virLogFilters[i]->flags;
 break;
@@ -1409,7 +1412,7 @@ virLogFilterNew(const char *match,
 virLogFilterPtr ret = NULL;
 char *mdup = NULL;
 
-virCheckFlags(VIR_LOG_STACK_TRACE, NULL);
+virCheckFlags(VIR_LOG_STACK_TRACE | VIR_LOG_GLOB, NULL);
 
 if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) {
 virReportError(VIR_ERR_INVALID_ARG, _("Invalid log priority %d"),
@@ -1718,6 +1721,16 @@ virLogParseFilter(const char *src)
 goto cleanup;
 }
 
+/* Only turn on fnmatch usage if we see special glob
+ * characters, so we use more efficient strstr()
+ * by default
+ */
+if (strchr(match, '*') ||
+strchr(match, '?') ||
+strchr(match, '[')) {
+flags |= VIR_LOG_GLOB;
+}
+
 if (!(ret = virLogFilterNew(match, prio, flags)))
 goto cleanup;
 
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 35dba16cfe..95c405bac0 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -174,6 +174,7 @@ typedef void (*virLogCloseFunc) (void *data);
 
 typedef enum {
 VIR_LOG_STACK_TRACE = (1 << 0),
+VIR_LOG_GLOB = (1 << 1),
 } virLogFilterFlags;
 
 int virLogGetNbFilters(void);
-- 
2.14.3

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

[libvirt] [PATCH 4/4] log: update docs for daemons to improve user understanding

[Daniel P . Berrangé]

Strongly recommend against use of the log_levels setting since it
creates overly verbose logs and has a serious performance impact.

Describe the log filter syntax better and mention use of shell
glob syntax. Also provide more realistic example of good settings
to use. The libvirtd example is biased towards QEMU, but when the
drivers split off each daemon can get its own more appropriate
example.

Signed-off-by: Daniel P. Berrangé 
---
 src/locking/test_virtlockd.aug.in |  2 +-
 src/locking/virtlockd.conf| 70 +++--
 src/logging/test_virtlogd.aug.in  |  2 +-
 src/logging/virtlogd.conf | 73 ---
 src/remote/libvirtd.conf  | 70 +++--
 src/remote/test_libvirtd.aug.in   |  2 +-
 6 files changed, 150 insertions(+), 69 deletions(-)

diff --git a/src/locking/test_virtlockd.aug.in 
b/src/locking/test_virtlockd.aug.in
index ad75286be6..f2f6979ef5 100644
--- a/src/locking/test_virtlockd.aug.in
+++ b/src/locking/test_virtlockd.aug.in
@@ -3,7 +3,7 @@ module Test_virtlockd =
 
test Virtlockd.lns get conf =
 { "log_level" = "3" }
-{ "log_filters" = "3:remote 4:event" }
+{ "log_filters" = "1:locking 4:object 4:json 4:event 1:util" }
 { "log_outputs" = "3:syslog:virtlockd" }
 { "max_clients" = "1024" }
 { "admin_max_clients" = "5" }
diff --git a/src/locking/virtlockd.conf b/src/locking/virtlockd.conf
index 1a2b27d0b9..29cefbd74a 100644
--- a/src/locking/virtlockd.conf
+++ b/src/locking/virtlockd.conf
@@ -8,47 +8,79 @@
 
 # Logging level: 4 errors, 3 warnings, 2 information, 1 debug
 # basically 1 will log everything possible
+#
+# WARNING: USE OF THIS IS STRONGLY DISCOURAGED.
+#
+# WARNING: It outputs too much information to practically read.
+# WARNING: The "log_filters" setting is recommended instead.
+#
+# WARNING: Journald may employ rate limiting of the messages logged
+# WARNING: and thus lock up the libvirt daemon. To use the debug
+# WARNING: level with journald you have to specify it explicitly in
+# WARNING: 'log_outputs', otherwise only information level messages
+# WARNING: will be logged.
+#
+# WARNING: USE OF THIS IS STRONGLY DISCOURAGED.
 #log_level = 3
 
 # Logging filters:
 # A filter allows to select a different logging level for a given category
-# of logs
-# The format for a filter is one of:
-#x:name
-#x:+name
-#  where name is a string which is matched against source file name,
-#  e.g., "remote", "qemu", or "util/json", the optional "+" prefix
-#  tells libvirt to log stack trace for each message matching name,
-#  and x is the minimal level where matching messages should be logged:
+# of logs. The format for a filter is one of:
+#
+#level:match
+#level:+match
+#
+# where 'match' is a string which is matched against the category
+# given in the VIR_LOG_INIT() at the top of each libvirt source
+# file, e.g., "remote", "qemu", or "util.json". The 'match' in the
+# filter matches using shell wildcard syntax (see 'man glob(7)').
+# The 'match' is always treated a substring match. IOW a match
+# string 'foo' is equivalent to '*foo*'.
+#
+# If 'match' contains the optional "+" prefix, it tells libvirt
+# to log stack trace for each message matching name.
+#
+# 'level' is the minimal level where matching messages should
+#  be logged:
+#
 #1: DEBUG
 #2: INFO
 #3: WARNING
 #4: ERROR
 #
-# Multiple filter can be defined in a single @filters, they just need to be
-# separated by spaces.
+# Multiple filters can be defined in a single @filters, they just need to be
+# separated by spaces. Note that libvirt performs "first" match, i.e. if
+# there are concurrent filters, the first one that matches will be applied,
+# given the order in log_filters.
+#
+# For the virtlockd daemon, a typical need is to capture information
+# from the locking code and some of the utility code. Some utility
+# code is very verbose and is generally not desired. Taking the QEMU
+# hypervisor as an example, a suitable filter string for debugging
+# might be to turn off object, json & event logging, but enable the
+# rest of the util code:
 #
-# e.g. to only get warning or errors from the remote layer and only errors
-# from the event layer:
-#log_filters="3:remote 4:event"
+#log_filters="1:locking 4:object 4:json 4:event 1:util"
 
 # Logging outputs:
 # An output is one of the places to save logging information
 # The format for an output can be:
-#x:stderr
+#level:stderr
 #  output goes to stderr
-#x:syslog:name
+#level:syslog:name
 #  use syslog for the output and use the given name as the ident
-#x:file:file_path
+#level:file:file_path
 #  output to a file, with the given filepath
-# In all case the x prefix is the minimal level, acting as a filter
+#level:journald
+#  output to journald logging system
+# In all cases 'level' is the minimal priority, acting as a filter
 #

[libvirt] [PATCH 0/4] Reimplement logging wildcards to use full shell glob syntax

[Daniel P . Berrangé]

This series provides a more generalized support for wildcards
in logging and removes the ordering constraint.

Daniel P. Berrangé (4):
  Revert "util: virlog: Introduce wildcard to log filters"
  log: rename virLogFlags to virLogFilterFlags to match docs
  log: support logging using shell wildcard syntax
  log: update docs for daemons to improve user understanding

 src/locking/test_virtlockd.aug.in |  2 +-
 src/locking/virtlockd.conf| 70 ++--
 src/logging/test_virtlogd.aug.in  |  2 +-
 src/logging/virtlogd.conf | 73 +++---
 src/remote/libvirtd.conf  | 74 ---
 src/remote/test_libvirtd.aug.in   |  2 +-
 src/util/virlog.c | 61 +---
 src/util/virlog.h |  4 +--
 8 files changed, 169 insertions(+), 119 deletions(-)

-- 
2.14.3

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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Markus Armbruster]

Laszlo Ersek  writes:

> On 04/20/18 14:53, Markus Armbruster wrote:
>> Laszlo Ersek  writes:
>
> [snip]
>
>>> The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386,
>>> lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel,
>>> moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4,
>>> sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb.
>>>
>>> That is, at least the following constants in CpuInfoArch have no
>>> corresponding (identical) mapping -- I'll state to the right of the
>>> arrow the emulation targets which I know or presume to be associated
>>> with the CpuInfoArch constant:
>>> - x86 -> i386, x86_64
>>> - sparc -> sparc, sparc64
>>> - ppc -> ppc, ppc64, ppcemb
>>> - mips -> mips, mips64, mips64el, mipsel
>>> - s390 -> s390x
>>> - riscv -> riscv32, riscv64
>>>
>>> The only constant that seems to have a 1:1 mapping is "tricore", but I
>>> could perfectly well be thinking even that just because I have no clue
>>> about "tricore".
>>>
>>> So, I don't think CpuInfoArch can be updated so that it both remains
>>> compatible with current QMP clients, and serves "firmware.json". In the
>>> firmware schema we don't just need CPU architecture, but actual emulator
>>> names (and board / machine types).
>> 
>> The completely orthodox fix for CpuInfo would be:
>> 
>> * Keep @arch unchanged.  In particular, keep "other" for all targets
>>   other than 'x86', 'sparc', 'ppc', 'mips', 'tricore'.
>> 
>> * Add a new member @target of new enum type Target, whose values match
>>   configures idea of targets exactly.
>> 
>> * Make the new member the union tag.
>> 
>> It's too late for complete orthodoxy; we already changed @arch.
>> 
>> A common enum type Target makes sense anyway, I think.
>> 
>> Using it in CpuInfo as described above may make sense, too.  Could be
>> left to a follow-up patch.
>> 
>>> I grepped 'qapi/*json' for the whole word "x86_64", and the only thing
>>> that remotely matches is the @TargetInfo structure, in which the @arch
>>> field is a string, coming with the example "x86_64". The example also
>>> names "i386" separately.
>> 
>> Well spotted.
>> 
>> TargetInfo member @arch is set to TARGET_NAME, which matches configure's
>> idea of the target.  If we add enum Target, we should change @arch's
>> type from str to Target.
>> 
>>> So what might make sense is to introduce a separate enum in
>>> "qapi/misc.json" with all the softmmu targets I listed above, and change
>>> the type of @TargetInfo.@arch to that enum.
>> 
>> I arrived at this conclusion, too.
>> 
>>> In parallel,
>>> qmp_query_target() would have to be updated to look up the enum value
>>> matching TARGET_NAME. (I do think we can ignore linux-user etc emulators
>>> for collecting the relevant arches here: @TargetInfo is only used in the
>>> "query-target" QMP command, and Markus said above that QMP is only used
>>> with system emulation.)
>>>
>>> Should I do this?
>> 
>> Yes, please.
>
> OK, so here's my understanding:
>
> (1) I'll introduce a new @Target enum in misc.json. I'll inherit /
> include it in firmware.json. This is compatible with what Dan said.
>
> (2) I'll change @TargetInfo.@arch to the new type. I believe this will
> break the compilation of qmp_query_target(); I'll hack on that until it
> builds again, with the lookup. :)
>
> (3) Regarding the addition of @target to CpuInfo (accompanying @arch)
> doesn't look hard; what *does* look hard is changing the union
> discriminator from @arch to @target. @target has many more values, and I
> would have to map all of them to the (fewer) @arch values that currently
> do *not* select @CpuInfoOther. Here's an example:
>
> - Both @i386 and @x86_64 from @target mean @x86 in @arch,
> - because @x86 currently selects @CpuInfoX86, not @CpuInfoOther,
>   both @i386 and @x86_64 must be assigned @CpuInfoX86.
>
> This depends on the knowledge that "x86" actually *means* "i386 plus
> x86_64", and I totally don't have that knowledge for the rest of the
> arches / targets.

Start with qmp_query_cpus()'s code to initialize @arch:

#if defined(TARGET_I386)
info->value->arch = CPU_INFO_ARCH_X86;
info->value->u.x86.pc = env->eip + env->segs[R_CS].base;
#elif defined(TARGET_PPC)
info->value->arch = CPU_INFO_ARCH_PPC;
info->value->u.ppc.nip = env->nip;
#elif defined(TARGET_SPARC)
info->value->arch = CPU_INFO_ARCH_SPARC;
info->value->u.q_sparc.pc = env->pc;
info->value->u.q_sparc.npc = env->npc;
#elif defined(TARGET_MIPS)
info->value->arch = CPU_INFO_ARCH_MIPS;
info->value->u.q_mips.PC = env->active_tc.PC;
#elif defined(TARGET_TRICORE)
info->value->arch = CPU_INFO_ARCH_TRICORE;
info->value->u.tricore.PC = env->PC;
#elif defined(TARGET_S390X)
info->value->arch = CPU_INFO_ARCH_S390;
info->value->u.s390.cpu_state = env->c

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/20/18 14:53, Markus Armbruster wrote:
> Laszlo Ersek  writes:

[snip]

>> The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386,
>> lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel,
>> moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4,
>> sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb.
>>
>> That is, at least the following constants in CpuInfoArch have no
>> corresponding (identical) mapping -- I'll state to the right of the
>> arrow the emulation targets which I know or presume to be associated
>> with the CpuInfoArch constant:
>> - x86 -> i386, x86_64
>> - sparc -> sparc, sparc64
>> - ppc -> ppc, ppc64, ppcemb
>> - mips -> mips, mips64, mips64el, mipsel
>> - s390 -> s390x
>> - riscv -> riscv32, riscv64
>>
>> The only constant that seems to have a 1:1 mapping is "tricore", but I
>> could perfectly well be thinking even that just because I have no clue
>> about "tricore".
>>
>> So, I don't think CpuInfoArch can be updated so that it both remains
>> compatible with current QMP clients, and serves "firmware.json". In the
>> firmware schema we don't just need CPU architecture, but actual emulator
>> names (and board / machine types).
> 
> The completely orthodox fix for CpuInfo would be:
> 
> * Keep @arch unchanged.  In particular, keep "other" for all targets
>   other than 'x86', 'sparc', 'ppc', 'mips', 'tricore'.
> 
> * Add a new member @target of new enum type Target, whose values match
>   configures idea of targets exactly.
> 
> * Make the new member the union tag.
> 
> It's too late for complete orthodoxy; we already changed @arch.
> 
> A common enum type Target makes sense anyway, I think.
> 
> Using it in CpuInfo as described above may make sense, too.  Could be
> left to a follow-up patch.
> 
>> I grepped 'qapi/*json' for the whole word "x86_64", and the only thing
>> that remotely matches is the @TargetInfo structure, in which the @arch
>> field is a string, coming with the example "x86_64". The example also
>> names "i386" separately.
> 
> Well spotted.
> 
> TargetInfo member @arch is set to TARGET_NAME, which matches configure's
> idea of the target.  If we add enum Target, we should change @arch's
> type from str to Target.
> 
>> So what might make sense is to introduce a separate enum in
>> "qapi/misc.json" with all the softmmu targets I listed above, and change
>> the type of @TargetInfo.@arch to that enum.
> 
> I arrived at this conclusion, too.
> 
>> In parallel,
>> qmp_query_target() would have to be updated to look up the enum value
>> matching TARGET_NAME. (I do think we can ignore linux-user etc emulators
>> for collecting the relevant arches here: @TargetInfo is only used in the
>> "query-target" QMP command, and Markus said above that QMP is only used
>> with system emulation.)
>>
>> Should I do this?
> 
> Yes, please.

OK, so here's my understanding:

(1) I'll introduce a new @Target enum in misc.json. I'll inherit /
include it in firmware.json. This is compatible with what Dan said.

(2) I'll change @TargetInfo.@arch to the new type. I believe this will
break the compilation of qmp_query_target(); I'll hack on that until it
builds again, with the lookup. :)

(3) Regarding the addition of @target to CpuInfo (accompanying @arch)
doesn't look hard; what *does* look hard is changing the union
discriminator from @arch to @target. @target has many more values, and I
would have to map all of them to the (fewer) @arch values that currently
do *not* select @CpuInfoOther. Here's an example:

- Both @i386 and @x86_64 from @target mean @x86 in @arch,
- because @x86 currently selects @CpuInfoX86, not @CpuInfoOther,
  both @i386 and @x86_64 must be assigned @CpuInfoX86.

This depends on the knowledge that "x86" actually *means* "i386 plus
x86_64", and I totally don't have that knowledge for the rest of the
arches / targets.

So, the modification of @CpuInfo I would indeed like to pass off to
someone else. (Well, if all the architecture maintainers follow up and
tell me what emulators exactly fall under the umbrella of each
individual @arch value, I can post the patch.) BTW... I wonder how
compatibility would be affected if we just added @target to @CpuInfo,
even without making it the new discriminator.

... Anyway, I think I've gotten a huge amount of useful and actionable
feedback; thanks everyone, let me work on RFCv3 and post it soon. :)

Thanks!
Laszlo

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

[libvirt] [PATCH 0/3] qemu: Allow disabling ROM for PCI devices

[Andrea Bolognani]

All the details are in the bug report referenced in 2/3.

Andrea Bolognani (3):
  conf: Add rom.enabled attribute for PCI devices
  qemu: Format rom.enabled attribute for PCI devices
  news: Document rom.enabled attribute for PCI devices

 docs/formatdomain.html.in |  3 +++
 docs/news.xml | 10 +
 docs/schemas/domaincommon.rng |  5 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c| 26 +++-
 src/qemu/qemu_command.c   | 13 ++--
 tests/qemuxml2argvdata/pci-rom-disabled.args  | 26 
 tests/qemuxml2argvdata/pci-rom-disabled.xml   | 20 ++
 tests/qemuxml2argvtest.c  |  1 +
 tests/qemuxml2xmloutdata/pci-rom-disabled.xml | 29 +++
 tests/qemuxml2xmltest.c   |  1 +
 11 files changed, 132 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/pci-rom-disabled.args
 create mode 100644 tests/qemuxml2argvdata/pci-rom-disabled.xml
 create mode 100644 tests/qemuxml2xmloutdata/pci-rom-disabled.xml

-- 
2.14.3

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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/20/18 12:41, Gerd Hoffmann wrote:
 Since your schema is likely to end up just being a file in docs/specs,
> 
>>> I think it would be useful to have this as part of the schema.  Should
>>> be easy then to write up a small utility then which takes a json file as
>>> input and translates that into qemu command line options.
>>
>> Currently we have two distinct QAPI schemas, one covering the system
>> emulator for QMP and subset of CLI args, and one covering the guest
>> agent.
> 
> Yep, we can make it a third one, fine with me.
> But it likewise wouldn't end up in docs/ then.

Let's please focus on the schema itself for now, with the stupid little
test command included. Once the schema enjoys universal popularity, I'll
be glad to pass it on to QMP wizards -- because, if we ultimately wanted
to keep the schema code-enabled, then Markus's earlier suggestions apply
too, regarding QMP test cases. I'll admit I haven't dared look at those
yet (in particular because Paolo originally suggested to move this under
docs/interop). Looking at my todo list, I'd really like to focus on the
schema itself.

Thanks
Laszlo

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

[libvirt] [PATCH 3/3] news: Document rom.enabled attribute for PCI devices

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 docs/news.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index dec3f134ce..de95d919a2 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -56,6 +56,16 @@
   host-passthrough CPU model.
 
   
+  
+
+  qemu: Optionally disable ROM loading for PCI devices
+
+
+  The enabled attribute of the rom
+  sub-element, usable for PCI devices, can be used for situations
+  where loading a PCI ROM is not desiderable.
+
+  
 
 
   
-- 
2.14.3

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

[libvirt] [PATCH 2/3] qemu: Format rom.enabled attribute for PCI devices

[Andrea Bolognani]

The attribute can be used to disable ROM loading completely
for a device.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1425058
Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_command.c   | 13 ++--
 tests/qemuxml2argvdata/pci-rom-disabled.args  | 26 
 tests/qemuxml2argvdata/pci-rom-disabled.xml   | 20 ++
 tests/qemuxml2argvtest.c  |  1 +
 tests/qemuxml2xmloutdata/pci-rom-disabled.xml | 29 +++
 tests/qemuxml2xmltest.c   |  1 +
 6 files changed, 88 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/pci-rom-disabled.args
 create mode 100644 tests/qemuxml2argvdata/pci-rom-disabled.xml
 create mode 100644 tests/qemuxml2xmloutdata/pci-rom-disabled.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b666f3715f..84c4e1e350 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -442,13 +442,20 @@ static int
 qemuBuildRomStr(virBufferPtr buf,
 virDomainDeviceInfoPtr info)
 {
-if (info->rombar || info->romfile) {
+if (info->romenabled || info->rombar || info->romfile) {
 if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   "%s", _("rombar and romfile are supported only for 
PCI devices"));
+   "%s", _("ROM tuning is only supported for PCI 
devices"));
 return -1;
 }
 
+/* Passing an empty romfile= tells QEMU to disable ROM entirely for
+ * this device, and makes other settings irrelevant */
+if (info->romenabled == VIR_TRISTATE_BOOL_NO) {
+virBufferAddLit(buf, ",romfile=");
+goto done;
+}
+
 switch (info->rombar) {
 case VIR_TRISTATE_SWITCH_OFF:
 virBufferAddLit(buf, ",rombar=0");
@@ -464,6 +471,8 @@ qemuBuildRomStr(virBufferPtr buf,
virQEMUBuildBufferEscapeComma(buf, info->romfile);
 }
 }
+
+ done:
 return 0;
 }
 
diff --git a/tests/qemuxml2argvdata/pci-rom-disabled.args 
b/tests/qemuxml2argvdata/pci-rom-disabled.args
new file mode 100644
index 00..8c9dc2fb80
--- /dev/null
+++ b/tests/qemuxml2argvdata/pci-rom-disabled.args
@@ -0,0 +1,26 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-guest/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot c \
+-netdev user,id=hostnet0 \
+-device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:24:a5:9f,bus=pci.0,\
+addr=0x3,romfile=
diff --git a/tests/qemuxml2argvdata/pci-rom-disabled.xml 
b/tests/qemuxml2argvdata/pci-rom-disabled.xml
new file mode 100644
index 00..1c12052382
--- /dev/null
+++ b/tests/qemuxml2argvdata/pci-rom-disabled.xml
@@ -0,0 +1,20 @@
+
+  guest
+  c7a5fdbd-edaf-9466-926a-d65c16db1809
+  219100
+  1
+  
+hvm
+  
+  
+/usr/bin/qemu-system-x86_64
+
+
+
+  
+  
+  
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 74d930ebe2..ae9893a84e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1554,6 +1554,7 @@ mymain(void)
 DO_TEST_PARSE_ERROR("hostdev-mdev-invalid-target-address",
 QEMU_CAPS_DEVICE_VFIO_PCI);
 DO_TEST("pci-rom", NONE);
+DO_TEST("pci-rom-disabled", NONE);
 
 DO_TEST_FULL("restore-v2", "exec:cat", 7, 0, 0, GIC_NONE, NONE);
 DO_TEST_FULL("restore-v2-fd", "stdio", 7, 0, 0, GIC_NONE, NONE);
diff --git a/tests/qemuxml2xmloutdata/pci-rom-disabled.xml 
b/tests/qemuxml2xmloutdata/pci-rom-disabled.xml
new file mode 100644
index 00..6a95064ebf
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/pci-rom-disabled.xml
@@ -0,0 +1,29 @@
+
+  guest
+  c7a5fdbd-edaf-9466-926a-d65c16db1809
+  219100
+  219100
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+
+
+  
+  
+  
+  
+
+
+
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 9e77b9fb13..6c1f0b0fa6 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -474,6 +474,7 @@ mymain(void)
 DO_TEST("hostdev-vfio", NONE);
 DO_TEST("hostdev-mdev-precreated", NONE);
 DO_TEST("pci-rom", NONE);
+DO_TEST("pci-rom-disabled", NONE);
 DO_TEST("pci-serial-dev-chardev", NONE);
 
 DO_TEST("encrypted-disk", NONE);
-- 
2.14.3

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

[libvirt] [PATCH 1/3] conf: Add rom.enabled attribute for PCI devices

[Andrea Bolognani]

The attribute can be used to disable ROM loading completely
for a device.

Signed-off-by: Andrea Bolognani 
---
 docs/formatdomain.html.in |  3 +++
 docs/schemas/domaincommon.rng |  5 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c| 26 +-
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ada0df227f..0afc310e25 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4476,6 +4476,9 @@
 virtual function of an sr-iov capable ethernet device (which
 has no boot ROMs for the VFs).
 Since 0.9.10 (QEMU and KVM only).
+The optional enabled attribute can be set to
+no to disable PCI ROM loading completely for the device.
+Since 4.3.0 (QEMU and KVM only).
   
   address
   The address element for USB devices has a
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4cab55f05d..3569b92127 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5108,6 +5108,11 @@
 
   
 
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index f87d6f1fc6..a31ce9c376 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -153,6 +153,7 @@ struct _virDomainDeviceInfo {
 } master;
 /* rombar and romfile are only used for pci hostdev and network
  * devices. */
+int romenabled; /* enum virTristateBool */
 int rombar; /* enum virTristateSwitch */
 char *romfile;
 /* bootIndex is only used for disk, network interface, hostdev
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35666c1347..3c152441df 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6095,9 +6095,17 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 }
 
 if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) &&
-(info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
+(info->romenabled != VIR_TRISTATE_BOOL_ABSENT ||
+ info->rombar != VIR_TRISTATE_SWITCH_ABSENT ||
+ info->romfile)) {
 
 virBufferAddLit(buf, "romenabled != VIR_TRISTATE_BOOL_ABSENT) {
+const char *romenabled = 
virTristateBoolTypeToString(info->romenabled);
+
+if (romenabled)
+virBufferAsprintf(buf, " enabled='%s'", romenabled);
+}
 if (info->rombar != VIR_TRISTATE_SWITCH_ABSENT) {
 const char *rombar = virTristateSwitchTypeToString(info->rombar);
 
@@ -6738,6 +6746,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt 
ATTRIBUTE_UNUSED,
 xmlNodePtr boot = NULL;
 xmlNodePtr rom = NULL;
 char *type = NULL;
+char *romenabled = NULL;
 char *rombar = NULL;
 char *aliasStr = NULL;
 int ret = -1;
@@ -6791,6 +6800,12 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt 
ATTRIBUTE_UNUSED,
 }
 
 if (rom) {
+if ((romenabled = virXMLPropString(rom, "enabled")) &&
+((info->romenabled = virTristateBoolTypeFromString(romenabled)) <= 
0)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown rom enabled value '%s'"), romenabled);
+goto cleanup;
+}
 if ((rombar = virXMLPropString(rom, "bar")) &&
 ((info->rombar = virTristateSwitchTypeFromString(rombar)) <= 0)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -6798,6 +6813,14 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt 
ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 info->romfile = virXMLPropString(rom, "file");
+
+if (info->romenabled == VIR_TRISTATE_BOOL_NO &&
+(info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   "%s",
+   _("ROM tuning is not supported when ROM is 
disabled"));
+goto cleanup;
+}
 }
 
 if (address &&
@@ -6811,6 +6834,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt 
ATTRIBUTE_UNUSED,
 virDomainDeviceInfoClear(info);
 VIR_FREE(type);
 VIR_FREE(rombar);
+VIR_FREE(romenabled);
 VIR_FREE(aliasStr);
 return ret;
 }
-- 
2.14.3

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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/20/18 11:34, Daniel P. Berrangé wrote:
> On Fri, Apr 20, 2018 at 10:11:08AM +0200, Laszlo Ersek wrote:
>> On 04/19/18 11:12, Daniel P. Berrangé wrote:
>>> On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote:
 On 04/19/18 09:56, Daniel P. Berrangé wrote:
> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote:
>> Laszlo Ersek  writes:
>>
>>> On 04/18/18 10:47, Markus Armbruster wrote:
 Laszlo Ersek  writes:
>> Replacing CpuInfoArch by such an enum will change the discriminator
>> value from "other" to the real architecture, with the obvious
>> compatibility concerns.  But we've accepted similar changes twice
>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.
>>
>> "other" was a bad idea.  Hindsight 20/20.
>>
>> Getting rid of it in one go rather than piecemeal seems like the least
>> bad way out.  Too late for 2.12, though.  Eric, what do you think?
>
> Given the context in which this "other" value is used, I think it is
> reasonable to kill it and put a full arch list in there.
>
> No app is likely to be accessing the struct under "other" because it
> is just an empty placeholder.

 Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the
 potential to confuse QMP clients that didn't expect "s390", but
 otherwise it didn't mess with preexistent enum values / structures.
>>>
>>> NB, qemu-system-s390x would previously have returned "other" in
>>> this field, and now it returns "s390".  So while it didn't
>>> remove "other" from the list of things that could potentially
>>> exist, it did change what the s390x binary will actually report.
>>>
 The same applies to commit 25fa194b7b1, just with "riscv" /
 "CpuInfoRISCV" substituted.

 Removing "other" might confuse QMP clients that expect it, except
 (according to Daniel) no such client exists, probably.
>>>
>>> When I say removing "other", I imply that we add an explicit arch
>>> for all those which we currently are missing. IOW, all qemu-system-XXX
>>> binaries which currently report "other" would change to report their
>>> respective "XXX" values.
>>>
>>> So in this way, it is exactly the same as what we did when we
>>> introduced "s390" as an option.
>>>
>>> The only difference is that once we have every binary reporting the
>>> correct arch, we can now also remove "other" from the schema itself
>>> as it will then be unused.
>>
>> Can we please translate this into more actionable items for me, because
>> I'm getting confused :)
>>
>> First, if I add "i386" and "x86_64" to the enum list, we'll have all
>> three of "i386", "x86_64" and "x86". Is that useful? How will that work?
> 
> Hmm, yes, on closer look this is a big mess as it is. We've been using
> generic terms for covering multiple architectures :-(  'x86' for both
> i386 and x86_64,  'sparc' for sparc and sparc64, etc. If we try to fix
> that we'll be entering a world of backcompat hurt :-(
> 
> Since your schema is likely to end up just being a file in docs/specs,
> rather than directly part of our existnig qapi schema, I suggest we just
> ignore whats there. Just define an arch enum in your spec which is right,
> and let someone else worry about fixing the mess

I can't tell you how much I love this idea. :)

Thanks!
Laszlo

> 
>> Second, assuming I add constants for the ~10 (?) softmmu arches, can I
>> still use @CpuInfoOther as the type for the corresponding new members in
>> @CpuInfo? What C code changes will be necessary?
> 
> Yes, we could still use the CpuInfoOther struct, since struct names are
> invisible to consumers, but as above, lets ignore the mess
> 
> Regards,
> Daniel
> 

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

Re: [libvirt] [dbus PATCH] Fix GError memory leaks

[Ján Tomko]

On Fri, Apr 20, 2018 at 02:14:43PM +0200, Pavel Hrdina wrote:

Signed-off-by: Pavel Hrdina 
---
src/gdbus.c | 8 
src/main.c  | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [dbus PATCH 3/3] domain: Implement SetPerfEvents method

[Katerina Koukiou]

On Fri, 2018-04-20 at 14:17 +0200, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
>  data/org.libvirt.Domain.xml |  6 ++
>  src/domain.c| 31 +++
>  2 files changed, 37 insertions(+)

Reviewed-by: Katerina Koukiou 

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

Re: [libvirt] --disable-nls is ignored in master

[Daniel P . Berrangé]

On Fri, Apr 20, 2018 at 04:37:34PM +0200, Olaf Hering wrote:
> It seems some of the recent changes for 'po' broke the --disable-nls 
> configure option.
> I expect no locales will be installed with --disable-nls, but they are 
> installed anyway.
> This leads to the well known "error: Installed (but unpackaged) file(s) 
> found:" failure.
> I tested libvirt-20180420T081114.3c66d5108.

Opps, I disabled use of NLS in the code but forgot to stop installing
the gmo files :-)


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [dbus PATCH 2/3] util: Introduce virtDBusUtilGVariantToTypedParams

[Katerina Koukiou]

On Fri, 2018-04-20 at 14:17 +0200, Pavel Hrdina wrote:
> This converts D-Bus dictionary into virTypedParameters.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  src/util.c | 83
> ++
>  src/util.h |  6 +
>  2 files changed, 89 insertions(+)

Reviewed-by: Katerina Koukiou 

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

[libvirt] --disable-nls is ignored in master

[Olaf Hering]

It seems some of the recent changes for 'po' broke the --disable-nls configure 
option.
I expect no locales will be installed with --disable-nls, but they are 
installed anyway.
This leads to the well known "error: Installed (but unpackaged) file(s) found:" 
failure.
I tested libvirt-20180420T081114.3c66d5108.

Olaf


pgp2BN1C8BC98.pgp
Description: Digitale Signatur von OpenPGP
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [dbus PATCH 1/3] util: Introduce virtDBusUtilTypedParams

[Katerina Koukiou]

On Fri, 2018-04-20 at 14:17 +0200, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
>  src/util.c |  6 ++
>  src/util.h | 11 +++
>  2 files changed, 17 insertions(+)

Reviewed-by: Katerina Koukiou 

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

Re: [libvirt] [PATCH v2 4/4] qemu: remove pointless connect retry logic in agent

[John Ferlan]

On 04/18/2018 01:30 PM, Daniel P. Berrangé wrote:
> When the agent code was first introduced back in
> 
>   commit c160ce3316852a797d7b06b4ee101233866e69a9
>   Author: Daniel P. Berrange 
>   Date:   Wed Oct 5 18:31:54 2011 +0100
> 
> QEMU guest agent support
> 
> there was code that would loop and retry the connection when opening
> the agent socket. At this time, the only thing done in between the
> opening of the monitor socket & opening of the agent socket was a
> call to set the monitor capabilities. This was a no-op on non-QMP
> versions, so in theory there could be a race which let us connect
> to the monitor while the agent socket was still not created by QEMU.
> 
> In the modern world, however, we long ago mandated the use of QMP
> for managing QEMU, so we're guaranteed to have a set capabilities
> QMP call. Once we've seen a reply to this, we're guaranteed that
> QEMU has fully initialized all backends and is in its event loop.
> 
> We can thus be sure the QEMU agent socket is present and don't need
> to retry connections to it, even without having the chardev FD passing
> feature.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/qemu/qemu_agent.c | 84 
> +--
>  1 file changed, 7 insertions(+), 77 deletions(-)
> 

Reviewed-by: John Ferlan 

John

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

Re: [libvirt] [PATCH v2 3/4] qemu: don't retry connect() if doing FD passing

[John Ferlan]

On 04/18/2018 01:30 PM, Daniel P. Berrangé wrote:
> Since libvirt called bind() and listen() on the UNIX socket, it is
> guaranteed that connect() will immediately succeed, if QEMU is running
> normally. It will only fail if QEMU has closed the monitor socket by
> mistake or if QEMU has exited, letting the kernel close it.
> 
> With this in mind we can remove the retry loop and timeout when
> connecting to the QEMU monitor if we are doing FD passing. Libvirt can
> go straight to sending the QMP greeting and will simply block waiting
> for a reply until QEMU is ready.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/qemu/qemu_capabilities.c |  2 +-
>  src/qemu/qemu_monitor.c  | 54 
> ++--
>  src/qemu/qemu_monitor.h  |  1 +
>  src/qemu/qemu_process.c  | 27 --
>  tests/qemumonitortestutils.c |  1 +
>  5 files changed, 55 insertions(+), 30 deletions(-)
> 

Reviewed-by: John Ferlan 

John

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

Re: [libvirt] [PATCH v2 2/4] qemu: support passing pre-opened UNIX socket listen FD

[John Ferlan]

On 04/18/2018 01:30 PM, Daniel P. Berrangé wrote:
> There is a race condition when spawning QEMU where libvirt has spawned
> QEMU but the monitor socket is not yet open. Libvirt has to repeatedly
> try to connect() to QEMU's monitor until eventually it succeeds, or
> times out. We use kill() to check if QEMU is still alive so we avoid
> waiting a long time if QEMU exited, but having a timeout at all is still
> unpleasant.
> 
> With QEMU 2.12 we can pass in a pre-opened FD for UNIX domain or TCP
> sockets. If libvirt has called bind() and listen() on this FD, then we
> have a guarantee that libvirt can immediately call connect() and
> succeed without any race.
> 
> Although we only really care about this for the monitor socket and agent
> socket, this patch does FD passing for all UNIX socket based character
> devices since there appears to be no downside to it.
> 
> We don't do FD passing for TCP sockets, however, because it is only
> possible to pass a single FD, while some hostnames may require listening
> on multiple FDs to cover IPv4 and IPv6 concurrently.
> 
> Reviewed-by: John Ferlan 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/qemu/qemu_command.c | 54 
> +++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
> 

This one is now affected by Peter's recent xml2argv adjustment for:

tests/qemuxml2argvdata/disk-drive-write-cache.x86_64-latest.args

which now fails w/ :

253) QEMU XML-2-ARGV disk-drive-write-cache.x86_64-latest
... libvirt: QEMU Driver error : Unable to bind to UNIX socket path
'/tmp/lib/domain--1-QEMUGuest1/monitor.sock': No such file or directory
FAILED

because the qemuProcessCreatePretendCmd doesn't allow this code to
distinguish that it shouldn't attempt a bind/listen 

I "think" this may require moving the socket setup code into
qemuProcessLaunch or somehow passing a flag into BuildCommandLine to
tell it not to do anything "real".

John

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 0c109c63e7..9fc48eb829 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5034,8 +5034,58 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>  break;
>  
>  case VIR_DOMAIN_CHR_TYPE_UNIX:
> -virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
> -virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
> +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) {
> +struct sockaddr_un addr;
> +socklen_t addrlen = sizeof(addr);
> +int fd;
> +
> +if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
> +virReportSystemError(errno, "%s",
> + _("Unable to create UNIX socket"));
> +goto cleanup;
> +}
> +
> +memset(&addr, 0, sizeof(addr));
> +addr.sun_family = AF_UNIX;
> +if (virStrcpyStatic(addr.sun_path, dev->data.nix.path) == NULL) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("UNIX socket path '%s' too long"),
> +   dev->data.nix.path);
> +VIR_FORCE_CLOSE(fd);
> +goto cleanup;
> +}
> +
> +if (unlink(dev->data.nix.path) < 0 && errno != ENOENT) {
> +virReportSystemError(errno,
> + _("Unable to unlink %s"),
> + dev->data.nix.path);
> +VIR_FORCE_CLOSE(fd);
> +goto cleanup;
> +}
> +
> +if (bind(fd, (struct sockaddr *)&addr, addrlen) < 0) {
> +virReportSystemError(errno,
> + _("Unable to bind to UNIX socket path 
> '%s'"),
> + dev->data.nix.path);
> +VIR_FORCE_CLOSE(fd);
> +goto cleanup;
> +}
> +
> +if (listen(fd, 1) < 0) {
> +virReportSystemError(errno,
> + _("Unable to listen to UNIX socket path 
> '%s'"),
> + dev->data.nix.path);
> +VIR_FORCE_CLOSE(fd);
> +goto cleanup;
> +}
> +
> +virBufferAsprintf(&buf, "socket,id=%s,fd=%d", charAlias, fd);
> +
> +virCommandPassFD(cmd, fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +} else {
> +virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
> +virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
> +}
>  if (dev->data.nix.listen)
>  virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
>  
> 

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

Re: [libvirt] [dbus PATCH v2] Implement GetAllDomainStats method for Connect Interface

[Pavel Hrdina]

On Fri, Apr 20, 2018 at 02:56:18PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou 
> ---
>  data/org.libvirt.Connect.xml |  7 +++
>  src/connect.c| 47 
> 
>  2 files changed, 54 insertions(+)

Reviewed-by: Pavel Hrdina 


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

[libvirt] [PATCH v2 0/2] Rename/remove virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

These were actually posted before as part of a larger series to
rework/rename virDomainObjListFindBy{UUID|ID}Ref as patches 19 and 20:

https://www.redhat.com/archives/libvir-list/2018-March/msg00508.html
https://www.redhat.com/archives/libvir-list/2018-March/msg00509.html

Even though it's only 6 weeks ago, attempting to remember what (if
anything) has changed is challenging. In the long run, the point is
that the *Ref specific functions are now removed. This makes it much
easier for the next step which will be to modify the Add/Remove logic
to be simpler and more realistic w/r/t expectations as well as remove
a few comments in the code that indicate some day we need to fix this
crazy problem (some day is getting closer).

John Ferlan (2):
  conf: Rework/rename virDomainObjListFindByUUIDRef
  conf: Rework/rename virDomainObjListFindByIDRef

 src/bhyve/bhyve_driver.c |  6 ++--
 src/conf/virdomainobjlist.c  | 66 ++--
 src/conf/virdomainobjlist.h  |  4 ---
 src/libvirt_private.syms |  2 --
 src/libxl/libxl_domain.c |  2 +-
 src/libxl/libxl_driver.c |  6 ++--
 src/lxc/lxc_driver.c |  7 ++---
 src/openvz/openvz_driver.c   |  4 +--
 src/qemu/qemu_driver.c   |  9 +++---
 src/test/test_driver.c   |  6 ++--
 src/uml/uml_driver.c |  6 ++--
 src/util/virclosecallbacks.c |  4 +--
 src/vmware/vmware_driver.c   |  4 +--
 src/vz/vz_driver.c   |  6 ++--
 src/vz/vz_sdk.c  | 14 +-
 src/vz/vz_utils.c|  2 +-
 16 files changed, 51 insertions(+), 97 deletions(-)

-- 
2.13.6

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

[libvirt] [PATCH v2 2/2] conf: Rework/rename virDomainObjListFindByIDRef

[John Ferlan]

Rework the code such that virDomainObjListFindByID will always
return a locked/ref counted object so that the callers can
always do the same cleanup logic to call virDomainObjEndAPI.
Makes accessing the objects much more consistent.

NB:
There were 2 callers (lxcDomainLookupByID and qemuDomainLookupByID)
that were already using the ByID name, but not virDomainObjEndAPI -
these were changed as well in this update/patch.

Signed-off-by: John Ferlan 
---
 src/bhyve/bhyve_driver.c|  2 +-
 src/conf/virdomainobjlist.c | 35 +--
 src/conf/virdomainobjlist.h |  2 --
 src/libvirt_private.syms|  1 -
 src/libxl/libxl_domain.c|  2 +-
 src/libxl/libxl_driver.c|  2 +-
 src/lxc/lxc_driver.c|  3 +--
 src/openvz/openvz_driver.c  |  2 +-
 src/qemu/qemu_driver.c  |  5 ++---
 src/test/test_driver.c  |  2 +-
 src/uml/uml_driver.c|  2 +-
 src/vmware/vmware_driver.c  |  2 +-
 src/vz/vz_driver.c  |  2 +-
 13 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index bd8c6b30ed..768578a43f 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -844,7 +844,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByIDRef(privconn->domains, id);
+vm = virDomainObjListFindByID(privconn->domains, id);
 
 if (!vm) {
 virReportError(VIR_ERR_NO_DOMAIN,
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 02b52465d0..d57ed10a5f 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -109,44 +109,27 @@ static int virDomainObjListSearchID(const void *payload,
 return want;
 }
 
-static virDomainObjPtr
-virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
- int id,
- bool ref)
+
+virDomainObjPtr
+virDomainObjListFindByID(virDomainObjListPtr doms,
+ int id)
 {
 virDomainObjPtr obj;
+
 virObjectRWLockRead(doms);
 obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL);
-if (ref) {
-virObjectRef(obj);
-virObjectRWUnlock(doms);
-}
+virObjectRef(obj);
+virObjectRWUnlock(doms);
 if (obj) {
 virObjectLock(obj);
 if (obj->removing) {
 virObjectUnlock(obj);
-if (ref)
-virObjectUnref(obj);
+virObjectUnref(obj);
 obj = NULL;
 }
 }
-if (!ref)
-virObjectRWUnlock(doms);
-return obj;
-}
-
-virDomainObjPtr
-virDomainObjListFindByID(virDomainObjListPtr doms,
- int id)
-{
-return virDomainObjListFindByIDInternal(doms, id, false);
-}
 
-virDomainObjPtr
-virDomainObjListFindByIDRef(virDomainObjListPtr doms,
-int id)
-{
-return virDomainObjListFindByIDInternal(doms, id, true);
+return obj;
 }
 
 
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
index 1b77a95ba9..7e2dece3aa 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -34,8 +34,6 @@ virDomainObjListPtr virDomainObjListNew(void);
 
 virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,
  int id);
-virDomainObjPtr virDomainObjListFindByIDRef(virDomainObjListPtr doms,
-int id);
 virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
const unsigned char *uuid);
 virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f6c76d4341..d2728749fb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -952,7 +952,6 @@ virDomainObjListCollect;
 virDomainObjListConvert;
 virDomainObjListExport;
 virDomainObjListFindByID;
-virDomainObjListFindByIDRef;
 virDomainObjListFindByName;
 virDomainObjListFindByUUID;
 virDomainObjListForEach;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index e401327df4..d4859d6707 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -450,7 +450,7 @@ libxlDomainShutdownThread(void *opaque)
 
 cfg = libxlDriverConfigGet(driver);
 
-vm = virDomainObjListFindByIDRef(driver->domains, ev->domid);
+vm = virDomainObjListFindByID(driver->domains, ev->domid);
 if (!vm) {
 VIR_INFO("Received event for unknown domain ID %d", ev->domid);
 goto cleanup;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 24553c7272..55a93a489b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1082,7 +1082,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id)
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByIDRef(driver->domains, id);
+vm = virDomainObjListFi

[libvirt] [PATCH v2 1/2] conf: Rework/rename virDomainObjListFindByUUIDRef

[John Ferlan]

Now that every caller is using virDomainObjListFindByUUIDRef,
let's just remove it and keep the name as virDomainObjListFindByUUID.

Signed-off-by: John Ferlan 
---
 src/bhyve/bhyve_driver.c |  4 ++--
 src/conf/virdomainobjlist.c  | 35 +++
 src/conf/virdomainobjlist.h  |  2 --
 src/libvirt_private.syms |  1 -
 src/libxl/libxl_driver.c |  4 ++--
 src/lxc/lxc_driver.c |  4 ++--
 src/openvz/openvz_driver.c   |  2 +-
 src/qemu/qemu_driver.c   |  4 ++--
 src/test/test_driver.c   |  4 ++--
 src/uml/uml_driver.c |  4 ++--
 src/util/virclosecallbacks.c |  4 ++--
 src/vmware/vmware_driver.c   |  2 +-
 src/vz/vz_driver.c   |  4 ++--
 src/vz/vz_sdk.c  | 14 +++---
 src/vz/vz_utils.c|  2 +-
 15 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 24c4a9c80e..bd8c6b30ed 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -168,7 +168,7 @@ bhyveDomObjFromDomain(virDomainPtr domain)
 bhyveConnPtr privconn = domain->conn->privateData;
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid);
+vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
 if (!vm) {
 virUUIDFormat(domain->uuid, uuidstr);
 virReportError(VIR_ERR_NO_DOMAIN,
@@ -791,7 +791,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
 virDomainObjPtr vm;
 virDomainPtr dom = NULL;
 
-vm = virDomainObjListFindByUUIDRef(privconn->domains, uuid);
+vm = virDomainObjListFindByUUID(privconn->domains, uuid);
 
 if (!vm) {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index c09e5e5d2f..02b52465d0 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -149,10 +149,10 @@ virDomainObjListFindByIDRef(virDomainObjListPtr doms,
 return virDomainObjListFindByIDInternal(doms, id, true);
 }
 
-static virDomainObjPtr
-virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms,
-   const unsigned char *uuid,
-   bool ref)
+
+virDomainObjPtr
+virDomainObjListFindByUUID(virDomainObjListPtr doms,
+   const unsigned char *uuid)
 {
 char uuidstr[VIR_UUID_STRING_BUFLEN];
 virDomainObjPtr obj;
@@ -161,41 +161,20 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr 
doms,
 virUUIDFormat(uuid, uuidstr);
 
 obj = virHashLookup(doms->objs, uuidstr);
-if (ref) {
-virObjectRef(obj);
-virObjectRWUnlock(doms);
-}
+virObjectRef(obj);
+virObjectRWUnlock(doms);
 if (obj) {
 virObjectLock(obj);
 if (obj->removing) {
 virObjectUnlock(obj);
-if (ref)
-virObjectUnref(obj);
+virObjectUnref(obj);
 obj = NULL;
 }
 }
-if (!ref)
-virObjectRWUnlock(doms);
 return obj;
 }
 
 
-virDomainObjPtr
-virDomainObjListFindByUUID(virDomainObjListPtr doms,
-   const unsigned char *uuid)
-{
-return virDomainObjListFindByUUIDInternal(doms, uuid, false);
-}
-
-
-virDomainObjPtr
-virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
-  const unsigned char *uuid)
-{
-return virDomainObjListFindByUUIDInternal(doms, uuid, true);
-}
-
-
 virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
const char *name)
 {
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
index bb186bde30..1b77a95ba9 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -38,8 +38,6 @@ virDomainObjPtr 
virDomainObjListFindByIDRef(virDomainObjListPtr doms,
 int id);
 virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
const unsigned char *uuid);
-virDomainObjPtr virDomainObjListFindByUUIDRef(virDomainObjListPtr doms,
-  const unsigned char *uuid);
 virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
const char *name);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b31f599bd2..f6c76d4341 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -955,7 +955,6 @@ virDomainObjListFindByID;
 virDomainObjListFindByIDRef;
 virDomainObjListFindByName;
 virDomainObjListFindByUUID;
-virDomainObjListFindByUUIDRef;
 virDomainObjListForEach;
 virDomainObjListGetActiveIDs;
 virDomainObjListGetInactiveNames;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b66a1de5f1..24553c7272 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -313,7 +313,7 @@ libxlDomObjFromDomain(vi

Re: [libvirt] [jenkins-ci PATCH 4/5] guests: Create ccache symlink farm

[Andrea Bolognani]

On Thu, 2018-04-19 at 17:36 +0200, Martin Kletzander wrote:
> > Replicate the setup in the user's ~/.ccache directory so
> > that it works across all guests. With the linke farm in
> 
> s/linke/link/ ?

Close: I actually meant to write Linkle[1] :P

> > - name: '{{ flavor }}: Configure ccache'
> >   file:
> > -path: /home/{{ flavor }}/.ccache
> > +path: /home/{{ flavor }}/.{{ item }}
> > state: directory
> > owner: '{{ flavor }}'
> > group: '{{ flavor }}'
> > +  with_items:
> > +- ccache
> > +- ccache/bin
> 
> With new enough ansible, you don't need the 'ccache' entry as this
> behaves similarly to `mkdir -p`:
> 
>   "If directory, all immediate subdirectories will be created if they do
>   not exist, since 1.7 they will be created with the supplied
>   permissions." [1]

If you dig through the playbooks, you'll certainly find no lack of
suboptimal or roundabout approaches to the problem at hand, mostly
as a consequence of me pretty much figuring out Ansible as I went.

I'll be make to get rid of this specific instance before pushing.
More patches in the spirit would be very much welcome O:-)


[1] https://zelda.gamepedia.com/Linkle
-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] remote: remove obsolete & incorrect comment from libvirtd.conf

[Andrea Bolognani]

On Fri, 2018-04-20 at 12:34 +0100, Daniel P. Berrangé wrote:
> The libvirtd.conf file has a comment pointing people to format.html
> which has nothing todo with the configuration file format.

s/todo/to do/

> It also has a comment about tests/daemon-conf which no longer exists,
> and even if it did exist such comment is not relevant to end users
> when this file is installed in /etc/.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/remote/libvirtd.conf | 5 -

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [dbus PATCH v2] Implement GetAllDomainStats method for Connect Interface

[Katerina Koukiou]

Signed-off-by: Katerina Koukiou 
---
 data/org.libvirt.Connect.xml |  7 +++
 src/connect.c| 47 
 2 files changed, 54 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 14e66e4..ee7bfdc 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -94,6 +94,13 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectGetAllDomainStats"/>
+  
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetCapabilities"/>
diff --git a/src/connect.c b/src/connect.c
index 1ef5c93..48db724 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -503,6 +503,52 @@ virtDBusConnectFindStoragePoolSources(GVariant *inArgs,
 *outArgs = g_variant_new("(s)", ret);
 }
 
+static void
+virtDBusConnectGetAllDomainStats(GVariant *inArgs,
+ GUnixFDList *inFDs G_GNUC_UNUSED,
+ const gchar *objectPath G_GNUC_UNUSED,
+ gpointer userData,
+ GVariant **outArgs,
+ GUnixFDList **outFDs G_GNUC_UNUSED,
+ GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virDomainStatsRecordPtr) records = NULL;
+guint stats;
+gint nstats;
+guint flags;
+GVariant *gret;
+GVariantBuilder builder;
+
+g_variant_get(inArgs, "(uu)", &stats, &flags);
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+nstats = virConnectGetAllDomainStats(connect->connection,
+ stats, &records, flags);
+if (nstats < 0)
+return virtDBusUtilSetLastVirtError(error);
+
+g_variant_builder_init(&builder, G_VARIANT_TYPE("a(sa{sv})"));
+
+for (gint i = 0; i < nstats; i++) {
+const gchar* name;
+GVariant *grecords;
+
+g_variant_builder_open(&builder, G_VARIANT_TYPE("(sa{sv})"));
+name = virDomainGetName(records[i]->dom);
+grecords = virtDBusUtilTypedParamsToGVariant(records[i]->params,
+ records[i]->nparams);
+g_variant_builder_add(&builder, "s", name);
+g_variant_builder_add_value(&builder, grecords);
+g_variant_builder_close(&builder);
+}
+gret = g_variant_builder_end(&builder);
+
+*outArgs = g_variant_new_tuple(&gret, 1);
+}
+
 static void
 virtDBusConnectGetCPUModelNames(GVariant *inArgs,
 GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -808,6 +854,7 @@ static virtDBusGDBusMethodTable 
virtDBusConnectMethodTable[] = {
 { "DomainRestore", virtDBusConnectDomainRestoreFlags },
 { "DomainSaveImageDefineXML", virtDBusConnectDomainSaveImageDefineXML },
 { "FindStoragePoolSources", virtDBusConnectFindStoragePoolSources },
+{ "GetAllDomainStats", virtDBusConnectGetAllDomainStats },
 { "GetCapabilities", virtDBusConnectGetCapabilities },
 { "GetCPUModelNames", virtDBusConnectGetCPUModelNames },
 { "GetDomainCapabilities", virtDBusConnectGetDomainCapabilities },
-- 
2.15.0

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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Markus Armbruster]

Laszlo Ersek  writes:

> On 04/19/18 09:56, Daniel P. Berrangé wrote:
>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote:
>>> Laszlo Ersek  writes:
>>>
 On 04/18/18 10:47, Markus Armbruster wrote:
> Laszlo Ersek  writes:

>> +##
>> +# @FirmwareArchitecture:
>> +#
>> +# Defines the target architectures (emulator binaries) that firmware may
>> +# execute on.
>> +#
>> +# @aarch64: The firmware can be executed by the qemu-system-aarch64 
>> emulator.
>> +#
>> +# @arm: The firmware can be executed by the qemu-system-arm emulator.
>> +#
>> +# @i386: The firmware can be executed by the qemu-system-i386 emulator.
>> +#
>> +# @x86_64: The firmware can be executed by the qemu-system-x86_64 
>> emulator.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'enum' : 'FirmwareArchitecture',
>> +  'data' : [ 'aarch64', 'arm', 'i386', 'x86_64' ] }
>
> Partial dupe of CpuInfoArch from misc.json.  Neither covers all our
> target architectures.  Should we have one that does in common.json
> instead?

 If we had one there, I'd use it here :)

 For collecting the arch identifiers, is it OK to run "./configure
 --help", and look for the "*-softmmu" options under
 "--target-list=LIST"? Because that's what I need here; the qemu-system-*
 emulators.
>>>
>>> configure gets its list like this:
>>>
>>> default_target_list=""
>>>
>>> mak_wilds=""
>>>
>>> if [ "$softmmu" = "yes" ]; then
>>> mak_wilds="${mak_wilds} $source_path/default-configs/*-softmmu.mak"
>>> fi
>>> if [ "$linux_user" = "yes" ]; then
>>> mak_wilds="${mak_wilds} 
>>> $source_path/default-configs/*-linux-user.mak"
>>> fi
>>> if [ "$bsd_user" = "yes" ]; then
>>> mak_wilds="${mak_wilds} $source_path/default-configs/*-bsd-user.mak"
>>> fi
>>>
>>> for config in $mak_wilds; do
>>> default_target_list="${default_target_list} $(basename "$config" 
>>> .mak)"
>>> done
>>>
>>> Since we use QMP only in system emulation, a QAPI enum limited to the
>>> system emulation targets makes sense.
>>>
>>> Replacing CpuInfoArch by such an enum will change the discriminator
>>> value from "other" to the real architecture, with the obvious
>>> compatibility concerns.  But we've accepted similar changes twice
>>> already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.
>>>
>>> "other" was a bad idea.  Hindsight 20/20.
>>>
>>> Getting rid of it in one go rather than piecemeal seems like the least
>>> bad way out.  Too late for 2.12, though.  Eric, what do you think?
>> 
>> Given the context in which this "other" value is used, I think it is
>> reasonable to kill it and put a full arch list in there.
>> 
>> No app is likely to be accessing the struct under "other" because it
>> is just an empty placeholder.
>
> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the
> potential to confuse QMP clients that didn't expect "s390", but
> otherwise it didn't mess with preexistent enum values / structures.
>
> The same applies to commit 25fa194b7b1, just with "riscv" /
> "CpuInfoRISCV" substituted.
>
> Removing "other" might confuse QMP clients that expect it, except
> (according to Daniel) no such client exists, probably.

It's a done deal anyway; we're not going to hold 2.12 to avoid this QMP
compatibility break.

> However, replacing the current list of CpuInfoArch constants with the
> system emulation target list would be more intrusive than all three of
> the above. In particular there is no "x86" target; only i386 and x86_64
> targets exist. For the firmware schema, this distinction is important,
> but it would break QMP clients that expect "x86" (and such clients must
> certainly exist).

You're right.  Another nice mess.

> The targets with softmmu are: aarch64, alpha, arm, cris, hppa, i386,
> lm32, m68k, microblaze, microblazeel, mips, mips64, mips64el, mipsel,
> moxie, nios2, or1k, ppc, ppc64, ppcemb, riscv32, riscv64, s390x, sh4,
> sh4eb, sparc, sparc64, tricore, unicore32, x86_64, xtensa, xtensaeb.
>
> That is, at least the following constants in CpuInfoArch have no
> corresponding (identical) mapping -- I'll state to the right of the
> arrow the emulation targets which I know or presume to be associated
> with the CpuInfoArch constant:
> - x86 -> i386, x86_64
> - sparc -> sparc, sparc64
> - ppc -> ppc, ppc64, ppcemb
> - mips -> mips, mips64, mips64el, mipsel
> - s390 -> s390x
> - riscv -> riscv32, riscv64
>
> The only constant that seems to have a 1:1 mapping is "tricore", but I
> could perfectly well be thinking even that just because I have no clue
> about "tricore".
>
> So, I don't think CpuInfoArch can be updated so that it both remains
> compatible with current QMP clients, and serves "firmware.json". In the
> firmware schema we don't just need CPU architecture, but actual emulator
> names (and board / machine types).

The completely orthodox fix 

Re: [libvirt] [dbus PATCH 7/8] Implement GetCPUModelNames method for Connect Interface

[Katerina Koukiou]

On Fri, 2018-04-20 at 13:39 +0200, Ján Tomko wrote:
> On Thu, Apr 19, 2018 at 03:58:06PM +0200, Katerina Koukiou wrote:
> > Signed-off-by: Katerina Koukiou 
> > ---
> > data/org.libvirt.Connect.xml |  7 +++
> > src/connect.c| 35
> > +++
> > tests/test_connect.py|  4 
> > 3 files changed, 46 insertions(+)
> > 
> > diff --git a/data/org.libvirt.Connect.xml
> > b/data/org.libvirt.Connect.xml
> > index 3ef965d..43a6241 100644
> > --- a/data/org.libvirt.Connect.xml
> > +++ b/data/org.libvirt.Connect.xml
> > @@ -106,6 +106,13 @@
> > value="See https://libvirt.org/html/libvirt-libvirt-host.ht
> > ml#virConnectGetCapabilities"/>;
> >   
> > 
> > +
> > +   > +value="See https://libvirt.org/html/libvirt-libvirt-host.h
> > tml#virConnectGetCPUModelNames"/>;
> > +  
> > +  
> > +  
> > +
> > 
> >> value="See https://libvirt.org/html/libvirt-libvirt-host.ht
> > ml#virConnectGetSysinfo"/>;
> > diff --git a/src/connect.c b/src/connect.c
> > index e774fa4..169a070 100644
> > --- a/src/connect.c
> > +++ b/src/connect.c
> > @@ -542,6 +542,40 @@ virtDBusConnectGetAllDomainStats(GVariant
> > *inArgs,
> > *outArgs = g_variant_new_tuple(&gret, 1);
> > }
> > 
> > +static void
> > +virtDBusConnectGetCPUModelNames(GVariant *inArgs,
> > +GUnixFDList *inFDs G_GNUC_UNUSED,
> > +const gchar *objectPath
> > G_GNUC_UNUSED,
> > +gpointer userData,
> > +GVariant **outArgs,
> > +GUnixFDList **outFDs
> > G_GNUC_UNUSED,
> > +GError **error)
> > +{
> > +virtDBusConnect *connect = userData;
> > +const gchar *arch;
> > +guint flags;
> > +g_autoptr(GCharArray) models = NULL;
> 
> This will need to be changed.
> 
> > +gint nmodels;
> > +GVariant *gret;
> > +GVariantBuilder builder;
> > +
> > +g_variant_get(inArgs, "(&su)", &arch, &flags);
> > +
> > +if (!virtDBusConnectOpen(connect, error))
> > +return;
> > +
> > +nmodels = virConnectGetCPUModelNames(connect->connection,
> > arch, &models, flags);
> 
> The API documentation also states:
> Pass NULL if only the list length is needed.
> But that does not seem like useful functionality.

Anyway, we can't support it, since @models is output and not input
variable, so we can't pass NULL there somehow.

Katerina 

> 
> > +if (nmodels < 0)
> > +return virtDBusUtilSetLastVirtError(error);
> > +
> > +g_variant_builder_init(&builder, G_VARIANT_TYPE("as"));
> > +for (gint i = 0; i < nmodels; i++)
> > +g_variant_builder_add(&builder, "s", models[i]);
> > +gret = g_variant_builder_end(&builder);
> > +
> > +*outArgs = g_variant_new_tuple(&gret, 1);
> > +}
> > +
> 
> Reviewed-by: Ján Tomko 
> 
> Jano

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

Re: [libvirt] [RFC v2] external (pull) backup API

[Nikolay Shirokovskiy]

On 19.04.2018 20:28, John Snow wrote:
> 
> 
> On 04/16/2018 06:20 AM, Vladimir Sementsov-Ogievskiy wrote:
>>
>> So my point is: if we are going to implement something complicated,
>> let's implement entirely what we want, not a semi-solution. Otherwise,
>> implement a minimal and simple thing, to just make it all work (my
>> current solution).
> 
> So basically:
> 
> (1) Using bitmap names: It's a hack, but it works; and
> (2) Adding parentage information to QEMU bitmaps is also a hack, but a
> more permanent commitment to the hack.
> 
> And further, both (1) and (2) leave the same problem that if a third
> party utility deletes the bitmap, they are checkpoint-unaware and will
> ruin the metadata.
> 
> (Though QEMU could be taught to disallow the deleting of bitmaps with
> parents/children, unless you specify --force or --mergeleft or
> --mergeright or some such. That's not an option with the
> name-as-metadata strategy.)
> 
> Why is option 3 unworkable, exactly?:
> 
> (3) Checkpoints exist as structures only with libvirt. They are saved
> and remembered in the XML entirely.
> 
> Or put another way:
> 
> Can you explain to me why it's important for libvirt to be able to
> reconstruct checkpoint information from a qcow2 file?
> 

In short it take extra effort for metadata to be consistent when 
libvirtd crashes occurs. See for more detailed explanation 
in [1] starting from words "Yes it is possible".

[1] https://www.redhat.com/archives/libvir-list/2018-April/msg01001.html

Nikolay

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

[libvirt] [dbus PATCH 0/3] Introduce GVariant to TypedParams helper

[Pavel Hrdina]

Pavel Hrdina (3):
  util: Introduce virtDBusUtilTypedParams
  util: Introduce virtDBusUtilGVariantToTypedParams
  domain: Implement SetPerfEvents method

 data/org.libvirt.Domain.xml |  6 +++
 src/domain.c| 31 
 src/util.c  | 89 +
 src/util.h  | 17 +
 4 files changed, 143 insertions(+)

-- 
2.14.3

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

[libvirt] [dbus PATCH 3/3] domain: Implement SetPerfEvents method

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 data/org.libvirt.Domain.xml |  6 ++
 src/domain.c| 31 +++
 2 files changed, 37 insertions(+)

diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index 0f3d926..a57b05a 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -348,6 +348,12 @@
   
   
 
+
+  https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetPerfEvents"/>
+  
+  
+
 
   https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetUserPassword"/>
diff --git a/src/domain.c b/src/domain.c
index ae1b68f..7eb43f4 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -1699,6 +1699,36 @@ virtDBusDomainSetMetadata(GVariant *inArgs,
 virtDBusUtilSetLastVirtError(error);
 }
 
+static void
+virtDBusDomainSetPerfEvents(GVariant *inArgs,
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath,
+gpointer userData,
+GVariant **outArgs G_GNUC_UNUSED,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+g_autoptr(virDomain) domain = NULL;
+g_autoptr(GVariantIter) iter = NULL;
+g_auto(virtDBusUtilTypedParams) params = { 0 };
+guint flags;
+
+g_variant_get(inArgs, "(a{sv}u)", &iter, &flags);
+
+if (!virtDBusUtilGVariantToTypedParams(iter, ¶ms.params,
+   ¶ms.nparams, error)) {
+return;
+}
+
+domain = virtDBusDomainGetVirDomain(connect, objectPath, error);
+if (!domain)
+return;
+
+if (virDomainSetPerfEvents(domain, params.params, params.nparams, flags) < 
0)
+virtDBusUtilSetLastVirtError(error);
+}
+
 static void
 virtDBusDomainSetUserPassword(GVariant *inArgs,
   GUnixFDList *inFDs G_GNUC_UNUSED,
@@ -1930,6 +1960,7 @@ static virtDBusGDBusMethodTable 
virtDBusDomainMethodTable[] = {
 { "SetMemory", virtDBusDomainSetMemory },
 { "SetMemoryStatsPeriod", virtDBusDomainSetMemoryStatsPeriod },
 { "SetMetadata", virtDBusDomainSetMetadata },
+{ "SetPerfEvents", virtDBusDomainSetPerfEvents },
 { "SetTime", virtDBusDomainSetTime },
 { "SetUserPassword", virtDBusDomainSetUserPassword },
 { "Shutdown", virtDBusDomainShutdown },
-- 
2.14.3

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

[libvirt] [dbus PATCH 2/3] util: Introduce virtDBusUtilGVariantToTypedParams

[Pavel Hrdina]

This converts D-Bus dictionary into virTypedParameters.

Signed-off-by: Pavel Hrdina 
---
 src/util.c | 83 ++
 src/util.h |  6 +
 2 files changed, 89 insertions(+)

diff --git a/src/util.c b/src/util.c
index ac965c4..5a69c89 100644
--- a/src/util.c
+++ b/src/util.c
@@ -69,6 +69,89 @@ virtDBusUtilTypedParamsToGVariant(virTypedParameterPtr 
params,
 return g_variant_builder_end(&builder);
 }
 
+gboolean
+virtDBusUtilGVariantToTypedParams(GVariantIter *iter,
+  virTypedParameterPtr *params,
+  gint *nparams,
+  GError **error)
+{
+g_autofree gchar *name = NULL;
+g_autoptr(GVariant) value = NULL;
+gint maxParams = 0;
+
+while (g_variant_iter_loop(iter, "{sv}", &name, &value)) {
+const gchar *type = g_variant_get_type_string(value);
+
+switch (type[0]) {
+case 'i':
+if (virTypedParamsAddInt(params, nparams, &maxParams, name,
+ g_variant_get_int32(value)) < 0) {
+virtDBusUtilSetLastVirtError(error);
+return FALSE;
+}
+break;
+
+case 'u':
+if (virTypedParamsAddUInt(params, nparams, &maxParams, name,
+  g_variant_get_uint32(value)) < 0) {
+virtDBusUtilSetLastVirtError(error);
+return FALSE;
+}
+break;
+
+case 'x':
+if (virTypedParamsAddLLong(params, nparams, &maxParams, name,
+   g_variant_get_int64(value)) < 0) {
+virtDBusUtilSetLastVirtError(error);
+return FALSE;
+}
+break;
+
+case 't':
+if (virTypedParamsAddULLong(params, nparams, &maxParams, name,
+g_variant_get_uint64(value)) < 0) {
+virtDBusUtilSetLastVirtError(error);
+return FALSE;
+}
+break;
+
+case 'd':
+if (virTypedParamsAddDouble(params, nparams, &maxParams, name,
+g_variant_get_double(value)) < 0) {
+virtDBusUtilSetLastVirtError(error);
+return FALSE;
+}
+break;
+
+case 'b':
+if (virTypedParamsAddBoolean(params, nparams, &maxParams, name,
+ g_variant_get_boolean(value)) < 0) {
+virtDBusUtilSetLastVirtError(error);
+return FALSE;
+}
+break;
+
+case 's':
+if (virTypedParamsAddString(params, nparams, &maxParams, name,
+g_variant_get_string(value, NULL)) < 
0) {
+virtDBusUtilSetLastVirtError(error);
+return FALSE;
+}
+break;
+
+default:
+g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT,
+"Invalid typed parameter '%s'.", type);
+return FALSE;
+}
+}
+
+name = NULL;
+value = NULL;
+
+return TRUE;
+}
+
 void
 virtDBusUtilSetLastVirtError(GError **error)
 {
diff --git a/src/util.h b/src/util.h
index 4ffc15c..86d2edc 100644
--- a/src/util.h
+++ b/src/util.h
@@ -31,6 +31,12 @@ GVariant *
 virtDBusUtilTypedParamsToGVariant(virTypedParameterPtr params,
   gint nparams);
 
+gboolean
+virtDBusUtilGVariantToTypedParams(GVariantIter *iter,
+  virTypedParameterPtr *params,
+  gint *nparams,
+  GError **error);
+
 void
 virtDBusUtilSetLastVirtError(GError **error);
 
-- 
2.14.3

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

[libvirt] [dbus PATCH 1/3] util: Introduce virtDBusUtilTypedParams

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/util.c |  6 ++
 src/util.h | 11 +++
 2 files changed, 17 insertions(+)

diff --git a/src/util.c b/src/util.c
index b21f2c5..ac965c4 100644
--- a/src/util.c
+++ b/src/util.c
@@ -20,6 +20,12 @@ virtDBusErrorQuark(void)
 return (GQuark) quarkVolatile;
 }
 
+void
+virtDBusUtilTypedParamsClear(virtDBusUtilTypedParams *params)
+{
+virTypedParamsFree(params->params, params->nparams);
+}
+
 GVariant *
 virtDBusUtilTypedParamsToGVariant(virTypedParameterPtr params,
   gint nparams)
diff --git a/src/util.h b/src/util.h
index 7bb4ad6..4ffc15c 100644
--- a/src/util.h
+++ b/src/util.h
@@ -16,6 +16,17 @@ typedef enum {
 GQuark
 virtDBusErrorQuark(void);
 
+struct _virtDBusUtilTypedParams {
+virTypedParameterPtr params;
+gint nparams;
+};
+typedef struct _virtDBusUtilTypedParams virtDBusUtilTypedParams;
+
+void
+virtDBusUtilTypedParamsClear(virtDBusUtilTypedParams *params);
+
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(virtDBusUtilTypedParams, 
virtDBusUtilTypedParamsClear);
+
 GVariant *
 virtDBusUtilTypedParamsToGVariant(virTypedParameterPtr params,
   gint nparams);
-- 
2.14.3

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

[libvirt] [dbus PATCH] Fix GError memory leaks

[Pavel Hrdina]

Signed-off-by: Pavel Hrdina 
---
 src/gdbus.c | 8 
 src/main.c  | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/gdbus.c b/src/gdbus.c
index cb06813..3407116 100644
--- a/src/gdbus.c
+++ b/src/gdbus.c
@@ -86,7 +86,7 @@ virtDBusGDBusHandlePropertyGet(GVariant *parameters,
 const gchar *interface;
 const gchar *name;
 GVariant *value = NULL;
-GError *error = NULL;
+g_autoptr(GError) error = NULL;
 
 g_variant_get(parameters, "(&s&s)", &interface, &name);
 
@@ -128,7 +128,7 @@ virtDBusGDBusHandlePropertySet(GVariant *parameters,
 const gchar *interface;
 const gchar *name;
 g_autoptr(GVariant) value = NULL;
-GError *error = NULL;
+g_autoptr(GError) error = NULL;
 
 g_variant_get(parameters, "(&s&sv)", &interface, &name, &value);
 
@@ -162,7 +162,7 @@ virtDBusGDBusHandlePropertyGetAll(GDBusMethodInvocation 
*invocation,
 {
 GVariant *value;
 g_auto(GVariantBuilder) builder;
-GError *error = NULL;
+g_autoptr(GError) error = NULL;
 
 g_variant_builder_init(&builder, G_VARIANT_TYPE("(a{sv})"));
 
@@ -202,7 +202,7 @@ virtDBusGDBusHandleMethod(GVariant *parameters,
 GUnixFDList *inFDs = NULL;
 GVariant *outArgs = NULL;
 GUnixFDList *outFDs = NULL;
-GError *error = NULL;
+g_autoptr(GError) error = NULL;
 
 for (gint i = 0; data->methods[i].name; i++) {
 if (g_str_equal(methodName, data->methods[i].name)) {
diff --git a/src/main.c b/src/main.c
index b3baf76..eb30ef8 100644
--- a/src/main.c
+++ b/src/main.c
@@ -52,7 +52,7 @@ virtDBusAcquired(GDBusConnection *connection,
  gpointer opaque)
 {
 virtDBusRegisterData *data = opaque;
-GError *error = NULL;
+g_autoptr(GError) error = NULL;
 
 for (gsize i = 0; i < data->ndrivers; i += 1) {
 virtDBusConnectNew(&data->connectList[i], connection,
-- 
2.14.3

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

Re: [libvirt] [REPOST PATCH 2/2] vz: Use virDomainObjListFindBy{UUID|ID}Ref

[Nikolay Shirokovskiy]

On 20.04.2018 14:44, John Ferlan wrote:
> 
> 
> On 04/20/2018 02:53 AM, Nikolay Shirokovskiy wrote:
>>
>>
>> On 02.04.2018 16:21, John Ferlan wrote:
>>> For vzDomainLookupByID and vzDomainLookupByUUID let's
>>> return a locked and referenced @vm object so that callers
>>> can then use the common and more consistent virDomainObjEndAPI
>>> in order to handle cleanup rather than needing to know that the
>>> returned object is locked and calling virObjectUnlock.
>>>
>>> The LookupByName already returns the ref counted and locked object,
>>> so this will make things more consistent.
>>>
>>> Also adjust the prlsdkHandle{VmState|VmRemoved|Perf}Event APIs
>>> in the same manner.
>>>
>>> Signed-off-by: John Ferlan 
>>> ---
>>>  src/vz/vz_driver.c |  8 
>>>  src/vz/vz_sdk.c| 15 ---
>>>  2 files changed, 12 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
>>> index bcbccf6cc8..736424897a 100644
>>> --- a/src/vz/vz_driver.c
>>> +++ b/src/vz/vz_driver.c
>>> @@ -578,7 +578,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
>>>  virDomainPtr ret = NULL;
>>>  virDomainObjPtr dom;
>>>  
>>> -dom = virDomainObjListFindByID(privconn->driver->domains, id);
>>> +dom = virDomainObjListFindByIDRef(privconn->driver->domains, id);
>>>  
>>>  if (dom == NULL) {
>>>  virReportError(VIR_ERR_NO_DOMAIN, NULL);
>>> @@ -591,7 +591,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
>>>  ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id);
>>>  
>>>   cleanup:
>>> -virObjectUnlock(dom);
>>> +virDomainObjEndAPI(&dom);
>>>  return ret;
>>>  }
>>>  
>>> @@ -602,7 +602,7 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned 
>>> char *uuid)
>>>  virDomainPtr ret = NULL;
>>>  virDomainObjPtr dom;
>>>  
>>> -dom = virDomainObjListFindByUUID(privconn->driver->domains, uuid);
>>> +dom = virDomainObjListFindByUUIDRef(privconn->driver->domains, uuid);
>>>  
>>>  if (dom == NULL) {
>>>  char uuidstr[VIR_UUID_STRING_BUFLEN];
>>> @@ -618,7 +618,7 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned 
>>> char *uuid)
>>>  ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id);
>>>  
>>>   cleanup:
>>> -virObjectUnlock(dom);
>>> +virDomainObjEndAPI(&dom);
>>>  return ret;
>>>  }
>>>  
>>> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
>>> index a5b9f2da67..b8f13f88a7 100644
>>> --- a/src/vz/vz_sdk.c
>>> +++ b/src/vz/vz_sdk.c
>>> @@ -2144,7 +2144,7 @@ prlsdkHandleVmStateEvent(vzDriverPtr driver,
>>>  virDomainEventType lvEventType = 0;
>>>  int lvEventTypeDetails = 0;
>>>  
>>> -dom = virDomainObjListFindByUUID(driver->domains, uuid);
>>> +dom = virDomainObjListFindByUUIDRef(driver->domains, uuid);
>>>  if (dom == NULL)
>>>  return;
>>>  
>>> @@ -2166,7 +2166,7 @@ prlsdkHandleVmStateEvent(vzDriverPtr driver,
>>>  
>>>   cleanup:
>>>  PrlHandle_Free(eventParam);
>>> -virObjectUnlock(dom);
>>> +virObjectEndAPI(&dom);
>>
>> s/virObjectEndAPI/virDomainObjEndAPI/
>>
> 
> Oh right - it wasn't the first one of those too /-|... Just realized vz
> wasn't building on my host...
> 
>>>  return;
>>>  }
>>>  
>>> @@ -2225,7 +2225,7 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
>>>  {
>>>  virDomainObjPtr dom = NULL;
>>>  
>>> -dom = virDomainObjListFindByUUID(driver->domains, uuid);
>>> +dom = virDomainObjListFindByUUIDRef(driver->domains, uuid);
>>>  /* domain was removed from the list from the libvirt
>>>   * API function in current connection */
>>>  if (dom == NULL)
>>> @@ -2235,6 +2235,7 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
>>>  VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
>>>  
>>>  virDomainObjListRemove(driver->domains, dom);
>>> +virDomainObjEndAPI(&dom);
>>
>> virDomainObjListRemove unlocks dom object so we should only unref here.
>>
> 
> Actually, I'd prefer to follow what I've done elsewhere which is add the
> virObjectLock(dom); prior to the EndAPI call. Since we don't fail on our

Ok

> virObjectUnlock calls the fact that it wasn't locked before unlocking
> doesn't really register; however, upcoming changes to how *ListRemove

We use 'PTHREAD_MUTEX_NORMAL' mutexes and unlocking unlocked is undefined 
behaviour
for such mutexes...

> works will do the "right" and "expected" thing, so having all the
> current consumers be consistent is preferred.
> 
> Oh and by right and expected I mean since it receives a locked and
> reffed object, it should return a locked and reffed object to let the
> caller decide how to dispose.
> 
>>
>> Also I doubt of usefullness of using ref/end api in vz_sdk.c as these 
>> functions
>> are not API calls so it looks strange to see virDomainObjEndAPI name here.
>>
> 
> The long winded explanation just in case you've missed it in other
> related series...
> 
> In the long run virDomainObjEndAPI is just a name chosen long ago - i

Re: [libvirt] [PATCH jenkins-ci] Add missing job dependencies for libvirt-glib/virt-viewer mingw builds

[Andrea Bolognani]

On Fri, 2018-04-20 at 12:27 +0100, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  projects/libvirt-glib.yaml | 4 ++--
>  projects/virt-viewer.yaml  | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Whoops.

Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [RFC PATCH 22/30] tests: qemublock: Add tests for all other format without special options

[Kevin Wolf]

Am 19.04.2018 um 17:25 hat Peter Krempa geschrieben:
> Similarly to the 'raw' case add tests for bochs, cloop, dmg, ploop, vdi
> vhd, and vpc. Covering all supproted non-backing formats.
> 
> Note that the JSON name for 'ploop' maps to 'parallels' and 'vhd' maps
> to 'vhdx'.

Your -drive lines below mention format=ploop/vhd, though. That wouldn't
actually work.

(Also 'vhd' as an alias for 'vhdx' is super confusing, because VHD is
really the name of the format implemented by QEMU's 'vpc' driver - which
is already a source of confusion on its own.)

> Files added here would result in the followint configs:
> 
> file-bochs-noopts.xml:
> -drive file=/path/to/i.img,format=bochs,if=none,id=drive-dummy
> -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
> 
> file-cloop-noopts.xml:
> -drive file=/path/to/i.img,format=cloop,if=none,id=drive-dummy
> -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
> 
> file-dmg-noopts.xml:
> -drive file=/path/to/i.img,format=dmg,if=none,id=drive-dummy
> -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
> 
> file-ploop-noopts.xml:
> -drive file=/path/to/i.img,format=ploop,if=none,id=drive-dummy
> -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
> 
> file-vdi-noopts.xml:
> -drive file=/path/to/i.img,format=vdi,if=none,id=drive-dummy
> -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
> 
> file-vhd-noopts.xml:
> -drive file=/path/to/i.img,format=vhd,if=none,id=drive-dummy
> -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
> 
> file-vpc-noopts.xml:
> -drive file=/path/to/i.img,format=vpc,if=none,id=drive-dummy
> -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
> 
> Signed-off-by: Peter Krempa 

Some of the cases tested here or in later patches are for configurations
that wouldn't work, but I guess for a test of parsing and storing XML,
that doesn't matter.

Kevin

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

Re: [libvirt] [REPOST PATCH 2/2] vz: Use virDomainObjListFindBy{UUID|ID}Ref

[John Ferlan]

On 04/20/2018 02:53 AM, Nikolay Shirokovskiy wrote:
> 
> 
> On 02.04.2018 16:21, John Ferlan wrote:
>> For vzDomainLookupByID and vzDomainLookupByUUID let's
>> return a locked and referenced @vm object so that callers
>> can then use the common and more consistent virDomainObjEndAPI
>> in order to handle cleanup rather than needing to know that the
>> returned object is locked and calling virObjectUnlock.
>>
>> The LookupByName already returns the ref counted and locked object,
>> so this will make things more consistent.
>>
>> Also adjust the prlsdkHandle{VmState|VmRemoved|Perf}Event APIs
>> in the same manner.
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/vz/vz_driver.c |  8 
>>  src/vz/vz_sdk.c| 15 ---
>>  2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
>> index bcbccf6cc8..736424897a 100644
>> --- a/src/vz/vz_driver.c
>> +++ b/src/vz/vz_driver.c
>> @@ -578,7 +578,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
>>  virDomainPtr ret = NULL;
>>  virDomainObjPtr dom;
>>  
>> -dom = virDomainObjListFindByID(privconn->driver->domains, id);
>> +dom = virDomainObjListFindByIDRef(privconn->driver->domains, id);
>>  
>>  if (dom == NULL) {
>>  virReportError(VIR_ERR_NO_DOMAIN, NULL);
>> @@ -591,7 +591,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
>>  ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id);
>>  
>>   cleanup:
>> -virObjectUnlock(dom);
>> +virDomainObjEndAPI(&dom);
>>  return ret;
>>  }
>>  
>> @@ -602,7 +602,7 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned 
>> char *uuid)
>>  virDomainPtr ret = NULL;
>>  virDomainObjPtr dom;
>>  
>> -dom = virDomainObjListFindByUUID(privconn->driver->domains, uuid);
>> +dom = virDomainObjListFindByUUIDRef(privconn->driver->domains, uuid);
>>  
>>  if (dom == NULL) {
>>  char uuidstr[VIR_UUID_STRING_BUFLEN];
>> @@ -618,7 +618,7 @@ vzDomainLookupByUUID(virConnectPtr conn, const unsigned 
>> char *uuid)
>>  ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id);
>>  
>>   cleanup:
>> -virObjectUnlock(dom);
>> +virDomainObjEndAPI(&dom);
>>  return ret;
>>  }
>>  
>> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
>> index a5b9f2da67..b8f13f88a7 100644
>> --- a/src/vz/vz_sdk.c
>> +++ b/src/vz/vz_sdk.c
>> @@ -2144,7 +2144,7 @@ prlsdkHandleVmStateEvent(vzDriverPtr driver,
>>  virDomainEventType lvEventType = 0;
>>  int lvEventTypeDetails = 0;
>>  
>> -dom = virDomainObjListFindByUUID(driver->domains, uuid);
>> +dom = virDomainObjListFindByUUIDRef(driver->domains, uuid);
>>  if (dom == NULL)
>>  return;
>>  
>> @@ -2166,7 +2166,7 @@ prlsdkHandleVmStateEvent(vzDriverPtr driver,
>>  
>>   cleanup:
>>  PrlHandle_Free(eventParam);
>> -virObjectUnlock(dom);
>> +virObjectEndAPI(&dom);
> 
> s/virObjectEndAPI/virDomainObjEndAPI/
> 

Oh right - it wasn't the first one of those too /-|... Just realized vz
wasn't building on my host...

>>  return;
>>  }
>>  
>> @@ -2225,7 +2225,7 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
>>  {
>>  virDomainObjPtr dom = NULL;
>>  
>> -dom = virDomainObjListFindByUUID(driver->domains, uuid);
>> +dom = virDomainObjListFindByUUIDRef(driver->domains, uuid);
>>  /* domain was removed from the list from the libvirt
>>   * API function in current connection */
>>  if (dom == NULL)
>> @@ -2235,6 +2235,7 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
>>  VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
>>  
>>  virDomainObjListRemove(driver->domains, dom);
>> +virDomainObjEndAPI(&dom);
> 
> virDomainObjListRemove unlocks dom object so we should only unref here.
> 

Actually, I'd prefer to follow what I've done elsewhere which is add the
virObjectLock(dom); prior to the EndAPI call. Since we don't fail on our
virObjectUnlock calls the fact that it wasn't locked before unlocking
doesn't really register; however, upcoming changes to how *ListRemove
works will do the "right" and "expected" thing, so having all the
current consumers be consistent is preferred.

Oh and by right and expected I mean since it receives a locked and
reffed object, it should return a locked and reffed object to let the
caller decide how to dispose.

> 
> Also I doubt of usefullness of using ref/end api in vz_sdk.c as these 
> functions
> are not API calls so it looks strange to see virDomainObjEndAPI name here.
> 

The long winded explanation just in case you've missed it in other
related series...

In the long run virDomainObjEndAPI is just a name chosen long ago - it
ends up being the complementary call for the *FindBy* and *ListAdd
calls. In the long run the *EndAPI just Unlocks and Unrefs the object.

The *FindBy* calls were "inconsistent" w/r/t *Name always returned
locked/reffed objects, while FindBy{UUID|ID} returned unreffed, but
locked while the FindBy{UUID

Re: [libvirt] [dbus PATCH 7/8] Implement GetCPUModelNames method for Connect Interface

[Ján Tomko]

On Thu, Apr 19, 2018 at 03:58:06PM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou 
---
data/org.libvirt.Connect.xml |  7 +++
src/connect.c| 35 +++
tests/test_connect.py|  4 
3 files changed, 46 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index 3ef965d..43a6241 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -106,6 +106,13 @@
value="See 
https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetCapabilities"/>
  

+
+  https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetCPUModelNames"/>
+  
+  
+  
+

  https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetSysinfo"/>
diff --git a/src/connect.c b/src/connect.c
index e774fa4..169a070 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -542,6 +542,40 @@ virtDBusConnectGetAllDomainStats(GVariant *inArgs,
*outArgs = g_variant_new_tuple(&gret, 1);
}

+static void
+virtDBusConnectGetCPUModelNames(GVariant *inArgs,
+GUnixFDList *inFDs G_GNUC_UNUSED,
+const gchar *objectPath G_GNUC_UNUSED,
+gpointer userData,
+GVariant **outArgs,
+GUnixFDList **outFDs G_GNUC_UNUSED,
+GError **error)
+{
+virtDBusConnect *connect = userData;
+const gchar *arch;
+guint flags;
+g_autoptr(GCharArray) models = NULL;


This will need to be changed.


+gint nmodels;
+GVariant *gret;
+GVariantBuilder builder;
+
+g_variant_get(inArgs, "(&su)", &arch, &flags);
+
+if (!virtDBusConnectOpen(connect, error))
+return;
+
+nmodels = virConnectGetCPUModelNames(connect->connection, arch, &models, 
flags);


The API documentation also states:
Pass NULL if only the list length is needed.
But that does not seem like useful functionality.


+if (nmodels < 0)
+return virtDBusUtilSetLastVirtError(error);
+
+g_variant_builder_init(&builder, G_VARIANT_TYPE("as"));
+for (gint i = 0; i < nmodels; i++)
+g_variant_builder_add(&builder, "s", models[i]);
+gret = g_variant_builder_end(&builder);
+
+*outArgs = g_variant_new_tuple(&gret, 1);
+}
+


Reviewed-by: Ján Tomko 

Jano


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

[libvirt] [PATCH] remote: remove obsolete & incorrect comment from libvirtd.conf

[Daniel P . Berrangé]

The libvirtd.conf file has a comment pointing people to format.html
which has nothing todo with the configuration file format.

It also has a comment about tests/daemon-conf which no longer exists,
and even if it did exist such comment is not relevant to end users
when this file is installed in /etc/.

Signed-off-by: Daniel P. Berrangé 
---
 src/remote/libvirtd.conf | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/remote/libvirtd.conf b/src/remote/libvirtd.conf
index 7d823cf1ad..28df4021aa 100644
--- a/src/remote/libvirtd.conf
+++ b/src/remote/libvirtd.conf
@@ -1,10 +1,5 @@
 # Master libvirt daemon configuration file
 #
-# For further information consult https://libvirt.org/format.html
-#
-# NOTE: the tests/daemon-conf regression test script requires
-# that each "PARAMETER = VALUE" line in this file have the parameter
-# name just after a leading "#".
 
 #
 #
-- 
2.14.3

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

[libvirt] [PATCH jenkins-ci] Add missing job dependencies for libvirt-glib/virt-viewer mingw builds

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 projects/libvirt-glib.yaml | 4 ++--
 projects/virt-viewer.yaml  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/projects/libvirt-glib.yaml b/projects/libvirt-glib.yaml
index 2d0e721..286d25b 100644
--- a/projects/libvirt-glib.yaml
+++ b/projects/libvirt-glib.yaml
@@ -15,13 +15,13 @@
   parent_jobs: 'libvirt-glib-master-check'
   machines: '{rpm_machines}'
   - autotools-build-job:
-  parent_jobs:
+  parent_jobs: 'libvirt-master-build-mingw32'
   variant: -mingw32
   local_env: '{mingw32_local_env}'
   autogen_args: '{mingw32_autogen_args}'
   machines: '{mingw_machines}'
   - autotools-build-job:
-  parent_jobs:
+  parent_jobs: 'libvirt-master-build-mingw64'
   variant: -mingw64
   local_env: '{mingw64_local_env}'
   autogen_args: '{mingw64_autogen_args}'
diff --git a/projects/virt-viewer.yaml b/projects/virt-viewer.yaml
index 3726cd0..633ac9a 100644
--- a/projects/virt-viewer.yaml
+++ b/projects/virt-viewer.yaml
@@ -14,13 +14,13 @@
   parent_jobs: 'virt-viewer-master-check'
   machines: '{rpm_machines}'
   - autotools-build-job:
-  parent_jobs:
+  parent_jobs: 'libvirt-glib-master-build-mingw32'
   variant: -mingw32
   local_env: '{mingw32_local_env}'
   autogen_args: '{mingw32_autogen_args}'
   machines: '{mingw_machines}'
   - autotools-build-job:
-  parent_jobs:
+  parent_jobs: 'libvirt-glib-master-build-mingw64'
   variant: -mingw64
   local_env: '{mingw64_local_env}'
   autogen_args: '{mingw64_autogen_args}'
-- 
2.14.3

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

[libvirt] [PATCH] remote: always build generated source files

[Daniel P . Berrangé]

The generated source files for dispatching libvirtd RPC messages contain
translations and are thus listed in POTFILES. This means they are
required in order to build libvirt.pot. Rather than changing the files
that go into libvirt.pot dynamically, just unconditionally build the
remote driver sources so they are always available for building
libvirt.pot. This ensures we don't silently loose translation messages
based on configure args.

This fixes the mingw build which needs to create libvirt.pot but has
libvirtd disabled.

Signed-off-by: Daniel P. Berrangé 
---

Pushed to fix mingw build

 src/remote/Makefile.inc.am | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am
index 12600b8bb5..eb8d6feb31 100644
--- a/src/remote/Makefile.inc.am
+++ b/src/remote/Makefile.inc.am
@@ -81,6 +81,17 @@ EXTRA_DIST += \
remote/libvirtd.sysctl \
$(NULL)
 
+# Needed to build libvirt.pot, so must be listed outside
+# the WITH_REMOTE/WITH_LIBVIRTD conditionals
+BUILT_SOURCES += \
+   $(REMOTE_DRIVER_GENERATED) \
+   $(LIBVIRTD_GENERATED) \
+   $(NULL)
+MAINTAINERCLEANFILES += \
+   $(REMOTE_DRIVER_GENERATED) \
+   $(LIBVIRTD_GENERATED) \
+   $(NULL)
+
 if WITH_REMOTE
 noinst_LTLIBRARIES += libvirt_driver_remote.la
 libvirt_la_BUILT_LIBADD += libvirt_driver_remote.la
@@ -93,9 +104,6 @@ libvirt_driver_remote_la_CFLAGS = \
 libvirt_driver_remote_la_LDFLAGS = $(AM_LDFLAGS)
 libvirt_driver_remote_la_SOURCES = $(REMOTE_DRIVER_SOURCES)
 
-BUILT_SOURCES += $(REMOTE_DRIVER_GENERATED)
-MAINTAINERCLEANFILES += $(REMOTE_DRIVER_GENERATED)
-
 endif WITH_REMOTE
 
 if WITH_REMOTE
@@ -108,9 +116,6 @@ if WITH_LIBVIRTD
 
 sbin_PROGRAMS += libvirtd
 
-BUILT_SOURCES += $(LIBVIRTD_GENERATED)
-MAINTAINERCLEANFILES += $(LIBVIRTD_GENERATED)
-
 augeas_DATA += remote/libvirtd.aug
 
 augeastest_DATA += test_libvirtd.aug
-- 
2.14.3

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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Gerd Hoffmann]

> > > Since your schema is likely to end up just being a file in docs/specs,

> > I think it would be useful to have this as part of the schema.  Should
> > be easy then to write up a small utility then which takes a json file as
> > input and translates that into qemu command line options.
> 
> Currently we have two distinct QAPI schemas, one covering the system
> emulator for QMP and subset of CLI args, and one covering the guest
> agent.

Yep, we can make it a third one, fine with me.
But it likewise wouldn't end up in docs/ then.

cheers,
  Gerd

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

Re: [libvirt] [PATCH] git: add config file telling git-publish how to send patches

[Daniel P . Berrangé]

On Fri, Apr 20, 2018 at 12:22:35PM +0200, Martin Kletzander wrote:
> On Thu, Apr 19, 2018 at 05:29:22PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Apr 19, 2018 at 06:11:20PM +0200, Martin Kletzander wrote:
> > > On Thu, Apr 19, 2018 at 10:06:45AM +0100, Daniel P. Berrangé wrote:
> > > > On Thu, Apr 19, 2018 at 10:55:01AM +0200, Peter Krempa wrote:
> > > > > On Wed, Apr 18, 2018 at 18:35:18 +0100, Daniel Berrange wrote:
> > > > > > The "git-publish" tool is a useful git extension for sending patch
> > > > > > series for code review. It automatically creates versioned tags
> > > > > > each time code on a branch is sent, so that there is a record of
> > > > > > each version. It also remembers the cover letter so it does not
> > > > > > need re-entering each time the series is reposted.
> > > > > >
> > > > > > With this config file present it is now sufficient[1] to run
> > > > > >
> > > > > >   $ git publish
> > > > > >
> > > > > > to send all patches in a branch to the list for review
> > > > > >
> > > > > > [1] Assuming your $HOME/.gitconfig has an SMTP server listed
> > > > > > at least e.g.
> > > > > >
> > > > > >[sendemail]
> > > > > > smtpserver = smtp.example.com
> > > > > >
> > > > > > Signed-off-by: Daniel P. Berrangé 
> > > > > > ---
> > > > > >  .gitpublish | 3 +++
> > > > > >  1 file changed, 3 insertions(+)
> > > > > >  create mode 100644 .gitpublish
> > > > > >
> > > > > > diff --git a/.gitpublish b/.gitpublish
> > > > > > new file mode 100644
> > > > > > index 00..857f0d552c
> > > > > > --- /dev/null
> > > > > > +++ b/.gitpublish
> > > > > > @@ -0,0 +1,3 @@
> > > > > > +[gitpublishprofile "default"]
> > > > > > +base = master
> > > > > > +to = libvir-list@redhat.com
> > > > >
> > > > > ACK
> > > > >
> > > > > As a side-question. Does git-publish have the option to send 
> > > > > GPG-signed
> > > > > mails? I always wanted that, but not enough to hack it into
> > > > > git-send-email.
> > > >
> > > > Not that I'm aware of. The only common use of GPG is for signing tags
> > > > associated with pull requests.  eg when you do  git publish --pull,
> > > > it will create a signed tag for that pull request. We don't use the
> > > > submaintainer model though so that's not really relevant to us.
> > > >
> > > > Stefan takes features requests / patches on github/stefanha/git-publish
> > > > though :-)
> > > >
> > > 
> > > Yeah, I always wondered how to do it nicely.  I have signed commits 
> > > turned on,
> > > but if I take a commit that is even verified by github, (and looks great 
> > > with
> > > `git show --show-signature`), I have no way how to send them using 
> > > send-email so
> > > that the signing is preserved.  Would be nice to know if there's a way.
> > 
> > Based on what I've read, I don't think signatures can work with an email
> > based patch workflow, nor with a fast-forward based process.
> > 
> > If someone signs a patch, IIUC that covers the body, the comit message, and
> > all the metadata including committer and author fields and parent commit
> > hash. When you send a patch and then a reviewer applies the patch, the
> > committer field gets changed to name of the person applying it. The parent
> > commit hash also gets changed to whatever is HEAD when you apply it. Both
> > these changes neccessarily invalidate any signature that could be attached
> > to the mail.
> > 
> > The only way I can see signatures working is if you exchange patches using
> > "git bundle" so you're sending the full git object, not just a patch file.
> > This will neccessitate you using merge commits in the repository, not
> > fast-forward.
> > 
> > So commit signatures look unusable for a development model based around
> > exchange of patches on the list as libvirt does.
> > 
> > They could be used to some extent with a sub-maintainer model, but only
> > the sub-maintainers would be able to sign commits, for those going into
> > the pull request. Individual contributors still wouldn't be able to sign
> > in general since sub-maintainers just pull patches off the mailing list
> > and git am them - only the final pull requests as done with merge commits.
> > 
> 
> Sure, I never meant the commits should stay signed.  It doesn't really
> make much sense in most scenarios to have individual commits signed.
> Signed tag is an inherent sign for all parent commits, which is more
> than enough.  What I wanted to have established was a way for the people
> receiving the commit to be able to see that it really came from me and
> it was not tampered with.  For that case I only care about the medium
> (through which the commit is sent) to be covered.  That's the only part
> of the chain where I'm missing some effort to enhance integrity and
> accountability.

Oh so you don't really need git commit signing for that then. Essentially
you can just git-format the patches and do normal "email" signing when
sending them, so the receipient verifies the email, rather than the commit
patch. That sounds 

Re: [libvirt] [PATCH] git: add config file telling git-publish how to send patches

[Martin Kletzander]

On Thu, Apr 19, 2018 at 05:29:22PM +0100, Daniel P. Berrangé wrote:

On Thu, Apr 19, 2018 at 06:11:20PM +0200, Martin Kletzander wrote:

On Thu, Apr 19, 2018 at 10:06:45AM +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 19, 2018 at 10:55:01AM +0200, Peter Krempa wrote:
> > On Wed, Apr 18, 2018 at 18:35:18 +0100, Daniel Berrange wrote:
> > > The "git-publish" tool is a useful git extension for sending patch
> > > series for code review. It automatically creates versioned tags
> > > each time code on a branch is sent, so that there is a record of
> > > each version. It also remembers the cover letter so it does not
> > > need re-entering each time the series is reposted.
> > >
> > > With this config file present it is now sufficient[1] to run
> > >
> > >   $ git publish
> > >
> > > to send all patches in a branch to the list for review
> > >
> > > [1] Assuming your $HOME/.gitconfig has an SMTP server listed
> > > at least e.g.
> > >
> > >[sendemail]
> > > smtpserver = smtp.example.com
> > >
> > > Signed-off-by: Daniel P. Berrangé 
> > > ---
> > >  .gitpublish | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >  create mode 100644 .gitpublish
> > >
> > > diff --git a/.gitpublish b/.gitpublish
> > > new file mode 100644
> > > index 00..857f0d552c
> > > --- /dev/null
> > > +++ b/.gitpublish
> > > @@ -0,0 +1,3 @@
> > > +[gitpublishprofile "default"]
> > > +base = master
> > > +to = libvir-list@redhat.com
> >
> > ACK
> >
> > As a side-question. Does git-publish have the option to send GPG-signed
> > mails? I always wanted that, but not enough to hack it into
> > git-send-email.
>
> Not that I'm aware of. The only common use of GPG is for signing tags
> associated with pull requests.  eg when you do  git publish --pull,
> it will create a signed tag for that pull request. We don't use the
> submaintainer model though so that's not really relevant to us.
>
> Stefan takes features requests / patches on github/stefanha/git-publish
> though :-)
>

Yeah, I always wondered how to do it nicely.  I have signed commits turned on,
but if I take a commit that is even verified by github, (and looks great with
`git show --show-signature`), I have no way how to send them using send-email so
that the signing is preserved.  Would be nice to know if there's a way.


Based on what I've read, I don't think signatures can work with an email
based patch workflow, nor with a fast-forward based process.

If someone signs a patch, IIUC that covers the body, the comit message, and
all the metadata including committer and author fields and parent commit
hash. When you send a patch and then a reviewer applies the patch, the
committer field gets changed to name of the person applying it. The parent
commit hash also gets changed to whatever is HEAD when you apply it. Both
these changes neccessarily invalidate any signature that could be attached
to the mail.

The only way I can see signatures working is if you exchange patches using
"git bundle" so you're sending the full git object, not just a patch file.
This will neccessitate you using merge commits in the repository, not
fast-forward.

So commit signatures look unusable for a development model based around
exchange of patches on the list as libvirt does.

They could be used to some extent with a sub-maintainer model, but only
the sub-maintainers would be able to sign commits, for those going into
the pull request. Individual contributors still wouldn't be able to sign
in general since sub-maintainers just pull patches off the mailing list
and git am them - only the final pull requests as done with merge commits.



Sure, I never meant the commits should stay signed.  It doesn't really
make much sense in most scenarios to have individual commits signed.
Signed tag is an inherent sign for all parent commits, which is more
than enough.  What I wanted to have established was a way for the people
receiving the commit to be able to see that it really came from me and
it was not tampered with.  For that case I only care about the medium
(through which the commit is sent) to be covered.  That's the only part
of the chain where I'm missing some effort to enhance integrity and
accountability.

Martin


Regards,
Daniel
--
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|


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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Fri, Apr 20, 2018 at 11:46:24AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > Since your schema is likely to end up just being a file in docs/specs,
> > rather than directly part of our existnig qapi schema, I suggest we just
> > ignore whats there. Just define an arch enum in your spec which is right,
> > and let someone else worry about fixing the mess
> 
> I think it would be useful to have this as part of the schema.  Should
> be easy then to write up a small utility then which takes a json file as
> input and translates that into qemu command line options.

Currently we have two distinct QAPI schemas, one covering the system
emulator for QMP and subset of CLI args, and one covering the guest
agent.


This new schema isn't mapping to anything in QMP / CLI right now though.
It can be used to decide how to generate CLI args, but it isn't describing
those CLI args. So to me this is a 3rd distinct schema.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Gerd Hoffmann]

  Hi,

> Since your schema is likely to end up just being a file in docs/specs,
> rather than directly part of our existnig qapi schema, I suggest we just
> ignore whats there. Just define an arch enum in your spec which is right,
> and let someone else worry about fixing the mess

I think it would be useful to have this as part of the schema.  Should
be easy then to write up a small utility then which takes a json file as
input and translates that into qemu command line options.

cheers,
  Gerd

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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Daniel P . Berrangé]

On Fri, Apr 20, 2018 at 10:11:08AM +0200, Laszlo Ersek wrote:
> On 04/19/18 11:12, Daniel P. Berrangé wrote:
> > On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote:
> >> On 04/19/18 09:56, Daniel P. Berrangé wrote:
> >>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote:
>  Laszlo Ersek  writes:
> 
> > On 04/18/18 10:47, Markus Armbruster wrote:
> >> Laszlo Ersek  writes:
>  Replacing CpuInfoArch by such an enum will change the discriminator
>  value from "other" to the real architecture, with the obvious
>  compatibility concerns.  But we've accepted similar changes twice
>  already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.
> 
>  "other" was a bad idea.  Hindsight 20/20.
> 
>  Getting rid of it in one go rather than piecemeal seems like the least
>  bad way out.  Too late for 2.12, though.  Eric, what do you think?
> >>>
> >>> Given the context in which this "other" value is used, I think it is
> >>> reasonable to kill it and put a full arch list in there.
> >>>
> >>> No app is likely to be accessing the struct under "other" because it
> >>> is just an empty placeholder.
> >>
> >> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the
> >> potential to confuse QMP clients that didn't expect "s390", but
> >> otherwise it didn't mess with preexistent enum values / structures.
> > 
> > NB, qemu-system-s390x would previously have returned "other" in
> > this field, and now it returns "s390".  So while it didn't
> > remove "other" from the list of things that could potentially
> > exist, it did change what the s390x binary will actually report.
> > 
> >> The same applies to commit 25fa194b7b1, just with "riscv" /
> >> "CpuInfoRISCV" substituted.
> >>
> >> Removing "other" might confuse QMP clients that expect it, except
> >> (according to Daniel) no such client exists, probably.
> > 
> > When I say removing "other", I imply that we add an explicit arch
> > for all those which we currently are missing. IOW, all qemu-system-XXX
> > binaries which currently report "other" would change to report their
> > respective "XXX" values.
> > 
> > So in this way, it is exactly the same as what we did when we
> > introduced "s390" as an option.
> > 
> > The only difference is that once we have every binary reporting the
> > correct arch, we can now also remove "other" from the schema itself
> > as it will then be unused.
> 
> Can we please translate this into more actionable items for me, because
> I'm getting confused :)
> 
> First, if I add "i386" and "x86_64" to the enum list, we'll have all
> three of "i386", "x86_64" and "x86". Is that useful? How will that work?

Hmm, yes, on closer look this is a big mess as it is. We've been using
generic terms for covering multiple architectures :-(  'x86' for both
i386 and x86_64,  'sparc' for sparc and sparc64, etc. If we try to fix
that we'll be entering a world of backcompat hurt :-(

Since your schema is likely to end up just being a file in docs/specs,
rather than directly part of our existnig qapi schema, I suggest we just
ignore whats there. Just define an arch enum in your spec which is right,
and let someone else worry about fixing the mess

> Second, assuming I add constants for the ~10 (?) softmmu arches, can I
> still use @CpuInfoOther as the type for the corresponding new members in
> @CpuInfo? What C code changes will be necessary?

Yes, we could still use the CpuInfoOther struct, since struct names are
invisible to consumers, but as above, lets ignore the mess

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 2/2] storage: Check qemu-img encryption type capability

[Daniel P . Berrangé]

On Thu, Apr 19, 2018 at 02:21:43PM -0400, John Ferlan wrote:
> 
> [...]
> 
> >> Oh, OK - well I didn't find that to be obvious... So there is a way
> >> using secret objects to create a qcow[2] encrypted volume?
> > 
> > Sure, the exact same syntax as with luks volumes - you just specify
> > "qcow" instead of "luks" as the type.
> > 
> 
> So I've been working on doing as suggested, there's slight differences
> for qcow:

Sorry, when it say it is the same as luks volumes, i mean qcow encryption
in qcow2, is the same syntax as luks encryption in qcow2. I didn't mean
to refer to luks on raw, which does have different syntax.

> 
>1. Usage of "encrypt.key-secret" instead of just "key-secret"
>2. Usage of "encrypt.format=aes" (or qcow works too) instead of
> "encryption=on"
>3. Don't need change the "type" value like is done for "luks" in
> virStorageBackendCreateQemuImgCmdFromVol.
> 
> The result is (testing mode only):
> 
>qemu-img create -f qcow2 \
>   --object secret,id=x0,format=raw,data=letmein \
>   -o encrypt.format=aes,encrypt.key-secret=x0 \
>   x0.img 1048576K

Yes, that is right.

> 
> NB: Using "-b /dev/null" and ",backing_fmt=raw" works just fine as would
> usage of other -o options such as "compat=1.1,", "compat=0.10",
> "lazy_refcounts,", and "preallocation={off|metadata|falloc|full}".
> 
> 
> However, storagevolxml2argvtest.c also generates qemu-img convert
> options.  And this is where things go down hill. The former commands
> would for example do:

[snip]

Ok, yes, the convert command gets difficult due to qemu code limitations.

Essentially the problem is that 'convert' has to do two jobs with the
target file, create it and then open it to write data.

Unfortunately you need two incompatible syntaxes for these jobs :-(

When creating the file, the filename must be a plain name, and any
options given via  '-o', but to open the file, the filename must be
in the dotted syntax with options inline. There is no way to convert
between these two syntaxes in QEMU.

Essentially you can't run the convert command as you are describing.

All is not lost though, you simply have to turn it into a two stage
process. First, use 'qemu-img create' to create the target image with
the right size, as we already have code todo.

Then you pass the '-n' flag to 'convert' which tells it that the image
has been pre-created, so it can skip the create step. Now it only has
one job todo, which is to open the target image, so you can use the
dotted filename syntax.

Note, you must pass the '--image-opts' flag to tell it that the source
filename is using dotted syntax, and also pass '--target-image-opts'
to tell it the target filanem is using dotted syntax.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v2 4/4] vmware: Use virDomainObjListFindBy{UUID|ID}Ref

[Pavel Hrdina]

On Mon, Apr 02, 2018 at 09:16:40AM -0400, John Ferlan wrote:
> For vmwareDomObjFromDomainLocked and vmwareDomainLookupByID
> let's return a locked and referenced @vm object so that callers
> can then use the common and more consistent virDomainObjEndAPI
> in order to handle cleanup rather than needing to know that the
> returned object is locked and calling virObjectUnlock.
> 
> The LookupByName already returns the ref counted and locked object,
> so this will make things more consistent.
> 
> For vmwareDomainUndefineFlags and vmwareDomainShutdownFlags since
> virDomainObjListRemove will return an unlocked object, we need to
> relock before making the EndAPI call.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/vmware/vmware_driver.c | 53 
> +-
>  1 file changed, 19 insertions(+), 34 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [RFC PATCH 05/30] qemu: block: Extract formatting of props for 'file' backend

[Kevin Wolf]

Am 20.04.2018 um 10:50 hat Peter Krempa geschrieben:
> On Fri, Apr 20, 2018 at 10:41:50 +0200, Kevin Wolf wrote:
> > Am 20.04.2018 um 09:56 hat Peter Krempa geschrieben:
> > > Is there a special need to use 'host_cdrom' explicitly if the CDROM
> > > drive is used? That would complicate things since we don't know when
> > > that will happen.
> > 
> > You don't get the full CD-ROM passthrough functionality with
> > host_device. Specifically, if you eject the virtual drive, the physical
> > drive will only be ejected with host_cdrom, and the same is true for
> > lock_medium.
> > 
> > I think we may also make an attempt at detecting physical media change
> > ocassionally with host_cdrom, but I doubt that this is working reliably
> > anyway (the driver function is .bdrv_is_inserted, so it would be polling
> > rather than processing an event; and I'm not sure when it's called).
> 
> Hmm, okay, it seems that we in fact should use it. Is there any drawback
> if host_cdrom is used with a device which is not a CDROM? (e.g. a
> logical volume containing the image?)
> 
> Basically the question is whether we need to add some "smart" handling
> or we can just assume that if the guest device is a block-backed CDROM
> we can use that.

Hm, looking at the code, it seems that failure is silently ignored in
.bdrv_eject and .bdrv_lock_medium, but .bdrv_is_inserted is implemented
like this:

ret = ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
return ret == CDS_DISC_OK;

Which probably means that a block device not supporting that ioctl won't
be functional at all because the block layer will return -ENOMEDIUM
almost everywhere.

Kevin


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

[libvirt] [PATCH v4 5/5] qemu: Introduce memoryBacking/discard

[Michal Privoznik]

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

QEMU has this new feature memory-backend-file.discard-data=yes
which is a nifty optimization. Basically, when qemu is quitting
or on memory hotplug it calls munmap() and close() on the file
that is backing the memory. However, this does not mean kernel
won't stop touching that part of memory. It still might. With
this feature enabled we tell kernel: "we don't need this memory
nor data stored in it". This makes kernel drop the memory
immediately without trying to sync memory with the mapped file.

Unfortunately, this cannot be turned on by default because we
can't be sure when users really don't care about what happens to
data after qemu dies. So it has to be opt-in. As usual, there are
three places where one can configure memory attributes. This
patch adds the feature to all of them.

Signed-off-by: Michal Privoznik 
---
 docs/formatdomain.html.in| 34 ++--
 docs/schemas/cputypes.rng|  5 
 docs/schemas/domaincommon.rng| 10 +++
 src/conf/domain_conf.c   | 39 ++--
 src/conf/domain_conf.h   |  3 +++
 src/conf/numa_conf.c | 27 +++
 src/conf/numa_conf.h |  3 +++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  | 27 ---
 tests/qemuxml2argvdata/hugepages-pages7.args |  3 ++-
 tests/qemuxml2argvdata/hugepages-pages7.xml  |  4 +--
 tests/qemuxml2argvtest.c |  3 ++-
 12 files changed, 148 insertions(+), 11 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ada0df227f..ea9d77bd18 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1016,6 +1016,7 @@
 
 
 
+
   
   ...
 
@@ -1070,6 +1071,14 @@
  numa node by memAccess
allocation
Specify when allocate the memory
+   discard
+   When set and supported by hypervisor the memory
+ content is discarded just before guest shuts down (or
+ when DIMM module is unplugged). Please note that this is
+ just an optimization and is not guaranteed to work in
+ all cases (e.g. when hypervisor crashes).
+ Since 4.3.0 (QEMU/KVM only)
+   
 
 
 
@@ -1608,7 +1617,7 @@
 
   ...
   
-
+
 
   
   ...
@@ -1634,6 +1643,13 @@
   memAccess can control whether the memory is to be
   mapped as "shared" or "private".  This is valid only for
   hugepages-backed memory and nvdimm modules.
+
+  Each cell element can have an optional
+  discard attribute which fine tunes the discard
+  feature for given numa node as described under
+  Memory Backing.
+  Accepted values are yes and no.
+  Since 4.3.0
 
 
 
@@ -7849,7 +7865,7 @@ qemu-kvm -net nic,model=? /dev/null
 
 ...
 
-  
+  
 
   524287
   0
@@ -7903,6 +7919,20 @@ qemu-kvm -net nic,model=? /dev/null
 
   
 
+  discard
+  
+
+  An optional attribute discard
+  (since 4.3.0) that provides
+  capability to fine tune discard of data on per module
+  basis. Accepted values are yes and
+  no. The feature is described here:
+  Memory Backing.
+  This attribute is allowed only for
+  model='dimm'.
+
+  
+
   source
   
 
diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng
index c45b6dfb28..1f1e0e36d5 100644
--- a/docs/schemas/cputypes.rng
+++ b/docs/schemas/cputypes.rng
@@ -129,6 +129,11 @@
   
 
   
+  
+
+  
+
+  
   
 
   
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4cab55f05d..9650a779b7 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -633,6 +633,11 @@
 
   
 
+
+  
+
+  
+
   
 
   
@@ -5138,6 +5143,11 @@
   
 
   
+  
+
+  
+
+  
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35666c1347..9585e38bc1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5508,6 +5508,20 @@ virDomainVideoDefValidate(const v

[libvirt] [PATCH v4 1/5] qemuMonitorJSONGetDeviceProps: Separate props processing

[Michal Privoznik]

The code that processes list of device properties is going to be
reused. Therefore put it into a separate function.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 59 +++-
 1 file changed, 36 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 24d37eb41d..95b9d60aff 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6053,35 +6053,19 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
 #undef MAKE_SET_CMD
 
 
-int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
-  const char *device,
-  char ***props)
+static int
+qemuMonitorJSONParsePropsList(virJSONValuePtr cmd,
+  virJSONValuePtr reply,
+  char ***props)
 {
-int ret = -1;
-virJSONValuePtr cmd;
-virJSONValuePtr reply = NULL;
 virJSONValuePtr data;
 char **proplist = NULL;
 ssize_t n = 0;
 size_t i;
-
-*props = NULL;
-
-if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
-   "s:typename", device,
-   NULL)))
-return -1;
-
-if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
-goto cleanup;
-
-if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
-ret = 0;
-goto cleanup;
-}
+int ret = -1;
 
 if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
-goto cleanup;
+return ret;
 
 data = virJSONValueObjectGetArray(reply, "return");
 n = virJSONValueArraySize(data);
@@ -6110,8 +6094,37 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
 
  cleanup:
 virStringListFree(proplist);
-virJSONValueFree(cmd);
+return ret;
+}
+
+
+int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
+  const char *device,
+  char ***props)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+*props = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
+   "s:typename", device,
+   NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+goto cleanup;
+
+if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+ret = 0;
+goto cleanup;
+}
+
+ret = qemuMonitorJSONParsePropsList(cmd, reply, props);
+ cleanup:
 virJSONValueFree(reply);
+virJSONValueFree(cmd);
 return ret;
 }
 
-- 
2.16.1

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

[libvirt] [PATCH v4 0/5] Enable memory-backend-file.discard-data

[Michal Privoznik]

v4 of:
https://www.redhat.com/archives/libvir-list/2018-April/msg01897.html

diff to v3:
- patches 1 and 2 from the original set are pushed,
- patch 3 is split into two (1 and 2 in this series),
- Introduced new QEMU_CAPS_QOM_LIST_PROPERTIES
- virQEMUCapsProbeQMPGenericProps is called iff the capability is set

Michal Privoznik (5):
  qemuMonitorJSONGetDeviceProps: Separate props processing
  qemu_monitor: Introduce qemuMonitorGetObjectProps
  qemu_capabilities: Introduce QEMU_CAPS_QOM_LIST_PROPERTIES
  qemu_capabilities: Introduce QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD
  qemu: Introduce memoryBacking/discard

 docs/formatdomain.html.in  |   34 +-
 docs/schemas/cputypes.rng  |5 +
 docs/schemas/domaincommon.rng  |   10 +
 src/conf/domain_conf.c |   39 +-
 src/conf/domain_conf.h |3 +
 src/conf/numa_conf.c   |   27 +
 src/conf/numa_conf.h   |3 +
 src/libvirt_private.syms   |1 +
 src/qemu/qemu_capabilities.c   |   20 +
 src/qemu/qemu_capabilities.h   |2 +
 src/qemu/qemu_command.c|   27 +-
 src/qemu/qemu_monitor.c|   13 +
 src/qemu/qemu_monitor.h|3 +
 src/qemu/qemu_monitor_json.c   |   89 +-
 src/qemu/qemu_monitor_json.h   |4 +
 .../caps_2.12.0.aarch64.replies|  416 +-
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |4 +-
 .../qemucapabilitiesdata/caps_2.12.0.ppc64.replies | 4454 ++--
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |4 +-
 .../qemucapabilitiesdata/caps_2.12.0.s390x.replies |   78 +-
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |2 +
 .../caps_2.12.0.x86_64.replies |  804 ++--
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |4 +-
 tests/qemuxml2argvdata/hugepages-pages7.args   |3 +-
 tests/qemuxml2argvdata/hugepages-pages7.xml|4 +-
 tests/qemuxml2argvtest.c   |3 +-
 26 files changed, 3252 insertions(+), 2804 deletions(-)

-- 
2.16.1

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

[libvirt] [PATCH v4 4/5] qemu_capabilities: Introduce QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD

[Michal Privoznik]

This capability tracks if memory-backend-file has discard-data
attribute or not.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_capabilities.c   |   18 +
 src/qemu/qemu_capabilities.h   |1 +
 .../caps_2.12.0.aarch64.replies|  416 +-
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml |3 +-
 .../qemucapabilitiesdata/caps_2.12.0.ppc64.replies | 4454 ++--
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   |3 +-
 .../qemucapabilitiesdata/caps_2.12.0.s390x.replies |   78 +-
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   |1 +
 .../caps_2.12.0.x86_64.replies |  804 ++--
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  |3 +-
 10 files changed, 3010 insertions(+), 2771 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5b9ada81dd..ea2402ba81 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -474,6 +474,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "query-cpus-fast",
   "disk-write-cache",
   "qom-list-properties",
+  "memory-backend-file.discard-data",
 );
 
 
@@ -1368,6 +1369,15 @@ static virQEMUCapsObjectTypeProps 
virQEMUCapsDeviceProps[] = {
   QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW },
 };
 
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMemoryBackendFile[] 
= {
+{ "discard-data", QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD },
+};
+
+static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = {
+{ "memory-backend-file", virQEMUCapsObjectPropsMemoryBackendFile,
+  ARRAY_CARDINALITY(virQEMUCapsObjectPropsMemoryBackendFile),
+  QEMU_CAPS_OBJECT_MEMORY_FILE },
+};
 
 static void
 virQEMUCapsProcessStringFlags(virQEMUCapsPtr qemuCaps,
@@ -2125,6 +2135,14 @@ virQEMUCapsProbeQMPDevices(virQEMUCapsPtr qemuCaps,
 qemuMonitorGetDeviceProps) < 0)
 return -1;
 
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QOM_LIST_PROPERTIES) &&
+virQEMUCapsProbeQMPGenericProps(qemuCaps,
+mon,
+virQEMUCapsObjectProps,
+
ARRAY_CARDINALITY(virQEMUCapsObjectProps),
+qemuMonitorGetObjectProps) < 0)
+return -1;
+
 return 0;
 }
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e3f749a026..daf60a3a1d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -458,6 +458,7 @@ typedef enum {
 QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */
 QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache 
param */
 QEMU_CAPS_QOM_LIST_PROPERTIES, /* qom-list-properties monitor command */
+QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD, /* -object 
memory-backend-dile,discard-data= */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies 
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
index 3512d88b67..3ca0ea13fa 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies
@@ -5275,6 +5275,60 @@
   "id": "libvirt-35"
 }
 
+{
+  "return": [
+{
+  "name": "policy",
+  "type": "HostMemPolicy"
+},
+{
+  "name": "dump",
+  "type": "bool"
+},
+{
+  "name": "share",
+  "type": "bool"
+},
+{
+  "name": "prealloc",
+  "type": "bool"
+},
+{
+  "name": "size",
+  "type": "int"
+},
+{
+  "name": "host-nodes",
+  "type": "int"
+},
+{
+  "name": "id",
+  "type": "string"
+},
+{
+  "name": "merge",
+  "type": "bool"
+},
+{
+  "name": "align",
+  "type": "int"
+},
+{
+  "name": "mem-path",
+  "type": "string"
+},
+{
+  "name": "discard-data",
+  "type": "bool"
+},
+{
+  "name": "type",
+  "type": "string"
+}
+  ],
+  "id": "libvirt-36"
+}
+
 {
   "return": [
 {
@@ -5569,198 +5623,198 @@
   "cpu-max": 1
 }
   ],
-  "id": "libvirt-36"
-}
-
-{
-  "return": [
-{
-  "name": "pxa262",
-  "typename": "pxa262-arm-cpu",
-  "static": false
-},
-{
-  "name": "pxa270-a0",
-  "typename": "pxa270-a0-arm-cpu",
-  "static": false
-},
-{
-  "name": "arm1136",
-  "typename": "arm1136-arm-cpu",
-  "static": false
-},
-{
-  "name": "cortex-a15",
-  "typename": "cortex-a15-arm-cpu",
-  "static": false
-},
-{
-  "name": "pxa260",
-  "typename": "pxa260-arm-cpu",
-  "static": false
-},
-{
-  "name": "arm1136-r2",
-  "typename": "arm1136-r2-arm-cpu",
-  "static": false
-},
-{
-  "name": "pxa261",
-  "typename": "pxa261

[libvirt] [PATCH v4 3/5] qemu_capabilities: Introduce QEMU_CAPS_QOM_LIST_PROPERTIES

[Michal Privoznik]

This capability tracks if qemu has "qom-list-properties" monitor
command.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_capabilities.c   | 2 ++
 src/qemu/qemu_capabilities.h   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml  | 1 +
 6 files changed, 7 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 833c75514c..5b9ada81dd 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -473,6 +473,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   /* 290 */
   "query-cpus-fast",
   "disk-write-cache",
+  "qom-list-properties",
 );
 
 
@@ -981,6 +982,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
 { "query-cpu-definitions", QEMU_CAPS_QUERY_CPU_DEFINITIONS },
 { "query-named-block-nodes", QEMU_CAPS_QUERY_NAMED_BLOCK_NODES },
 { "query-cpus-fast", QEMU_CAPS_QUERY_CPUS_FAST },
+{ "qom-list-properties", QEMU_CAPS_QOM_LIST_PROPERTIES },
 };
 
 struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f08cfc2611..e3f749a026 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -457,6 +457,7 @@ typedef enum {
 /* 290 */
 QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */
 QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache 
param */
+QEMU_CAPS_QOM_LIST_PROPERTIES, /* qom-list-properties monitor command */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
index 31c5d0dd23..f85cab6f64 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml
@@ -157,6 +157,7 @@
   
   
   
+  
   2011090
   0
   342346
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index 7dead4a1f4..4fb1205162 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -154,6 +154,7 @@
   
   
   
+  
   2011090
   0
   419215
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
index 70ae8f91c7..cb4f0d0d5b 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml
@@ -119,6 +119,7 @@
   
   
   
+  
   2011090
   0
   0
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
index d809a78380..f7c394a218 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml
@@ -195,6 +195,7 @@
   
   
   
+  
   2011090
   0
   390060
-- 
2.16.1

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

[libvirt] [PATCH v4 2/5] qemu_monitor: Introduce qemuMonitorGetObjectProps

[Michal Privoznik]

Now that we've gotten rid of misleading names we can introduce
qemuMonitorGetObjectProps() function which queries -object
properties. Again, some parts of code can be reused.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor.c  | 13 +
 src/qemu/qemu_monitor.h  |  3 +++
 src/qemu/qemu_monitor_json.c | 32 
 src/qemu/qemu_monitor_json.h |  4 
 4 files changed, 52 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f642d9a51a..a1e2e40d0f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3932,6 +3932,19 @@ qemuMonitorGetDeviceProps(qemuMonitorPtr mon,
 }
 
 
+int
+qemuMonitorGetObjectProps(qemuMonitorPtr mon,
+  const char *object,
+  char ***props)
+{
+VIR_DEBUG("object=%s props=%p", object, props);
+
+QEMU_CHECK_MONITOR_JSON(mon);
+
+return qemuMonitorJSONGetObjectProps(mon, object, props);
+}
+
+
 char *
 qemuMonitorGetTargetArch(qemuMonitorPtr mon)
 {
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d6b68b44ca..0f648ce27c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1048,6 +1048,9 @@ int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
 int qemuMonitorGetDeviceProps(qemuMonitorPtr mon,
   const char *device,
   char ***props);
+int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
+  const char *object,
+  char ***props);
 char *qemuMonitorGetTargetArch(qemuMonitorPtr mon);
 
 int qemuMonitorNBDServerStart(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 95b9d60aff..c6b9ccce39 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6129,6 +6129,38 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
 }
 
 
+int
+qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
+  const char *object,
+  char ***props)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+*props = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("qom-list-properties",
+   "s:typename", object,
+   NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+goto cleanup;
+
+if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
+ret = 0;
+goto cleanup;
+}
+
+ret = qemuMonitorJSONParsePropsList(cmd, reply, props);
+ cleanup:
+virJSONValueFree(reply);
+virJSONValueFree(cmd);
+return ret;
+}
+
+
 char *
 qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon)
 {
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 665da27d6d..b4863a454a 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -445,6 +445,10 @@ int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
   const char *device,
   char ***props)
 ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon,
+  const char *object,
+  char ***props)
+ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
 char *qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon);
 
 int qemuMonitorJSONNBDServerStart(qemuMonitorPtr mon,
-- 
2.16.1

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

Re: [libvirt] [PATCH v2 3/4] vmware: Add more descriptive error message on Find failure

[Pavel Hrdina]

On Mon, Apr 02, 2018 at 09:16:39AM -0400, John Ferlan wrote:
> If vmwareDomainLookupByID or vmwareDomainLookupByName fails
> to find a vm, let's be a bit more descriptive by providing
> the failing id or name in the error message.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/vmware/vmware_driver.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH v2 2/4] vmware: Create accessors to virDomainObjListFindByUUID

[Pavel Hrdina]

On Mon, Apr 02, 2018 at 09:16:38AM -0400, John Ferlan wrote:
> Rather than repeat code throughout, create and use a couple of
> accessors in order to lookup by UUID.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/vmware/vmware_driver.c | 180 
> +++--
>  1 file changed, 61 insertions(+), 119 deletions(-)

Reviewed-by: Pavel Hrdina 


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

[libvirt] [PATCH] Qemu driver: Support network-backed pflash disks.

[Prerna Saxena]

So far libvirt domain XML only allows local filepaths that can be
used to specify a loader element or its matching NVRAM disk.
Given that Vms may themselves move across hypervisor hosts, it should be
possible to allocate loaders/NVRAM disks on network storage for
uninterrupted access.

Signed-off-by: Prerna Saxena 
---
 docs/schemas/domaincommon.rng   | 108 +++
 src/conf/domain_conf.c  | 188 
 src/conf/domain_conf.h  |   7 +-
 src/qemu/qemu_cgroup.c  |  13 ++-
 src/qemu/qemu_command.c |  21 +++--
 src/qemu/qemu_domain.c  |  16 ++--
 src/qemu/qemu_driver.c  |   7 +-
 src/qemu/qemu_parse_command.c   |  30 ++-
 src/qemu/qemu_process.c |  33 ---
 src/security/security_dac.c |   6 +-
 src/security/security_selinux.c |   6 +-
 11 files changed, 361 insertions(+), 74 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4cab55f..32db395 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -276,7 +276,42 @@
 
   
 
-
+
+  
+
+  file
+  network
+
+  
+
+
+  
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+  
+
   
 
 
@@ -287,7 +322,40 @@
   
 
 
-  
+  
+
+  file
+  network
+
+  
+
+
+  
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+ 
+  
+ 
+  
 
   
 
@@ -1494,25 +1562,29 @@
   
 
 
-  
-
-  
-
-  
-
-
-  
-
-
-  
-
-
-  
-
-  
+  
 
   
 
+  
+
+  
+
+  
+
+  
+  
+
+  
+  
+
+  
+  
+
+  
+
+  
+
   
 
   block
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35666c1..c80f9d9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2883,8 +2883,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader)
 if (!loader)
 return;
 
-VIR_FREE(loader->path);
-VIR_FREE(loader->nvram);
+virStorageSourceFree(loader->loader_src);
+virStorageSourceFree(loader->nvram);
 VIR_FREE(loader->templt);
 VIR_FREE(loader);
 }
@@ -17961,17 +17961,59 @@ 
virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def)
 
 static int
 virDomainLoaderDefParseXML(xmlNodePtr node,
+   xmlXPathContextPtr ctxt,
virDomainLoaderDefPtr loader)
 {
 int ret = -1;
 char *readonly_str = NULL;
 char *secure_str = NULL;
 char *type_str = NULL;
+char *tmp = NULL;
+xmlNodePtr cur;
+xmlXPathContextPtr cur_ctxt = ctxt;
+
+if (VIR_ALLOC(loader->loader_src)) {
+goto cleanup;
+}
+loader->loader_src->type = VIR_STORAGE_TYPE_LAST;
 
 readonly_str = virXMLPropString(node, "readonly");
 secure_str = virXMLPropString(node, "secure");
 type_str = virXMLPropString(node, "type");
-loader->path = (char *) xmlNodeGetContent(node);
+
+if ((tmp = virXMLPropString(node, "backing")) &&
+(loader->loader_src->type = virStorageTypeFromString(tmp)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown loader type '%s'"), tmp);
+goto cleanup;
+}
+VIR_FREE(tmp);
+
+for (cur = node->children; cur != NULL; cur = cur->next) {
+if (cur->type != XML_ELEMENT_NODE) {
+continue;
+}
+
+if (virXMLNodeNameEqual(cur, "source")) {
+if (virDomainStorageSourceParse(cur, cur_ctxt, loader->loader_src, 
0) < 0) {
+virReportError(VIR_ERR_XML_DETAIL,
+   _("Error parsing Loader s

Re: [libvirt] [qemu RFC v2] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/20/18 10:47, Paolo Bonzini wrote:
> On 20/04/2018 03:03, David Gibson wrote:
>>> This also implies I shouldn't add "openbios" separately, which was
>>> suggested earlier by Gerd -- according to
>>> , OpenBIOS is another
>>> implementation of OFW.
>>
>> Right.  Although I think OpenBIOS and SLOF support a disjoint set of
>> machines.  Openhackware which is (was?) used on some machines is yet
>> another (very partial) openfirmware implementation.
> 
> We can:
> 
> 1a) replace the architecture field with an ABI field (seems wrong to me)
> 
> 1b) add an "ABI" field (containing e.g. prep, spapr, etc.) to complement
> the architecture field
> 
> 2) we include directly a glob pattern for the QEMU machine types (also
> seems wrong).

Using globs in machine types was what Gerd and Daniel agreed upon, and I
was planning to do for v3. :(

> 3) just like we effectively moved the guest ABI to the features field,

No, we didn't -- while it's true that a single firmware image can
provide multiple guest ABIs, the implementation quality of those guest
ABIs is not identical, and there is a preference order between them. The
latest idea was to keep the @type field, but turn it into an ordered
list of enum constants. Whichever constant is near the top is the more
preferred ABI, and mgmt tools, when looking for "the" type of the
firware, would consider the topmost (first) element.

> we split the features into host-features and guest-features, and the
> host ABI (prep, spapr, etc.; but also OVMF's SMM requirement fits here
> for example) moves into host-features.  Again, just like 1a/1b, this can
> be replace or complement the architecture field, and I think I'd prefer
> the latter

I'd prefer if we could mostly stick with the structure of the schema as
seen in RFCv2 -- all the feedback until now implies that's possible,
with minor tweaks. Let me post an RFCv3 soon, so that we have a more
up-to-date basis for the discussion. (I've been waiting for some of the
architecture (emulation target) technicalities to clear up, but I guess
I had better postpone that now, because I see the discussion diverging.)

Thanks!
Laszlo

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

Re: [libvirt] [PATCH v2 1/4] vmware: Properly clean up in vmwareDomainLookupByName

[Pavel Hrdina]

On Mon, Apr 02, 2018 at 09:16:37AM -0400, John Ferlan wrote:
> The virDomainObjListFindByName returns a locked and reffed
> domain object, all we did was unlock it, leaving an extra
> ref. Use the virDomainObjEndAPI to cleanup instead.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/vmware/vmware_driver.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Pavel Hrdina 


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

[libvirt] [PATCH] Qemu driver: Support network-backed pflash disks.

[Prerna Saxena]

This implements support for firmware loader & NVRAM disks over network-backed 
disks.
As discussed in 
https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html,
the patch embeds the  spec for disks in  and  elements 
as well.

Currently, the source type is annotated by introducing a new attribute 
"backing" for both
'loader' and 'nvram' elements. Hence, a sample XML with new annotation looks 
like this:


  

  
  


The patche automatically re-formats any older-stype declaration into this new 
style.
Templates can be used to create a new NVRAM only if the nvram backing = 'file'.

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

Re: [libvirt] [RFC PATCH 05/30] qemu: block: Extract formatting of props for 'file' backend

[Peter Krempa]

On Fri, Apr 20, 2018 at 10:41:50 +0200, Kevin Wolf wrote:
> Am 20.04.2018 um 09:56 hat Peter Krempa geschrieben:
> > Is there a special need to use 'host_cdrom' explicitly if the CDROM
> > drive is used? That would complicate things since we don't know when
> > that will happen.
> 
> You don't get the full CD-ROM passthrough functionality with
> host_device. Specifically, if you eject the virtual drive, the physical
> drive will only be ejected with host_cdrom, and the same is true for
> lock_medium.
> 
> I think we may also make an attempt at detecting physical media change
> ocassionally with host_cdrom, but I doubt that this is working reliably
> anyway (the driver function is .bdrv_is_inserted, so it would be polling
> rather than processing an event; and I'm not sure when it's called).

Hmm, okay, it seems that we in fact should use it. Is there any drawback
if host_cdrom is used with a device which is not a CDROM? (e.g. a
logical volume containing the image?)

Basically the question is whether we need to add some "smart" handling
or we can just assume that if the guest device is a block-backed CDROM
we can use that.


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

Re: [libvirt] [qemu RFC v2] qapi: add "firmware.json"

[Paolo Bonzini]

On 20/04/2018 03:03, David Gibson wrote:
>> This also implies I shouldn't add "openbios" separately, which was
>> suggested earlier by Gerd -- according to
>> , OpenBIOS is another
>> implementation of OFW.
> 
> Right.  Although I think OpenBIOS and SLOF support a disjoint set of
> machines.  Openhackware which is (was?) used on some machines is yet
> another (very partial) openfirmware implementation.

We can:

1a) replace the architecture field with an ABI field (seems wrong to me)

1b) add an "ABI" field (containing e.g. prep, spapr, etc.) to complement
the architecture field

2) we include directly a glob pattern for the QEMU machine types (also
seems wrong).

3) just like we effectively moved the guest ABI to the features field,
we split the features into host-features and guest-features, and the
host ABI (prep, spapr, etc.; but also OVMF's SMM requirement fits here
for example) moves into host-features.  Again, just like 1a/1b, this can
be replace or complement the architecture field, and I think I'd prefer
the latter

Paolo



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

Re: [libvirt] [RFC PATCH 02/30] qemu: domain: Format storage source node names into private data

[Peter Krempa]

On Fri, Apr 20, 2018 at 10:32:11 +0200, Kevin Wolf wrote:
> Am 20.04.2018 um 09:45 hat Peter Krempa geschrieben:
> > On Fri, Apr 20, 2018 at 09:30:16 +0200, Kevin Wolf wrote:
> > > Am 19.04.2018 um 17:24 hat Peter Krempa geschrieben:
> > > > Save and restore node names if we know them in the status XML so that we
> > > > don't need to recalculate them or don't lose them in some cases.
> > > > 
> > > > Signed-off-by: Peter Krempa 
> > > > ---
> > > >  src/qemu/qemu_domain.c | 12 
> > > >  1 file changed, 12 insertions(+)
> > > > 
> > > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > > > index e2a8450e2e..224b3b0478 100644
> > > > --- a/src/qemu/qemu_domain.c
> > > > +++ b/src/qemu/qemu_domain.c
> > > > @@ -1934,6 +1934,9 @@ static int
> > > >  qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
> > > >virStorageSourcePtr src)
> > > >  {
> > > > +src->nodestorage = virXPathString("string(./nodename/@storage)", 
> > > > ctxt);
> > > > +src->nodeformat = virXPathString("string(./nodename/@format)", 
> > > > ctxt);
> > > > +
> > > >  if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0)
> > > >  return -1;
> > > 
> > > This format doesn't look very future-proof. I can see that using
> > > x->nodestorage and x->nodeformat internally are probably easier for now,
> > > but while the assumption of exactly two layers (except for backing
> > > files) is probably correct for most cases, it will become less and less
> > > common the more we use filter drivers.
> > > 
> > > Representing an arbitrary tree (or even non-tree graph) in the XML may
> > > be unpractical now, but please be aware that you'll probably need this
> > > change in the long run.
> > 
> > Yes. It will need to change e.g. if we allow IO throttling for other
> > than the top layer. On the other hand it would be impractical to change
> > to the topology qemu uses where a storage file is two layers minimum, so
> > we will need to track everyting for one layer of XML.
> 
> The two layers minimum is not correct as far as QEMU is concerned. A
> two-layer setup is what -drive creates by default if you don't specify a
> protocol driver as driver=... (or format=... for older versions)
> explicitly, but if you want, you can directly assign a 'file' node to a
> guest device without adding a format layer on top.
> 
> > As the above uses attributes in the XML we can add an arbitrary amount
> > of them if required.
> 
> Quorum takes an array of children, and VMDK needs a number of extents
> that depends on the image file, so attributes probably won't do. If

Adding support for quorum will not be much fun for the poor soul having
to do it since the XML design will be very hard to do. The problem is
in the fact that the  element contains few things related to the
source image outside of the  tag.

> we didn't have to consider compatibility, I think I'd choose something
> like:

Well this is private data, so as long as we add a compatibility layer
with the old format we can change it anytime we want.

I'll try to use a more modular approach though in the next version.

> 
> 
> 
> 
> 
> 
> Kevin




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

Re: [libvirt] [RFC PATCH 05/30] qemu: block: Extract formatting of props for 'file' backend

[Kevin Wolf]

Am 20.04.2018 um 09:56 hat Peter Krempa geschrieben:
> Is there a special need to use 'host_cdrom' explicitly if the CDROM
> drive is used? That would complicate things since we don't know when
> that will happen.

You don't get the full CD-ROM passthrough functionality with
host_device. Specifically, if you eject the virtual drive, the physical
drive will only be ejected with host_cdrom, and the same is true for
lock_medium.

I think we may also make an attempt at detecting physical media change
ocassionally with host_cdrom, but I doubt that this is working reliably
anyway (the driver function is .bdrv_is_inserted, so it would be polling
rather than processing an event; and I'm not sure when it's called).

Kevin


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

Re: [libvirt] [RFC PATCH 02/30] qemu: domain: Format storage source node names into private data

[Kevin Wolf]

Am 20.04.2018 um 09:45 hat Peter Krempa geschrieben:
> On Fri, Apr 20, 2018 at 09:30:16 +0200, Kevin Wolf wrote:
> > Am 19.04.2018 um 17:24 hat Peter Krempa geschrieben:
> > > Save and restore node names if we know them in the status XML so that we
> > > don't need to recalculate them or don't lose them in some cases.
> > > 
> > > Signed-off-by: Peter Krempa 
> > > ---
> > >  src/qemu/qemu_domain.c | 12 
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > > index e2a8450e2e..224b3b0478 100644
> > > --- a/src/qemu/qemu_domain.c
> > > +++ b/src/qemu/qemu_domain.c
> > > @@ -1934,6 +1934,9 @@ static int
> > >  qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
> > >virStorageSourcePtr src)
> > >  {
> > > +src->nodestorage = virXPathString("string(./nodename/@storage)", 
> > > ctxt);
> > > +src->nodeformat = virXPathString("string(./nodename/@format)", ctxt);
> > > +
> > >  if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0)
> > >  return -1;
> > 
> > This format doesn't look very future-proof. I can see that using
> > x->nodestorage and x->nodeformat internally are probably easier for now,
> > but while the assumption of exactly two layers (except for backing
> > files) is probably correct for most cases, it will become less and less
> > common the more we use filter drivers.
> > 
> > Representing an arbitrary tree (or even non-tree graph) in the XML may
> > be unpractical now, but please be aware that you'll probably need this
> > change in the long run.
> 
> Yes. It will need to change e.g. if we allow IO throttling for other
> than the top layer. On the other hand it would be impractical to change
> to the topology qemu uses where a storage file is two layers minimum, so
> we will need to track everyting for one layer of XML.

The two layers minimum is not correct as far as QEMU is concerned. A
two-layer setup is what -drive creates by default if you don't specify a
protocol driver as driver=... (or format=... for older versions)
explicitly, but if you want, you can directly assign a 'file' node to a
guest device without adding a format layer on top.

> As the above uses attributes in the XML we can add an arbitrary amount
> of them if required.

Quorum takes an array of children, and VMDK needs a number of extents
that depends on the image file, so attributes probably won't do. If
we didn't have to consider compatibility, I think I'd choose something
like:






Kevin


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

Re: [libvirt] [RFC PATCH 15/30] qemu: block: Add support for creating 'format' layer for blockdev-add

[Kevin Wolf]

Am 19.04.2018 um 17:25 hat Peter Krempa geschrieben:
> When using blockdev-add and friends, libvirt will need to create also
> properties for the qcow2/raw/... format handler in qemu. This patch adds
> the infrastructure and implements all formats known to libvirt including
> all properties which are expressed at the format level in qemu.
> 
> Signed-off-by: Peter Krempa 
> ---
>  src/qemu/qemu_block.c | 296 
> ++
>  src/qemu/qemu_block.h |   3 +
>  2 files changed, 299 insertions(+)
> 
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 0357d93384..87f3cf9cbe 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -1168,3 +1168,299 @@ 
> qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src,
>  virJSONValueFree(fileprops);
>  return ret;
>  }
> +
> +
> +static int
> +qemuBlockStorageSourceGetFormatRawProps(virStorageSourcePtr src,
> +virJSONValuePtr props)
> +{
> +qemuDomainStorageSourcePrivatePtr srcPriv = 
> QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
> +const char *driver = "raw";
> +const char *secretalias = NULL;
> +
> +if (src->encryption &&
> +src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
> +srcPriv &&
> +srcPriv->encinfo) {
> +driver = "luks";
> +secretalias = srcPriv->encinfo->s.aes.alias;
> +}

Hm, so libvirt treats luks images as "encrypted raw" (which is a
contradiction in itself)? I suppose it's too late to change this, but
it's a bit unfortunate, because...

> +/* currently unhandled properties for the 'raw' driver:
> + * 'offset'
> + * 'size'
> + */

...if you ever want to implement these features, the luks driver won't
be offering them and you'll need to create two layers at once in this
function (raw over luks, or luks over raw - and it's not clear which
one).

Kevin

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

Re: [libvirt] [RFC PATCH 14/30] [RFC] qemu: block: Always set discard for storage nodes

[Kevin Wolf]

Am 19.04.2018 um 17:25 hat Peter Krempa geschrieben:
> According to my research it seems that qemu always sets discard for the
> storage nodes. Replicate this in our generator.
> ---
>  src/qemu/qemu_block.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 44662a4603..0357d93384 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -1155,7 +1155,10 @@ 
> qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src,
>  if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, fileprops) < 
> 0)
>  goto cleanup;
> 
> -if (virJSONValueObjectAdd(fileprops, "b:read-only", src->readonly, 
> NULL) < 0)
> +if (virJSONValueObjectAdd(fileprops,
> +  "b:read-only", src->readonly,
> +  "s:discard", "unmap",
> +  NULL) < 0)
>  goto cleanup;
>  }

Looks correct to me. The QEMU default means that discard requests made
by the guest are ignored (already at the format level), but discard
requests made by a format driver to the protocol layer are honoured
(e.g. qcow2 discarding data when an internal snapshot is deleted).

Kevin

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

Re: [libvirt] [Qemu-devel] [qemu RFC v2] qapi: add "firmware.json"

[Laszlo Ersek]

On 04/19/18 11:12, Daniel P. Berrangé wrote:
> On Thu, Apr 19, 2018 at 10:39:32AM +0200, Laszlo Ersek wrote:
>> On 04/19/18 09:56, Daniel P. Berrangé wrote:
>>> On Thu, Apr 19, 2018 at 09:48:36AM +0200, Markus Armbruster wrote:
 Laszlo Ersek  writes:

> On 04/18/18 10:47, Markus Armbruster wrote:
>> Laszlo Ersek  writes:
 Replacing CpuInfoArch by such an enum will change the discriminator
 value from "other" to the real architecture, with the obvious
 compatibility concerns.  But we've accepted similar changes twice
 already: commit 9d0306dfdfb and commit 25fa194b7b1, both v2.12.0-rc0.

 "other" was a bad idea.  Hindsight 20/20.

 Getting rid of it in one go rather than piecemeal seems like the least
 bad way out.  Too late for 2.12, though.  Eric, what do you think?
>>>
>>> Given the context in which this "other" value is used, I think it is
>>> reasonable to kill it and put a full arch list in there.
>>>
>>> No app is likely to be accessing the struct under "other" because it
>>> is just an empty placeholder.
>>
>> Commit 9d0306dfdfb added "s390" and "CpuInfoS390", which I guess had the
>> potential to confuse QMP clients that didn't expect "s390", but
>> otherwise it didn't mess with preexistent enum values / structures.
> 
> NB, qemu-system-s390x would previously have returned "other" in
> this field, and now it returns "s390".  So while it didn't
> remove "other" from the list of things that could potentially
> exist, it did change what the s390x binary will actually report.
> 
>> The same applies to commit 25fa194b7b1, just with "riscv" /
>> "CpuInfoRISCV" substituted.
>>
>> Removing "other" might confuse QMP clients that expect it, except
>> (according to Daniel) no such client exists, probably.
> 
> When I say removing "other", I imply that we add an explicit arch
> for all those which we currently are missing. IOW, all qemu-system-XXX
> binaries which currently report "other" would change to report their
> respective "XXX" values.
> 
> So in this way, it is exactly the same as what we did when we
> introduced "s390" as an option.
> 
> The only difference is that once we have every binary reporting the
> correct arch, we can now also remove "other" from the schema itself
> as it will then be unused.

Can we please translate this into more actionable items for me, because
I'm getting confused :)

First, if I add "i386" and "x86_64" to the enum list, we'll have all
three of "i386", "x86_64" and "x86". Is that useful? How will that work?

Second, assuming I add constants for the ~10 (?) softmmu arches, can I
still use @CpuInfoOther as the type for the corresponding new members in
@CpuInfo? What C code changes will be necessary?

Thanks
Laszlo

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

Re: [libvirt] [RFC PATCH 05/30] qemu: block: Extract formatting of props for 'file' backend

[Peter Krempa]

On Fri, Apr 20, 2018 at 09:45:40 +0200, Kevin Wolf wrote:
> Am 19.04.2018 um 17:25 hat Peter Krempa geschrieben:
> > 'file' backend in qemu supports few more options than the current
> > implementation. Extract it so that changes don't pollute the code.
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> >  src/qemu/qemu_block.c | 16 +---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> > index c0cf6a95ad..e7bd6c909d 100644
> > --- a/src/qemu/qemu_block.c
> > +++ b/src/qemu/qemu_block.c
> > @@ -974,6 +974,18 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr 
> > src)
> >  }
> > 
> > 
> > +static virJSONValuePtr
> > +qemuBlockStorageSourceGetFileProps(virStorageSourcePtr src)
> > +{
> > +virJSONValuePtr ret = NULL;
> > +
> > +ignore_value(virJSONValueObjectCreate(&ret,
> > +  "s:driver", "file",
> > +  "s:filename", src->path, NULL) < 
> > 0);
> > +return ret;
> > +}
> > +
> > +
> >  /**
> >   * qemuBlockStorageSourceGetBackendProps:
> >   * @src: disk source
> > @@ -991,9 +1003,7 @@ 
> > qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src)
> >  case VIR_STORAGE_TYPE_BLOCK:
> >  case VIR_STORAGE_TYPE_FILE:
> >  case VIR_STORAGE_TYPE_DIR:
> > -if (virJSONValueObjectCreate(&fileprops,
> > - "s:driver", "file",
> > - "s:filename", src->path, NULL) < 0)
> > +if (!(fileprops = qemuBlockStorageSourceGetFileProps(src)))
> >  return NULL;
> 
> Preexisting, but I wonder whether 'driver': 'file' is correct for all
> these cases. I see that you handle _DIR separately with a vvfat function
> later in this series, but shouldn't _BLOCK be 'host_device' rather than
> 'file', too?

It should. I just did not notice it. If I try the block storage now I
get:

{
  "iops_rd": 0,
  "detect_zeroes": "off",
  "image": {
"virtual-size": 10737418240,
"filename": "/dev/mapper/angien--storage-ble",
"format": "host_device",
"actual-size": 0,
"dirty-flag": false
  },
  "iops_wr": 0,
  "ro": false,
  "node-name": "#block093",
  "backing_file_depth": 0,
  "drv": "host_device",
  "iops": 0,
  "bps_wr": 0,
  "write_threshold": 0,
  "encrypted": true,
  "bps": 0,
  "bps_rd": 0,
  "cache": {
"no-flush": false,
"direct": true,
"writeback": true
  },
  "file": "/dev/mapper/angien--storage-ble",
  "encryption_key_missing": false
}

Is there a special need to use 'host_cdrom' explicitly if the CDROM
drive is used? That would complicate things since we don't know when
that will happen.



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

Re: [libvirt] [RFC PATCH 05/30] qemu: block: Extract formatting of props for 'file' backend

[Kevin Wolf]

Am 19.04.2018 um 17:25 hat Peter Krempa geschrieben:
> 'file' backend in qemu supports few more options than the current
> implementation. Extract it so that changes don't pollute the code.
> 
> Signed-off-by: Peter Krempa 
> ---
>  src/qemu/qemu_block.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index c0cf6a95ad..e7bd6c909d 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -974,6 +974,18 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr 
> src)
>  }
> 
> 
> +static virJSONValuePtr
> +qemuBlockStorageSourceGetFileProps(virStorageSourcePtr src)
> +{
> +virJSONValuePtr ret = NULL;
> +
> +ignore_value(virJSONValueObjectCreate(&ret,
> +  "s:driver", "file",
> +  "s:filename", src->path, NULL) < 
> 0);
> +return ret;
> +}
> +
> +
>  /**
>   * qemuBlockStorageSourceGetBackendProps:
>   * @src: disk source
> @@ -991,9 +1003,7 @@ 
> qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src)
>  case VIR_STORAGE_TYPE_BLOCK:
>  case VIR_STORAGE_TYPE_FILE:
>  case VIR_STORAGE_TYPE_DIR:
> -if (virJSONValueObjectCreate(&fileprops,
> - "s:driver", "file",
> - "s:filename", src->path, NULL) < 0)
> +if (!(fileprops = qemuBlockStorageSourceGetFileProps(src)))
>  return NULL;

Preexisting, but I wonder whether 'driver': 'file' is correct for all
these cases. I see that you handle _DIR separately with a vvfat function
later in this series, but shouldn't _BLOCK be 'host_device' rather than
'file', too?

Kevin

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

Re: [libvirt] [RFC PATCH 02/30] qemu: domain: Format storage source node names into private data

[Peter Krempa]

On Fri, Apr 20, 2018 at 09:30:16 +0200, Kevin Wolf wrote:
> Am 19.04.2018 um 17:24 hat Peter Krempa geschrieben:
> > Save and restore node names if we know them in the status XML so that we
> > don't need to recalculate them or don't lose them in some cases.
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> >  src/qemu/qemu_domain.c | 12 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> > index e2a8450e2e..224b3b0478 100644
> > --- a/src/qemu/qemu_domain.c
> > +++ b/src/qemu/qemu_domain.c
> > @@ -1934,6 +1934,9 @@ static int
> >  qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
> >virStorageSourcePtr src)
> >  {
> > +src->nodestorage = virXPathString("string(./nodename/@storage)", ctxt);
> > +src->nodeformat = virXPathString("string(./nodename/@format)", ctxt);
> > +
> >  if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0)
> >  return -1;
> 
> This format doesn't look very future-proof. I can see that using
> x->nodestorage and x->nodeformat internally are probably easier for now,
> but while the assumption of exactly two layers (except for backing
> files) is probably correct for most cases, it will become less and less
> common the more we use filter drivers.
> 
> Representing an arbitrary tree (or even non-tree graph) in the XML may
> be unpractical now, but please be aware that you'll probably need this
> change in the long run.

Yes. It will need to change e.g. if we allow IO throttling for other
than the top layer. On the other hand it would be impractical to change
to the topology qemu uses where a storage file is two layers minimum, so
we will need to track everyting for one layer of XML.

As the above uses attributes in the XML we can add an arbitrary amount
of them if required.


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

Re: [libvirt] [RFC PATCH 02/30] qemu: domain: Format storage source node names into private data

[Kevin Wolf]

Am 19.04.2018 um 17:24 hat Peter Krempa geschrieben:
> Save and restore node names if we know them in the status XML so that we
> don't need to recalculate them or don't lose them in some cases.
> 
> Signed-off-by: Peter Krempa 
> ---
>  src/qemu/qemu_domain.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e2a8450e2e..224b3b0478 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1934,6 +1934,9 @@ static int
>  qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt,
>virStorageSourcePtr src)
>  {
> +src->nodestorage = virXPathString("string(./nodename/@storage)", ctxt);
> +src->nodeformat = virXPathString("string(./nodename/@format)", ctxt);
> +
>  if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0)
>  return -1;

This format doesn't look very future-proof. I can see that using
x->nodestorage and x->nodeformat internally are probably easier for now,
but while the assumption of exactly two layers (except for backing
files) is probably correct for most cases, it will become less and less
common the more we use filter drivers.

Representing an arbitrary tree (or even non-tree graph) in the XML may
be unpractical now, but please be aware that you'll probably need this
change in the long run.

Kevin

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

Re: [libvirt] [PATCH 0/2] vmx: start reading/writing CPU topology

[Ján Tomko]

On Thu, Apr 19, 2018 at 03:03:36PM +0200, Pino Toscano wrote:

Read/write only cpuid.coresPerSocket for now.

Pino Toscano (2):
 vmx: convert cpuid.coresPerSocket for CPU topology
 vmx: write cpuid.coresPerSocket back from CPU topology

src/vmx/vmx.c   | 57 
tests/vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml |  3 ++
tests/vmx2xmldata/vmx2xml-esx-in-the-wild-9.vmx | 72 +
tests/vmx2xmldata/vmx2xml-esx-in-the-wild-9.xml | 37 +
tests/vmx2xmltest.c |  1 +
tests/xml2vmxdata/xml2vmx-esx-in-the-wild-9.vmx | 39 ++
tests/xml2vmxdata/xml2vmx-esx-in-the-wild-9.xml | 37 +
tests/xml2vmxtest.c |  1 +
8 files changed, 247 insertions(+)
create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-9.vmx
create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-9.xml
create mode 100644 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-9.vmx
create mode 100644 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-9.xml



Reviewed-by: Ján Tomko 

and pushed

Jano


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

Re: [libvirt] [PATCH v3 3/5] qemu_monitor: Introduce qemuMonitorGetObjectProps

[Michal Privoznik]

On 04/19/2018 06:04 PM, Ján Tomko wrote:
> On Thu, Apr 19, 2018 at 04:00:25PM +0200, Michal Privoznik wrote:
>> Now that we've gotten rid of misleading names we can introduce
>> qemuMonitorGetObjectProps() function which queries -object
>> properties. Again, some parts of code can be reused.
>>
> 
> This should be two commits:
> * split out qemuMonitorJSONParsePropsList
> * introduce qemuMonitorGetObjectProps

Well, I can split it, but one patch without the other does not make
sense. That's the reason I've merged them. If somebody is going to
backport this, they will have to backport both patches or none.

> 
>> Signed-off-by: Michal Privoznik 
>> ---
>> src/qemu/qemu_monitor.c  | 13 ++
>> src/qemu/qemu_monitor.h  |  3 +++
>> src/qemu/qemu_monitor_json.c | 62
>> ++--
>> src/qemu/qemu_monitor_json.h |  4 +++
>> 4 files changed, 68 insertions(+), 14 deletions(-)
>>
>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>> index f642d9a51a..a1e2e40d0f 100644
>> --- a/src/qemu/qemu_monitor.c
>> +++ b/src/qemu/qemu_monitor.c
>> @@ -3932,6 +3932,19 @@ qemuMonitorGetDeviceProps(qemuMonitorPtr mon,
>> }
>>
>>
>> +int
>> +qemuMonitorGetObjectProps(qemuMonitorPtr mon,
>> +  const char *object,
>> +  char ***props)
>> +{
>> +    VIR_DEBUG("object=%s props=%p", object, props);
>> +
>> +    QEMU_CHECK_MONITOR_JSON(mon);
>> +
>> +    return qemuMonitorJSONGetObjectProps(mon, object, props);
>> +}
>> +
>> +
>> char *
>> qemuMonitorGetTargetArch(qemuMonitorPtr mon)
>> {
>> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
>> index d6b68b44ca..0f648ce27c 100644
>> --- a/src/qemu/qemu_monitor.h
>> +++ b/src/qemu/qemu_monitor.h
>> @@ -1048,6 +1048,9 @@ int qemuMonitorGetObjectTypes(qemuMonitorPtr mon,
>> int qemuMonitorGetDeviceProps(qemuMonitorPtr mon,
>>   const char *device,
>>   char ***props);
>> +int qemuMonitorGetObjectProps(qemuMonitorPtr mon,
>> +  const char *object,
>> +  char ***props);
>> char *qemuMonitorGetTargetArch(qemuMonitorPtr mon);
>>
>> int qemuMonitorNBDServerStart(qemuMonitorPtr mon,
>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>> index 24d37eb41d..bd1729488c 100644
>> --- a/src/qemu/qemu_monitor_json.c
>> +++ b/src/qemu/qemu_monitor_json.c
>> @@ -6053,29 +6053,23 @@ int
>> qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon,
>> #undef MAKE_SET_CMD
>>
>>
>> -int qemuMonitorJSONGetDeviceProps(qemuMonitorPtr mon,
>> -  const char *device,
>> -  char ***props)
>> +static int
>> +qemuMonitorJSONParsePropsList(qemuMonitorPtr mon,
>> +  virJSONValuePtr cmd,
>> +  char ***props)
>> {
>> -    int ret = -1;
>> -    virJSONValuePtr cmd;
>>     virJSONValuePtr reply = NULL;
>>     virJSONValuePtr data;
>>     char **proplist = NULL;
>>     ssize_t n = 0;
>>     size_t i;
>> -
>> -    *props = NULL;
>> -
>> -    if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
>> -   "s:typename", device,
>> -   NULL)))
>> -    return -1;
>> +    int ret = -1;
>>
>>     if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
>>     goto cleanup;
>>
>> -    if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) {
>> +    if (qemuMonitorJSONHasError(reply, "DeviceNotFound") ||
>> +    qemuMonitorJSONHasError(reply, "CommandNotFound")) {
> 
> One of the errors is appropriate for device-list-properties,
> the other one for qom-list-properties.

Yes. But it doesn't really matter. If device-list-properties returns
CommandNotFound we will not add any capability. So it's safe.

> 
> Each caller should take care of their error and
> qemuMonitorJSONParsePropsList
> would work with virJSON replies data.

I guess it's just a matter of preference, but okay. I'll rework this patch.

Michal

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

Re: [libvirt] [PATCH] tests: Xen: use qemu-system-i386 for emulator

[Ján Tomko]

On Wed, Apr 11, 2018 at 01:25:10PM -0600, Jim Fehlig wrote:

Many of the old xm and sexpr test files used qemu-dm as the emulator.
Modern Xen systems no longer use the old, forked qemu-dm, instead
preferring the distro provided qemu or an "upstream" qemu that is
built when the Xen tools are built. This qemu is typically installed
in /usr/lib/xen/bin/qemu-system-i386.

The libxl test files already use /usr/lib/xen/bin/qemu-system-i386.
For consistency, change the old test files to use the same emulator

Signed-off-by: Jim Fehlig 
---

These config files can serve as examples too, so best not to
encourage use of outdated/deprecated config.



Reviewed-by: Ján Tomko 

Jano


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