Re: [libvirt] [PATCH] Fix documentation errors about the path of conf files

[Andrea Bolognani]

On Wed, 2017-06-21 at 10:47 +0800, Lily Zhu wrote:
> The conf files, for example, libvirtd.conf, virtlockd.conf and
> virtlogd.conf, should be located under the directory
> "/etc/libvirt", rather than "/etc".
> 
> Signed-off-by: Lily Zhu 
> ---
>  daemon/libvirtd.pod   | 2 +-
>  src/locking/virtlockd.pod | 2 +-
>  src/logging/virtlogd.pod  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/libvirtd.pod b/daemon/libvirtd.pod
> index 3b819a2..1bca509 100644
> --- a/daemon/libvirtd.pod
> +++ b/daemon/libvirtd.pod
> @@ -81,7 +81,7 @@ On receipt of B libvirtd will reload its 
> configuration.
>  
>  =over
>  
> -=item F
> +=item F

Good catch :)

However, further down in each of the files you can find
something along the lines of

  =item F<$XDG_CONFIG_HOME/libvirtd.conf>

which suffers from the same issue. Would you mind sending
a v2 that takes care of that as well?

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] Fix documentation errors about the path of conf files

[Lily Zhu]

The conf files, for example, libvirtd.conf, virtlockd.conf and
virtlogd.conf, should be located under the directory
"/etc/libvirt", rather than "/etc".

Signed-off-by: Lily Zhu 
---
 daemon/libvirtd.pod   | 2 +-
 src/locking/virtlockd.pod | 2 +-
 src/logging/virtlogd.pod  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/daemon/libvirtd.pod b/daemon/libvirtd.pod
index 3b819a2..1bca509 100644
--- a/daemon/libvirtd.pod
+++ b/daemon/libvirtd.pod
@@ -81,7 +81,7 @@ On receipt of B libvirtd will reload its 
configuration.
 
 =over
 
-=item F
+=item F
 
 The default configuration file used by libvirtd, unless overridden on the
 command line using the B<-f>|B<--config> option.
diff --git a/src/locking/virtlockd.pod b/src/locking/virtlockd.pod
index a3bb268..2e155f8 100644
--- a/src/locking/virtlockd.pod
+++ b/src/locking/virtlockd.pod
@@ -67,7 +67,7 @@ upgrades of the virtlockd service.
 
 =over
 
-=item F
+=item F
 
 The default configuration file used by virtlockd, unless overridden on the
 command line using the B<-f>|B<--config> option.
diff --git a/src/logging/virtlogd.pod b/src/logging/virtlogd.pod
index 7e55c9e..5fc20db 100644
--- a/src/logging/virtlogd.pod
+++ b/src/logging/virtlogd.pod
@@ -67,7 +67,7 @@ upgrades of the virtlogd service.
 
 =over
 
-=item F
+=item F
 
 The default configuration file used by virtlogd, unless overridden on the
 command line using the B<-f>|B<--config> option.
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] qemu: capabilities: Move comments separating groups of capabilities

[Andrea Bolognani]

On Tue, 2017-06-20 at 16:17 +0200, Peter Krempa wrote:
> Similarly to how we specify the groups of 5 capabilities in the header
> file move the labels to separate line also for the VIR_ENUM_IMPL part.
> 
> This simplifies rebase conflict resolution in the capability file since
> only lines have to be shuffled around, but they don't need to be edited.
> ---
>  src/qemu/qemu_capabilities.c | 159 
> ---
>  1 file changed, 106 insertions(+), 53 deletions(-)

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] [PATCH v2] util: fix locale problem with virStrToDouble().

[Julio Faracco]

Hi Martin,

2017-06-20 11:42 GMT-03:00 Martin Kletzander :
> On Sun, Jun 18, 2017 at 03:20:11PM -0300, Julio Faracco wrote:
>>
>> This commit fixes a locale problem with locales that use comma as a
>> mantissa
>> separator. Example: 12.34 en_US = 12,34 pt_BR. Since strtod() is a
>> non-safe
>> function, virStrToDouble() will have problems to parse double numbers from
>> kernel settings and other double numbers from static files (XMLs, JSONs,
>> etc).
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457634
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457481
>>
>> Signed-off-by: Julio Faracco 
>> ---
>> src/util/virstring.c | 27 +++
>> 1 file changed, 27 insertions(+)
>>
>
> I don't really like the duplication of the data and code.  I would

I totally agree. I needed to duplicate the code because virDoubleToStr()
is inside virutil as you mentioned.

> rather see virDoubleToStr move to virstring (it can be called
> virStrFromDouble if some don't like the move), it is called from one
> place anyway and that way it can share some of the code and data at
> least.
>

I will join the methods inside virstring and submit a v3.

> I apologize for asking you for yet another version (and possibly
> splitting it into two patches -- the move and the fix), but I haven't
> notice in the previous submission.
>
> Have a nice day,
> Martin

Thanks!

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

[libvirt] [PATCH 3/7] qemu: Implement qemuDomainManagedSaveGetXMLDesc

[Kothapally Madhu Pavan]

This commit adds qemu driver implementaion to get xml description
for managed save state domain.

Signed-off-by: Kothapally Madhu Pavan 
---
 src/qemu/qemu_driver.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e91663c..c9b3ef3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6798,6 +6798,45 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
 return ret;
 }
 
+static char *
+qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm;
+char *path = NULL;
+char *ret = NULL;
+virDomainDefPtr def = NULL;
+int fd = -1;
+virQEMUSaveDataPtr data = NULL;
+
+/* We only take subset of virDomainDefFormat flags.  */
+virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return ret;
+
+path = qemuDomainManagedSavePath(driver, vm);
+
+if (!path)
+goto cleanup;
+
+fd = qemuDomainSaveImageOpen(driver, path, , ,
+ false, NULL, false, false);
+if (fd < 0)
+goto cleanup;
+if (virDomainManagedSaveGetXMLDescEnsureACL(dom->conn, def, flags) < 0)
+goto cleanup;
+ret = qemuDomainDefFormatXML(driver, def, flags);
+
+ cleanup:
+virQEMUSaveDataFree(data);
+virDomainDefFree(def);
+VIR_FORCE_CLOSE(fd);
+virDomainObjEndAPI();
+VIR_FREE(path);
+return ret;
+}
+
 /* Return 0 on success, 1 if incomplete saved image was silently unlinked,
  * and -1 on failure with error raised.  */
 static int
@@ -20810,6 +20849,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainManagedSave = qemuDomainManagedSave, /* 0.8.0 */
 .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */
+.domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.5.0 */
 .domainSnapshotCreateXML = qemuDomainSnapshotCreateXML, /* 0.8.0 */
 .domainSnapshotGetXMLDesc = qemuDomainSnapshotGetXMLDesc, /* 0.8.0 */
 .domainSnapshotNum = qemuDomainSnapshotNum, /* 0.8.0 */
-- 
1.8.3.1

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

[libvirt] [PATCH 5/7] virsh: Implement managedsave-define command

[Kothapally Madhu Pavan]

Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan 
---
 tools/virsh-domain.c | 79 
 tools/virsh.pod  | 14 ++
 2 files changed, 93 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 91bdb58..aadacef 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4705,6 +4705,79 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * "managedsave-define" command
+ */
+static const vshCmdInfo info_managed_save_define[] = {
+{.name = "help",
+ .data = N_("redefine the XML for a domain's managed save state file")
+},
+{.name = "desc",
+ .data = N_("Replace the domain XML associated with a managed save state 
file")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_managed_save_define[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL,
+{.name = "xml",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("filename containing updated XML for the target")
+},
+{.name = "running",
+ .type = VSH_OT_BOOL,
+ .help = N_("set domain to be running on start")
+},
+{.name = "paused",
+ .type = VSH_OT_BOOL,
+ .help = N_("set domain to be paused on start")
+},
+{.name = NULL}
+};
+
+static bool
+cmdManagedSaveDefine(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+virDomainPtr dom = NULL;
+const char *xmlfile = NULL;
+char *xml = NULL;
+unsigned int flags = 0;
+
+if (vshCommandOptBool(cmd, "running"))
+flags |= VIR_DOMAIN_SAVE_RUNNING;
+if (vshCommandOptBool(cmd, "paused"))
+flags |= VIR_DOMAIN_SAVE_PAUSED;
+
+VSH_EXCLUSIVE_OPTIONS("running", "paused");
+
+if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
+return false;
+
+if (virFileReadAll(xmlfile, VSH_MAX_XML_FILE, ) < 0)
+return false;
+
+dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (dom == NULL)
+goto cleanup;
+
+if (virDomainManagedSaveDefineXML(dom, xml, flags) < 0) {
+vshError(ctl, _("Failed to update %s XML configuration"),
+virDomainGetName(dom));
+goto cleanup;
+}
+
+vshPrintExtra(ctl, _("Managed save state file of domain %s updated.\n"),
+ virDomainGetName(dom));
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+VIR_FREE(xml);
+return ret;
+}
+
+/*
  * "schedinfo" command
  */
 static const vshCmdInfo info_schedinfo[] = {
@@ -13789,6 +13862,12 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_managedsaveremove,
  .flags = 0
 },
+{.name = "managedsave-define",
+ .handler = cmdManagedSaveDefine,
+ .opts = opts_managed_save_define,
+ .info = info_managed_save_define,
+ .flags = 0
+},
 {.name = "memtune",
  .handler = cmdMemtune,
  .opts = opts_memtune,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index c5bf168..46b4d72 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1624,6 +1624,20 @@ has any managed save image.
 Remove the B state file for a domain, if it exists.  This
 ensures the domain will do a full boot the next time it is started.
 
+=item B I I [{I<--running> | I<--paused>}]
+
+Update the domain XML that will be used when I is later
+started. The I argument must be a file name containing
+the alternative XML, with changes only in the host-specific portions of
+the domain XML. For example, it can be used to account for file naming
+differences resulting from creating disk snapshots of underlying storage
+after the guest was saved.
+
+The managed save image records whether the domain should be started to a
+running or paused state.  Normally, this command does not alter the
+recorded state; passing either the I<--running> or I<--paused> flag
+will allow overriding which state the B should use.
+
 =item B [I]
 
 Provide the maximum number of virtual CPUs supported for a guest VM on
-- 
1.8.3.1

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

[libvirt] [PATCH 4/7] qemu: Implement qemuDomainManagedSaveDefineXML

[Kothapally Madhu Pavan]

This commit adds qemu driver implementation to edit xml
configuration of managed save state file of a domain.

Signed-off-by: Kothapally Madhu Pavan 
---
 src/qemu/qemu_driver.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c9b3ef3..7ce6464 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6837,6 +6837,43 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, 
unsigned int flags)
 return ret;
 }
 
+static int
+qemuDomainManagedSaveDefineXML(virDomainPtr dom, const char *dxml,
+   unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virConnectPtr conn = dom->conn;
+virDomainObjPtr vm;
+char *path = NULL;
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return -1;
+
+path = qemuDomainManagedSavePath(driver, vm);
+if (!path)
+goto error;
+
+virDomainObjEndAPI();
+
+if (conn->driver->domainSaveImageDefineXML) {
+int ret;
+ret = qemuDomainSaveImageDefineXML(conn, path, dxml, flags);
+
+VIR_FREE(path);
+
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+VIR_FREE(path);
+virDispatchError(conn);
+return -1;
+}
+
 /* Return 0 on success, 1 if incomplete saved image was silently unlinked,
  * and -1 on failure with error raised.  */
 static int
@@ -20850,6 +20887,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */
 .domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.5.0 */
+.domainManagedSaveDefineXML = qemuDomainManagedSaveDefineXML, /* 3.5.0 */
 .domainSnapshotCreateXML = qemuDomainSnapshotCreateXML, /* 0.8.0 */
 .domainSnapshotGetXMLDesc = qemuDomainSnapshotGetXMLDesc, /* 0.8.0 */
 .domainSnapshotNum = qemuDomainSnapshotNum, /* 0.8.0 */
-- 
1.8.3.1

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

[libvirt] [PATCH 7/7] virsh: Implement managedsave-edit command

[Kothapally Madhu Pavan]

Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan 
---
 tools/virsh-domain.c | 72 
 tools/virsh.pod  | 21 +++
 2 files changed, 93 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 874cf49..12721e7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4705,6 +4705,72 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * "managedsave-edit" command
+ */
+static const vshCmdInfo info_managed_save_edit[] = {
+   {.name = "help",
+.data = N_("edit XML for a domain's managed save state file")
+   },
+   {.name = "desc",
+.data = N_("Edit the domain XML associated with the managed save state 
file")
+   },
+   {.name = NULL}
+};
+
+static const vshCmdOptDef opts_managed_save_edit[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL,
+{.name = "running",
+ .type = VSH_OT_BOOL,
+ .help = N_("set domain to be running on start")
+},
+{.name = "paused",
+ .type = VSH_OT_BOOL,
+ .help = N_("set domain to be paused on start")
+},
+{.name = NULL}
+};
+
+static bool
+cmdManagedSaveEdit(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+virDomainPtr dom = NULL;
+unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE;
+unsigned int define_flags = 0;
+
+if (vshCommandOptBool(cmd, "running"))
+define_flags |= VIR_DOMAIN_SAVE_RUNNING;
+if (vshCommandOptBool(cmd, "paused"))
+define_flags |= VIR_DOMAIN_SAVE_PAUSED;
+
+VSH_EXCLUSIVE_OPTIONS("running", "paused");
+
+dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (dom == NULL)
+goto cleanup;
+
+#define EDIT_GET_XML virDomainManagedSaveGetXMLDesc(dom, getxml_flags)
+#define EDIT_NOT_CHANGED   
   \
+do {   
   \
+vshPrintExtra(ctl, _("Managed save image of domain %s XML 
configuration " \
+ "not changed.\n"), virDomainGetName(dom));
   \
+ret = true;
   \
+goto edit_cleanup; 
   \
+} while (0)
+#define EDIT_DEFINE \
+(virDomainManagedSaveDefineXML(dom, doc_edited, define_flags) == 0)
+#include "virsh-edit.c"
+
+vshPrintExtra(ctl, _("Managed save image of Domain %s XML configuration 
edited.\n"),
+  virDomainGetName(dom));
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+return ret;
+}
+
+/*
  * "managedsave-dumpxml" command
  */
 static const vshCmdInfo info_managed_save_dumpxml[] = {
@@ -13912,6 +13978,12 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_managedsaveremove,
  .flags = 0
 },
+{.name = "managedsave-edit",
+ .handler = cmdManagedSaveEdit,
+ .opts = opts_managed_save_edit,
+ .info = info_managed_save_edit,
+ .flags = 0
+},
 {.name = "managedsave-dumpxml",
  .handler = cmdManagedSaveDumpxml,
  .opts = opts_managed_save_dumpxml,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index e93460e..639391a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1644,6 +1644,27 @@ Extract the domain XML that was in effect at the time 
the saved state
 file I was created with the B command.  Using
 I<--security-info> will also include security sensitive information.
 
+=item B I [{I<--running> | I<--paused>}]
+
+Edit the XML configuration associated with a saved state file of a
+I was created by the B command.
+
+The managed save image records whether the domain should be started to a
+running or paused state.  Normally, this command does not alter the
+recorded state; passing either the I<--running> or I<--paused> flag
+will allow overriding which state the B should use.
+
+This is equivalent to:
+
+ virsh managedsave-dumpxml domain-name > state-file.xml
+ vi state-file.xml (or make changes with your other text editor)
+ virsh managedsave-define domain-name state-file-xml
+
+except that it does some error checking.
+
+The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment
+variables, and defaults to C.
+
 =item B [I]
 
 Provide the maximum number of virtual CPUs supported for a guest VM on
-- 
1.8.3.1

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

[libvirt] [PATCH 6/7] virsh: Implement managedsave-dumpxml command

[Kothapally Madhu Pavan]

Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan 
---
 tools/virsh-domain.c | 56 
 tools/virsh.pod  |  6 ++
 2 files changed, 62 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index aadacef..874cf49 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -4705,6 +4705,56 @@ cmdManagedSaveRemove(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * "managedsave-dumpxml" command
+ */
+static const vshCmdInfo info_managed_save_dumpxml[] = {
+   {.name = "help",
+.data = N_("Domain information of managed save state file in XML")
+   },
+   {.name = "desc",
+.data = N_("Dump XML of domain information for a managed save state file 
to stdout.")
+   },
+   {.name = NULL}
+};
+
+static const vshCmdOptDef opts_managed_save_dumpxml[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL,
+{.name = "security-info",
+ .type = VSH_OT_BOOL,
+ .help = N_("include security sensitive information in XML dump")
+},
+{.name = NULL}
+};
+
+static bool
+cmdManagedSaveDumpxml(vshControl *ctl, const vshCmd *cmd)
+{
+bool ret = false;
+virDomainPtr dom = NULL;
+unsigned int flags = 0;
+char *xml = NULL;
+
+if (vshCommandOptBool(cmd, "security-info"))
+flags |= VIR_DOMAIN_XML_SECURE;
+
+dom = virshCommandOptDomain(ctl, cmd, NULL);
+if (dom == NULL)
+goto cleanup;
+
+xml = virDomainManagedSaveGetXMLDesc(dom, flags);
+if (!xml)
+goto cleanup;
+
+vshPrint(ctl, "%s", xml);
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+VIR_FREE(xml);
+return ret;
+}
+
+/*
  * "managedsave-define" command
  */
 static const vshCmdInfo info_managed_save_define[] = {
@@ -13862,6 +13912,12 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_managedsaveremove,
  .flags = 0
 },
+{.name = "managedsave-dumpxml",
+ .handler = cmdManagedSaveDumpxml,
+ .opts = opts_managed_save_dumpxml,
+ .info = info_managed_save_dumpxml,
+ .flags = 0
+},
 {.name = "managedsave-define",
  .handler = cmdManagedSaveDefine,
  .opts = opts_managed_save_define,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 46b4d72..e93460e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1638,6 +1638,12 @@ running or paused state.  Normally, this command does 
not alter the
 recorded state; passing either the I<--running> or I<--paused> flag
 will allow overriding which state the B should use.
 
+=item B I [I<--security-info>]
+
+Extract the domain XML that was in effect at the time the saved state
+file I was created with the B command.  Using
+I<--security-info> will also include security sensitive information.
+
 =item B [I]
 
 Provide the maximum number of virtual CPUs supported for a guest VM on
-- 
1.8.3.1

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

[libvirt] [PATCH 1/7] lib: Add API to dump xml configuration of managed save state domain

[Kothapally Madhu Pavan]

Similar to domainSaveImageGetXMLDesc this commit adds 
domainManagedSaveGetXMLDesc
API which allows to get the xml of managed save state domain.

This allows to edit the managed save state domain xml.

Signed-off-by: Kothapally Madhu Pavan 
---
 include/libvirt/libvirt-domain.h |  2 ++
 src/driver-hypervisor.h  |  5 
 src/libvirt-domain.c | 49 
 src/libvirt_public.syms  |  5 
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 19 ++--
 src/remote_protocol-structs  |  8 +++
 7 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 45f939a..56ab5d7 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1209,6 +1209,8 @@ int
virDomainHasManagedSaveImage(virDomainPtr dom,
 unsigned int flags);
 intvirDomainManagedSaveRemove(virDomainPtr dom,
   unsigned int flags);
+char * virDomainManagedSaveGetXMLDesc(virDomainPtr domain,
+  unsigned int flags);
 
 /*
  * Domain core dump
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 3053d7a..598fc06 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -745,6 +745,10 @@ typedef int
 (*virDrvDomainManagedSaveRemove)(virDomainPtr domain,
  unsigned int flags);
 
+typedef char *
+(*virDrvDomainManagedSaveGetXMLDesc)(virDomainPtr domain,
+ unsigned int flags);
+
 typedef virDomainSnapshotPtr
 (*virDrvDomainSnapshotCreateXML)(virDomainPtr domain,
  const char *xmlDesc,
@@ -1422,6 +1426,7 @@ struct _virHypervisorDriver {
 virDrvDomainManagedSave domainManagedSave;
 virDrvDomainHasManagedSaveImage domainHasManagedSaveImage;
 virDrvDomainManagedSaveRemove domainManagedSaveRemove;
+virDrvDomainManagedSaveGetXMLDesc domainManagedSaveGetXMLDesc;
 virDrvDomainSnapshotCreateXML domainSnapshotCreateXML;
 virDrvDomainSnapshotGetXMLDesc domainSnapshotGetXMLDesc;
 virDrvDomainSnapshotNum domainSnapshotNum;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 9bda3c2..b67f8fd 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9302,6 +9302,55 @@ virDomainManagedSaveRemove(virDomainPtr dom, unsigned 
int flags)
 }
 
 
+/**
+ * virDomainManagedSaveGetXMLDesc:
+ * @domain: a domain object
+ * @flags: bitwise-OR of subset of virDomainXMLFlags
+ *
+ * This method will extract the XML description of the managed save
+ * state file of a domain.
+ *
+ * No security-sensitive data will be included unless @flags contains
+ * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
+ * connections.  For this API, @flags should not contain either
+ * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ *
+ * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of
+ * error.  The caller must free() the returned value.
+ */
+char *
+virDomainManagedSaveGetXMLDesc(virDomainPtr domain, unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "flags=%x", flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, NULL);
+conn = domain->conn;
+
+if ((conn->flags & VIR_CONNECT_RO) && (flags & VIR_DOMAIN_XML_SECURE)) {
+virReportError(VIR_ERR_OPERATION_DENIED, "%s",
+   _("virDomainManagedSaveGetXMLDesc with secure flag"));
+goto error;
+}
+
+if (conn->driver->domainManagedSaveGetXMLDesc) {
+char *ret;
+ret = conn->driver->domainManagedSaveGetXMLDesc(domain, flags);
+if (!ret)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain->conn);
+return NULL;
+}
+
 
 /**
  * virDomainOpenConsole:
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index fac77fb..acba9ea 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -768,4 +768,9 @@ LIBVIRT_3.4.0 {
 virStreamSparseSendAll;
 } LIBVIRT_3.1.0;
 
+LIBVIRT_3.5.0 {
+global:
+virDomainManagedSaveGetXMLDesc;
+} LIBVIRT_3.4.0;
+
 #  define new API here using predicted next version number 
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b452e8b..5b11147 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8410,6 +8410,7 @@ static virHypervisorDriver hypervisor_driver = {
 .domainManagedSave = remoteDomainManagedSave, /* 0.8.0 */
 .domainHasManagedSaveImage = remoteDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = remoteDomainManagedSaveRemove, /* 0.8.0 */
+.domainManagedSaveGetXMLDesc 

[libvirt] [PATCH 0/7] Add new APIs to edit xml configuration of managed save state of a domain

[Kothapally Madhu Pavan]

managedsave command offloads the user from managing the save state file.
It does not need the user to specify saved state file location, all it takes
is domain name to identify. This makes it much more comfortable to use in
emergency where immediate shutdowm is needed. But it doesn't provide a way
to edit XML description of the save state file without user going through an
extra effort to search manually where the file actually exists.

The series aims to overcome the above constraints by adding new APIs and
commands to seemlessly edit the managed save state XML description using
just the domain name. The Patches mainly make use of the save-image-edit
code flow only to simplify the above use case.

This patch set provides capability to Dump and Edit the XML configuration
associated with a saved state file of a domain which was created by the
managedsave command.

The new command carry the similar options as the save-image- commands
to change the running state as to paused state or running on start.

This is equivalent to:

 virsh managedsave-dumpxml domain-name > state-file.xml
 vi state-file.xml (or make changes with your other text editor)
 virsh managedsave-define domain-name state-file-xml

or you can simply use:

 virsh managedsave-edit domain-name

It is always better when we get more.

Kothapally Madhu Pavan (7):
  lib: Add API to dump xml configuration of managed save state domain
  lib: Add API to edit domain's managed save state xml configuration
  qemu: Implement qemuDomainManagedSaveGetXMLDesc
  qemu: Implement qemuDomainManagedSaveDefineXML
  virsh: Implement managedsave-define command
  virsh: Implement managedsave-dumpxml command
  virsh: Implement managedsave-edit command

 include/libvirt/libvirt-domain.h |   6 ++
 src/driver-hypervisor.h  |  11 +++
 src/libvirt-domain.c | 107 
 src/libvirt_public.syms  |   6 ++
 src/qemu/qemu_driver.c   |  78 +++
 src/remote/remote_driver.c   |   2 +
 src/remote/remote_protocol.x |  31 +-
 src/remote_protocol-structs  |  14 +++
 tools/virsh-domain.c | 207 +++
 tools/virsh.pod  |  41 
 10 files changed, 502 insertions(+), 1 deletion(-)

-- 
1.8.3.1

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

[libvirt] [PATCH 2/7] lib: Add API to edit domain's managed save state xml configuration

[Kothapally Madhu Pavan]

Similar to domainSaveImageDefineXML this commit adds domainManagedSaveDefineXML
API which allows to edit domain's managed save state xml configuration.

Signed-off-by: Kothapally Madhu Pavan 
---
 include/libvirt/libvirt-domain.h |  4 +++
 src/driver-hypervisor.h  |  6 +
 src/libvirt-domain.c | 58 
 src/libvirt_public.syms  |  1 +
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 16 ++-
 src/remote_protocol-structs  |  8 +-
 7 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 56ab5d7..53bebf1 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1211,6 +1211,10 @@ int
virDomainManagedSaveRemove(virDomainPtr dom,
   unsigned int flags);
 char * virDomainManagedSaveGetXMLDesc(virDomainPtr domain,
   unsigned int flags);
+intvirDomainManagedSaveDefineXML(virDomainPtr domain,
+ const char *dxml,
+ unsigned int flags);
+
 
 /*
  * Domain core dump
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 598fc06..0a4181e 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -749,6 +749,11 @@ typedef char *
 (*virDrvDomainManagedSaveGetXMLDesc)(virDomainPtr domain,
  unsigned int flags);
 
+typedef int
+(*virDrvDomainManagedSaveDefineXML)(virDomainPtr domain,
+const char *dxml,
+unsigned int flags);
+
 typedef virDomainSnapshotPtr
 (*virDrvDomainSnapshotCreateXML)(virDomainPtr domain,
  const char *xmlDesc,
@@ -1427,6 +1432,7 @@ struct _virHypervisorDriver {
 virDrvDomainHasManagedSaveImage domainHasManagedSaveImage;
 virDrvDomainManagedSaveRemove domainManagedSaveRemove;
 virDrvDomainManagedSaveGetXMLDesc domainManagedSaveGetXMLDesc;
+virDrvDomainManagedSaveDefineXML domainManagedSaveDefineXML;
 virDrvDomainSnapshotCreateXML domainSnapshotCreateXML;
 virDrvDomainSnapshotGetXMLDesc domainSnapshotGetXMLDesc;
 virDrvDomainSnapshotNum domainSnapshotNum;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index b67f8fd..fc5f4ed 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9353,6 +9353,64 @@ virDomainManagedSaveGetXMLDesc(virDomainPtr domain, 
unsigned int flags)
 
 
 /**
+ * virDomainManagedSaveDefineXML:
+ * @domain: a domain object
+ * @dxml: XML config for adjusting guest xml used on restore
+ * @flags: bitwise-OR of virDomainSaveRestoreFlags
+ *
+ * This updates the definition of a domain stored in a saved state
+ * file. @domain is used to extract the saved state file location.
+ *
+ * @dxml can be used to alter host-specific portions of the domain XML
+ * that will be used on the next start of the domain. For example, it is
+ * possible to alter the backing filename that is associated with a
+ * disk device, to match renaming done as part of backing up the disk
+ * device while the domain is stopped.
+ *
+ * Normally, the saved state file will remember whether the domain was
+ * running or paused, and restore defaults to the same state.
+ * Specifying VIR_DOMAIN_SAVE_RUNNING or VIR_DOMAIN_SAVE_PAUSED in
+ * @flags will override the default saved into the file; omitting both
+ * leaves the file's default unchanged.  These two flags are mutually
+ * exclusive.
+ *
+ * Returns 0 in case of success and -1 in case of failure.
+ */
+int
+virDomainManagedSaveDefineXML(virDomainPtr domain, const char *dxml,
+  unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "flags=%x", flags);
+
+virResetLastError();
+
+VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING,
+ VIR_DOMAIN_SAVE_PAUSED,
+ error);
+
+virCheckDomainReturn(domain, -1);
+conn = domain->conn;
+
+if (conn->driver->domainManagedSaveDefineXML) {
+int ret;
+ret = conn->driver->domainManagedSaveDefineXML(domain, dxml, flags);
+
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain->conn);
+return -1;
+}
+
+
+/**
  * virDomainOpenConsole:
  * @dom: a domain object
  * @dev_name: the console, serial or parallel port device alias, or NULL
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index acba9ea..8c33c6b 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -771,6 +771,7 @@ LIBVIRT_3.4.0 {
 LIBVIRT_3.5.0 {
 global:
 virDomainManagedSaveGetXMLDesc;
+   

[libvirt] [PATCH 3/3] add reference tests for hostsfiles

[aruiz]

From: Alberto Ruiz 

---
 src/libvirt_private.syms   |  1 +
 src/network/bridge_driver.c| 12 -
 src/network/bridge_driver.h|  1 +
 src/util/virdnsmasq.c  | 54 +-
 src/util/virdnsmasq.h  |  1 +
 .../dhcp6-nat-network.hostsfile|  7 +++
 tests/networkxml2confdata/dhcp6-network.hostsfile  |  5 ++
 .../dhcp6host-routed-network.hostsfile |  7 +++
 .../nat-network-dns-srv-record-minimal.hostsfile   |  2 +
 .../nat-network-dns-srv-record.hostsfile   |  2 +
 .../nat-network-dns-txt-record.hostsfile   |  2 +
 .../nat-network-name-with-quotes.hostsfile |  2 +
 tests/networkxml2confdata/nat-network.hostsfile|  2 +
 tests/networkxml2conftest.c| 39 
 14 files changed, 116 insertions(+), 21 deletions(-)
 create mode 100644 tests/networkxml2confdata/dhcp6-nat-network.hostsfile
 create mode 100644 tests/networkxml2confdata/dhcp6-network.hostsfile
 create mode 100644 tests/networkxml2confdata/dhcp6host-routed-network.hostsfile
 create mode 100644 
tests/networkxml2confdata/nat-network-dns-srv-record-minimal.hostsfile
 create mode 100644 
tests/networkxml2confdata/nat-network-dns-srv-record.hostsfile
 create mode 100644 
tests/networkxml2confdata/nat-network-dns-txt-record.hostsfile
 create mode 100644 
tests/networkxml2confdata/nat-network-name-with-quotes.hostsfile
 create mode 100644 tests/networkxml2confdata/nat-network.hostsfile

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 923afd187..c93385958 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1497,6 +1497,7 @@ dnsmasqCapsRefresh;
 dnsmasqContextFree;
 dnsmasqContextNew;
 dnsmasqDelete;
+dnsmasqDhcpHostsToString;
 dnsmasqReload;
 dnsmasqSave;
 
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 1cffd4dcf..1d2ada208 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -952,6 +952,7 @@ int
 networkDnsmasqConfContents(virNetworkObjPtr network,
const char *pidfile,
char **configstr,
+   char **hostsfilestr,
dnsmasqContext *dctx,
dnsmasqCapsPtr caps ATTRIBUTE_UNUSED)
 {
@@ -1311,6 +1312,15 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
 if (networkBuildDnsmasqDhcpHostsList(dctx, ipdef) < 0)
 goto cleanup;
 
+/* Return the contents of the hostsfile if requested */
+if (hostsfilestr) {
+*hostsfilestr = dnsmasqDhcpHostsToString (dctx->hostsfile->hosts,
+  dctx->hostsfile->nhosts);
+
+if (!hostsfilestr)
+goto cleanup;
+}
+
 /* Note: the following is IPv4 only */
 if (VIR_SOCKET_ADDR_IS_FAMILY(>address, AF_INET)) {
 if (ipdef->nranges || ipdef->nhosts)
@@ -1410,7 +1420,7 @@ 
networkBuildDhcpDaemonCommandLine(virNetworkDriverStatePtr driver,
 
 network->dnsmasqPid = -1;
 
-if (networkDnsmasqConfContents(network, pidfile, ,
+if (networkDnsmasqConfContents(network, pidfile, , NULL,
dctx, dnsmasq_caps) < 0)
 goto cleanup;
 if (!configstr)
diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h
index ff7f921ed..c653c507f 100644
--- a/src/network/bridge_driver.h
+++ b/src/network/bridge_driver.h
@@ -53,6 +53,7 @@ int networkGetActualType(virDomainNetDefPtr iface)
 int networkDnsmasqConfContents(virNetworkObjPtr network,
 const char *pidfile,
 char **configstr,
+char **hostsfilestr,
 dnsmasqContext *dctx,
 dnsmasqCapsPtr caps);
 
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 92f834fe7..94c9a3bb1 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -386,10 +386,9 @@ hostsfileWrite(const char *path,
dnsmasqDhcpHost *hosts,
unsigned int nhosts)
 {
-char *tmp;
+char *tmp, *content = NULL;
 FILE *f;
 bool istmp = true;
-size_t i;
 int rc = 0;
 
 /* even if there are 0 hosts, create a 0 length file, to allow
@@ -407,17 +406,21 @@ hostsfileWrite(const char *path,
 }
 }
 
-for (i = 0; i < nhosts; i++) {
-if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) {
-rc = -errno;
-VIR_FORCE_FCLOSE(f);
+if (!(content = dnsmasqDhcpHostsToString(hosts, nhosts))) {
+rc = -ENOMEM;
+goto cleanup;
+}
 
-if (istmp)
-unlink(tmp);
+if (fputs(content, f) == EOF) {
+rc = -errno;
+VIR_FORCE_FCLOSE(f);
+
+if (istmp)
+

[libvirt] [PATCH 2/3] lease time support per host in dnsmasq

[aruiz]

From: Alberto Ruiz 

---
 src/network/bridge_driver.c | 56 ++---
 src/util/virdnsmasq.c   | 52 +++--
 src/util/virdnsmasq.h   |  1 +
 3 files changed, 57 insertions(+), 52 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e51e469c8..1cffd4dcf 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -861,30 +861,6 @@ networkKillDaemon(pid_t pid, const char *daemonName, const 
char *networkName)
 return ret;
 }
 
-/* the following does not build a file, it builds a list
- * which is later saved into a file
- */
-
-static int
-networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
- virNetworkIPDefPtr ipdef)
-{
-size_t i;
-bool ipv6 = false;
-
-if (VIR_SOCKET_ADDR_IS_FAMILY(>address, AF_INET6))
-ipv6 = true;
-for (i = 0; i < ipdef->nhosts; i++) {
-virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
-if (VIR_SOCKET_ADDR_VALID(>ip))
-if (dnsmasqAddDhcpHost(dctx, host->mac, >ip,
-   host->name, host->id, ipv6) < 0)
-return -1;
-}
-
-return 0;
-}
-
 static int
 networkBuildDnsmasqHostsList(dnsmasqContext *dctx,
  virNetworkDNSDefPtr dnsdef)
@@ -940,6 +916,38 @@ networkDnsmasqConfLeaseValueToString (int64_t leasetime)
 return result;
 }
 
+/* the following does not build a file, it builds a list
+ * which is later saved into a file
+ */
+
+static int
+networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
+ virNetworkIPDefPtr ipdef)
+{
+int ret = -1;
+size_t i;
+bool ipv6 = false;
+char *leasetime = networkDnsmasqConfLeaseValueToString(ipdef->leasetime);
+
+if (!leasetime)
+goto cleanup;
+
+if (VIR_SOCKET_ADDR_IS_FAMILY(>address, AF_INET6))
+ipv6 = true;
+for (i = 0; i < ipdef->nhosts; i++) {
+virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
+if (VIR_SOCKET_ADDR_VALID(>ip))
+if (dnsmasqAddDhcpHost(dctx, host->mac, >ip,
+   host->name, host->id, leasetime, ipv6) < 0)
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FREE(leasetime);
+return ret;
+}
+
 int
 networkDnsmasqConfContents(virNetworkObjPtr network,
const char *pidfile,
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 1b78c1fad..92f834fe7 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -308,52 +308,47 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
  virSocketAddr *ip,
  const char *name,
  const char *id,
+ const char *leasetime,
  bool ipv6)
 {
+int ret = -1;
 char *ipstr = NULL;
+virBuffer hostbuf = VIR_BUFFER_INITIALIZER;
+
 if (VIR_REALLOC_N(hostsfile->hosts, hostsfile->nhosts + 1) < 0)
 goto error;
 
 if (!(ipstr = virSocketAddrFormat(ip)))
-return -1;
+goto error;
 
 /* the first test determines if it is a dhcpv6 host */
 if (ipv6) {
-if (name && id) {
-if (virAsprintf(>hosts[hostsfile->nhosts].host,
-"id:%s,%s,[%s]", id, name, ipstr) < 0)
-goto error;
-} else if (name && !id) {
-if (virAsprintf(>hosts[hostsfile->nhosts].host,
-"%s,[%s]", name, ipstr) < 0)
-goto error;
-} else if (!name && id) {
-if (virAsprintf(>hosts[hostsfile->nhosts].host,
-"id:%s,[%s]", id, ipstr) < 0)
-goto error;
-}
+if (name && id)
+virBufferAsprintf(, "id:%s,%s,[%s]", id, name, ipstr);
+else if (name && !id)
+virBufferAsprintf(, "%s,[%s]", name, ipstr);
+else if (!name && id)
+virBufferAsprintf(, "id:%s,[%s]", id, ipstr);
 } else if (name && mac) {
-if (virAsprintf(>hosts[hostsfile->nhosts].host, "%s,%s,%s",
-mac, ipstr, name) < 0)
-goto error;
+virBufferAsprintf(, "%s,%s,%s", mac, ipstr, name);
 } else if (name && !mac) {
-if (virAsprintf(>hosts[hostsfile->nhosts].host, "%s,%s",
-name, ipstr) < 0)
-goto error;
+virBufferAsprintf(, "%s,%s", name, ipstr);
 } else {
-if (virAsprintf(>hosts[hostsfile->nhosts].host, "%s,%s",
-mac, ipstr) < 0)
-goto error;
+virBufferAsprintf(, "%s,%s", mac, ipstr);
 }
-VIR_FREE(ipstr);
 
-hostsfile->nhosts++;
+/* The leasetime string already includes comma if there's any value at all 
*/
+virBufferAsprintf(, "%s", leasetime);
 
-return 0;
+if (!(hostsfile->hosts[hostsfile->nhosts].host = virBufferContentAndReset 
()))
+  goto 

[libvirt] [PATCH 1/3] leasetime support for globally

[aruiz]

From: Alberto Ruiz 

---
 docs/schemas/basictypes.rng   | 16 +
 docs/schemas/network.rng  |  8 +++
 src/conf/network_conf.c   | 78 ++-
 src/conf/network_conf.h   |  3 +-
 src/network/bridge_driver.c   | 49 +-
 tests/networkxml2confdata/leasetime-days.conf | 17 +
 tests/networkxml2confdata/leasetime-days.xml  | 18 ++
 tests/networkxml2confdata/leasetime-hours.conf| 17 +
 tests/networkxml2confdata/leasetime-hours.xml | 18 ++
 tests/networkxml2confdata/leasetime-infinite.conf | 17 +
 tests/networkxml2confdata/leasetime-infinite.xml  | 18 ++
 tests/networkxml2confdata/leasetime-minutes.conf  | 17 +
 tests/networkxml2confdata/leasetime-minutes.xml   | 18 ++
 tests/networkxml2confdata/leasetime-seconds.conf  | 17 +
 tests/networkxml2confdata/leasetime-seconds.xml   | 18 ++
 tests/networkxml2confdata/leasetime.conf  | 17 +
 tests/networkxml2confdata/leasetime.xml   | 18 ++
 tests/networkxml2conftest.c   |  7 ++
 18 files changed, 368 insertions(+), 3 deletions(-)
 create mode 100644 tests/networkxml2confdata/leasetime-days.conf
 create mode 100644 tests/networkxml2confdata/leasetime-days.xml
 create mode 100644 tests/networkxml2confdata/leasetime-hours.conf
 create mode 100644 tests/networkxml2confdata/leasetime-hours.xml
 create mode 100644 tests/networkxml2confdata/leasetime-infinite.conf
 create mode 100644 tests/networkxml2confdata/leasetime-infinite.xml
 create mode 100644 tests/networkxml2confdata/leasetime-minutes.conf
 create mode 100644 tests/networkxml2confdata/leasetime-minutes.xml
 create mode 100644 tests/networkxml2confdata/leasetime-seconds.conf
 create mode 100644 tests/networkxml2confdata/leasetime-seconds.xml
 create mode 100644 tests/networkxml2confdata/leasetime.conf
 create mode 100644 tests/networkxml2confdata/leasetime.xml

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 1b4f980e7..8a76c235a 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -518,4 +518,20 @@
 
   
 
+  
+
+  seconds
+  minutes
+  hours
+  days
+
+  
+
+  
+
+  -1
+  4294967295
+
+  
+
 
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 1a18e64b2..4b8056ab6 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -340,6 +340,14 @@
   
   
+
+  
+
+  
+
+
+  
+
 
   
 
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index aa397768c..6f051493f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -30,6 +30,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "virerror.h"
 #include "datatypes.h"
@@ -1031,12 +1033,82 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
 return ret;
 }
 
+static int64_t
+virNetworkDHCPDefGetLeaseTime (xmlNodePtr node,
+   xmlXPathContextPtr ctxt,
+   bool *error)
+{
+int64_t multiplier, result = -1;
+char *leaseString, *leaseUnit;
+xmlNodePtr save;
+
+*error = 0;
+
+save = ctxt->node;
+ctxt->node = node;
+
+leaseString = virXPathString ("string(./leasetime/text())", ctxt);
+leaseUnit   = virXPathString ("string(./leasetime/@unit)", ctxt);
+
+/* If value is not present we set the value to -2 */
+if (leaseString == NULL) {
+result = -2;
+goto cleanup;
+}
+
+if (leaseUnit == NULL || strcmp (leaseUnit, "seconds") == 0)
+multiplier = 1;
+else if (strcmp (leaseUnit, "minutes") == 0)
+multiplier = 60;
+else if (strcmp (leaseUnit, "hours") == 0)
+multiplier = 60 * 60;
+else if (strcmp (leaseUnit, "days") == 0)
+multiplier = 60 * 60 * 24;
+else {
+virReportError(VIR_ERR_XML_ERROR,
+   _("invalid value for unit parameter in  
element found in  network, "
+ "only 'seconds', 'minutes', 'hours' or 'days' are 
valid: %s"),
+   leaseUnit);
+*error = 1;
+goto cleanup;
+}
+
+errno = 0;
+result = (int64_t) strtoll((const char*)leaseString, NULL, 10);
+
+/* Report any errors parsing the string */
+if (errno != 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(" value could not be converted to a signed 
integer: %s"),
+  leaseString);
+*error = 1;
+goto cleanup;
+}
+
+result = result * multiplier;
+
+if (result > UINT32_MAX) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(" value 

Re: [libvirt] [PATCH 0/3] Workaround mdev uevent race affecting node device driver

[John Ferlan]

On 06/20/2017 11:03 AM, Erik Skultety wrote:
> This series resolves https://bugzilla.redhat.com/show_bug.cgi?id=1463285
> 
> Erik Skultety (3):
>   util: Report an error when virFileResolveLinkHelper's lstat fails
>   util: Introduce virFileWaitForAccess
>   nodedev: Work around the uevent race by hooking up
> virFileWaitForAccess
> 
>  src/libvirt_private.syms   |  1 +
>  src/node_device/node_device_udev.c | 48 
> +-
>  src/util/virfile.c | 40 ++-
>  src/util/virfile.h |  2 ++
>  4 files changed, 89 insertions(+), 2 deletions(-)
> 
> --
> 2.13.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

FWIW: This seemed a bit familiar to something for NPIV as well. Although
for NPIV the files exist, it's just that they have bogus data.  See:

https://www.redhat.com/archives/libvir-list/2016-June/msg02213.html

The referenced bz:

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

The settle code is used in a number of place in libvirt, search on
virWaitForDevices

John

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

[libvirt] [PATCH] qemu: Avoid fd leak on incoming tunneled migration

[Jiri Denemark]

While qemuProcessIncomingDefNew takes an fd argument and stores it in
qemuProcessIncomingDef structure, the caller is still responsible for
closing the file descriptor.

Introduced by commit v1.2.21-140-ge7c6f4575.

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

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8129dcd40..c23fffef2 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2690,7 +2690,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
   listenAddress, port,
   dataFD[0])))
 goto stopjob;
-dataFD[0] = -1; /* the FD is now owned by incoming */
 
 if (qemuProcessPrepareDomain(dconn, driver, vm,
  VIR_QEMU_PROCESS_START_AUTODESTROY) < 0)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fa9990e5d..443ec88fe 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4169,6 +4169,9 @@ qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc)
  * This function does not copy @path, the caller is responsible for keeping
  * the @path pointer valid during the lifetime of the allocated
  * qemuProcessIncomingDef structure.
+ *
+ * The caller is responsible for closing @fd, calling
+ * qemuProcessIncomingDefFree will NOT close it.
  */
 qemuProcessIncomingDefPtr
 qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps,
-- 
2.13.1

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

Re: [libvirt] [PATCH 2/3] util: Introduce virFileWaitForAccess

[Dawid Zamirski]

On Tue, 2017-06-20 at 17:45 +0200, Michal Privoznik wrote:
> On 06/20/2017 05:21 PM, Dawid Zamirski wrote:
> > On Tue, 2017-06-20 at 17:03 +0200, Erik Skultety wrote:
> > > Since we have a number of places where we workaround timing
> > > issues
> > > with
> > > devices, attributes (files in general) not being available at the
> > > time
> > > of processing them by calling usleep in a loop for a fixed number
> > > of
> > > tries, we could as well have a utility function that would do
> > > that.
> > > Therefore we won't have to duplicate this ugly workaround even
> > > more.
> > > 
> > > This is a prerequisite for
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1463285.
> > > 
> > > Signed-off-by: Erik Skultety 
> > > ---
> > >  src/libvirt_private.syms |  1 +
> > >  src/util/virfile.c   | 36
> > > 
> > >  src/util/virfile.h   |  2 ++
> > >  3 files changed, 39 insertions(+)
> > > 
> > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > > index c1e9471c5..53878a30f 100644
> > > --- a/src/libvirt_private.syms
> > > +++ b/src/libvirt_private.syms
> > > @@ -1698,6 +1698,7 @@ virFileStripSuffix;
> > >  virFileTouch;
> > >  virFileUnlock;
> > >  virFileUpdatePerm;
> > > +virFileWaitForAccess;
> > >  virFileWrapperFdClose;
> > >  virFileWrapperFdFree;
> > >  virFileWrapperFdNew;
> > > diff --git a/src/util/virfile.c b/src/util/virfile.c
> > > index 6bbcc3d15..0b1a91699 100644
> > > --- a/src/util/virfile.c
> > > +++ b/src/util/virfile.c
> > > @@ -4164,3 +4164,39 @@ virFileReadValueString(char **value, const
> > > char *format, ...)
> > >  VIR_FREE(str);
> > >  return ret;
> > >  }
> > > +
> > > +
> > > +/**
> > > + * virFileWaitForAccess:
> > > + * @path: absolute path to a sysfs attribute (can be a symlink)
> > > + * @ms: how long to wait (in milliseconds)
> > > + * @tries: how many times should we try to wait for @path to
> > > become
> > > accessible
> > > + *
> > > + * Checks the existence of @path. In case the file defined by
> > > @path
> > > + * doesn't exist, we wait for it to appear in 100ms (for up to
> > > @tries times).
> > > + *
> > > + * Returns 0 on success, -1 on error (ENOENT is fine here).
> > > + */
> > > +int
> > > +virFileWaitForAccess(const char *path, size_t ms, size_t tries)
> > > +{
> > > +errno = 0;
> > > +
> > > +/* wait for @path to be accessible in @ms milliseconds, up
> > > to
> > > @tries */
> > > +while (tries-- > 0 && !virFileExists(path)) {
> > > +if (errno != ENOENT) {
> > > +virReportSystemError(errno, "%s", path);
> > > +return -1;
> > > +} else if (tries == 10) {
> > > +virReportError(VIR_ERR_INTERNAL_ERROR,
> > > +   _("Failed to access '%s' after %zu
> > > tries"),
> > > +   path, tries);
> > > +return -1;
> > > +} else {
> > > +VIR_DEBUG("Failed to access '%s', re-try in %zu ms",
> > > path, ms);
> > > +usleep(ms * 1000);
> > > +}
> > > +}
> > > +
> > > +return 0;
> > > +}
> > 
> > Just FYI, there's another way to address it by calling udevadm
> > settle
> > before and after "touching" a block device, libguestfs is using
> > this
> > approach and it works very well:
> > 
> > https://github.com/libguestfs/libguestfs/search?utf8=%E2%9C%93=ud
> > ev_s
> > ettle=
> 
> Does it? udevadm settle waits for all the events to be processed, not
> just the one that we want. The wait time would be unpredictable IMO.
> 
> Michal

Well it's a kind of brute-force approach but at least guarantees the
device will be accessible once it exits. Why would setting a custom
arbitrary timeout be better which may be too short? Is the predictable
wait time here more important than being able to open the device at
all?

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

Re: [libvirt] [PATCH 3/3] nodedev: Work around the uevent race by hooking up virFileWaitForAccess

[Peter Krempa]

On Tue, Jun 20, 2017 at 17:03:32 +0200, Erik Skultety wrote:
> If we find ourselves in the situation that the 'add' uevent has been
> fired earlier than the sysfs tree for a device, we'd better wait for the
> attributes to be ready rather than discarding the device from our device
> list forever.

You'd have to wait for an infinite amount of time to guarantee this.

This patch just makes it slightly more probable that libvirt will find
the device.

> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1463285
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/node_device/node_device_udev.c | 48 
> +-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/src/node_device/node_device_udev.c 
> b/src/node_device/node_device_udev.c
> index 174124a1b..4f9ca0c67 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -60,6 +60,43 @@ struct _udevPrivate {
>  };
>  
>  
> +/**
> + * udevWaitForAttrs:
> + * @sys_path: node device base path
> + * @...: attributes to wait for, last attribute must be NULL
> + *
> + * Takes a list of attributes to wait for, waits until all of them are
> + * available, unless the max number of tries (10) has been reached.
> + *
> + * Returns 0 if all attributes became available, -1 on error.

Since this function waits in order to see whether the individual
components are available I don't really see a point in having the
varargs here. A simple helper that only waits for 1 file (basically
formats the path and waits) would be way better.

> + */
> +static int
> +udevWaitForAttrs(const char *sys_path, ...)
> +{
> +int ret = -1;
> +const char *attr = NULL;
> +char *attr_path = NULL;
> +va_list args;
> +
> +va_start(args, sys_path);
> +while ((attr = va_arg(args, char *))) {
> +if (virAsprintf(_path, "%s/%s", sys_path, attr) < 0)
> +goto cleanup;
> +
> +if (virFileWaitForAccess(attr_path, 100, 10) < 0)

So this waits up to 1 second per file in rather long increments (100
ms) which I don't think is really desired.

The only prior art here which I think is somewhat relevant is the
waiting code for netdevs, where a 1 ms timeout with 100 retries is used.

Also note that this will delay the event loop since the function is
called by udevEventHandleCallback which is registered in the event loop.
This is definitely unaceptable. NACK to this approach

> +goto cleanup;
> +
> +VIR_FREE(attr_path);
> +}
> +
> +ret = 0;
> + cleanup:
> +va_end(args);
> +VIR_FREE(attr_path);
> +return ret;
> +}
> +
> +
>  static bool
>  udevHasDeviceProperty(struct udev_device *dev,
>const char *key)
> @@ -1113,12 +1150,21 @@ udevProcessMediatedDevice(struct udev_device *dev,
>  {
>  int ret = -1;
>  const char *uuidstr = NULL;
> +const char *devpath = udev_device_get_syspath(dev);
>  int iommugrp = -1;
>  char *linkpath = NULL;
>  char *canonicalpath = NULL;
>  virNodeDevCapMdevPtr data = >caps->data.mdev;
>  
> -if (virAsprintf(, "%s/mdev_type", udev_device_get_syspath(dev)) 
> < 0)
> +/* Because of a kernel uevent race, we might get the 'add' event prior to
> + * the sysfs tree being ready, so any attempt to access any sysfs 
> attribute
> + * would result in ENOENT and us dropping the device, so let's work 
> around
> + * it by waiting for the attributes to become available.
> + */
> +if (udevWaitForAttrs(devpath, "mdev_type", "iommu_group", NULL) < 0)
> +goto cleanup;

This call formats the same string ...

> +
> +if (virAsprintf(, "%s/mdev_type", devpath) < 0)
>  goto cleanup;

.. as this and then throws it away, just to be reformatted in the same
way.



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

Re: [libvirt] [PATCH 2/3] util: Introduce virFileWaitForAccess

[Michal Privoznik]

On 06/20/2017 05:21 PM, Dawid Zamirski wrote:
> On Tue, 2017-06-20 at 17:03 +0200, Erik Skultety wrote:
>> Since we have a number of places where we workaround timing issues
>> with
>> devices, attributes (files in general) not being available at the
>> time
>> of processing them by calling usleep in a loop for a fixed number of
>> tries, we could as well have a utility function that would do that.
>> Therefore we won't have to duplicate this ugly workaround even more.
>>
>> This is a prerequisite for
>> https://bugzilla.redhat.com/show_bug.cgi?id=1463285.
>>
>> Signed-off-by: Erik Skultety 
>> ---
>>  src/libvirt_private.syms |  1 +
>>  src/util/virfile.c   | 36 
>>  src/util/virfile.h   |  2 ++
>>  3 files changed, 39 insertions(+)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index c1e9471c5..53878a30f 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -1698,6 +1698,7 @@ virFileStripSuffix;
>>  virFileTouch;
>>  virFileUnlock;
>>  virFileUpdatePerm;
>> +virFileWaitForAccess;
>>  virFileWrapperFdClose;
>>  virFileWrapperFdFree;
>>  virFileWrapperFdNew;
>> diff --git a/src/util/virfile.c b/src/util/virfile.c
>> index 6bbcc3d15..0b1a91699 100644
>> --- a/src/util/virfile.c
>> +++ b/src/util/virfile.c
>> @@ -4164,3 +4164,39 @@ virFileReadValueString(char **value, const
>> char *format, ...)
>>  VIR_FREE(str);
>>  return ret;
>>  }
>> +
>> +
>> +/**
>> + * virFileWaitForAccess:
>> + * @path: absolute path to a sysfs attribute (can be a symlink)
>> + * @ms: how long to wait (in milliseconds)
>> + * @tries: how many times should we try to wait for @path to become
>> accessible
>> + *
>> + * Checks the existence of @path. In case the file defined by @path
>> + * doesn't exist, we wait for it to appear in 100ms (for up to
>> @tries times).
>> + *
>> + * Returns 0 on success, -1 on error (ENOENT is fine here).
>> + */
>> +int
>> +virFileWaitForAccess(const char *path, size_t ms, size_t tries)
>> +{
>> +errno = 0;
>> +
>> +/* wait for @path to be accessible in @ms milliseconds, up to
>> @tries */
>> +while (tries-- > 0 && !virFileExists(path)) {
>> +if (errno != ENOENT) {
>> +virReportSystemError(errno, "%s", path);
>> +return -1;
>> +} else if (tries == 10) {
>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>> +   _("Failed to access '%s' after %zu
>> tries"),
>> +   path, tries);
>> +return -1;
>> +} else {
>> +VIR_DEBUG("Failed to access '%s', re-try in %zu ms",
>> path, ms);
>> +usleep(ms * 1000);
>> +}
>> +}
>> +
>> +return 0;
>> +}
> 
> Just FYI, there's another way to address it by calling udevadm settle
> before and after "touching" a block device, libguestfs is using this
> approach and it works very well:
> 
> https://github.com/libguestfs/libguestfs/search?utf8=%E2%9C%93=udev_s
> ettle=

Does it? udevadm settle waits for all the events to be processed, not
just the one that we want. The wait time would be unpredictable IMO.

Michal

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

Re: [libvirt] [PATCH 2/3] util: Introduce virFileWaitForAccess

[Peter Krempa]

On Tue, Jun 20, 2017 at 17:03:31 +0200, Erik Skultety wrote:
> Since we have a number of places where we workaround timing issues with
> devices, attributes (files in general) not being available at the time
> of processing them by calling usleep in a loop for a fixed number of
> tries, we could as well have a utility function that would do that.
> Therefore we won't have to duplicate this ugly workaround even more.
> 
> This is a prerequisite for
> https://bugzilla.redhat.com/show_bug.cgi?id=1463285.

This statement is useless. The helper function can be reused elsewhere.

> 
> Signed-off-by: Erik Skultety 
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virfile.c   | 36 
>  src/util/virfile.h   |  2 ++
>  3 files changed, 39 insertions(+)

[...]

> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 6bbcc3d15..0b1a91699 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -4164,3 +4164,39 @@ virFileReadValueString(char **value, const char 
> *format, ...)
>  VIR_FREE(str);
>  return ret;
>  }
> +
> +
> +/**
> + * virFileWaitForAccess:
> + * @path: absolute path to a sysfs attribute (can be a symlink)
> + * @ms: how long to wait (in milliseconds)
> + * @tries: how many times should we try to wait for @path to become 
> accessible
> + *
> + * Checks the existence of @path. In case the file defined by @path
> + * doesn't exist, we wait for it to appear in 100ms (for up to @tries times).
> + *
> + * Returns 0 on success, -1 on error (ENOENT is fine here).

This description does not make sense. You don't state that this reports
errors. Also the mention of ENOENT does not make sense.

This function in fact sometimes returns enoent if the file does not
appear until timeout.

> + */
> +int
> +virFileWaitForAccess(const char *path, size_t ms, size_t tries)
> +{
> +errno = 0;
> +
> +/* wait for @path to be accessible in @ms milliseconds, up to @tries */
> +while (tries-- > 0 && !virFileExists(path)) {
> +if (errno != ENOENT) {
> +virReportSystemError(errno, "%s", path);

This does not really explain stuff to users. You might want to add a
more comprehensive error message or leave error reporting to users.

> +return -1;
> +} else if (tries == 10) {

This does not make any sense. The while loop counts down and checks if
tries is more than 10. And this checks that tries is equal to 10.

So if somebody passes 11 as @tries he/she gets this error? If tries is
set to < 10 you get success even on timeout?

Did you modify the code without testing it?


> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to access '%s' after %zu tries"),
> +   path, tries);
> +return -1;
> +} else {
> +VIR_DEBUG("Failed to access '%s', re-try in %zu ms", path, ms);

This will spam logs too much. I think a debug prior to the while loop
and one after it succeeds should be enough.

> +usleep(ms * 1000);


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

Re: [libvirt] [PATCH 2/3] util: Introduce virFileWaitForAccess

[Dawid Zamirski]

On Tue, 2017-06-20 at 17:03 +0200, Erik Skultety wrote:
> Since we have a number of places where we workaround timing issues
> with
> devices, attributes (files in general) not being available at the
> time
> of processing them by calling usleep in a loop for a fixed number of
> tries, we could as well have a utility function that would do that.
> Therefore we won't have to duplicate this ugly workaround even more.
> 
> This is a prerequisite for
> https://bugzilla.redhat.com/show_bug.cgi?id=1463285.
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virfile.c   | 36 
>  src/util/virfile.h   |  2 ++
>  3 files changed, 39 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c1e9471c5..53878a30f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1698,6 +1698,7 @@ virFileStripSuffix;
>  virFileTouch;
>  virFileUnlock;
>  virFileUpdatePerm;
> +virFileWaitForAccess;
>  virFileWrapperFdClose;
>  virFileWrapperFdFree;
>  virFileWrapperFdNew;
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 6bbcc3d15..0b1a91699 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -4164,3 +4164,39 @@ virFileReadValueString(char **value, const
> char *format, ...)
>  VIR_FREE(str);
>  return ret;
>  }
> +
> +
> +/**
> + * virFileWaitForAccess:
> + * @path: absolute path to a sysfs attribute (can be a symlink)
> + * @ms: how long to wait (in milliseconds)
> + * @tries: how many times should we try to wait for @path to become
> accessible
> + *
> + * Checks the existence of @path. In case the file defined by @path
> + * doesn't exist, we wait for it to appear in 100ms (for up to
> @tries times).
> + *
> + * Returns 0 on success, -1 on error (ENOENT is fine here).
> + */
> +int
> +virFileWaitForAccess(const char *path, size_t ms, size_t tries)
> +{
> +errno = 0;
> +
> +/* wait for @path to be accessible in @ms milliseconds, up to
> @tries */
> +while (tries-- > 0 && !virFileExists(path)) {
> +if (errno != ENOENT) {
> +virReportSystemError(errno, "%s", path);
> +return -1;
> +} else if (tries == 10) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to access '%s' after %zu
> tries"),
> +   path, tries);
> +return -1;
> +} else {
> +VIR_DEBUG("Failed to access '%s', re-try in %zu ms",
> path, ms);
> +usleep(ms * 1000);
> +}
> +}
> +
> +return 0;
> +}

Just FYI, there's another way to address it by calling udevadm settle
before and after "touching" a block device, libguestfs is using this
approach and it works very well:

https://github.com/libguestfs/libguestfs/search?utf8=%E2%9C%93=udev_s
ettle=

> diff --git a/src/util/virfile.h b/src/util/virfile.h
> index 57ceb8072..42630ebb5 100644
> --- a/src/util/virfile.h
> +++ b/src/util/virfile.h
> @@ -347,6 +347,8 @@ int virFileReadValueScaledInt(unsigned long long
> *value, const char *format, ...
>  int virFileReadValueString(char **value, const char *format, ...)
>   ATTRIBUTE_FMT_PRINTF(2, 3);
>  
> +int virFileWaitForAccess(const char *path, size_t ms, size_t tries);
> +
>  
>  int virFileInData(int fd,
>int *inData,

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

Re: [libvirt] [PATCH 1/3] util: Report an error when virFileResolveLinkHelper's lstat fails

[Peter Krempa]

On Tue, Jun 20, 2017 at 17:03:30 +0200, Erik Skultety wrote:
> During investigation of [1] I saw nothing in the logs that would help me
> get to the root cause. Then I found out that we don't log anything when
> lstat fails. Sure, doesn't happen often, but if it happens we should
> reflect that in the logs to prevent spurious behaviour.
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1463285
> 
> Signed-off-by: Erik Skultety 
> ---
>  src/util/virfile.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index d444b32f8..6bbcc3d15 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -1560,8 +1560,10 @@ virFileResolveLinkHelper(const char *linkpath,
>   * directories, if linkpath is absolute and the basename is
>   * already a non-symlink.  */
>  if (IS_ABSOLUTE_FILE_NAME(linkpath) && !intermediatePaths) {
> -if (lstat(linkpath, ) < 0)
> +if (lstat(linkpath, ) < 0) {
> +virReportSystemError(errno, "%s", linkpath);
>  return -1;
> +}

NACK, this function is designed not to report errors. It's even
documented so. Some other callers even report their own errors.

At most a VIR_DEBUG is appropriate here.


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

[libvirt] [PATCH 0/3] Workaround mdev uevent race affecting node device driver

[Erik Skultety]

This series resolves https://bugzilla.redhat.com/show_bug.cgi?id=1463285

Erik Skultety (3):
  util: Report an error when virFileResolveLinkHelper's lstat fails
  util: Introduce virFileWaitForAccess
  nodedev: Work around the uevent race by hooking up
virFileWaitForAccess

 src/libvirt_private.syms   |  1 +
 src/node_device/node_device_udev.c | 48 +-
 src/util/virfile.c | 40 ++-
 src/util/virfile.h |  2 ++
 4 files changed, 89 insertions(+), 2 deletions(-)

--
2.13.1

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

[libvirt] [PATCH 3/3] nodedev: Work around the uevent race by hooking up virFileWaitForAccess

[Erik Skultety]

If we find ourselves in the situation that the 'add' uevent has been
fired earlier than the sysfs tree for a device, we'd better wait for the
attributes to be ready rather than discarding the device from our device
list forever.

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

Signed-off-by: Erik Skultety 
---
 src/node_device/node_device_udev.c | 48 +-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 174124a1b..4f9ca0c67 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -60,6 +60,43 @@ struct _udevPrivate {
 };
 
 
+/**
+ * udevWaitForAttrs:
+ * @sys_path: node device base path
+ * @...: attributes to wait for, last attribute must be NULL
+ *
+ * Takes a list of attributes to wait for, waits until all of them are
+ * available, unless the max number of tries (10) has been reached.
+ *
+ * Returns 0 if all attributes became available, -1 on error.
+ */
+static int
+udevWaitForAttrs(const char *sys_path, ...)
+{
+int ret = -1;
+const char *attr = NULL;
+char *attr_path = NULL;
+va_list args;
+
+va_start(args, sys_path);
+while ((attr = va_arg(args, char *))) {
+if (virAsprintf(_path, "%s/%s", sys_path, attr) < 0)
+goto cleanup;
+
+if (virFileWaitForAccess(attr_path, 100, 10) < 0)
+goto cleanup;
+
+VIR_FREE(attr_path);
+}
+
+ret = 0;
+ cleanup:
+va_end(args);
+VIR_FREE(attr_path);
+return ret;
+}
+
+
 static bool
 udevHasDeviceProperty(struct udev_device *dev,
   const char *key)
@@ -1113,12 +1150,21 @@ udevProcessMediatedDevice(struct udev_device *dev,
 {
 int ret = -1;
 const char *uuidstr = NULL;
+const char *devpath = udev_device_get_syspath(dev);
 int iommugrp = -1;
 char *linkpath = NULL;
 char *canonicalpath = NULL;
 virNodeDevCapMdevPtr data = >caps->data.mdev;
 
-if (virAsprintf(, "%s/mdev_type", udev_device_get_syspath(dev)) < 
0)
+/* Because of a kernel uevent race, we might get the 'add' event prior to
+ * the sysfs tree being ready, so any attempt to access any sysfs attribute
+ * would result in ENOENT and us dropping the device, so let's work around
+ * it by waiting for the attributes to become available.
+ */
+if (udevWaitForAttrs(devpath, "mdev_type", "iommu_group", NULL) < 0)
+goto cleanup;
+
+if (virAsprintf(, "%s/mdev_type", devpath) < 0)
 goto cleanup;
 
 if (virFileResolveLink(linkpath, ) < 0)
-- 
2.13.1

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

[libvirt] [PATCH 1/3] util: Report an error when virFileResolveLinkHelper's lstat fails

[Erik Skultety]

During investigation of [1] I saw nothing in the logs that would help me
get to the root cause. Then I found out that we don't log anything when
lstat fails. Sure, doesn't happen often, but if it happens we should
reflect that in the logs to prevent spurious behaviour.

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

Signed-off-by: Erik Skultety 
---
 src/util/virfile.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index d444b32f8..6bbcc3d15 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1560,8 +1560,10 @@ virFileResolveLinkHelper(const char *linkpath,
  * directories, if linkpath is absolute and the basename is
  * already a non-symlink.  */
 if (IS_ABSOLUTE_FILE_NAME(linkpath) && !intermediatePaths) {
-if (lstat(linkpath, ) < 0)
+if (lstat(linkpath, ) < 0) {
+virReportSystemError(errno, "%s", linkpath);
 return -1;
+}
 
 if (!S_ISLNK(st.st_mode))
 return VIR_STRDUP_QUIET(*resultpath, linkpath) < 0 ? -1 : 0;
-- 
2.13.1

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

[libvirt] [PATCH 2/3] util: Introduce virFileWaitForAccess

[Erik Skultety]

Since we have a number of places where we workaround timing issues with
devices, attributes (files in general) not being available at the time
of processing them by calling usleep in a loop for a fixed number of
tries, we could as well have a utility function that would do that.
Therefore we won't have to duplicate this ugly workaround even more.

This is a prerequisite for
https://bugzilla.redhat.com/show_bug.cgi?id=1463285.

Signed-off-by: Erik Skultety 
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 36 
 src/util/virfile.h   |  2 ++
 3 files changed, 39 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c1e9471c5..53878a30f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1698,6 +1698,7 @@ virFileStripSuffix;
 virFileTouch;
 virFileUnlock;
 virFileUpdatePerm;
+virFileWaitForAccess;
 virFileWrapperFdClose;
 virFileWrapperFdFree;
 virFileWrapperFdNew;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 6bbcc3d15..0b1a91699 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -4164,3 +4164,39 @@ virFileReadValueString(char **value, const char *format, 
...)
 VIR_FREE(str);
 return ret;
 }
+
+
+/**
+ * virFileWaitForAccess:
+ * @path: absolute path to a sysfs attribute (can be a symlink)
+ * @ms: how long to wait (in milliseconds)
+ * @tries: how many times should we try to wait for @path to become accessible
+ *
+ * Checks the existence of @path. In case the file defined by @path
+ * doesn't exist, we wait for it to appear in 100ms (for up to @tries times).
+ *
+ * Returns 0 on success, -1 on error (ENOENT is fine here).
+ */
+int
+virFileWaitForAccess(const char *path, size_t ms, size_t tries)
+{
+errno = 0;
+
+/* wait for @path to be accessible in @ms milliseconds, up to @tries */
+while (tries-- > 0 && !virFileExists(path)) {
+if (errno != ENOENT) {
+virReportSystemError(errno, "%s", path);
+return -1;
+} else if (tries == 10) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to access '%s' after %zu tries"),
+   path, tries);
+return -1;
+} else {
+VIR_DEBUG("Failed to access '%s', re-try in %zu ms", path, ms);
+usleep(ms * 1000);
+}
+}
+
+return 0;
+}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 57ceb8072..42630ebb5 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -347,6 +347,8 @@ int virFileReadValueScaledInt(unsigned long long *value, 
const char *format, ...
 int virFileReadValueString(char **value, const char *format, ...)
  ATTRIBUTE_FMT_PRINTF(2, 3);
 
+int virFileWaitForAccess(const char *path, size_t ms, size_t tries);
+
 
 int virFileInData(int fd,
   int *inData,
-- 
2.13.1

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

Re: [libvirt] [PATCH] util: storage: Make @backingFormat optional in virStorageFileGetMetadataInternal

[Martin Kletzander]

On Tue, Jun 20, 2017 at 04:37:21PM +0200, Peter Krempa wrote:

Some callers don't need to know the backing format. Make the argument
optional by using a dummy int if NULL is passed.
---
src/util/virstoragefile.c | 4 
src/util/virstoragefile.h | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)



I wanted to say this is weird, even though it works, but then I saw it
already used in virStorageFileGetMetadataFrom{FD,Buf}(), so I guess it's
fine :)


diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 8047d977f..042698872 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -966,9 +966,13 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
  size_t len,
  int *backingFormat)
{
+int format;


I would change this to 'int dummy' so that it's clearly visible it won't
be used (and it's the same as in the other functions).

ACK either way.  Oh, I'm sorry,

Reviewed-by: Martin Kletzander 


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

Re: [libvirt] [PATCH] util: storage: Make @backingFormat optional in virStorageFileGetMetadataInternal

[Jiri Denemark]

On Tue, Jun 20, 2017 at 16:37:21 +0200, Peter Krempa wrote:
> Some callers don't need to know the backing format. Make the argument
> optional by using a dummy int if NULL is passed.
> ---
>  src/util/virstoragefile.c | 4 
>  src/util/virstoragefile.h | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)

ACK

Jirka

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

Re: [libvirt] [PATCH v2] util: fix locale problem with virStrToDouble().

[Martin Kletzander]

On Sun, Jun 18, 2017 at 03:20:11PM -0300, Julio Faracco wrote:

This commit fixes a locale problem with locales that use comma as a mantissa
separator. Example: 12.34 en_US = 12,34 pt_BR. Since strtod() is a non-safe
function, virStrToDouble() will have problems to parse double numbers from
kernel settings and other double numbers from static files (XMLs, JSONs, etc).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457634
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457481

Signed-off-by: Julio Faracco 
---
src/util/virstring.c | 27 +++
1 file changed, 27 insertions(+)



I don't really like the duplication of the data and code.  I would
rather see virDoubleToStr move to virstring (it can be called
virStrFromDouble if some don't like the move), it is called from one
place anyway and that way it can share some of the code and data at
least.

I apologize for asking you for yet another version (and possibly
splitting it into two patches -- the move and the fix), but I haven't
notice in the previous submission.

Have a nice day,
Martin


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

[libvirt] [PATCH] util: storage: Make @backingFormat optional in virStorageFileGetMetadataInternal

[Peter Krempa]

Some callers don't need to know the backing format. Make the argument
optional by using a dummy int if NULL is passed.
---
 src/util/virstoragefile.c | 4 
 src/util/virstoragefile.h | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 8047d977f..042698872 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -966,9 +966,13 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
   size_t len,
   int *backingFormat)
 {
+int format;
 int ret = -1;
 size_t i;

+if (!backingFormat)
+backingFormat = 
+
 VIR_DEBUG("path=%s, buf=%p, len=%zu, meta->format=%d",
   meta->path, buf, len, meta->format);

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index ce54a19ce..0bff8671f 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -293,7 +293,7 @@ int virStorageFileGetMetadataInternal(virStorageSourcePtr 
meta,
   char *buf,
   size_t len,
   int *backingFormat)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

 virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path,
 int fd,
-- 
2.12.2

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

Re: [libvirt] [libvirt-python PATCH] Add details for shutdown event

[Peter Krempa]

On Tue, Jun 20, 2017 at 16:21:34 +0200, Martin Kletzander wrote:
> In commit a8eba5036cb4b0e2ec827e9e6e019ce70e451377, libvirt added
> support for two more details.  Follow that in python bindings as well.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1463188
> 
> Signed-off-by: Martin Kletzander 
> ---
>  examples/event-test.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/event-test.py b/examples/event-test.py
> index 3bca9e27c9be..4eb71425cff4 100755
> --- a/examples/event-test.py
> +++ b/examples/event-test.py
> @@ -477,7 +477,7 @@ def domDetailToString(event, detail):
>  ( "Paused", "Migrated", "IOError", "Watchdog", "Restored", 
> "Snapshot", "API error" ),
>  ( "Unpaused", "Migrated", "Snapshot" ),
>  ( "Shutdown", "Destroyed", "Crashed", "Migrated", "Saved", "Failed", 
> "Snapshot"),
> -( "Finished", ),
> +( "Finished", "On guest request", "On host request"),
>  ( "Memory", "Disk" ),
>  ( "Panicked", ),
>  )

ACK if you mention in the commit message that only the test/example file
was wrong. 


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

[libvirt] [libvirt-python PATCH] Add details for shutdown event

[Martin Kletzander]

In commit a8eba5036cb4b0e2ec827e9e6e019ce70e451377, libvirt added
support for two more details.  Follow that in python bindings as well.

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

Signed-off-by: Martin Kletzander 
---
 examples/event-test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/event-test.py b/examples/event-test.py
index 3bca9e27c9be..4eb71425cff4 100755
--- a/examples/event-test.py
+++ b/examples/event-test.py
@@ -477,7 +477,7 @@ def domDetailToString(event, detail):
 ( "Paused", "Migrated", "IOError", "Watchdog", "Restored", "Snapshot", 
"API error" ),
 ( "Unpaused", "Migrated", "Snapshot" ),
 ( "Shutdown", "Destroyed", "Crashed", "Migrated", "Saved", "Failed", 
"Snapshot"),
-( "Finished", ),
+( "Finished", "On guest request", "On host request"),
 ( "Memory", "Disk" ),
 ( "Panicked", ),
 )
-- 
2.13.1

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

[libvirt] [PATCH] qemu: capabilities: Move comments separating groups of capabilities

[Peter Krempa]

Similarly to how we specify the groups of 5 capabilities in the header
file move the labels to separate line also for the VIR_ENUM_IMPL part.

This simplifies rebase conflict resolution in the capability file since
only lines have to be shuffled around, but they don't need to be edited.
---
 src/qemu/qemu_capabilities.c | 159 ---
 1 file changed, 106 insertions(+), 53 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a9171893e..61c9a1066 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -61,319 +61,372 @@ VIR_LOG_INIT("qemu.qemu_capabilities");
  * daemon restarts
  */
 VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
-  "kqemu",  /* 0 */
+  /* 0 */
+  "kqemu",
   "vnc-colon",
   "no-reboot",
   "drive",
   "drive-boot",

-  "name", /* 5 */
+  /* 5 */
+  "name",
   "uuid",
   "domid",
   "vnet-hdr",
   "migrate-kvm-stdio",

-  "migrate-qemu-tcp", /* 10 */
+  /* 10 */
+  "migrate-qemu-tcp",
   "migrate-qemu-exec",
   "drive-cache-v2",
   "kvm",
   "drive-format",

-  "vga", /* 15 */
+  /* 15 */
+  "vga",
   "0.10",
   "pci-device",
   "mem-path",
   "drive-serial",

-  "xen-domid", /* 20 */
+  /* 20 */
+  "xen-domid",
   "migrate-qemu-unix",
   "chardev",
   "enable-kvm",
   "monitor-json",

-  "balloon", /* 25 */
+  /* 25 */
+  "balloon",
   "device",
   "sdl",
   "smp-topology",
   "netdev",

-  "rtc", /* 30 */
+  /* 30 */
+  "rtc",
   "vhost-net",
   "rtc-td-hack",
   "no-hpet",
   "no-kvm-pit",

-  "tdf", /* 35 */
+  /* 35 */
+  "tdf",
   "pci-configfd",
   "nodefconfig",
   "boot-menu",
   "enable-kqemu",

-  "fsdev", /* 40 */
+  /* 40 */
+  "fsdev",
   "nesting",
   "name-process",
   "drive-readonly",
   "smbios-type",

-  "vga-qxl", /* 45 */
+  /* 45 */
+  "vga-qxl",
   "spice",
   "vga-none",
   "migrate-qemu-fd",
   "boot-index",

-  "hda-duplex", /* 50 */
+  /* 50 */
+  "hda-duplex",
   "drive-aio",
   "pci-multibus",
   "pci-bootindex",
   "ccid-emulated",

-  "ccid-passthru", /* 55 */
+  /* 55 */
+  "ccid-passthru",
   "chardev-spicevmc",
   "device-spicevmc",
   "virtio-tx-alg",
   "device-qxl-vga",

-  "pci-multifunction", /* 60 */
+  /* 60 */
+  "pci-multifunction",
   "virtio-blk-pci.ioeventfd",
   "sga",
   "virtio-blk-pci.event_idx",
   "virtio-net-pci.event_idx",

-  "cache-directsync", /* 65 */
+  /* 65 */
+  "cache-directsync",
   "piix3-usb-uhci",
   "piix4-usb-uhci",
   "usb-ehci",
   "ich9-usb-ehci1",

-  "vt82c686b-usb-uhci", /* 70 */
+  /* 70 */
+  "vt82c686b-usb-uhci",
   "pci-ohci",
   "usb-redir",
   "usb-hub",
   "no-shutdown",

-  "cache-unsafe", /* 75 */
+  /* 75 */
+  "cache-unsafe",
   "rombar",
   "ich9-ahci",
   "no-acpi",
   "fsdev-readonly",

-  "virtio-blk-pci.scsi", /* 80 */
+  /* 80 */
+  "virtio-blk-pci.scsi",
   "blk-sg-io",
   "drive-copy-on-read",
   "cpu-host",
   "fsdev-writeout",

-  "drive-iotune", /* 85 */
+  /* 85 */
+  "drive-iotune",
   "system_wakeup",
   "scsi-disk.channel",
   "scsi-block",
   "transaction",

-  "block-job-sync", /* 90 */
+  /* 90 */
+  "block-job-sync",
   "block-job-async",
   "scsi-cd",
   "ide-cd",
   "no-user-config",

-  "hda-micro", /* 95 */
+  /* 95 */
+  "hda-micro",
   "dump-guest-memory",
   "nec-usb-xhci",
   "virtio-s390",
   "balloon-event",

-  "bridge", 

[libvirt] [PATCH] docs: Add callback-related info to virStream{Abort, Finish}

[Martin Kletzander]

When one has a non-blocking stream and aborts or finishes it without
removing the callback, any event loop invocation will trigger that
callback, but it cannot be removed any more.  We cannot remove the
callback automatically from virStream{Abort,Finish} functions due to
forward-compatibility.  So let's at least document this behaviour,
because it is not easy to find out the reason for.

Signed-off-by: Martin Kletzander 
---

Notes:
The discussion about the reasons:
https://www.redhat.com/archives/libvir-list/2017-June/msg00038.html

 src/libvirt-stream.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
index d7a8f581608f..c49f20264fd7 100644
--- a/src/libvirt-stream.c
+++ b/src/libvirt-stream.c
@@ -1131,6 +1131,9 @@ virStreamEventRemoveCallback(virStreamPtr stream)
  * errors, so if this returns a success code the application can
  * be sure that all data has been successfully processed.
  *
+ * If the stream is non-blocking, any callback must be removed
+ * beforehand.
+ *
  * Returns 0 on success, -1 upon error
  */
 int
@@ -1170,6 +1173,9 @@ virStreamFinish(virStreamPtr stream)
  * streams this can be used to inform the driver that it
  * should stop sending data.
  *
+ * If the stream is non-blocking, any callback must be removed
+ * beforehand.
+ *
  * Returns 0 on success, -1 upon error
  */
 int
-- 
2.13.1

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

Re: [libvirt] [PATCH RFC 2/2] Resctrl: Add uitls functions to operate sysfs resctrl

[Martin Kletzander]

On Mon, Jun 12, 2017 at 05:48:41PM +0800, Eli Qiao wrote:

This patch adds 3 major private interface.

virResctrlGetFreeCache: return free cache, default cache substract cache
   allocated.
virResctrlSetCachetunes: set cache banks which defined in a domain.
virResctrlRemoveCachetunes: remove cache allocation group from the
   host.

There's some existed issue when do syntax-check as I reference the cache
tune and cachebank definition (from conf/domain_conf.h) in
util/virresctrl.h.


Yes, util/ cannot depend on conf/ in libvirt due to various reasons.
All the data you want to use in util/ need to be defined there.  If that
corresponds to some XML, the parsers and formatters must be in conf/.
In rare cases, there might be need for two data structures, one in util/
and one in conf/.  I don't think that's needed in this case.


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

Re: [libvirt] [PATCH 5/5] qemu: snapshot: Load data necessary for relative block commit to work

[Pavel Hrdina]

On Tue, Jun 20, 2017 at 12:56:56PM +0200, Peter Krempa wrote:
> Commit 7456c4f5f introduced a regression by not reloading the backing
> chain of a disk after snapshot. The regression was caused as
> src->relPath was not set and thus the block commit code could not
> determine the relative path.
> 
> This patch adds code that will load the backing store string if
> VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT and store it in the correct place
> when a snapshot is successfully completed.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1461303
> ---
>  src/qemu/qemu_driver.c | 19 +--
>  1 file changed, 17 insertions(+), 2 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 4/5] storage: Add helper to retrieve the backing store string of a storage volume

[Pavel Hrdina]

On Tue, Jun 20, 2017 at 12:56:55PM +0200, Peter Krempa wrote:
> It is necessary for some parts of the code to refresh just data
> based on the based on the backing store string. Add a convenience

s/based on the //

> function that will retrieve this data.
> ---
>  src/storage/storage_driver.c | 44 
> 
>  src/storage/storage_driver.h |  3 +++
>  2 files changed, 47 insertions(+)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 3/5] util: storage: Export virStorageIsRelative

[Pavel Hrdina]

On Tue, Jun 20, 2017 at 12:56:54PM +0200, Peter Krempa wrote:
> ---
>  src/libvirt_private.syms  | 1 +
>  src/util/virstoragefile.c | 2 +-
>  src/util/virstoragefile.h | 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 2/5] qemu: block commit: Don't overwrite error when rolling back disk labels

[Pavel Hrdina]

On Tue, Jun 20, 2017 at 12:56:53PM +0200, Peter Krempa wrote:
> Calls to qemuDomainDiskChainElementPrepare resets the original error,
> thus we need to save it in the cleanup path of qemuDomainBlockCommit.
> ---
>  src/qemu/qemu_driver.c | 6 ++
>  1 file changed, 6 insertions(+)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 1/5] qemu: block commit: Determine relative path of images before initializing

[Pavel Hrdina]

On Tue, Jun 20, 2017 at 12:56:52PM +0200, Peter Krempa wrote:
> Changing labelling of the images does not need to happen after setting
> the labeling and lock manager access. This saves the cleanup of the
> labeling if the relative path can't be determined.
> ---
>  src/qemu/qemu_driver.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)

Reviewed-by: Pavel Hrdina 


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

[libvirt] [PATCH 4/5] storage: Add helper to retrieve the backing store string of a storage volume

[Peter Krempa]

It is necessary for some parts of the code to refresh just data
based on the based on the backing store string. Add a convenience
function that will retrieve this data.
---
 src/storage/storage_driver.c | 44 
 src/storage/storage_driver.h |  3 +++
 2 files changed, 47 insertions(+)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 967776698..ab1dc8f36 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3263,6 +3263,50 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
 }


+/**
+ * virStorageFileGetBackingStoreStr:
+ * @src: storage object
+ *
+ * Extracts the backing store string as stored in the storage volume described
+ * by @src and returns it to the user. Caller is responsible for freeing it.
+ * In case when the string can't be retrieved or does not exist NULL is
+ * returned.
+ */
+char *
+virStorageFileGetBackingStoreStr(virStorageSourcePtr src)
+{
+virStorageSourcePtr tmp = NULL;
+char *buf = NULL;
+ssize_t headerLen;
+char *ret = NULL;
+
+/* exit if we can't load information about the current image */
+if (!virStorageFileSupportsBackingChainTraversal(src))
+return NULL;
+
+if (virStorageFileAccess(src, F_OK) < 0)
+return NULL;
+
+if ((headerLen = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER,
+  )) < 0)
+return NULL;
+
+if (!(tmp = virStorageSourceCopy(src, false)))
+goto cleanup;
+
+if (virStorageFileGetMetadataInternal(tmp, buf, headerLen, NULL) < 0)
+goto cleanup;
+
+VIR_STEAL_PTR(ret, tmp->backingStoreRaw);
+
+ cleanup:
+VIR_FREE(buf);
+virStorageSourceFree(tmp);
+
+return ret;
+}
+
+
 static int
 virStorageAddISCSIPoolSourceHost(virDomainDiskDefPtr def,
  virStoragePoolDefPtr pooldef)
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index 530bc3388..ade05f715 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -54,6 +54,9 @@ int virStorageFileGetMetadata(virStorageSourcePtr src,
   bool report_broken)
 ATTRIBUTE_NONNULL(1);

+char *virStorageFileGetBackingStoreStr(virStorageSourcePtr src)
+ATTRIBUTE_NONNULL(1);
+
 int virStorageTranslateDiskSourcePool(virConnectPtr conn,
   virDomainDiskDefPtr def);

-- 
2.12.2

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

[libvirt] [PATCH 5/5] qemu: snapshot: Load data necessary for relative block commit to work

[Peter Krempa]

Commit 7456c4f5f introduced a regression by not reloading the backing
chain of a disk after snapshot. The regression was caused as
src->relPath was not set and thus the block commit code could not
determine the relative path.

This patch adds code that will load the backing store string if
VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT and store it in the correct place
when a snapshot is successfully completed.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1461303
---
 src/qemu/qemu_driver.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4f62c1a7a..e91663ca9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14091,6 +14091,7 @@ struct _qemuDomainSnapshotDiskData {
 bool created; /* @src was created by the snapshot code */
 bool prepared; /* @src was prepared using 
qemuDomainDiskChainElementPrepare */
 virDomainDiskDefPtr disk;
+char *relPath; /* relative path component to fill into original disk */

 virStorageSourcePtr persistsrc;
 virDomainDiskDefPtr persistdisk;
@@ -14124,6 +14125,7 @@ 
qemuDomainSnapshotDiskDataFree(qemuDomainSnapshotDiskDataPtr data,
 virStorageSourceFree(data[i].src);
 }
 virStorageSourceFree(data[i].persistsrc);
+VIR_FREE(data[i].relPath);
 }

 VIR_FREE(data);
@@ -14139,11 +14141,13 @@ 
qemuDomainSnapshotDiskDataFree(qemuDomainSnapshotDiskDataPtr data,
 static qemuDomainSnapshotDiskDataPtr
 qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
-  virDomainSnapshotObjPtr snap)
+  virDomainSnapshotObjPtr snap,
+  bool reuse)
 {
 size_t i;
 qemuDomainSnapshotDiskDataPtr ret;
 qemuDomainSnapshotDiskDataPtr dd;
+char *backingStoreStr;

 if (VIR_ALLOC_N(ret, snap->def->ndisks) < 0)
 return NULL;
@@ -14167,6 +14171,16 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr 
driver,

 dd->initialized = true;

+/* relative backing store paths need to be updated so that relative
+ * block commit still works */
+if (reuse &&
+(backingStoreStr = virStorageFileGetBackingStoreStr(dd->src))) {
+if (virStorageIsRelative(backingStoreStr))
+VIR_STEAL_PTR(dd->relPath, backingStoreStr);
+else
+VIR_FREE(backingStoreStr);
+}
+
 /* Note that it's unsafe to assume that the disks in the persistent
  * definition match up with the disks in the live definition just by
  * checking that the target name is the same. We've done that
@@ -14210,6 +14224,7 @@ 
qemuDomainSnapshotUpdateDiskSources(qemuDomainSnapshotDiskDataPtr dd,
 if (dd->initialized)
 virStorageFileDeinit(dd->src);

+VIR_STEAL_PTR(dd->disk->src->relPath, dd->relPath);
 VIR_STEAL_PTR(dd->src->backingStore, dd->disk->src);
 VIR_STEAL_PTR(dd->disk->src, dd->src);

@@ -14323,7 +14338,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr 
driver,

 /* prepare a list of objects to use in the vm definition so that we don't
  * have to roll back later */
-if (!(diskdata = qemuDomainSnapshotDiskDataCollect(driver, vm, snap)))
+if (!(diskdata = qemuDomainSnapshotDiskDataCollect(driver, vm, snap, 
reuse)))
 goto cleanup;

 cfg = virQEMUDriverGetConfig(driver);
-- 
2.12.2

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

[libvirt] [PATCH 3/5] util: storage: Export virStorageIsRelative

[Peter Krempa]

---
 src/libvirt_private.syms  | 1 +
 src/util/virstoragefile.c | 2 +-
 src/util/virstoragefile.h | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 044510f09..c1e9471c5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2593,6 +2593,7 @@ virStorageFileParseChainIndex;
 virStorageFileProbeFormat;
 virStorageFileResize;
 virStorageIsFile;
+virStorageIsRelative;
 virStorageNetHostDefClear;
 virStorageNetHostDefCopy;
 virStorageNetHostDefFree;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 3cbbb6c8f..8047d977f 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -801,7 +801,7 @@ virStorageIsFile(const char *backing)
 }


-static bool
+bool
 virStorageIsRelative(const char *backing)
 {
 if (backing[0] == '/')
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 9ebfc1108..ce54a19ce 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -332,6 +332,7 @@ int virStorageFileResize(const char *path,

 int virStorageFileIsClusterFS(const char *path);
 bool virStorageIsFile(const char *path);
+bool virStorageIsRelative(const char *backing);

 int virStorageFileGetLVMKey(const char *path,
 char **key);
-- 
2.12.2

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

[libvirt] [PATCH 1/5] qemu: block commit: Determine relative path of images before initializing

[Peter Krempa]

Changing labelling of the images does not need to happen after setting
the labeling and lock manager access. This saves the cleanup of the
labeling if the relative path can't be determined.
---
 src/qemu/qemu_driver.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3526da8c..0cf4aaa95 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17193,19 +17193,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
 goto endjob;
 }

-/* For the commit to succeed, we must allow qemu to open both the
- * 'base' image and the parent of 'top' as read/write; 'top' might
- * not have a parent, or might already be read-write.  XXX It
- * would also be nice to revert 'base' to read-only, as well as
- * revoke access to files removed from the chain, when the commit
- * operation succeeds, but doing that requires tracking the
- * operation in XML across libvirtd restarts.  */
-clean_access = true;
-if (qemuDomainDiskChainElementPrepare(driver, vm, baseSource, false) < 0 ||
-(top_parent && top_parent != disk->src &&
- qemuDomainDiskChainElementPrepare(driver, vm, top_parent, false) < 0))
-goto endjob;
-
 if (flags & VIR_DOMAIN_BLOCK_COMMIT_RELATIVE &&
 topSource != disk->src) {
 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
@@ -17225,6 +17212,19 @@ qemuDomainBlockCommit(virDomainPtr dom,
 }
 }

+/* For the commit to succeed, we must allow qemu to open both the
+ * 'base' image and the parent of 'top' as read/write; 'top' might
+ * not have a parent, or might already be read-write.  XXX It
+ * would also be nice to revert 'base' to read-only, as well as
+ * revoke access to files removed from the chain, when the commit
+ * operation succeeds, but doing that requires tracking the
+ * operation in XML across libvirtd restarts.  */
+clean_access = true;
+if (qemuDomainDiskChainElementPrepare(driver, vm, baseSource, false) < 0 ||
+(top_parent && top_parent != disk->src &&
+ qemuDomainDiskChainElementPrepare(driver, vm, top_parent, false) < 0))
+goto endjob;
+
 /* Start the commit operation.  Pass the user's original spelling,
  * if any, through to qemu, since qemu may behave differently
  * depending on whether the input was specified as relative or
-- 
2.12.2

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

[libvirt] [PATCH 2/5] qemu: block commit: Don't overwrite error when rolling back disk labels

[Peter Krempa]

Calls to qemuDomainDiskChainElementPrepare resets the original error,
thus we need to save it in the cleanup path of qemuDomainBlockCommit.
---
 src/qemu/qemu_driver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0cf4aaa95..4f62c1a7a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17270,10 +17270,16 @@ qemuDomainBlockCommit(virDomainPtr dom,

  endjob:
 if (ret < 0 && clean_access) {
+virErrorPtr orig_err = virSaveLastError();
 /* Revert access to read-only, if possible.  */
 qemuDomainDiskChainElementPrepare(driver, vm, baseSource, true);
 if (top_parent && top_parent != disk->src)
 qemuDomainDiskChainElementPrepare(driver, vm, top_parent, true);
+
+if (orig_err) {
+virSetError(orig_err);
+virFreeError(orig_err);
+}
 }
 virStorageSourceFree(mirror);
 qemuDomainObjEndJob(driver, vm);
-- 
2.12.2

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

[libvirt] [PATCH 0/5] Fix regression when relative block commit does not work after snapshots

[Peter Krempa]

See patch 5/5.

Peter Krempa (5):
  qemu: block commit: Determine relative path of images before
initializing
  qemu: block commit: Don't overwrite error when rolling back disk
labels
  util: storage: Export virStorageIsRelative
  storage: Add helper to retrieve the backing store string of a storage
volume
  qemu: snapshot: Load data necessary for relative block commit to work

 src/libvirt_private.syms |  1 +
 src/qemu/qemu_driver.c   | 51 +++-
 src/storage/storage_driver.c | 44 ++
 src/storage/storage_driver.h |  3 +++
 src/util/virstoragefile.c|  2 +-
 src/util/virstoragefile.h|  1 +
 6 files changed, 86 insertions(+), 16 deletions(-)

-- 
2.12.2

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

Re: [libvirt] [PATCH] check the return value of qemuBuildVirtioOptionsStr

[Pavel Hrdina]

On Tue, Jun 20, 2017 at 12:25:10PM +0200, Ján Tomko wrote:
> Only qemuBuildFSDevStr missed the return check.
> ---
>  src/qemu/qemu_command.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH] check the return value of qemuBuildVirtioOptionsStr

[Jiri Denemark]

On Tue, Jun 20, 2017 at 12:25:10 +0200, Ján Tomko wrote:
> Only qemuBuildFSDevStr missed the return check.
> ---
>  src/qemu/qemu_command.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 900239f05..57893cfda 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2527,7 +2527,8 @@ qemuBuildFSDevStr(const virDomainDef *def,
>QEMU_FSDEV_HOST_PREFIX, fs->info.alias);
>  virBufferAsprintf(, ",mount_tag=%s", fs->dst);
>  
> -qemuBuildVirtioOptionsStr(, fs->virtio, qemuCaps);
> +if (qemuBuildVirtioOptionsStr(, fs->virtio, qemuCaps) < 0)
> +goto error;
>  
>  if (qemuBuildDeviceAddressStr(, def, >info, qemuCaps) < 0)
>  goto error;

ACK

Jirka

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

[libvirt] [PATCH] check the return value of qemuBuildVirtioOptionsStr

[Ján Tomko]

Only qemuBuildFSDevStr missed the return check.
---
 src/qemu/qemu_command.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 900239f05..57893cfda 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2527,7 +2527,8 @@ qemuBuildFSDevStr(const virDomainDef *def,
   QEMU_FSDEV_HOST_PREFIX, fs->info.alias);
 virBufferAsprintf(, ",mount_tag=%s", fs->dst);
 
-qemuBuildVirtioOptionsStr(, fs->virtio, qemuCaps);
+if (qemuBuildVirtioOptionsStr(, fs->virtio, qemuCaps) < 0)
+goto error;
 
 if (qemuBuildDeviceAddressStr(, def, >info, qemuCaps) < 0)
 goto error;
-- 
2.13.0

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

Re: [libvirt] [PATCH 3/3] Use ATTRIBUTE_FALLTHROUGH

[Marc Hartmayer]

On Tue, Jun 13, 2017 at 01:17 AM +0200, John Ferlan  wrote:
> On 06/07/2017 04:46 AM, Marc Hartmayer wrote:
>> Use ATTRIBUTE_FALLTHROUGH, introduced by commit
>> 5d84f5961b8e28e802f600bb2d2c6903e219092e, instead of comments to
>> indicate that the fall through is an intentional behavior.
>> 
>> Signed-off-by: Marc Hartmayer 
>> Reviewed-by: Boris Fiuczynski 
>> Reviewed-by: Bjoern Walk 
>> ---
>>  src/conf/domain_conf.c   |  2 +-
>>  src/conf/nwfilter_conf.c | 14 +++---
>>  src/cpu/cpu_ppc64.c  |  2 +-
>>  src/libvirt-domain.c |  2 +-
>>  src/libxl/libxl_conf.c   |  2 +-
>>  src/network/leaseshelper.c   |  4 ++--
>>  src/qemu/qemu_command.c  |  2 +-
>>  src/qemu/qemu_domain.c   |  4 ++--
>>  src/qemu/qemu_driver.c   |  4 ++--
>>  src/qemu/qemu_hotplug.c  |  4 ++--
>>  src/qemu/qemu_migration.c|  2 +-
>>  src/remote/remote_driver.c   |  2 +-
>>  src/rpc/virnetservermdns.c   |  2 +-
>>  src/storage/storage_driver.c |  2 +-
>>  src/util/virconf.c   |  2 +-
>>  src/util/virhashcode.c   |  6 +++---
>>  src/util/virnetdevbridge.c   |  1 +
>>  src/util/virutil.c   | 10 +-
>>  tools/virsh-domain.c |  4 ++--
>>  tools/virsh.c|  2 +-
>>  tools/virt-admin.c   |  2 +-
>>  21 files changed, 38 insertions(+), 37 deletions(-)
>> 
>
> Reviewed-by: John Ferlan 
>
> John
>
> I've pushed the series...

Thanks!

> For some reason the brain cells never remember
> who has push access and who doesn't nor how to check ;-)

-- 
Beste Grüße / Kind regards
   Marc Hartmayer

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


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

Re: [libvirt] [PATCH v2 8/8] util: storage: adapt to changes in JSON format for sheepdog

[Peter Krempa]

On Tue, Jun 20, 2017 at 09:08:34 +0200, Pavel Hrdina wrote:
> On Mon, Jun 19, 2017 at 03:58:35PM +0200, Peter Krempa wrote:
> > Since qemu 2.9 the options changed from a monolithic string into fine
> > grained options for the json pseudo-protocol object.
> > ---
> > 
> > Notes:
> > v2:
> > - fixed server type to 'inet' in test
> > - removed spurious 'transport' value in test
> > 
> >  src/util/virstoragefile.c | 28 
> >  tests/virstoragetest.c| 11 +++
> >  2 files changed, 35 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Pavel Hrdina 

Thanks! I've pushed the series.


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

Re: [libvirt] [PATCH RFC 2/2] Resctrl: Add uitls functions to operate sysfs resctrl

[Eli Qiao]

hello,
ping 


On Monday, 12 June 2017 at 5:48 PM, Eli Qiao wrote:

> This patch adds 3 major private interface.
> 
> virResctrlGetFreeCache: return free cache, default cache substract cache
> allocated.
> virResctrlSetCachetunes: set cache banks which defined in a domain.
> virResctrlRemoveCachetunes: remove cache allocation group from the
> host.
> 
> There's some existed issue when do syntax-check as I reference the cache
> tune and cachebank definition (from conf/domain_conf.h) in
> util/virresctrl.h.
> ---
> include/libvirt/virterror.h | 1 +
> src/Makefile.am (http://Makefile.am) | 1 +
> src/libvirt_private.syms | 9 +
> src/qemu/qemu_process.c | 54 ++
> src/util/virerror.c | 1 +
> src/util/virresctrl.c | 851 ++
> src/util/virresctrl.h | 79 +++
> tests/Makefile.am (http://Makefile.am) | 8 +-
> tests/virresctrldata/L3-free.schemata | 1 +
> tests/virresctrldata/L3CODE-free.schemata | 1 +
> tests/virresctrldata/L3DATA-free.schemata | 1 +
> tests/virresctrldata/linux-resctrl | 1 +
> tests/virresctrldata/linux-resctrl-cdp | 1 +
> tests/virresctrltest.c | 119 +
> 14 files changed, 1127 insertions(+), 1 deletion(-)
> create mode 100644 src/util/virresctrl.c
> create mode 100644 src/util/virresctrl.h
> create mode 100644 tests/virresctrldata/L3-free.schemata
> create mode 100644 tests/virresctrldata/L3CODE-free.schemata
> create mode 100644 tests/virresctrldata/L3DATA-free.schemata
> create mode 12 tests/virresctrldata/linux-resctrl
> create mode 12 tests/virresctrldata/linux-resctrl-cdp
> create mode 100644 tests/virresctrltest.c
> 
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 2efee8f..4bc0c74 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -132,6 +132,7 @@ typedef enum {
> 
> VIR_FROM_PERF = 65, /* Error from perf */
> VIR_FROM_LIBSSH = 66, /* Error from libssh connection transport */
> + VIR_FROM_RESCTRL = 67, /* Error from resctrl */
> 
> # ifdef VIR_ENUM_SENTINELS
> VIR_ERR_DOMAIN_LAST
> diff --git a/src/Makefile.am (http://Makefile.am) b/src/Makefile.am 
> (http://Makefile.am)
> index eae32dc..8dbb778 100644
> --- a/src/Makefile.am (http://Makefile.am)
> +++ b/src/Makefile.am (http://Makefile.am)
> @@ -167,6 +167,7 @@ UTIL_SOURCES = \
> util/virprocess.c util/virprocess.h \
> util/virqemu.c util/virqemu.h \
> util/virrandom.h util/virrandom.c \
> + util/virresctrl.h util/virresctrl.c \
> util/virrotatingfile.h util/virrotatingfile.c \
> util/virscsi.c util/virscsi.h \
> util/virscsihost.c util/virscsihost.h \
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index b6c828f..7392cfa 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2440,6 +2440,15 @@ virRandomGenerateWWN;
> virRandomInt;
> 
> 
> +# util/virresctrl.h
> +virResctrlBitmap2String;
> +virResctrlFreeSchemata;
> +virResctrlGetFreeCache;
> +virResctrlRemoveCachetunes;
> +virResctrlSetCachetunes;
> +virResctrlTypeToString;
> +
> +
> # util/virrotatingfile.h
> virRotatingFileReaderConsume;
> virRotatingFileReaderFree;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 4a66f0d..8efeb19 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -70,6 +70,7 @@
> #include "virbitmap.h"
> #include "viratomic.h"
> #include "virnuma.h"
> +#include "virresctrl.h"
> #include "virstring.h"
> #include "virhostdev.h"
> #include "secret_util.h"
> @@ -5088,6 +5089,51 @@ qemuProcessSetupVcpus(virDomainObjPtr vm)
> return 0;
> }
> 
> +static int
> +qemuProcessSetCacheBanks(virCapsHostPtr caps, virDomainObjPtr vm)
> +{
> + size_t i, j;
> + virDomainCachetunePtr cachetune;
> + unsigned int max_vcpus = virDomainDefGetVcpusMax(vm->def);
> + pid_t *pids = NULL;
> + virDomainVcpuDefPtr vcpu;
> + size_t npid = 0;
> + size_t count = 0;
> + int ret = -1;
> +
> + cachetune = &(vm->def->cachetune);
> +
> + for (i = 0; i < cachetune->n_banks; i++) {
> + if (cachetune->cache_banks[i].vcpus) {
> + for (j = 0; j < max_vcpus; j++) {
> + if (virBitmapIsBitSet(cachetune->cache_banks[i].vcpus, j)) {
> +
> + vcpu = virDomainDefGetVcpu(vm->def, j);
> + if (!vcpu->online)
> + continue;
> +
> + if (VIR_RESIZE_N(pids, npid, count, 1) < 0)
> + goto cleanup;
> + pids[count ++] = qemuDomainGetVcpuPid(vm, j);
> + }
> + }
> + }
> + }
> +
> + /* If not specify vcpus in cachetune, add vm->pid */
> + if (pids == NULL) {
> + if (VIR_ALLOC_N(pids, 1) < 0)
> + goto cleanup;
> + pids[0] = vm->pid;
> + count = 1;
> + }
> + ret = virResctrlSetCachetunes(caps, cachetune, vm->def->uuid, pids, count);
> +
> + cleanup:
> + VIR_FREE(pids);
> + return ret;
> +}
> +
> 
> int
> qemuProcessSetupIOThread(virDomainObjPtr vm,
> @@ -5914,6 +5960,11 @@ qemuProcessLaunch(virConnectPtr conn,
> qemuProcessAutoDestroyAdd(driver, vm, conn) < 0)
> goto cleanup;
> 
> + VIR_WARN("Cache allocation");
> + if (qemuProcessSetCacheBanks(&(driver->caps->host),
> + vm) < 0)
> + goto cleanup;
> +
> ret = 0;
> 
> cleanup:
> @@ -6419,6 

Re: [libvirt] [PATCH v2 8/8] util: storage: adapt to changes in JSON format for sheepdog

[Pavel Hrdina]

On Mon, Jun 19, 2017 at 03:58:35PM +0200, Peter Krempa wrote:
> Since qemu 2.9 the options changed from a monolithic string into fine
> grained options for the json pseudo-protocol object.
> ---
> 
> Notes:
> v2:
> - fixed server type to 'inet' in test
> - removed spurious 'transport' value in test
> 
>  src/util/virstoragefile.c | 28 
>  tests/virstoragetest.c| 11 +++
>  2 files changed, 35 insertions(+), 4 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH v2 7/8] util: storage: adapt to changes in JSON format for ssh

[Pavel Hrdina]

On Mon, Jun 19, 2017 at 03:58:34PM +0200, Peter Krempa wrote:
> Since qemu 2.9 the options changed from a monolithic string into fine
> grained options for the json pseudo-protocol object.
> ---
> 
> Notes:
> v2:
> - tweaked error message
> 
>  src/util/virstoragefile.c | 21 +
>  tests/virstoragetest.c| 11 +++
>  2 files changed, 24 insertions(+), 8 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH v2 6/8] util: storage: adapt to changes in JSON format for ceph/rbd

[Pavel Hrdina]

On Mon, Jun 19, 2017 at 03:58:33PM +0200, Peter Krempa wrote:
> Since qemu 2.9 the options changed from a monolithic string into fine
> grained options for the json pseudo-protocol object.
> ---
>  src/util/virstoragefile.c | 50 
> +++
>  tests/virstoragetest.c| 19 ++
>  2 files changed, 65 insertions(+), 4 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH v2 5/8] util: storage: adapt to changes in JSON format for NBD

[Pavel Hrdina]

On Mon, Jun 19, 2017 at 03:58:32PM +0200, Peter Krempa wrote:
> Since 2.9 the host and port for NBD are no longer directly under the
> json pseudo-protocol object, but rather belong to a sub-object called
> 'server'.
> ---
> 
> Notes:
> v2:
> - changed type 'tcp' to 'inet' according to the qemu schema
> 
>  src/util/virstoragefile.c | 28 +---
>  tests/virstoragetest.c| 11 +++
>  2 files changed, 28 insertions(+), 11 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH v2 4/8] util: storage: Add JSON parser for new options in iSCSI protocol

[Pavel Hrdina]

On Mon, Jun 19, 2017 at 03:58:31PM +0200, Peter Krempa wrote:
> Starting from qemu 2.9, more granular options are supported. Add parser
> for the relevant bits.
> 
> With this patch libvirt is able to parse the host and target IQN of from
> the JSON pseudo-protocol specification.
> 
> This corresponds to BlockdevOptionsIscsi in qemu qapi.
> ---
> 
> Notes:
> v2:
> - parse 'port' from the portal string
> - treat 'lun' as an integer in json
> 
>  src/util/virstoragefile.c | 63 
> ---
>  tests/virstoragetest.c| 19 ++
>  2 files changed, 78 insertions(+), 4 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH v2 3/8] util: storage: Report errors when source host data is missing

[Pavel Hrdina]

On Mon, Jun 19, 2017 at 03:58:30PM +0200, Peter Krempa wrote:
> Merge the reporting of the missing source host data into the parser
> functions so that callers don't have to do it separately.
> ---
> 
> Notes:
> v2:
> - fixed merge conflicts
> 
>  src/util/virstoragefile.c | 29 +++--
>  1 file changed, 23 insertions(+), 6 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH v2 2/8] util: storage: Split out parsing of TCP network host from JSON pseudoprotocol

[Pavel Hrdina]

On Mon, Jun 19, 2017 at 03:58:29PM +0200, Peter Krempa wrote:
> Few backing protocols support only TCP. Split out the function which
> will correspond to parsing qemu's InetSocketAddressBase.
> ---
> 
> Notes:
> v2:
> - resolved merge conflict after adding a patch
> - changed ordering of setting of the transport type
> 
>  src/util/virstoragefile.c | 38 +-
>  1 file changed, 25 insertions(+), 13 deletions(-)

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH v2 1/8] util: storage: Add support for type 'inet' in virStorageSourceParseBackingJSONSocketAddress

[Pavel Hrdina]

On Mon, Jun 19, 2017 at 03:58:28PM +0200, Peter Krempa wrote:
> 'SocketAddress' structure was changed to contain 'inet' instead of
> 'tcp' since qemu commit c5f1ae3ae7b. Existing entries have a backward
> compatibility layer.
> 
> Libvirt will parse 'inet' and 'tcp' as equivalents.
> ---
>  src/util/virstoragefile.c | 23 +--
>  tests/virstoragetest.c|  2 +-
>  2 files changed, 10 insertions(+), 15 deletions(-)

Reviewed-by: Pavel Hrdina 


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