[libvirt] Snapshot: Request to review patch

2017-12-07 Thread Madhu Pavan

Hi all,

Could someone please review 
https://www.redhat.com/archives/libvir-list/2017-October/msg01333.html
This patch is based on comments from Jiri Denemark ( 
https://www.redhat.com/archives/libvir-list/2017-October/msg01275.html )
This patch potentially fixes 
https://bugzilla.redhat.com/show_bug.cgi?id=1494471


Thanks,

Madhu.

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


Re: [libvirt] [PATCH] qemu: snapshot: Keep non-persistent changes alive in snapshot

2017-10-30 Thread Madhu Pavan



On 10/27/2017 06:51 PM, Jiri Denemark wrote:

On Fri, Oct 27, 2017 at 18:47:51 +0530, Madhu Pavan wrote:

On 10/27/2017 02:51 PM, Jiri Denemark wrote:

I think this is actually a bit more complicated. When reverting to a
snapshot, when reverting a snapshot we should revert both active and
inactive configuration for backward compatibility and also because it
makes sense. Imagine you made a snapshot of a running domain, played
with the domain configuration and then reverted the state of the domain
to the snapshot. Once you shutdown the domain and start it again you'd
get a completely different machine. Of course, you actually may want
such behavior. Thus we can't really guess whether a user wants to revert
both active and inactive configuration or just one of them. The user
should be able to tell us what to do (and we should revert both configs
if no preference is given).

However, for this to be really useful we need to store both active and
inactive configurations when creating a snapshot of a running domain.

With the current behavior that I see from snapshot-list, I understood we
categorize the snapshots
as "Active (running, paused)" or "config(shutoff)" depending if the
snapshot was taken on an
active or inactive domain. With my use case what I observed was that the
revert of active
snapshot actually overwriting inactive domain configuration. I thought
only the "config" snapshot
alone can overwrite the inactive domain configuration. Hence this patch.
Are you suggesting we
should have both active and inactive domain configurations to be saved
for an active guest and
restore (both by default) OR (provide options to select)?

Yes, exactly.

I have sent a new patchset considering your suggestions.
Here is the link to it
https://www.redhat.com/archives/libvir-list/2017-October/msg01333.html

Madhu

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


[libvirt] [PATCH 7/7] tests: docs: Add schema and testcase for domainsnapshot

2017-10-30 Thread Kothapally Madhu Pavan
Alter the schema of domainsnapshot to add inactive XML of a snapshot.
As, snapshot already has active XML configuration of domain, the inactive
XMl is embedded in  tags. Sample XML is:

  





  
  

  

Alter the domainsnapshotxml2xmltest to validate the format.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 docs/schemas/domainsnapshot.rng| 19 +
 .../full_domain_withinactive.xml   | 83 ++
 tests/domainsnapshotxml2xmltest.c  |  1 +
 3 files changed, 103 insertions(+)
 create mode 100644 tests/domainsnapshotxml2xmlout/full_domain_withinactive.xml

diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng
index 2680887..2a58a84 100644
--- a/docs/schemas/domainsnapshot.rng
+++ b/docs/schemas/domainsnapshot.rng
@@ -84,6 +84,25 @@
   
 
 
+  
+
+  
+
+  
+
+  
+
+
+
+  
+
+  
+
+  
+
+
   
 
   
diff --git a/tests/domainsnapshotxml2xmlout/full_domain_withinactive.xml 
b/tests/domainsnapshotxml2xmlout/full_domain_withinactive.xml
new file mode 100644
index 000..d6d1b39
--- /dev/null
+++ b/tests/domainsnapshotxml2xmlout/full_domain_withinactive.xml
@@ -0,0 +1,83 @@
+
+  my snap name
+  !@#$%^
+  running
+  
+earlier_snap
+  
+  1272917631
+  
+  
+QEMUGuest1
+c7a5fdbd-edaf-9455-926a-d65c16db1809
+219100
+219100
+1
+
+  hvm
+  
+
+
+destroy
+restart
+destroy
+
+  /usr/bin/qemu-system-i686
+  
+
+
+
+  
+  
+
+  
+  
+
+  
+  
+  
+  
+  
+
+  
+
+  
+  
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219100
+  219100
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+  
+
+
+  
+
+
+  
+
+
+
+
+
+  
+
+  
+
+  
+  1
+
diff --git a/tests/domainsnapshotxml2xmltest.c 
b/tests/domainsnapshotxml2xmltest.c
index 3a6f86b..ebec2de 100644
--- a/tests/domainsnapshotxml2xmltest.c
+++ b/tests/domainsnapshotxml2xmltest.c
@@ -205,6 +205,7 @@ mymain(void)
 DO_TEST_OUT("all_parameters", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 
true);
 DO_TEST_OUT("disk_snapshot_redefine", 
"c7a5fdbd-edaf-9455-926a-d65c16db1809", true);
 DO_TEST_OUT("full_domain", "c7a5fdbd-edaf-9455-926a-d65c16db1809", true);
+DO_TEST_OUT("full_domain_withinactive", 
"c7a5fdbd-edaf-9455-926a-d65c16db1809", true);
 DO_TEST_OUT("noparent_nodescription_noactive", NULL, false);
 DO_TEST_OUT("noparent_nodescription", NULL, true);
 DO_TEST_OUT("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", false);
-- 
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: Allow restoring snapshot with non-persistent configuration

2017-10-30 Thread Kothapally Madhu Pavan
Now, snapshot-restore will allow restoring snapshots with non-persistent
configuration as both active and inactive XML configurations are saved
in snapshot. User can discard non-persistent configuratin of a domain
using --active-only flag. When --active-only flag is used, active XML
configuration of the snapshot is used as both the active and inactive
XML configuration of the domain after restore.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 tools/virsh-snapshot.c | 6 ++
 tools/virsh.pod| 9 -
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 48fc034..7f6a231 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1811,6 +1811,10 @@ static const vshCmdOptDef opts_snapshot_revert[] = {
  .type = VSH_OT_BOOL,
  .help = N_("try harder on risky reverts")
 },
+{.name = "active-only",
+ .type = VSH_OT_BOOL,
+ .help = N_("use only active snapshot configuration when restoring")
+},
 {.name = NULL}
 };
 
@@ -1835,6 +1839,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd 
*cmd)
  * when the error says it will make a difference.  */
 if (vshCommandOptBool(cmd, "force"))
 force = true;
+if (vshCommandOptBool(cmd, "active-only"))
+flags |= VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY;
 
 dom = virshCommandOptDomain(ctl, cmd, NULL);
 if (dom == NULL)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 0578f8f..791014e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -4526,7 +4526,7 @@ Output the name of the parent snapshot, if any, for the 
given
 I, or for the current snapshot with I<--current>.
 
 =item B I {I | I<--current>}
-[{I<--running> | I<--paused>}] [I<--force>]
+[{I<--running> | I<--paused>}] [I<--force>] [I<--active-only>]
 
 Revert the given domain to the snapshot specified by I, or to
 the current snapshot with I<--current>.  Be aware
@@ -4559,6 +4559,13 @@ snapshot that uses a provably incompatible 
configuration, as well as
 with an inactive snapshot that is combined with the I<--start> or
 I<--pause> flag.
 
+When inactive XML configuration of a snapshot is available along with
+active XML configuration both the inactive and active XMl configurations
+are used to restore the snapshot. This will keep the non-persistent
+configuration alive after restoring a snapshot. User can kill the
+non-persistent configuration by issuing I<--active-only> flag. This will
+use active XML configuraton alone to revert the snapshot.
+
 =item B I {I | I<--current>} [I<--metadata>]
 [{I<--children> | I<--children-only>}]
 
-- 
1.8.3.1

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


[libvirt] [PATCH 3/7] conf: Allow editing inactive snapshot configuration

2017-10-30 Thread Kothapally Madhu Pavan
This patch will allow user to edit the inactive XML snapshot
configuration when it is available in the snapshot.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 include/libvirt/libvirt-domain.h |  1 +
 src/conf/domain_conf.c   |  6 --
 src/conf/domain_conf.h   |  2 ++
 src/conf/snapshot_conf.c | 35 ++-
 src/qemu/qemu_driver.c   |  3 ++-
 5 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 4048acf..e70c664 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1546,6 +1546,7 @@ typedef enum {
 VIR_DOMAIN_XML_INACTIVE = (1 << 1), /* dump inactive domain 
information */
 VIR_DOMAIN_XML_UPDATE_CPU   = (1 << 2), /* update guest CPU requirements 
according to host CPU */
 VIR_DOMAIN_XML_MIGRATABLE   = (1 << 3), /* dump XML suitable for migration 
*/
+VIR_DOMAIN_XML_ACTIVE_ONLY  = (1 << 4), /* dump active XML and avoid 
inactive XML in snapshot */
 } virDomainXMLFlags;
 
 char *  virDomainGetXMLDesc (virDomainPtr domain,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 77c20c6..36cebe5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -25687,8 +25687,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
   VIR_DOMAIN_DEF_FORMAT_STATUS |
   VIR_DOMAIN_DEF_FORMAT_ACTUAL_NET |
   VIR_DOMAIN_DEF_FORMAT_PCI_ORIG_STATES |
-  VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST,
-  -1);
+  VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST |
+  VIR_DOMAIN_DEF_FORMAT_ACTIVE_ONLY, -1);
 
 if (!(type = virDomainVirtTypeToString(def->virtType))) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -26472,6 +26472,8 @@ unsigned int virDomainDefFormatConvertXMLFlags(unsigned 
int flags)
 formatFlags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
 if (flags & VIR_DOMAIN_XML_MIGRATABLE)
 formatFlags |= VIR_DOMAIN_DEF_FORMAT_MIGRATABLE;
+if (flags & VIR_DOMAIN_XML_ACTIVE_ONLY)
+formatFlags |= VIR_DOMAIN_DEF_FORMAT_ACTIVE_ONLY;
 
 return formatFlags;
 }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 38de70b..0659220 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2853,6 +2853,8 @@ typedef enum {
 VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM   = 1 << 6,
 VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT  = 1 << 7,
 VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST= 1 << 8,
+/* format active XML and avoid inactive XML */
+VIR_DOMAIN_DEF_FORMAT_ACTIVE_ONLY = 1 << 9,
 } virDomainDefFormatFlags;
 
 /* Use these flags to skip specific domain ABI consistency checks done
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index bfe3d6c..3cb7cd4 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -290,6 +290,29 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
 } else {
 VIR_WARN("parsing older snapshot that lacks domain");
 }
+
+/* Older snapshots were created without inactive domain configuration.
+ * In that case, leave the newDom NULL. */
+if ((tmp = virXPathString("string(./inactiveDomain/domain/@type)", 
ctxt))) {
+int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+  VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
+if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL)
+domainflags |= VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS;
+xmlNodePtr domainNode = virXPathNode("./inactiveDomain/domain", 
ctxt);
+
+VIR_FREE(tmp);
+if (domainNode) {
+def->newDom = virDomainDefParseNode(ctxt->node->doc, 
domainNode,
+caps, xmlopt, NULL, 
domainflags);
+if (!def->newDom)
+goto cleanup;
+} else {
+VIR_WARN("missing inactive domain in snapshot");
+}
+} else {
+VIR_WARN("parsing older snapshot that lacks inactive domain");
+}
+
 } else {
 def->creationTime = tv.tv_sec;
 }
@@ -705,7 +728,8 @@ virDomainSnapshotDefFormat(const char *domain_uuid,
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 size_t i;
 
-virCheckFlags(VIR_DOMAIN_DEF_FORMAT_SECURE, NULL);
+virCheckFlags(VIR_DOMAIN_DEF_FORMAT_SECURE |
+  VIR_DOMAIN_DEF_FORMAT_ACTIVE_ONLY, NULL);
 
 flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
 
@@ -757,6 +781,15 @@ virDomainSnapshotDefFormat(const char *domain_uuid,
 virBufferAddLit(, "\n");
 }
 
+if (def->newDom && !(flags & VIR_DOMAIN_DEF_FORMAT_ACTIVE_ONLY)) {
+   

[libvirt] [PATCH 2/7] qemu: Use active and inactive snapshot configuration on restore

2017-10-30 Thread Kothapally Madhu Pavan
By default, active and inactive XMl snapshot configurations are
assigned to domain definition. This will make sure that all the
non-persistent configurations of the snapshot are restored back
as it is. This patch will also make sure that user has a choice
to choose of using active XML configuration of snapshot as both
active and inactive XML configurations of the restoring domain.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 include/libvirt/libvirt-domain-snapshot.h | 10 +++---
 src/qemu/qemu_driver.c| 20 +++-
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt-domain-snapshot.h 
b/include/libvirt/libvirt-domain-snapshot.h
index 0f73f24..67ccb59 100644
--- a/include/libvirt/libvirt-domain-snapshot.h
+++ b/include/libvirt/libvirt-domain-snapshot.h
@@ -184,9 +184,13 @@ int virDomainSnapshotHasMetadata(virDomainSnapshotPtr 
snapshot,
  unsigned int flags);
 
 typedef enum {
-VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1 << 0, /* Run after revert */
-VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED  = 1 << 1, /* Pause after revert */
-VIR_DOMAIN_SNAPSHOT_REVERT_FORCE   = 1 << 2, /* Allow risky reverts */
+VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING   = 1 << 0, /* Run after revert */
+VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED= 1 << 1, /* Pause after revert */
+VIR_DOMAIN_SNAPSHOT_REVERT_FORCE = 1 << 2, /* Allow risky reverts 
*/
+VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY   = 1 << 3, /* Use active snapshot
+  configurations as 
both
+  active and inactive
+  domain 
configurations*/
 } virDomainSnapshotRevertFlags;
 
 /* Revert the domain to a point-in-time snapshot.  The
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4ffec70..aecfcff 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15577,6 +15577,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 qemuDomainObjPrivatePtr priv;
 int rc;
 virDomainDefPtr config = NULL;
+virDomainDefPtr newConfig = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 bool was_running = false;
@@ -15586,7 +15587,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 
 virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
   VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
-  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1);
+  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE |
+  VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY, -1);
 
 /* We have the following transitions, which create the following events:
  * 1. inactive -> inactive: none
@@ -15688,6 +15690,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 goto endjob;
 }
 
+/* Prepare to copy snapshot inactive xml as inactive configuration
+ * of this domain unless user exclusively specify not to copy it */
+if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY) &&
+snap->def->newDom) {
+newConfig = virDomainDefCopy(snap->def->newDom, caps,
+ driver->xmlopt, NULL, true);
+if (!newConfig)
+goto endjob;
+}
+
 cookie = (qemuDomainSaveCookiePtr) snap->def->cookie;
 
 switch ((virDomainState) snap->def->state) {
@@ -15785,12 +15797,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 virCPUDefFree(priv->origCPU);
 VIR_STEAL_PTR(priv->origCPU, origCPU);
 }
+if (newConfig)
+vm->newDef = newConfig;
 } else {
 /* Transitions 2, 3 */
 load:
 was_stopped = true;
 if (config)
 virDomainObjAssignDef(vm, config, false, NULL);
+if (newConfig)
+vm->newDef = newConfig;
 
 /* No cookie means libvirt which saved the domain was too old to
  * mess up the CPU definitions.
@@ -15884,6 +15900,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 }
 if (config)
 virDomainObjAssignDef(vm, config, false, NULL);
+if (newConfig)
+vm->newDef = newConfig;
 
 if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
  VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
-- 
1.8.3.1

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


[libvirt] [PATCH 0/7] Keep non-persistent changes alive in snapshot

2017-10-30 Thread Kothapally Madhu Pavan
Restoring to a snapshot should not overwrite the persistent XML configuration
of a snapshot as a side effect. This patchset fixes the same. Currently,
virDomainSnapshotDef only saves active domain definition of the guest.
And on restore the active domain definition is used as both active and
inactive domain definitions. This will make the non-persistent changes
persistent in snapshot image. This patchset allows to save inactive domain
definition as well and on snapshot-revert non-persistent configuration is
restored as is.

Currently, snapshot-revert is making non-presistent changes as persistent.
Here are the steps to reproduce.
Step1: virsh define $dom
Step2: virsh attach-device $dom $memory-device.xml --live
Step3: virsh snapshot-create $dom
Step4: virsh destroy $dom
Step5: virsh snapshot-revert $dom $snapshot-name
Step6: virsh destroy $dom
Step7: virsh start $dom
Here we still have $memory-device attached in Step2.

This patchset is attempting to solve this issue. This patchset will also
allow user to dump and edit inactive XML configuration of a snapshot.
Dumping inactive domain definition of a snapshot is important as
--redefine uses snapshot-dumpxml output to redefine a snapshot.

Kothapally Madhu Pavan (7):
  qemu: Store inactive domain configuration in snapshot
  qemu: Use active and inactive snapshot configuration on restore
  conf: Allow editing inactive snapshot configuration
  virsh: Dump inactive XML configuration of snapshot using
snapshot-dumpxml
  virsh: Edit inactive XML configuration of snapshot using snapshot-edit
  virsh: Allow restoring snapshot with non-persistent configuration
  tests: docs: Add schema and testcase for domainsnapshot

 docs/schemas/domainsnapshot.rng| 19 +
 include/libvirt/libvirt-domain-snapshot.h  | 10 ++-
 include/libvirt/libvirt-domain.h   |  1 +
 src/conf/domain_conf.c |  6 +-
 src/conf/domain_conf.h |  2 +
 src/conf/snapshot_conf.c   | 48 -
 src/conf/snapshot_conf.h   |  1 +
 src/qemu/qemu_driver.c | 33 -
 .../full_domain_withinactive.xml   | 83 ++
 tests/domainsnapshotxml2xmltest.c  |  1 +
 tools/virsh-snapshot.c | 20 ++
 tools/virsh.pod| 37 +-
 12 files changed, 251 insertions(+), 10 deletions(-)
 create mode 100644 tests/domainsnapshotxml2xmlout/full_domain_withinactive.xml

-- 
1.8.3.1

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


[libvirt] [PATCH 4/7] virsh: Dump inactive XML configuration of snapshot using snapshot-dumpxml

2017-10-30 Thread Kothapally Madhu Pavan
Now, snapshot-dumpxml will display inactive XML configuration of snapshot
along with active XML configuration. When --active-only flag is used the
inactive XML configuration will not be displayed.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 tools/virsh-snapshot.c |  7 +++
 tools/virsh.pod| 20 
 2 files changed, 27 insertions(+)

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 24cd4ab..4b0a18d 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1667,6 +1667,10 @@ static const vshCmdOptDef opts_snapshot_dumpxml[] = {
  .type = VSH_OT_BOOL,
  .help = N_("include security sensitive information in XML dump")
 },
+{.name = "active-only",
+ .type = VSH_OT_BOOL,
+ .help = N_("dump only active XML configuration and avoid inactive XML")
+},
 {.name = NULL}
 };
 
@@ -1683,6 +1687,9 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptBool(cmd, "security-info"))
 flags |= VIR_DOMAIN_XML_SECURE;
 
+if (vshCommandOptBool(cmd, "active-only"))
+flags |= VIR_DOMAIN_XML_ACTIVE_ONLY;
+
 if (vshCommandOptStringReq(ctl, cmd, "snapshotname", ) < 0)
 return false;
 
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 69cc423..f899da7 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -4489,11 +4489,31 @@ is specified, the list will be filtered to snapshots 
that use external
 files for disk images or memory state.
 
 =item B I I [I<--security-info>]
+[I<--active-only>]
 
 Output the snapshot XML for the domain's snapshot named I.
 Using I<--security-info> will also include security sensitive information.
 Use B to easily access the XML of the current snapshot.
 
+If I<--active-only> is specified, only active XML configuration of the
+snapshot is displayed. Otherwise, both active and inactive XML
+configuration of the snapshot will be displayed.
+
+When both active and inactive XML snapshot configurations are displayed,
+as inactive XML configuration will have same structure as active XML
+it is embedded within  tag as shown below:
+
+  
+
+
+
+  
+
+
+  
+
+  
+
 =item B I {I | I<--current>}
 
 Output the name of the parent snapshot, if any, for the given
-- 
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: Edit inactive XML configuration of snapshot using snapshot-edit

2017-10-30 Thread Kothapally Madhu Pavan
Now, snapshot-edit will allow editing inactive XML configuration of snapshot
along with active XML configuration. When --active-only flag is used the
inactive XML will not be displayed and will be removed from snapshot.
--active-only flag is used when user doesn't what any non-persistent
configuration in domain after restoring the snapshot.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 tools/virsh-snapshot.c | 7 +++
 tools/virsh.pod| 8 +++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 4b0a18d..48fc034 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -522,6 +522,10 @@ static const vshCmdOptDef opts_snapshot_edit[] = {
  .type = VSH_OT_BOOL,
  .help = N_("allow cloning to new name")
 },
+{.name = "active-only",
+ .type = VSH_OT_BOOL,
+ .help = N_("allow editing active XML configuration and remove inactive 
XML")
+},
 {.name = NULL}
 };
 
@@ -545,6 +549,9 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
 vshCommandOptBool(cmd, "snapshotname"))
 define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT;
 
+if (vshCommandOptBool(cmd, "active-only"))
+getxml_flags |= VIR_DOMAIN_XML_ACTIVE_ONLY;
+
 if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
 return false;
 
diff --git a/tools/virsh.pod b/tools/virsh.pod
index f899da7..0578f8f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -4400,7 +4400,7 @@ With I, this is a request to make the 
existing named
 snapshot become the current snapshot, without reverting the domain.
 
 =item B I [I] [I<--current>]
-{[I<--rename>] | [I<--clone>]}
+{[I<--rename>] | [I<--clone>]} [I<--active-only>]
 
 Edit the XML configuration file for I of a domain.  If
 both I and I<--current> are specified, also force the
@@ -4427,6 +4427,12 @@ a snapshot name must be done with care, since the 
contents of some
 snapshots, such as internal snapshots within a single qcow2 file, are
 accessible only from the original name.
 
+If I<--active-only> is specified, only active XML configuration of the
+snapshot is displayed to edit. Otherwise, both active and inactive XML
+configuration of the snapshot will be displayed to edit. When domain
+snapshot is edited with I<--active-only> flag, inactive XML configuration
+will be removed from snapshot. 
+
 =item B I {I | I<--current>}
 
 Output basic information about a named , or the current snapshot
-- 
1.8.3.1

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


[libvirt] [PATCH 1/7] qemu: Store inactive domain configuration in snapshot

2017-10-30 Thread Kothapally Madhu Pavan
Inorder to capture the exact state of domain, inactive configuration
is needed along with active configuration. This patch stores inactive
domain configuration when creating snapshot of a running domain. It
also captures the inactive snapshot configuration when a snapshot is
redefined.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/conf/snapshot_conf.c | 13 +
 src/conf/snapshot_conf.h |  1 +
 src/qemu/qemu_driver.c   | 10 ++
 3 files changed, 24 insertions(+)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f0e852c..bfe3d6c 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -102,6 +102,7 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
 virDomainSnapshotDiskDefClear(>disks[i]);
 VIR_FREE(def->disks);
 virDomainDefFree(def->dom);
+virDomainDefFree(def->newDom);
 virObjectUnref(def->cookie);
 VIR_FREE(def);
 }
@@ -1336,6 +1337,18 @@ virDomainSnapshotRedefinePrep(virDomainPtr domain,
 }
 }
 
+if (other->def->newDom) {
+if (def->newDom) {
+if (!virDomainDefCheckABIStability(other->def->newDom,
+   def->newDom, xmlopt))
+goto cleanup;
+} else {
+/* Transfer the inactive domain def */
+def->newDom = other->def->newDom;
+other->def->newDom = NULL;
+}
+}
+
 if (other == vm->current_snapshot) {
 *update_current = true;
 vm->current_snapshot = NULL;
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 1d663c7..0bc915f 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -75,6 +75,7 @@ struct _virDomainSnapshotDef {
 virDomainSnapshotDiskDef *disks;
 
 virDomainDefPtr dom;
+virDomainDefPtr newDom;
 
 virObjectPtr cookie;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 74fdfdb..4ffec70 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15035,6 +15035,16 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
  
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
 goto endjob;
 
+if (vm->newDef) {
+if (!(xml = qemuDomainDefFormatLive(driver, vm->newDef, 
priv->origCPU,
+true, true)) ||
+!(def->newDom = virDomainDefParseString(xml, caps, 
driver->xmlopt, NULL,
+
VIR_DOMAIN_DEF_PARSE_INACTIVE |
+
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+goto endjob;
+}
+
+
 if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
 align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
 align_match = false;
-- 
1.8.3.1

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


Re: [libvirt] [PATCH] qemu: snapshot: Keep non-persistent changes alive in snapshot

2017-10-27 Thread Madhu Pavan



On 10/27/2017 02:51 PM, Jiri Denemark wrote:

On Fri, Oct 27, 2017 at 14:02:23 +0530, Kothapally Madhu Pavan wrote:

Restoring to a snapshot should not overwrite the persistent configuration
xml of a snapshot as a side effect. This patch fixes the same. Currently,
virDomainSnapshotDef only saves active domain definition of the guest.
And on restore the active domain definition is used as both active and
inactive domain definitions. Thiswill make the non-persistent changes
persistent in snapshot image. This patch allows to save inactive domain
definition as well and on snapshot-revert non-persistent configuration is
restored as is.

Currently, snapshot-revert is making non-presistent changes as persistent.
Here are the steps to reproduce.
Step1: virsh define $dom
Step2: virsh attach-device $dom $memory-device.xml --live
Step3: virsh snapshot-create $dom
Step4: virsh destroy $dom
Step5: virsh snapshot-revert $dom $snapshot-name
Step6: virsh destroy $dom
Step7: virsh start $dom
Here we still have $memory-device attached in Step2.

I think this is actually a bit more complicated. When reverting to a
snapshot, when reverting a snapshot we should revert both active and
inactive configuration for backward compatibility and also because it
makes sense. Imagine you made a snapshot of a running domain, played
with the domain configuration and then reverted the state of the domain
to the snapshot. Once you shutdown the domain and start it again you'd
get a completely different machine. Of course, you actually may want
such behavior. Thus we can't really guess whether a user wants to revert
both active and inactive configuration or just one of them. The user
should be able to tell us what to do (and we should revert both configs
if no preference is given).

However, for this to be really useful we need to store both active and
inactive configurations when creating a snapshot of a running domain.
With the current behavior that I see from snapshot-list, I understood we 
categorize the snapshots
as "Active (running, paused)" or "config(shutoff)" depending if the 
snapshot was taken on an
active or inactive domain. With my use case what I observed was that the 
revert of active
snapshot actually overwriting inactive domain configuration. I thought 
only the "config" snapshot
alone can overwrite the inactive domain configuration. Hence this patch. 
Are you suggesting we
should have both active and inactive domain configurations to be saved 
for an active guest and

restore (both by default) OR (provide options to select)?

Madhu


Jirka



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

[libvirt] [PATCH] qemu: snapshot: Keep non-persistent changes alive in snapshot

2017-10-27 Thread Kothapally Madhu Pavan
Restoring to a snapshot should not overwrite the persistent configuration
xml of a snapshot as a side effect. This patch fixes the same. Currently,
virDomainSnapshotDef only saves active domain definition of the guest.
And on restore the active domain definition is used as both active and
inactive domain definitions. Thiswill make the non-persistent changes
persistent in snapshot image. This patch allows to save inactive domain
definition as well and on snapshot-revert non-persistent configuration is
restored as is.

Currently, snapshot-revert is making non-presistent changes as persistent.
Here are the steps to reproduce.
Step1: virsh define $dom
Step2: virsh attach-device $dom $memory-device.xml --live
Step3: virsh snapshot-create $dom
Step4: virsh destroy $dom
Step5: virsh snapshot-revert $dom $snapshot-name
Step6: virsh destroy $dom
Step7: virsh start $dom
Here we still have $memory-device attached in Step2.

This patch is attempting to solve this issue.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/conf/snapshot_conf.c |  1 +
 src/conf/snapshot_conf.h |  1 +
 src/qemu/qemu_driver.c   | 23 +++
 3 files changed, 25 insertions(+)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f0e852c..e32fb4d 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -102,6 +102,7 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
 virDomainSnapshotDiskDefClear(>disks[i]);
 VIR_FREE(def->disks);
 virDomainDefFree(def->dom);
+virDomainDefFree(def->newDom);
 virObjectUnref(def->cookie);
 VIR_FREE(def);
 }
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 1d663c7..0bc915f 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -75,6 +75,7 @@ struct _virDomainSnapshotDef {
 virDomainSnapshotDiskDef *disks;
 
 virDomainDefPtr dom;
+virDomainDefPtr newDom;
 
 virObjectPtr cookie;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 74fdfdb..c7cdb43 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15035,6 +15035,15 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
  
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
 goto endjob;
 
+if (vm->newDef) {
+if (!(xml = qemuDomainDefFormatLive(driver, vm->newDef, 
priv->origCPU,
+true, true)) ||
+!(def->newDom = virDomainDefParseString(xml, caps, 
driver->xmlopt, NULL,
+
VIR_DOMAIN_DEF_PARSE_INACTIVE |
+
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)))
+goto endjob;
+}
+
 if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) {
 align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
 align_match = false;
@@ -15567,6 +15576,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 qemuDomainObjPrivatePtr priv;
 int rc;
 virDomainDefPtr config = NULL;
+virDomainDefPtr newConfig = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 bool was_running = false;
@@ -15678,6 +15688,13 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 goto endjob;
 }
 
+if (snap->def->newDom) {
+newConfig = virDomainDefCopy(snap->def->newDom, caps,
+ driver->xmlopt, NULL, true);
+if (!newConfig)
+goto endjob;
+}
+
 cookie = (qemuDomainSaveCookiePtr) snap->def->cookie;
 
 switch ((virDomainState) snap->def->state) {
@@ -15775,12 +15792,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 virCPUDefFree(priv->origCPU);
 VIR_STEAL_PTR(priv->origCPU, origCPU);
 }
+if (newConfig)
+vm->newDef = newConfig;
 } else {
 /* Transitions 2, 3 */
 load:
 was_stopped = true;
 if (config)
 virDomainObjAssignDef(vm, config, false, NULL);
+if (newConfig)
+vm->newDef = newConfig;
 
 /* No cookie means libvirt which saved the domain was too old to
  * mess up the CPU definitions.
@@ -15874,6 +15895,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 }
 if (config)
 virDomainObjAssignDef(vm, config, false, NULL);
+if (newConfig)
+vm->newDef = newConfig;
 
 if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
  VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
-- 
1.8.3.1

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


[libvirt] [PATCH] qemu: Move qemuFreeKeywords into qemu_parse_command.c

2017-10-16 Thread Kothapally Madhu Pavan
Move qemuFreeKeywords into qemu_parse_command.c as
qemuKeywordsFree and call it rather than inline code
in multiple places.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_monitor_json.c  | 15 ++---
 src/qemu/qemu_parse_command.c | 52 ---
 src/qemu/qemu_parse_command.h |  5 +
 3 files changed, 26 insertions(+), 46 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 5546d1a..af66967 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -468,17 +468,6 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char 
*cmdname, ...)
 #define qemuMonitorJSONMakeCommand(cmdname, ...) \
 qemuMonitorJSONMakeCommandRaw(false, cmdname, __VA_ARGS__)
 
-static void
-qemuFreeKeywords(int nkeywords, char **keywords, char **values)
-{
-size_t i;
-for (i = 0; i < nkeywords; i++) {
-VIR_FREE(keywords[i]);
-VIR_FREE(values[i]);
-}
-VIR_FREE(keywords);
-VIR_FREE(values);
-}
 
 static virJSONValuePtr
 qemuMonitorJSONKeywordStringToJSON(const char *str, const char *firstkeyword)
@@ -513,11 +502,11 @@ qemuMonitorJSONKeywordStringToJSON(const char *str, const 
char *firstkeyword)
 }
 }
 
-qemuFreeKeywords(nkeywords, keywords, values);
+qemuKeywordsFree(nkeywords, keywords, values);
 return ret;
 
  error:
-qemuFreeKeywords(nkeywords, keywords, values);
+qemuKeywordsFree(nkeywords, keywords, values);
 virJSONValueFree(ret);
 return NULL;
 }
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 9f739ae..d5ea5aa 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -381,6 +381,20 @@ static const char *qemuFindEnv(char **progenv,
 return NULL;
 }
 
+
+void
+qemuKeywordsFree(int nkeywords, char **keywords, char **values)
+{
+size_t i;
+for (i = 0; i < nkeywords; i++) {
+VIR_FREE(keywords[i]);
+VIR_FREE(values[i]);
+}
+VIR_FREE(keywords);
+VIR_FREE(values);
+}
+
+
 /*
  * Takes a string containing a set of key=value,key=value,key...
  * parameters and splits them up, returning two arrays with
@@ -401,7 +415,6 @@ qemuParseKeywords(const char *str,
 char **values = NULL;
 const char *start = str;
 const char *end;
-size_t i;
 
 *retkeywords = NULL;
 *retvalues = NULL;
@@ -479,12 +492,7 @@ qemuParseKeywords(const char *str,
 return 0;
 
  error:
-for (i = 0; i < keywordCount; i++) {
-VIR_FREE(keywords[i]);
-VIR_FREE(values[i]);
-}
-VIR_FREE(keywords);
-VIR_FREE(values);
+qemuKeywordsFree(keywordCount, keywords, values);
 return -1;
 }
 
@@ -949,12 +957,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
 }
 
  cleanup:
-for (i = 0; i < nkeywords; i++) {
-VIR_FREE(keywords[i]);
-VIR_FREE(values[i]);
-}
-VIR_FREE(keywords);
-VIR_FREE(values);
+qemuKeywordsFree(nkeywords, keywords, values);
 return def;
 
  error:
@@ -1132,12 +1135,7 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt,
 virDomainNetGenerateMAC(xmlopt, >mac);
 
  cleanup:
-for (i = 0; i < nkeywords; i++) {
-VIR_FREE(keywords[i]);
-VIR_FREE(values[i]);
-}
-VIR_FREE(keywords);
-VIR_FREE(values);
+qemuKeywordsFree(nkeywords, keywords, values);
 return def;
 
  error:
@@ -1704,13 +1702,7 @@ qemuParseCommandLineMem(virDomainDefPtr dom,
 ret = 0;
 
  cleanup:
-for (i = 0; i < nkws; i++) {
-VIR_FREE(kws[i]);
-VIR_FREE(vals[i]);
-}
-VIR_FREE(kws);
-VIR_FREE(vals);
-
+qemuKeywordsFree(nkws, kws, vals);
 return ret;
 }
 
@@ -1795,13 +1787,7 @@ qemuParseCommandLineSmp(virDomainDefPtr dom,
 ret = 0;
 
  cleanup:
-for (i = 0; i < nkws; i++) {
-VIR_FREE(kws[i]);
-VIR_FREE(vals[i]);
-}
-VIR_FREE(kws);
-VIR_FREE(vals);
-
+qemuKeywordsFree(nkws, kws, vals);
 return ret;
 
  syntax:
diff --git a/src/qemu/qemu_parse_command.h b/src/qemu/qemu_parse_command.h
index ed65342..4c553f2 100644
--- a/src/qemu/qemu_parse_command.h
+++ b/src/qemu/qemu_parse_command.h
@@ -43,6 +43,11 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps,
 virDomainChrSourceDefPtr *monConfig,
 bool *monJSON);
 
+void
+qemuKeywordsFree(int nkeywords,
+ char **keywords,
+ char **values);
+
 int
 qemuParseKeywords(const char *str,
   char ***retkeywords,
-- 
2.7.4

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


Re: [libvirt] [PATCH] qemu: Floppy disks not supported on pSeries machines

2017-10-10 Thread Madhu Pavan



On 10/10/2017 02:05 PM, Ján Tomko wrote:

On Sun, Oct 08, 2017 at 03:26:13PM +0530, Kothapally Madhu Pavan wrote:

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
src/qemu/qemu_parse_command.c | 5 +
1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_parse_command.c 
b/src/qemu/qemu_parse_command.c

index 7c409b0..d5745ce 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -760,6 +760,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr 
xmlopt,

    } else if (STREQ(values[i], "floppy")) {
    def->bus = VIR_DOMAIN_DISK_BUS_FDC;
    def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
+    if (qemuDomainIsPSeries(dom)) {
+    virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("pseries systems do not support 
floppy devices '%s'"), val);

+    goto error;


Do we actually need to worry about that here?

I would expect that if pseries does not support floppy devices, you will
not be able to run QEMU with such command line in the first place.

So this case would never happen with QEMU that is actually running.
This is definitely not a possible use case for qemu-attach as you point 
out.  I intended
this patch for "virsh domxml-from-native qemu-argv ". I expect 
domxml-from-native
command to be equally capable of validating qemu-argv as that of 
qemu-attach command

which otherwise errors out from qemu.

Madhu



+    }
    } else if (STREQ(values[i], "virtio")) {
    def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO;
    } else if (STREQ(values[i], "xen")) {
--
1.8.3.1

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


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


[libvirt] [PATCH v3] qemu: argv: parse qemu commandline memory arguments

2017-10-09 Thread Kothapally Madhu Pavan
Existing qemuParseCommandLineMem() will parse "-m 4G" format string.
This patch allows it to parse "-m size=8126464k,slots=32,maxmem=33554432k"
format along with existing format. And adds a testcase to validate the changes.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_parse_command.c  | 89 +++---
 .../qemuargv2xml-mem-scale-maxmemory.args  | 22 ++
 .../qemuargv2xml-mem-scale-maxmemory.xml   | 38 +
 tests/qemuargv2xmltest.c   |  1 +
 4 files changed, 138 insertions(+), 12 deletions(-)
 create mode 100644 tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.args
 create mode 100644 tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.xml

diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 37e1149..cd2a32a 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -1629,26 +1629,91 @@ static int
 qemuParseCommandLineMem(virDomainDefPtr dom,
 const char *val)
 {
-unsigned long long mem;
+unsigned long long mem = 0;
+unsigned long long size = 0;
+unsigned long long maxmem = 0;
+unsigned int slots = 0;
 char *end;
+size_t i;
+int nkws;
+char **kws;
+char **vals;
+int n;
+int ret = -1;
 
-if (virStrToLong_ull(val, , 10, ) < 0) {
+if (qemuParseKeywords(val, , , , 1) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot parse memory level '%s'"), val);
-return -1;
+   _("cannot parse memory '%s'"), val);
+goto cleanup;
 }
 
-if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot scale memory: %s"),
-   virGetLastErrorMessage());
-return -1;
+for (i = 0; i < nkws; i++) {
+if (vals[i] == NULL) {
+if (i > 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory '%s'"), val);
+goto cleanup;
+}
+if (virStrToLong_ull(kws[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory level '%s'"), kws[i]);
+goto cleanup;
+}
+if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot scale memory: %s"),
+   virGetLastErrorMessage());
+goto cleanup;
+}
+
+size = mem;
+
+} else {
+if (STREQ(kws[i], "size") || STREQ(kws[i], "maxmem")) {
+if (virStrToLong_ull(vals[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory level '%s'"), 
vals[i]);
+goto cleanup;
+}
+if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot scale memory: %s"),
+   virGetLastErrorMessage());
+goto cleanup;
+}
+
+STREQ(kws[i], "size") ? (size = mem) : (maxmem = mem);
+
+}
+if (STREQ(kws[i], "slots")) {
+if (virStrToLong_i(vals[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse slots value '%s'"), 
vals[i]);
+goto cleanup;
+}
+
+   slots = n;
+
+}
+}
 }
 
-virDomainDefSetMemoryTotal(dom, mem / 1024);
-dom->mem.cur_balloon = mem / 1024;
+virDomainDefSetMemoryTotal(dom, size / 1024);
+dom->mem.cur_balloon = size / 1024;
+dom->mem.memory_slots = slots;
+dom->mem.max_memory = maxmem / 1024;
 
-return 0;
+ret = 0;
+
+ cleanup:
+for (i = 0; i < nkws; i++) {
+VIR_FREE(kws[i]);
+VIR_FREE(vals[i]);
+}
+VIR_FREE(kws);
+VIR_FREE(vals);
+
+return ret;
 }
 
 
diff --git a/tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.args 
b/tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.args
new file mode 100644
index 000..7bce841
--- /dev/null
+++ b/tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.args
@@ -0,0 +1,22 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 8G,slots=16,maxmem=16G \
+-sm

[libvirt] [PATCH v2] qemu: argv: parse qemu commandline memory arguments

2017-10-09 Thread Kothapally Madhu Pavan
Existing qemuParseCommandLineMem() will parse "-m 4G" format string.
This patch allows it to parse "-m size=8126464k,slots=32,maxmem=33554432k"
format along with existing format. And adds a testcase to validate the changes.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_parse_command.c  | 89 +++---
 .../qemuargv2xml-mem-scale-maxmemory.args  | 22 ++
 .../qemuargv2xml-mem-scale-maxmemory.xml   | 38 +
 tests/qemuargv2xmltest.c   |  1 +
 4 files changed, 138 insertions(+), 12 deletions(-)
 create mode 100644 tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.args
 create mode 100644 tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.xml

diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 37e1149..f8b6f8b 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -1629,26 +1629,91 @@ static int
 qemuParseCommandLineMem(virDomainDefPtr dom,
 const char *val)
 {
-unsigned long long mem;
+unsigned long long mem = 0;
+unsigned long long size = 0;
+unsigned long long maxmem = 0;
+unsigned int slots = 0;
 char *end;
+size_t i;
+int nkws;
+char **kws;
+char **vals;
+int n;
+int ret = -1;
 
-if (virStrToLong_ull(val, , 10, ) < 0) {
+if (qemuParseKeywords(val, , , , 1) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot parse memory level '%s'"), val);
-return -1;
+   _("cannot parse memory '%s'"), val);
+goto cleanup;
 }
 
-if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot scale memory: %s"),
-   virGetLastErrorMessage());
-return -1;
+for (i = 0; i < nkws; i++) {
+if (vals[i] == NULL) {
+if (i > 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory '%s'"), val);
+goto cleanup;
+}
+if (virStrToLong_ull(kws[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory level '%s'"), kws[i]);
+goto cleanup;
+}
+if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot scale memory: %s"),
+   virGetLastErrorMessage());
+goto cleanup;
+}
+
+size = mem;
+
+} else {
+if (STREQ(kws[i], "size") || STREQ(kws[i], "maxmem")) {
+if (virStrToLong_ull(vals[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory level '%s'"), 
vals[i]);
+goto cleanup;
+}
+if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot scale memory: %s"),
+   virGetLastErrorMessage());
+goto cleanup;
+}
+
+STREQ(kws[i], "size") ? (size = mem) : (maxmem = mem);
+
+}
+if (STREQ(kws[i], "slots")) {
+if (virStrToLong_i(vals[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse slots value '%s'"), 
vals[i]);
+goto cleanup;
+}
+
+   slots = n;
+
+}
+}
 }
 
-virDomainDefSetMemoryTotal(dom, mem / 1024);
-dom->mem.cur_balloon = mem / 1024;
+virDomainDefSetMemoryTotal(dom, size / 1024);
+dom->mem.cur_balloon = size / 1024;
+dom->mem.memory_slots = slots;
+dom->mem.max_memory = maxmem;
 
-return 0;
+ret = 0;
+
+ cleanup:
+for (i = 0; i < nkws; i++) {
+VIR_FREE(kws[i]);
+VIR_FREE(vals[i]);
+}
+VIR_FREE(kws);
+VIR_FREE(vals);
+
+return ret;
 }
 
 
diff --git a/tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.args 
b/tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.args
new file mode 100644
index 000..7bce841
--- /dev/null
+++ b/tests/qemuargv2xmldata/qemuargv2xml-mem-scale-maxmemory.args
@@ -0,0 +1,22 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 8G,slots=16,maxmem=16G \
+-smp 1,max

Re: [libvirt] [PATCH] qemu: Changes in parsing qemu commandline memory section

2017-10-09 Thread Madhu Pavan



On 10/09/2017 04:34 PM, Daniel P. Berrange wrote:

On Mon, Oct 09, 2017 at 04:32:38PM +0530, Kothapally Madhu Pavan wrote:

Existing qemuParseCommandLineMem() will parse "-m 4G" format string.
This patch allows it to parse "-m size=8126464k,slots=32,maxmem=33554432k"
format along with existing format.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
  src/qemu/qemu_parse_command.c | 89 +--
  1 file changed, 77 insertions(+), 12 deletions(-)

This needs to have a corresponding  addition to tests/qemuargv2xmltest.c to
validate the code is operating correctly.

Sure. I will add it and post a v2.

Thanks,
Madhu.

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


[libvirt] [PATCH] qemu: Changes in parsing qemu commandline memory section

2017-10-09 Thread Kothapally Madhu Pavan
Existing qemuParseCommandLineMem() will parse "-m 4G" format string.
This patch allows it to parse "-m size=8126464k,slots=32,maxmem=33554432k"
format along with existing format.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_parse_command.c | 89 +--
 1 file changed, 77 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 37e1149..c30e8ed 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -1629,26 +1629,91 @@ static int
 qemuParseCommandLineMem(virDomainDefPtr dom,
 const char *val)
 {
-unsigned long long mem;
+unsigned long long mem = 0;
+unsigned long long size = 0;
+unsigned long long maxmem = 0;
+unsigned int slots = 0;
 char *end;
+size_t i;
+int nkws;
+char **kws;
+char **vals;
+int n;
+int ret = -1;
 
-if (virStrToLong_ull(val, , 10, ) < 0) {
+if (qemuParseKeywords(val, , , , 1) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot parse memory level '%s'"), val);
-return -1;
+   _("cannot parse memory '%s'"), val);
+goto cleanup;
 }
 
-if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cannot scale memory: %s"),
-   virGetLastErrorMessage());
-return -1;
+for (i = 0; i < nkws; i++) {
+if (vals[i] == NULL) {
+if (i > 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory '%s'"), val);
+goto cleanup;
+}
+if (virStrToLong_ull(kws[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory level '%s'"), kws[i]);
+goto cleanup;
+}
+if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot scale memory: %s"),
+   virGetLastErrorMessage());
+goto cleanup;
+}
+
+size = mem;
+
+} else {
+if (STREQ(kws[i], "size") || STREQ(kws[i], "maxmem")) {
+if (virStrToLong_ull(vals[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse memory level '%s'"), 
vals[i]);
+goto cleanup;
+}
+if (virScaleInteger(, end, 1024*1024, ULLONG_MAX) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot scale memory: %s"),
+   virGetLastErrorMessage());
+goto cleanup;
+}
+
+STREQ(kws[i], "size") ? (size = mem) : (maxmem = mem);
+
+}
+if (STREQ(kws[i], "slots")) {
+if (virStrToLong_i(vals[i], , 10, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cannot parse slots value '%s'"), 
vals[i]);
+goto cleanup;
+}
+ 
+   slots = n;
+
+}
+}
 }
 
-virDomainDefSetMemoryTotal(dom, mem / 1024);
-dom->mem.cur_balloon = mem / 1024;
+virDomainDefSetMemoryTotal(dom, size / 1024);
+dom->mem.cur_balloon = size / 1024;
+dom->mem.memory_slots = slots;
+dom->mem.max_memory = maxmem;
 
-return 0;
+ret = 0;
+
+ cleanup:
+for (i = 0; i < nkws; i++) {
+VIR_FREE(kws[i]);
+VIR_FREE(vals[i]);
+}
+VIR_FREE(kws);
+VIR_FREE(vals);
+
+return ret;
 }
 
 
-- 
1.8.3.1

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


[libvirt] [PATCH] qemu: Floppy disks not supported on pSeries machines

2017-10-08 Thread Kothapally Madhu Pavan
Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_parse_command.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 7c409b0..d5745ce 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -760,6 +760,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
 } else if (STREQ(values[i], "floppy")) {
 def->bus = VIR_DOMAIN_DISK_BUS_FDC;
 def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
+if (qemuDomainIsPSeries(dom)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("pseries systems do not support floppy 
devices '%s'"), val);
+goto error;
+}
 } else if (STREQ(values[i], "virtio")) {
 def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO;
 } else if (STREQ(values[i], "xen")) {
-- 
1.8.3.1

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


[libvirt] [PATCH] qemu: Remove redundant code in qemuParseCommandLineDisk

2017-10-08 Thread Kothapally Madhu Pavan
Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_parse_command.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 37e1149..7c409b0 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -945,9 +945,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
 if (virDomainDiskDefAssignAddress(xmlopt, def, dom) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("invalid device name '%s'"), def->dst);
-virDomainDiskDefFree(def);
-def = NULL;
-goto cleanup;
+goto error;
 }
 
  cleanup:
-- 
1.8.3.1

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


[libvirt] [PATCH] util: Free a pointer in virPolkitCheckAuth

2017-10-04 Thread Kothapally Madhu Pavan
Free DBusMessage pointer in virPolkitCheckAuth

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/util/virpolkit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/util/virpolkit.c b/src/util/virpolkit.c
index c735ca9..4559431 100644
--- a/src/util/virpolkit.c
+++ b/src/util/virpolkit.c
@@ -138,6 +138,7 @@ int virPolkitCheckAuth(const char *actionid,
 
  cleanup:
 virStringListFreeCount(retdetails, nretdetails);
+virDBusMessageUnref(reply);
 return ret;
 }
 
-- 
1.8.3.1

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


Re: [libvirt] RFC: Detaching interface from guest fails with improper error message if no mac given in xml

2017-09-29 Thread Madhu Pavan



On 09/29/2017 01:27 PM, Peter Krempa wrote:

On Fri, Sep 29, 2017 at 12:45:30 +0530, Madhu Pavan wrote:

This RFC is regarding https://bugzilla.redhat.com/show_bug.cgi?id=1497054

Let's say we have a network interface
     
   
   
   
   
     

and we are trying to detach the above interface with the following
xml(vfpool.xml):
     
   
   
   
     
Detaching the interface returns error:
# virsh detach-device vffuest vfpool.xml
error: Failed to detach device from vfpool.xml
error: operation failed: no device matching mac address 52:54:00:54:f6:61
found

Here the mac address is auto-generated as we haven't given in the
vfpool.xml.
And virDomainNetFindIdx will try to match the auto-generated mac address
with
the domain xml. It fails as there will be no match and the error message
says
"no device matching mac address 52:54:00:54:f6:61 found".

Here in this scenario I see two possible improvements.
1. As virDomainNetFindIdx search according to mac address and guest side
    PCI address (if specified), we can search for PCI address and avoid
    mac address search if mac is not given in the xml. As the PCI address
    is unique we don't compromise in performance.
2. Improve error message by saying mac address is missing in the device xml.

I think the problem is that the mac address is always generated in the
device XML parser. Auto assignment of the mac address is necessary when
the device is added to the VM. When removing, generating the mac address
is bogus.

I think we need to make sure that no unique ids are added to the parser
at least in the cases when the XML is parsed for the unplug code path.

I have a patch ready to search according to PCI address in the absence
of mac address. I can make changes to it so that mac address is not
auto-generated. Is this good to proceed in this direction?

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


[libvirt] RFC: Detaching interface from guest fails with improper error message if no mac given in xml

2017-09-29 Thread Madhu Pavan

This RFC is regarding https://bugzilla.redhat.com/show_bug.cgi?id=1497054

Let's say we have a network interface
    
  
  
  
  function='0x0'/>

    

and we are trying to detach the above interface with the following 
xml(vfpool.xml):

    
  
  
  function='0x0'/>

    
Detaching the interface returns error:
# virsh detach-device vffuest vfpool.xml
error: Failed to detach device from vfpool.xml
error: operation failed: no device matching mac address 
52:54:00:54:f6:61 found


Here the mac address is auto-generated as we haven't given in the 
vfpool.xml.
And virDomainNetFindIdx will try to match the auto-generated mac address 
with
the domain xml. It fails as there will be no match and the error message 
says

"no device matching mac address 52:54:00:54:f6:61 found".

Here in this scenario I see two possible improvements.
1. As virDomainNetFindIdx search according to mac address and guest side
   PCI address (if specified), we can search for PCI address and avoid
   mac address search if mac is not given in the xml. As the PCI address
   is unique we don't compromise in performance.
2. Improve error message by saying mac address is missing in the device xml.

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

[libvirt] RFC: libvirt support for QEMU live patching

2017-09-15 Thread Madhu Pavan

Hi,
QEMU live patching should be just a matter of updating the QEMU RPM package
and then live migrating the VMs to another QEMU instance on the same host
(which would point to the just installed new QEMU executable).
I think it will be useful to support it from libvirt side. After some 
searching I found a

RFC patch posted in Nov 2013. Here is the link to it
https://www.redhat.com/archives/libvir-list/2013-November/msg00372.html
Approach followed in above mentioned link is as follows:
1. newDef = deep copy oldVm definition
2. newVm = create VM using newDef, start QEMU process with all vCPUs paused
3. oldVm migrate to newVm using unix socket
4. shutdown oldVm
5. newPid = newVm->pid
6. finalDef = live deep copy of newVm definition
7. Drop the newVm from qemu domain table without shutting down QEMU process
8. Assign finalDef to oldVm
9. oldVm attaches to QEMU process newPid using finalDef
10.resume all vCPUs in oldVm

I can see it didn't get communities approval for having problems in 
handling UUID
of the vm's. To fix the problem we need to teach libvirt to manage two 
qemu processes
at once both tied to same UUID. I would like to know if there is any 
interested approach

to get this done. I would like to send patches on this.

Is there any specific reason why it is not been pursued for the last 4 year?

I plan to use postcopy along with the local migration as part of 
migration, that way the

guest need not be even paused for a busy guest.

Thanks,
Madhu Pavan.

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


Re: [libvirt] [PATCH 3/3] doc: Document managedsave-dumpxml support

2017-08-30 Thread Madhu Pavan



On 08/30/2017 04:12 PM, Peter Krempa wrote:

On Wed, Aug 30, 2017 at 15:44:37 +0530, Kothapally Madhu Pavan wrote:

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
  docs/news.xml | 9 +
  1 file changed, 9 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 119c478..fd64094 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,15 @@
  

  
+  qemu: Add managedsave-dumpxml command
+
+
+  Using managedsave-dumpxml, now we can dump the XML
+  configuration of a domain with managedsave image.
+
+  
+  
+


2/3 and 3/3 basically document the same feature. Please add all the docs
in one entry.
I have sent v2 patchset with patch 2/2 documenting the feature in single 
entry.


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


[libvirt] [PATCH v2 0/2] Fix docs/news.xml template structure and add new features

2017-08-30 Thread Kothapally Madhu Pavan
This patchset will fix docs/news.xml template structure which is broken by
commit b7e779c1a51793 and add support for managedsave-define, managedsave-edit
and managedsave-dumpxml commands.

Kothapally Madhu Pavan (2):
  docs: Fix docs/news.xml template structure
  docs: Document managedsave-edit commands support

 docs/news.xml | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

-- 
1.8.3.1

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


[libvirt] [PATCH v2 2/2] docs: Document managedsave-edit commands support

2017-08-30 Thread Kothapally Madhu Pavan
This patch documents support for managedsave-dumpxml,
managedsave-define and managedsave-edit commands.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 docs/news.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 088966d..1f4eb3e 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,16 @@
 
   
 
+  qemu: Add managedsave-edit commands
+
+
+  Using managedsave-dumpxml, managedsave-define and managedsave-edit
+  commands, now we can dump and edit the XML configuration of domain
+  which has managedsave image.
+
+  
+  
+
   qemu: Add migrate-getmaxdowntime command
 
 
-- 
1.8.3.1

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


[libvirt] [PATCH v2 1/2] docs: Fix docs/news.xml template structure

2017-08-30 Thread Kothapally Madhu Pavan
Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 docs/news.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/news.xml b/docs/news.xml
index 05152b8..088966d 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -35,8 +35,8 @@
 
   
 
-   
- 
+  
+
   qemu: Add migrate-getmaxdowntime command
 
 
-- 
1.8.3.1

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


[libvirt] [PATCH 1/3] doc: Fix docs/news.xml template structure

2017-08-30 Thread Kothapally Madhu Pavan
Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 docs/news.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/news.xml b/docs/news.xml
index 05152b8..088966d 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -35,8 +35,8 @@
 
   
 
-   
- 
+  
+
   qemu: Add migrate-getmaxdowntime command
 
 
-- 
1.8.3.1

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


[libvirt] [PATCH 2/3] doc: Document managedsave-define and managedsave-edit support

2017-08-30 Thread Kothapally Madhu Pavan
Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 docs/news.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 088966d..119c478 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,16 @@
 
   
 
+  qemu: Add managedsave-define and managedsave-edit commands
+
+
+  Using managedsave-define and managedsave-edit commands, now
+  we can edit the host-specific portions of the domain XMl
+  which has managedsave image.
+
+  
+  
+
   qemu: Add migrate-getmaxdowntime command
 
 
-- 
1.8.3.1

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


[libvirt] [PATCH 3/3] doc: Document managedsave-dumpxml support

2017-08-30 Thread Kothapally Madhu Pavan
Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 docs/news.xml | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 119c478..fd64094 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,15 @@
 
   
 
+  qemu: Add managedsave-dumpxml command
+
+
+  Using managedsave-dumpxml, now we can dump the XML
+  configuration of a domain with managedsave image.
+
+  
+  
+
   qemu: Add managedsave-define and managedsave-edit commands
 
 
-- 
1.8.3.1

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


[libvirt] [PATCH 0/3] Fix docs/news.xml template structure and add new features

2017-08-30 Thread Kothapally Madhu Pavan
This patchset will fix docs/news.xml template structure which is broken by
commit b7e779c1a51793 and add support for managedsave-define, managedsave-edit
and managedsave-dumpxml commands.

Kothapally Madhu Pavan (3):
  doc: Fix docs/news.xml template structure
  doc: Document managedsave-define and managedsave-edit support
  doc: Document managedsave-dumpxml support

 docs/news.xml | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

-- 
1.8.3.1

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


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

2017-08-08 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 512804c..8baea2a 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[] = {
@@ -13845,6 +13918,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 43d6f0c..11e2321 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1630,6 +1630,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 v4 3/7] qemu: Implement qemuDomainManagedSaveGetXMLDesc

2017-08-08 Thread Kothapally Madhu Pavan
This commit adds qemu driver implementation to get xml description
for managed save state domain.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_driver.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b3f65f4..ec73dc1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6797,6 +6797,51 @@ 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;
+
+if (!virFileExists(path)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",_("domain does not have managed save image"));
+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
@@ -20839,6 +20884,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainManagedSave = qemuDomainManagedSave, /* 0.8.0 */
 .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */
+.domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.7.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 v4 1/7] lib: Add API to dump xml configuration of managed save state domain

2017-08-08 Thread Kothapally Madhu Pavan
Similar to domainSaveImageGetXMLDesc this commit adds 
domainManagedSaveGetXMLDesc
API which allows to get the xml of managed save state domain.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 a3bb9cb..8ede912 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 87fca29..cb260e2 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9298,6 +9298,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..b38d0bb 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.7.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 a57d25f..7a74b87 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 */
+.

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

2017-08-08 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 8baea2a..ce15090 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[] = {
@@ -13918,6 +13968,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 11e2321..1bda44f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1644,6 +1644,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 v4 0/7] Add new APIs to edit xml configuration of managed save state of a domain

2017-08-08 Thread 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's always better when we get more.

Changes since v3:
- refracted version references from 3.6.0 to 3.7.0
- fixed typo in error message.

Changes since v2:
- refracted version references from 3.5.0 to 3.6.0

Changes since v1:
- qemu implementation called directly rather than going through
  driver pointer in qemuDomainManagedSaveDefineXML.
- check whether the managed save state file exists and report a
  error if it doesn't.

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   |  87 
 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, 511 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 v4 4/7] qemu: Implement qemuDomainManagedSaveDefineXML

2017-08-08 Thread 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 <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_driver.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ec73dc1..b6db435 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6842,6 +6842,46 @@ 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;
+int ret;
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return -1;
+
+path = qemuDomainManagedSavePath(driver, vm);
+virDomainObjEndAPI();
+
+if (!path)
+goto error;
+
+if (!virFileExists(path)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",_("domain does not have managed save image"));
+goto error;
+}
+
+ret = qemuDomainSaveImageDefineXML(conn, path, dxml, flags);
+
+VIR_FREE(path);
+
+if (ret < 0)
+goto error;
+
+return ret;
+
+ 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
@@ -20885,6 +20925,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */
 .domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.7.0 */
+.domainManagedSaveDefineXML = qemuDomainManagedSaveDefineXML, /* 3.7.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 v4 7/7] virsh: Implement managedsave-edit command

2017-08-08 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 ce15090..6152661 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[] = {
@@ -13968,6 +14034,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 1bda44f..dbad9a0 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1650,6 +1650,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 v4 2/7] lib: Add API to edit domain's managed save state xml configuration

2017-08-08 Thread 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 <k...@linux.vnet.ibm.com>
---
 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 8ede912..ed2c2b6 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 cb260e2..918e81a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9349,6 +9349,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 b38d0bb..fc4efd9 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -771,6 +771,7 @@ LIBVIRT_3.4.0 {
 LIBVIRT_3.7.0 {
 global:
  

[libvirt] [PATCH v3 2/2] qemu: Default hwclock source for sPAPR to RTC

2017-07-25 Thread Kothapally Madhu Pavan
QEMU fails to launch a sPAPR guest with clock sources other that RTC.
Internally qemu only uses RTC timer for hwclock. This patch reports
the right error message instead of qemu erroring out when any other
timer other than RTC is used.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_domain.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e5e4208..b3cfb6c 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3103,6 +3103,14 @@ qemuDomainDefValidate(const virDomainDef *def,
virArchToString(def->os.arch));
 goto cleanup;
 }
+/* /* Only RTC timer is supported as hwclock for sPAPR machines */
+if (ARCH_IS_PPC64(def->os.arch) && timer->name != 
VIR_DOMAIN_TIMER_NAME_RTC) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unsupported clock timer '%s' for %s 
architecture"),
+   
virDomainTimerNameTypeToString(def->clock.timers[i]->name),
+   virArchToString(def->os.arch));
+goto cleanup;
+}
 }
 
 if (def->mem.min_guarantee) {
-- 
1.8.3.1

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


[libvirt] [PATCH v3 1/2] qemu: Restrict usage of hpet and kvm.pit timers by unsupported architectures

2017-07-25 Thread Kothapally Madhu Pavan
hpet and kvm.pit clock timers are specific to x86 architecture and
are not suppose to be used in other architectures. This patch restricts
the usage of hpet and kvm.pit timers in unsupported architectures.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_domain.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2c8c9a7..e5e4208 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3083,12 +3083,28 @@ qemuDomainDefValidate(const virDomainDef *def,
 virQEMUCapsPtr qemuCaps = NULL;
 unsigned int topologycpus;
 int ret = -1;
+size_t i;
 
 if (!(qemuCaps = virQEMUCapsCacheLookup(caps,
 driver->qemuCapsCache,
 def->emulator)))
 goto cleanup;
 
+/* Restrict usage of unsupported clock sources */
+for (i = 0; i < def->clock.ntimers; i++) {
+virDomainTimerDefPtr timer = def->clock.timers[i];
+if ((!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) &&
+ (timer->name == VIR_DOMAIN_TIMER_NAME_HPET)) ||
+(!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) &&
+ (timer->name == VIR_DOMAIN_TIMER_NAME_PIT))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unsupported clock timer '%s' for %s 
architecture"),
+   
virDomainTimerNameTypeToString(def->clock.timers[i]->name),
+   virArchToString(def->os.arch));
+goto cleanup;
+}
+}
+
 if (def->mem.min_guarantee) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Parameter 'min_guarantee' not supported by QEMU."));
-- 
1.8.3.1

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


[libvirt] [PATCH v3 0/2] Restrict usage of unsupported clock timers

2017-07-25 Thread Kothapally Madhu Pavan
hpet and kvm.pit clock timers are specific to x86 architecture and
are not to be used by unsuported architectures. Similarly sPAPR
guests only allow RTC timer. This patchset will restrict the usage
of unsupported clock timers.

Kothapally Madhu Pavan (2):
  qemu: Restrict usage of hpet and kvm.pit timers by unsupported
architectures
  qemu: Default hwclock source for sPAPR to RTC

 src/qemu/qemu_domain.c | 24 
 1 file changed, 24 insertions(+)

-- 
1.8.3.1

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


[libvirt] [PATCH v2] qemu: Default hwclock source for sPAPR to RTC

2017-07-14 Thread Kothapally Madhu Pavan
QEMU fails to launch a sPAPR guest with clock sources other that RTC.
Internally qemu only uses RTC timer for hwclock. This patch reports
the right error message instead of qemu erroring out when any other
timer other than RTC is used.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_domain.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8e7404d..b74800d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3025,6 +3025,7 @@ qemuDomainDefValidate(const virDomainDef *def,
 virQEMUCapsPtr qemuCaps = NULL;
 unsigned int topologycpus;
 int ret = -1;
+size_t i;
 
 if (!(qemuCaps = virQEMUCapsCacheLookup(caps,
 driver->qemuCapsCache,
@@ -3037,6 +3038,18 @@ qemuDomainDefValidate(const virDomainDef *def,
 goto cleanup;
 }
 
+/* Only RTC timer is supported as hwclock for sPAPR machines */
+for (i = 0; i < def->clock.ntimers; i++) {
+virDomainTimerDefPtr timer = def->clock.timers[i];
+if (ARCH_IS_PPC64(def->os.arch) && timer->name != 
VIR_DOMAIN_TIMER_NAME_RTC) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unsupported clock timer '%s' for %s 
architecture"),
+ 
virDomainTimerNameTypeToString(def->clock.timers[i]->name),
+ virArchToString(def->os.arch));
+goto cleanup;
+}
+}
+
 /* On x86, UEFI requires ACPI */
 if (def->os.loader &&
 def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH &&
-- 
1.8.3.1

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


Re: [libvirt] [PATCH] qemu: Default hwclock source for sPAPR to RTC

2017-07-13 Thread Madhu Pavan



On 07/13/2017 05:49 PM, Cole Robinson wrote:

On 07/13/2017 04:36 AM, Kothapally Madhu Pavan wrote:

QEMU fails to launch a sPAPR guest with clock sources other that RTC.
Internally qemu only uses RTC timer for hwclock. This patch reports
the right error message instead of qemu erroring out when any other
timer other than RTC is used.


How does it fail exactly? Is it a qemu error message or a guest OS failure?

If it's from qemu, and the error message is reasonably clear what hardware/xml
config option is at fauly, then these checks don't add much functional
benefit, just more code to maintain.


If it's from qemu, and the error message is reasonably clear what hardware/xml
config option is at fauly, then these checks don't add much functional
benefit, just more code to maintain.

When we use kvmclock timer in domain xml as:
  

  
Domain fails to start with following error:
#virsh start --console virt-tests-vm1
error: Failed to start domain virt-tests-vm1
error: internal error: process exited while connecting to monitor: 
2017-04-25T09:31:58.180062Z qemu-system-ppc64: Unable to find CPU definition: 
qemu64

This is because the qemu cpu command line generated when kvmclock
timer is used is:
 -cpu qemu64,+kvmclock
This happens because in qemuBuildCpuCommandLine has default_model = qemu64,
When I corrected the default model to "host" for ppc64 machine,
qemu cpu commandline generated is:
 -cpu host,+kvmclock
This is a valid qemu command for ppc64 machine. Now the qemu
fails to start with folloeing error:
qemu-system-ppc64: Expected key=value format, found +kvmclock.

Similarly when kvm-pit timer is used qemu warns as below:
sudo ./qemu-system-ppc64 -name migrate_qemu -boot strict=on -device 
nec-usb-xhci,id=usb,bus=pci.0,addr=0xf -device spapr-vscsi,id=scsi0,reg=0x2000 
-smp 1,maxcpus=4,sockets=4,cores=1,threads=1 --machine 
pseries,accel=kvm,kvm-type=HV,usb=off,dump-guest-core=off -m 
4G,slots=32,maxmem=32G -drive 
file=/home/danielhb/vm_imgs/ubuntu1704.qcow2,format=qcow2,if=none,id=drive-virtio-disk0,cache=none
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x2,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -nographic -global kvm-pit.lost_tick_policy=discard

qemu-system-ppc64: Warning: global kvm-pit.lost_tick_policy has invalid class 
name

Basically, RTC is the only valid clocksource for sPAPR guests. For other clock 
sources
qemu either errors out or internally considers RTC as default.



A general point, these types of checks should be considered for
qemuDomainDefValidate which adds the benefit of rejecting the config at XML
define time.
I was of the opinion, the existing the domain definitions would fail to 
be parsed if I add
in qemuDomainDefValidate(). Now, while I reply to you I realise, there 
is no way someone
would have attempted to use non-RTC clock sources. So, its perfectly 
safe to move these

checks to qemuDomainDefValidate().

Will attempt it in V2.


Thanks,
Cole


Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
  src/qemu/qemu_command.c | 28 +++-
  1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c53ab97..31561ce 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6440,6 +6440,15 @@ qemuBuildClockCommandLine(virCommandPtr cmd,
  break;
  
  case VIR_DOMAIN_TIMER_NAME_PIT:

+/* Only RTC timer is supported as hwclock for sPAPR machines */
+if (ARCH_IS_PPC64(def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unsupported clock timer '%s' for '%s' 
architecture"),
+ 
virDomainTimerNameTypeToString(def->clock.timers[i]->name),
+ virArchToString(def->os.arch));
+return -1;
+}
+
  switch (def->clock.timers[i]->tickpolicy) {
  case -1:
  case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY:
@@ -6483,13 +6492,21 @@ qemuBuildClockCommandLine(virCommandPtr cmd,
  break;
  
  case VIR_DOMAIN_TIMER_NAME_HPET:

+/* Only RTC timer is supported as hwclock for sPAPR machines */
+if (ARCH_IS_PPC64(def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unsupported clock timer '%s' for '%s' 
architecture"),
+ 
virDomainTimerNameTypeToString(def->clock.timers[i]->name),
+ virArchToString(def->os.arch));
+return -1;
+}
+
  /* the only meaningful attribute for hpet is "present". If
   * present is -1, that means it wasn't specified, and
   * should be left at the default for the
   * hypervisor. "default" when -no-hpet

[libvirt] [PATCH] qemu: Default hwclock source for sPAPR to RTC

2017-07-13 Thread Kothapally Madhu Pavan
QEMU fails to launch a sPAPR guest with clock sources other that RTC.
Internally qemu only uses RTC timer for hwclock. This patch reports
the right error message instead of qemu erroring out when any other
timer other than RTC is used.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_command.c | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c53ab97..31561ce 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6440,6 +6440,15 @@ qemuBuildClockCommandLine(virCommandPtr cmd,
 break;
 
 case VIR_DOMAIN_TIMER_NAME_PIT:
+/* Only RTC timer is supported as hwclock for sPAPR machines */
+if (ARCH_IS_PPC64(def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unsupported clock timer '%s' for '%s' 
architecture"),
+ 
virDomainTimerNameTypeToString(def->clock.timers[i]->name),
+ virArchToString(def->os.arch));
+return -1;
+}
+
 switch (def->clock.timers[i]->tickpolicy) {
 case -1:
 case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY:
@@ -6483,13 +6492,21 @@ qemuBuildClockCommandLine(virCommandPtr cmd,
 break;
 
 case VIR_DOMAIN_TIMER_NAME_HPET:
+/* Only RTC timer is supported as hwclock for sPAPR machines */
+if (ARCH_IS_PPC64(def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unsupported clock timer '%s' for '%s' 
architecture"),
+ 
virDomainTimerNameTypeToString(def->clock.timers[i]->name),
+ virArchToString(def->os.arch));
+return -1;
+}
+
 /* the only meaningful attribute for hpet is "present". If
  * present is -1, that means it wasn't specified, and
  * should be left at the default for the
  * hypervisor. "default" when -no-hpet exists is "yes",
  * and when -no-hpet doesn't exist is "no". "confusing"?
  * "yes"! */
-
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) {
 if (def->clock.timers[i]->present == 0)
 virCommandAddArg(cmd, "-no-hpet");
@@ -7047,6 +7064,15 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
 for (i = 0; i < def->clock.ntimers; i++) {
 virDomainTimerDefPtr timer = def->clock.timers[i];
 
+/* Only RTC timer is supported as hwclock for sPAPR machines */
+if (ARCH_IS_PPC64(def->os.arch) && timer->name != 
VIR_DOMAIN_TIMER_NAME_RTC) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unsupported clock timer '%s' for '%s' 
architecture"),
+ 
virDomainTimerNameTypeToString(def->clock.timers[i]->name),
+ virArchToString(def->os.arch));
+return -1;
+}
+
 if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK &&
 timer->present != -1) {
 virBufferAsprintf(, "%s,%ckvmclock",
-- 
1.8.3.1

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


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

2017-07-05 Thread 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's always better when we get more.

Changes since v2:
- refracted version references from 3.5.0 to 3.6.0

Changes since v1:
- qemu implementation called directly rather than going through
  driver pointer in qemuDomainManagedSaveDefineXML.
- check whether the managed save state file exists and report a
  error if it doesn't.

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   |  87 
 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, 511 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 v3 4/7] qemu: Implement qemuDomainManagedSaveDefineXML

2017-07-05 Thread 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 <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_driver.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 63bdf77..3ecd334 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6843,6 +6843,46 @@ 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;
+int ret;
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return -1;
+
+path = qemuDomainManagedSavePath(driver, vm);
+virDomainObjEndAPI();
+
+if (!path)
+goto error;
+
+if (!virFileExists(path)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",_("domain doesnot have managed save image"));
+goto error;
+}
+
+ret = qemuDomainSaveImageDefineXML(conn, path, dxml, flags);
+
+VIR_FREE(path);
+
+if (ret < 0)
+goto error;
+
+return ret;
+
+ 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
@@ -20856,6 +20896,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */
 .domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.6.0 */
+.domainManagedSaveDefineXML = qemuDomainManagedSaveDefineXML, /* 3.6.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 v3 7/7] virsh: Implement managedsave-edit command

2017-07-05 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 6e33921..7333e7d 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[] = {
@@ -13937,6 +14003,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 1bda44f..dbad9a0 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1650,6 +1650,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 v3 2/7] lib: Add API to edit domain's managed save state xml configuration

2017-07-05 Thread 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 <k...@linux.vnet.ibm.com>
---
 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 5fadd26..01eb682 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 0cde357..e5b48e4 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -771,6 +771,7 @@ LIBVIRT_3.4.0 {
 LIBVIRT_3.6.0 {
 global:
  

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

2017-07-05 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 af02bab..6e33921 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[] = {
@@ -13887,6 +13937,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 11e2321..1bda44f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1644,6 +1644,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 v3 5/7] virsh: Implement managedsave-define command

2017-07-05 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 55f4e14..af02bab 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[] = {
@@ -13814,6 +13887,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 43d6f0c..11e2321 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1630,6 +1630,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 v3 1/7] lib: Add API to dump xml configuration of managed save state domain

2017-07-05 Thread Kothapally Madhu Pavan
Similar to domainSaveImageGetXMLDesc this commit adds 
domainManagedSaveGetXMLDesc
API which allows to get the xml of managed save state domain.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 4033ae8..5fadd26 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..0cde357 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.6.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 a57d25f..e530fe5 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 */
+.

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

2017-07-05 Thread Kothapally Madhu Pavan
This commit adds qemu driver implementation to get xml description
for managed save state domain.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_driver.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cdb727b..63bdf77 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6798,6 +6798,51 @@ 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;
+
+if (!virFileExists(path)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",_("domain doesnot have managed save image"));
+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 +20855,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainManagedSave = qemuDomainManagedSave, /* 0.8.0 */
 .domainHasManagedSaveImage = qemuDomainHasManagedSaveImage, /* 0.8.0 */
 .domainManagedSaveRemove = qemuDomainManagedSaveRemove, /* 0.8.0 */
+.domainManagedSaveGetXMLDesc = qemuDomainManagedSaveGetXMLDesc, /* 3.6.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] [resend PATCH v2 6/7] virsh: Implement managedsave-dumpxml command

2017-06-22 Thread Madhu Pavan

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

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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] [resend PATCH v2 7/7] virsh: Implement managedsave-edit command

2017-06-22 Thread Madhu Pavan

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

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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] [resend PATCH v2 5/7] virsh: Implement managedsave-define command

2017-06-22 Thread Madhu Pavan

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

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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] [resend PATCH v2 4/7] qemu: Implement qemuDomainManagedSaveDefineXML

2017-06-22 Thread 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 <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_driver.c | 41 +
 1 file changed, 41 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5b21cf7..93c62a1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6843,6 +6843,46 @@ 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;
+int ret;
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+return -1;
+
+path = qemuDomainManagedSavePath(driver, vm);
+virDomainObjEndAPI();
+
+if (!path)
+goto error;
+
+if (!virFileExists(path)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",_("domain doesnot have managed save image"));
+goto error;
+}
+
+ret = qemuDomainSaveImageDefineXML(conn, path, dxml, flags);
+
+VIR_FREE(path);
+
+if (ret < 0)
+goto error;
+
+return ret;
+
+ 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
@@ -20856,6 +20896,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] [resend PATCH v2 2/7] lib: Add API to edit domain's managed save state xml configuration

2017-06-22 Thread 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 <k...@linux.vnet.ibm.com>
---
 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:
  

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

2017-06-22 Thread Madhu Pavan

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

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_driver.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e91663c..5b21cf7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6798,6 +6798,51 @@ 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;
+
+if (!virFileExists(path)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s",_("domain doesnot have managed save image"));
+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 +20855,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] [resend PATCH v2 1/7] lib: Add API to dump xml configuration of managed save state domain

2017-06-22 Thread Madhu Pavan

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

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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 */
+.

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

2017-06-22 Thread 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's always better when we get more.

Changes since v1:
- qemu implementation called directly rather than going through
  driver pointer in qemuDomainManagedSaveDefineXML.
- check whether the managed save state file exists and report a
  error if it doesn't.

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   |  87 
 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, 511 insertions(+), 1 deletion(-)

--
1.8.3.1

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


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

2017-06-21 Thread Madhu Pavan



On 06/21/2017 01:02 PM, Peter Krempa wrote:

On Wed, Jun 21, 2017 at 05:07:49 +0530, Kothapally Madhu Pavan wrote:

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

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
  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;
+}

This implementation is horrible. Please call the qemu implementation
directly or extract useful parts rather than going through the driver
pointers.
I was trying to follow the pattern of other APIs and I didn't realize 
the pointers are not necessary. Thanks for the comments. Fixing that in 
the next version.


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

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

2017-06-21 Thread Madhu Pavan



On 06/21/2017 01:05 PM, Peter Krempa wrote:

On Wed, Jun 21, 2017 at 05:07:48 +0530, Kothapally Madhu Pavan wrote:

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

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
  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)

This will report a horrible error message in case when the VM is not
manage-saved. You need to check whether the file exists and report a
better one.

(error message will be reported by qemuOpenFileAs, thus will be
Failed to open file '%s')

Thanks for the comments. Fixing it in the next version.

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


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

2017-06-20 Thread Kothapally Madhu Pavan
This commit adds qemu driver implementaion to get xml description
for managed save state domain.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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

2017-06-20 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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

2017-06-20 Thread 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 <k...@linux.vnet.ibm.com>
---
 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

2017-06-20 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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

2017-06-20 Thread Kothapally Madhu Pavan
Add a simple virsh command handler which makes use of the new API.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 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

2017-06-20 Thread 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 <k...@linux.vnet.ibm.com>
---
 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 = rem

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

2017-06-20 Thread 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

2017-06-20 Thread 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 <k...@linux.vnet.ibm.com>
---
 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:
  

[libvirt] [PATCH] qemu: Remove unused variables in qemuDomainUpdateDeviceConfig

2017-05-16 Thread Kothapally Madhu Pavan
priv and qemuCaps variables are not used anymore.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_driver.c |   11 ---
 1 file changed, 11 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1c4873e..4721356 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8172,8 +8172,6 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
 bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0;
 int ret = -1;
-virQEMUCapsPtr qemuCaps = NULL;
-qemuDomainObjPrivatePtr priv;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
@@ -8192,8 +8190,6 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
 
-priv = vm->privateData;
-
 if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
 goto cleanup;
 
@@ -8220,12 +8216,6 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 goto endjob;
 }
 
-if (priv->qemuCaps)
-qemuCaps = virObjectRef(priv->qemuCaps);
-else if (!(qemuCaps = virQEMUCapsCacheLookup(caps, driver->qemuCapsCache,
- vm->def->emulator)))
-goto endjob;
-
 if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
 /* Make a copy for updated domain. */
 vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
@@ -8273,7 +8263,6 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 qemuDomainObjEndJob(driver, vm);
 
  cleanup:
-virObjectUnref(qemuCaps);
 virDomainDefFree(vmdef);
 if (dev != dev_copy)
 virDomainDeviceDefFree(dev_copy);

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


[libvirt] [PATCH] Adding POWER9 cpu model to cpu_map.xml

2017-05-09 Thread Kothapally Madhu Pavan
As POWER9 model is not available in cpu_map.xml virsh capabilities
donot display the cpu model and vendor details. This patch
provides those details
---
 src/cpu/cpu_map.xml |5 +
 1 file changed, 5 insertions(+)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 7d5540a..29b5b59 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -1571,6 +1571,11 @@
   
 
 
+
+  
+  
+
+
 
 
   

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


Re: [libvirt] [PATCH] qemu: Check for maximum vcpus exceeding cpu topology

2017-02-20 Thread Madhu Pavan



On 02/07/2017 06:41 PM, Peter Krempa wrote:

On Tue, Feb 07, 2017 at 07:56:29 -0500, Kothapally Madhu Pavan wrote:

This patch will prevent guest to start when the maximum
vcpus are greater than cpu topology limit. Currently similar
checks do exist only during setvcpus. The patch adds the
missing check. The c9cb35c255222 reverted similar check to
avoid older configs from vanishing. The current patch adds
it in domain validation part to be consistent with the
original intent.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
  src/qemu/qemu_process.c |7 +++
  1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 184440d..f0d42b8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3738,6 +3738,13 @@ qemuValidateCpuCount(virDomainDefPtr def,
  return -1;
  }
  
+if (def->cpu->sockets && virDomainDefGetVcpusMax(def) >

+def->cpu->sockets * def->cpu->cores * def->cpu->threads) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Maximum CPUs greater than topology limit"));
+return -1;
+}

This exact logic is done in qemuDomainDefValidate:

+if (virDomainDefGetVcpusTopology(def, ) == 0 &&
+topologycpus != virDomainDefGetVcpusMax(def)) {
+/* presence of query-hotpluggable-cpus should be a good enough witness 
*/
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("CPU topology doesn't match maximum vcpu count"));
+goto cleanup;
  }
  }

See commit 043ba4a40a4ae26cf616146d0d1c129d65b156b8. The only difference
is that the code is validated only for certain versions of qemu known to
reject such configuration.

Is that not enough?

64




# virsh start one-0
error: Failed to start domain one-0
error: internal error: process exited while connecting to monitor: 
2017-02-08T06:41:59.615692Z qemu-system-ppc64: cpu topology: sockets (1) 
* cores (4) * threads (8) < smp_cpus (64)


I have qemu version 2.5.0 which doesn't have cpu hotplug capability 
hence I get this error. Any ways we can improve and error out using libvirt?


I tried with qemu-2.4.1 and still see the error, from this I can say smp 
topology check is maintained by qemu even before cpu hotplug feature is 
added.


When we had vcpu count validation in XML parser, domains with above 
mentioned config were rejected irrespective of qemu capabilities.


Thanks,
Madhu.

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


[libvirt] [PATCH] qemu: Check for maximum vcpus exceeding cpu topology

2017-02-07 Thread Kothapally Madhu Pavan
This patch will prevent guest to start when the maximum
vcpus are greater than cpu topology limit. Currently similar
checks do exist only during setvcpus. The patch adds the
missing check. The c9cb35c255222 reverted similar check to
avoid older configs from vanishing. The current patch adds
it in domain validation part to be consistent with the
original intent.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_process.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 184440d..f0d42b8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3738,6 +3738,13 @@ qemuValidateCpuCount(virDomainDefPtr def,
 return -1;
 }
 
+if (def->cpu->sockets && virDomainDefGetVcpusMax(def) >
+def->cpu->sockets * def->cpu->cores * def->cpu->threads) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Maximum CPUs greater than topology limit"));
+return -1;
+}
+
 if (maxCpus > 0 && virDomainDefGetVcpusMax(def) > maxCpus) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Maximum CPUs greater than specified machine type 
limit"));

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


[libvirt] [PATCH] domain_event.c: Fix a typo

2016-10-31 Thread Kothapally Madhu Pavan
Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/conf/domain_event.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 63ae9e1..f1249ad 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1839,7 +1839,7 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn,
 virDomainEventJobCompletedPtr ev;
 
 ev = (virDomainEventJobCompletedPtr) event;
-((virConnectDomainEventJobCompletedCallback) cb)(conn, dom,
+((virConnectDomainEventJobCompletedCallback)cb)(conn, dom,
  ev->params,
  ev->nparams,
  cbopaque);

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


[libvirt] [PATCH v2] vsh: Using VSH_REQUIRE_OPTION rather than virReportError

2016-10-18 Thread Kothapally Madhu Pavan
Correcting the error reporting method by using VSH_REQUIRE_OPTION
instead of virReportError

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 tools/virsh-domain.c |7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 050e7fb..e1cb2ac 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10443,6 +10443,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
 
 VSH_EXCLUSIVE_OPTIONS("live", "offline");
 VSH_EXCLUSIVE_OPTIONS("timeout-suspend", "timeout-postcopy");
+VSH_REQUIRE_OPTION("postcopy-after-precopy", "postcopy");
 
 if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
 return false;
@@ -10474,12 +10475,6 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
 }
 
 if (vshCommandOptBool(cmd, "postcopy-after-precopy")) {
-if (!vshCommandOptBool(cmd, "postcopy")) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
-  _("--postcopy-after-precopy can only be used with "
-"--postcopy"));
-goto cleanup;
-}
 iterEvent = virConnectDomainEventRegisterAny(
 priv->conn, dom,
 VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION,

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


Re: [libvirt] [PATCH] vsh: Using vshError rather than virReportError

2016-10-18 Thread Madhu Pavan



On 10/18/2016 07:33 PM, Peter Krempa wrote:

On Tue, Oct 18, 2016 at 07:39:16 -0400, Kothapally Madhu Pavan wrote:

Correcting the error reporting method by using vshError
instead of virReportError

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
  tools/virsh-domain.c |6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 050e7fb..c9fabf2 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10475,9 +10475,9 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
  
  if (vshCommandOptBool(cmd, "postcopy-after-precopy")) {

  if (!vshCommandOptBool(cmd, "postcopy")) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
-  _("--postcopy-after-precopy can only be used with "
-"--postcopy"));
+vshError(ctl, "%s",
+ _("argument unsupported: --postcopy-after-precopy can only 
"
+   "be used with --postcopy"));
  goto cleanup;

I think the above code can be avoided by simply using
VSH_REQUIRE_OPTION.

Yes, it can be avoided by using VSH_REQUIRE_OPTION. Sending a new patch.

Thanks,
Madhu Pavan.

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


[libvirt] [PATCH] vsh: Using vshError rather than virReportError

2016-10-18 Thread Kothapally Madhu Pavan
Correcting the error reporting method by using vshError
instead of virReportError

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 tools/virsh-domain.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 050e7fb..c9fabf2 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10475,9 +10475,9 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
 
 if (vshCommandOptBool(cmd, "postcopy-after-precopy")) {
 if (!vshCommandOptBool(cmd, "postcopy")) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
-  _("--postcopy-after-precopy can only be used with "
-"--postcopy"));
+vshError(ctl, "%s",
+ _("argument unsupported: --postcopy-after-precopy can 
only "
+   "be used with --postcopy"));
 goto cleanup;
 }
 iterEvent = virConnectDomainEventRegisterAny(

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


[libvirt] [PATCH] Make --postcopy flag mandatory with --postcopy-after-precopy

2016-09-02 Thread Kothapally Madhu Pavan
--postcopy-after-precopy is just an aditional flag for
postcopy migration.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 tools/virsh-domain.c |6 +++---
 tools/virsh.pod  |4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index c9ad0d7..a614512 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10318,10 +10318,10 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
 }
 
 if (vshCommandOptBool(cmd, "postcopy-after-precopy")) {
-if (!live_flag) {
+if (!vshCommandOptBool(cmd, "postcopy")) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
-  _("post-copy migration is not supported with "
-"non-live or paused migration"));
+  _("--postcopy-after-precopy can only be used with "
+"--postcopy"));
 goto cleanup;
 }
 iterEvent = virConnectDomainEventRegisterAny(
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1e36ee1..3670ace 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1617,8 +1617,8 @@ migration. I<--postcopy> enables post-copy logic in 
migration, but does not
 actually start post-copy, i.e., migration is started in pre-copy mode.
 Once migration is running, the user may switch to post-copy using the
 B command sent from another virsh instance or use
-I<--postcopy-after-precopy> to let libvirt automatically switch to
-post-copy after the first pass of pre-copy is finished.
+I<--postcopy-after-precopy> along with I<--postcopy> to let libvirt 
automatically
+switch to post-copy after the first pass of pre-copy is finished.
 
 I<--auto-converge> forces convergence during live migration. The initial
 guest CPU throttling rate can be set with I. If the

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


Re: [libvirt] [PATCH] Check for --live flag for postcopy-after-precopy migration

2016-08-29 Thread Madhu Pavan



On 08/27/2016 02:21 AM, Jiri Denemark wrote:

On Fri, Aug 26, 2016 at 21:41:31 +0200, Michal Privoznik wrote:

On 26.08.2016 11:25, Kothapally Madhu Pavan wrote:

Unlike postcopy migration there is no --live flag check for
postcopy-after-precopy.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
  tools/virsh-domain.c |6 ++
  1 file changed, 6 insertions(+)


ACKed and pushed.

This doesn't make any sense. First, post-copy migration is enabled with
--postcopy option to migrate command and --postcopy-after-precopy is
just an additional flag for post-copy migration. So if virsh was to
report such an error, it should check for --postcopy option. But such
check doesn't belong to libvirt at all, the appropriate libvirt driver
is supposed to check for the flags and report invalid combinations.
I have proposed this patch as the qemu driver doesn't have 
postcopy-after-precopy
flag and this bug can be fixed by minimal changes in libvirt. If we have 
to check for
invalid combinations in appropriate libvirt drivers, we need to create a 
flag for
postcopy-after-precopy migration. I will be happy to send another patch 
if this is what

needed.

Thanks,
Madhu Pavan.




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


[libvirt] [PATCH] Check for --live flag for postcopy-after-precopy migration

2016-08-26 Thread Kothapally Madhu Pavan
Unlike postcopy migration there is no --live flag check for
postcopy-after-precopy.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 tools/virsh-domain.c |6 ++
 1 file changed, 6 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index de2a22c..798a1ff 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10317,6 +10317,12 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd)
 }
 
 if (vshCommandOptBool(cmd, "postcopy-after-precopy")) {
+if (!live_flag) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+  _("post-copy migration is not supported with "
+"non-live or paused migration"));
+goto cleanup;
+}
 iterEvent = virConnectDomainEventRegisterAny(
 priv->conn, dom,
 VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION,

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


[libvirt] [PATCH] conf: Skip MAC checks when using an auto-generated one during detach-device

2016-05-30 Thread Kothapally Madhu Pavan
When we try to detach a network device without specifying the
mac address, random mac address is generated. As the generated
mac address will not be available in the running vm, detaching
device will fail erroring out "error: operation failed: no
device matching mac address xx:xx:xx:xx:xx:xx found".
This patch allows to match DetachDeviec xml using PCI address
when mac address is not specified.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/conf/domain_conf.c   |  116 +-
 src/conf/domain_conf.h   |6 ++
 src/libxl/libxl_driver.c |4 +-
 src/lxc/lxc_driver.c |6 +-
 src/qemu/qemu_driver.c   |   40 ++--
 src/qemu/qemu_hotplug.c  |   16 --
 src/qemu/qemu_hotplug.h  |3 +
 7 files changed, 132 insertions(+), 59 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 568c699..f07f178 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13613,14 +13613,20 @@ int virDomainNetInsert(virDomainDefPtr def, 
virDomainNetDefPtr net)
 return 0;
 }
 
-/* virDomainNetFindIdx: search according to mac address and guest side
- *  PCI address (if specified)
+/* virDomainNetFindIdx:
+ * search according to mac address and guest side PCI address (if specified).
+ *
+ * @def: pointer to domain definition
+ * @net: pointer to network definition that has to be looked up for
+ * @MACAddrNotSpecified: when detaching a device it takes "true" when mac 
address is
+ *   not specified in device xml and a random mac is 
generated which
+ *   cannot be found in the domain def.
  *
  * Return: index of match if unique match found
  * -1 otherwise and an error is logged
  */
 int
-virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net)
+virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net, bool 
MACAddrNotSpecified)
 {
 size_t i;
 int matchidx = -1;
@@ -13628,50 +13634,76 @@ virDomainNetFindIdx(virDomainDefPtr def, 
virDomainNetDefPtr net)
 bool PCIAddrSpecified = virDomainDeviceAddressIsValid(>info,
   
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI);
 
-for (i = 0; i < def->nnets; i++) {
-if (virMacAddrCmp(>nets[i]->mac, >mac))
-continue;
-
-if ((matchidx >= 0) && !PCIAddrSpecified) {
-/* there were multiple matches on mac address, and no
- * qualifying guest-side PCI address was given, so we must
- * fail (NB: a USB address isn't adequate, since it may
- * specify only vendor and product ID, and there may be
- * multiples of those.
- */
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("multiple devices matching mac address %s found"),
-   virMacAddrFormat(>mac, mac));
-return -1;
-}
+/* MAC address is not specified in device xml. So, we need not check for 
it.
+ * Here we check for matching PCI address if specified. */
+if (MACAddrNotSpecified) {
 if (PCIAddrSpecified) {
-if (virPCIDeviceAddressEqual(>nets[i]->info.addr.pci,
- >info.addr.pci)) {
-/* exit early if the pci address was specified and
- * it matches, as this guarantees no duplicates.
+for (i = 0; i < def->nnets; i++) {
+if (virPCIDeviceAddressEqual(>nets[i]->info.addr.pci,
+ >info.addr.pci)) {
+matchidx = i;
+break;
+}
+}
+if (matchidx < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("no device found on "
+   "%.4x:%.2x:%.2x.%.1x"),
+   net->info.addr.pci.domain,
+   net->info.addr.pci.bus,
+   net->info.addr.pci.slot,
+   net->info.addr.pci.function);
+}
+} else {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("mac address and pci address not specified in 
device xml"));
+}
+} else {
+for (i = 0; i < def->nnets; i++) {
+if (virMacAddrCmp(>nets[i]->mac, >mac))
+continue;
+
+if ((matchidx >= 0) && !PCIAddrSpecified) {
+/* there were multiple matches on mac address, and no
+ * qualifying guest-side PCI address was given, so we must
+ * fail (NB: a USB address isn't adequate, since it may
+

[libvirt] [PATCH] conf: Skip generating random MAC in DetachDevice xml

2016-04-18 Thread Kothapally Madhu Pavan
When we try to detach a network device without specifying
the mac address, random mac address is generated. As the
generated mac address will not be available in the running
vm, detaching device will fail erroring out "error:
operation failed: no device matching mac address
xx:xx:xx:xx:xx:xx found".

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/conf/domain_conf.c   |4 
 src/conf/domain_conf.h   |3 +++
 src/libxl/libxl_driver.c |   12 ++--
 src/lxc/lxc_driver.c |7 ---
 src/qemu/qemu_driver.c   |2 ++
 src/uml/uml_driver.c |6 --
 src/vbox/vbox_common.c   |6 --
 src/vz/vz_driver.c   |4 +++-
 src/xen/xend_internal.c  |6 --
 src/xen/xm_internal.c|8 
 10 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 28248c8..512d877 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8784,6 +8784,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
(const char *)macaddr);
 goto error;
 }
+} else if (flags & VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("mac address not specified in the device xml"));
+goto error;
 } else {
 virDomainNetGenerateMAC(xmlopt, >mac);
 }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1986f53..74692f1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2679,6 +2679,9 @@ typedef enum {
 VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS = 1 << 8,
 /* allow updates in post parse callback that would break ABI otherwise */
 VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 9,
+/* don't generate random mac address when a network device without mac 
address
+ * is detached */
+VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR = 1 << 10,
 } virDomainDefParseFlags;
 
 typedef enum {
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index bf97c9c..507edcf 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3726,6 +3726,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char 
*xml,
 virDomainDefPtr vmdef = NULL;
 virDomainDeviceDefPtr dev = NULL;
 int ret = -1;
+unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+   VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
 
 virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE |
   VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
@@ -3743,9 +3745,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char 
*xml,
 goto endjob;
 
 if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) {
-if (!(dev = virDomainDeviceDefParse(xml, vm->def,
-cfg->caps, driver->xmlopt,
-VIR_DOMAIN_DEF_PARSE_INACTIVE)))
+if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps,
+driver->xmlopt, parse_flags)))
 goto endjob;
 
 /* Make a copy for updated domain. */
@@ -3760,9 +3761,8 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char 
*xml,
 if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
 /* If dev exists it was created to modify the domain config. Free it. 
*/
 virDomainDeviceDefFree(dev);
-if (!(dev = virDomainDeviceDefParse(xml, vm->def,
-cfg->caps, driver->xmlopt,
-VIR_DOMAIN_DEF_PARSE_INACTIVE)))
+if (!(dev = virDomainDeviceDefParse(xml, vm->def, cfg->caps,
+driver->xmlopt, parse_flags)))
 goto endjob;
 
 if (libxlDomainDetachDeviceLive(driver, vm, dev) < 0)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index ef48812..23f0d80 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5196,6 +5196,8 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
 virDomainDefPtr vmdef = NULL;
 virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
 int ret = -1;
+unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+   VIR_DOMAIN_DEF_PARSE_SKIP_GENERATE_MACADDR;
 virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
@@ -5213,9 +5215,8 @@ static int lxcDomainDetachDeviceFlags(virDomainPtr dom,
 if (!(caps = virLXCDriverGetCapabilities(driver, false)))
 goto cleanup;
 
-dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
- caps, driver->xmlopt,
- VIR_DOMAIN_DEF_PARSE_INACTIVE);
+dev = dev_copy = virDomainDeviceDefParse(xm

[libvirt] [PATCH] qemu: ignore EACCES error on hot unplug of PCI devices with VFIO

2016-03-07 Thread Kothapally Madhu Pavan
When the hostdev managed=true, the VFIO devices are reattached
to the host driver before coming to qemuTeardownHostdevCgroup().
So, the /dev/vfio/ path would not exist. We ignore the
EACCES error here.

Signed-off-by: Kothapally Madhu Pavan <k...@linux.vnet.ibm.com>
---
 src/qemu/qemu_cgroup.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index c76d585..2850346 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -427,7 +427,7 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
 
 VIR_DEBUG("Cgroup deny %s for PCI device assignment", path);
 rv = virCgroupDenyDevicePath(priv->cgroup, path,
- VIR_CGROUP_DEVICE_RWM, false);
+ VIR_CGROUP_DEVICE_RWM, 
dev->managed);
 virDomainAuditCgroupPath(vm, priv->cgroup,
  "deny", path, "rwm", rv == 0);
 if (rv < 0)

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


Re: [libvirt] [PATCH v2 2/3] Avoid starting a PowerPC VM with floppy disk

2015-08-03 Thread madhu pavan



On 08/03/2015 06:23 PM, Ján Tomko wrote:

On Thu, Jul 30, 2015 at 07:55:36AM -0400, Kothapally Madhu Pavan wrote:

PowerPC pseries based VMs do not support a floppy disk controller.

Here the commit message mentions a controller, but the code only checks
for the disk type='floppy'/ device,
not for controller type='fdc'/

Should we forbid that one too? AFAIK we've auto-added it to the XML
only if there was at least floppy.
We need not forbid fdc, as checking for floppy device will fulfill the 
need. Yes, fdc is added only if there was at least one floppy device.




Either way, the series looks good to me.

Jan


This prohibits libvirt from creating qemu command with floppy device.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
  src/qemu/qemu_command.c |8 
  1 file changed, 8 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 09f30c4..501c7df 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9767,6 +9767,14 @@ qemuBuildCommandLine(virConnectPtr conn,
  continue;
  }
  
+/* PowerPC pseries based VMs do not support floppy device */

+if ((disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) 
+ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support floppy 
device));
+goto error;
+}
+
  switch (disk-device) {
  case VIR_DOMAIN_DISK_DEVICE_CDROM:
  bootindex = bootCD;

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

Thanks,
Madhu Pavan.

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


[libvirt] [PATCH v2 2/3] Avoid starting a PowerPC VM with floppy disk

2015-07-30 Thread Kothapally Madhu Pavan
PowerPC pseries based VMs do not support a floppy disk controller.
This prohibits libvirt from creating qemu command with floppy device.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
 src/qemu/qemu_command.c |8 
 1 file changed, 8 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 09f30c4..501c7df 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9767,6 +9767,14 @@ qemuBuildCommandLine(virConnectPtr conn,
 continue;
 }
 
+/* PowerPC pseries based VMs do not support floppy device */
+if ((disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) 
+ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}
+
 switch (disk-device) {
 case VIR_DOMAIN_DISK_DEVICE_CDROM:
 bootindex = bootCD;

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


[libvirt] [PATCH v2 1/3] Caps: Disable floppy disk for PowerPC VM

2015-07-30 Thread Kothapally Madhu Pavan
PowerPC pseries based VMs do not support a floppy disk controller.
This prohibits libvirt from adding floppy disk for a PowerPC pseries VM.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
 src/qemu/qemu_capabilities.c |   15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d8cb32d..e304473 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3919,25 +3919,32 @@ virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps,
 
 static int
 virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps,
+const char *machine,
 virDomainCapsDeviceDiskPtr disk)
 {
 disk-device.supported = true;
 /* QEMU supports all of these */
 VIR_DOMAIN_CAPS_ENUM_SET(disk-diskDevice,
  VIR_DOMAIN_DISK_DEVICE_DISK,
- VIR_DOMAIN_DISK_DEVICE_CDROM,
- VIR_DOMAIN_DISK_DEVICE_FLOPPY);
+ VIR_DOMAIN_DISK_DEVICE_CDROM);
+
+/* PowerPC pseries based VMs do not support floppy device */
+if (!(ARCH_IS_PPC64(qemuCaps-arch)  STRPREFIX(machine, pseries)))
+VIR_DOMAIN_CAPS_ENUM_SET(disk-diskDevice, 
VIR_DOMAIN_DISK_DEVICE_FLOPPY);
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO))
 VIR_DOMAIN_CAPS_ENUM_SET(disk-diskDevice, VIR_DOMAIN_DISK_DEVICE_LUN);
 
 VIR_DOMAIN_CAPS_ENUM_SET(disk-bus,
  VIR_DOMAIN_DISK_BUS_IDE,
- VIR_DOMAIN_DISK_BUS_FDC,
  VIR_DOMAIN_DISK_BUS_SCSI,
  VIR_DOMAIN_DISK_BUS_VIRTIO,
  /* VIR_DOMAIN_DISK_BUS_SD */);
 
+/* PowerPC pseries based VMs do not support floppy device */
+if (!(ARCH_IS_PPC64(qemuCaps-arch)  STRPREFIX(machine, pseries)))
+VIR_DOMAIN_CAPS_ENUM_SET(disk-bus, VIR_DOMAIN_DISK_BUS_FDC);
+
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE))
 VIR_DOMAIN_CAPS_ENUM_SET(disk-bus, VIR_DOMAIN_DISK_BUS_USB);
 return 0;
@@ -4008,7 +4015,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
 
 if (virQEMUCapsFillDomainOSCaps(qemuCaps, os,
 loader, nloader)  0 ||
-virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk)  0 ||
+virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, domCaps-machine, disk) 
 0 ||
 virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev)  0)
 return -1;
 return 0;

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


[libvirt] [PATCH v2 3/3] test: Introduce test case to disallow floppy disk on PowerPC VM

2015-07-30 Thread Kothapally Madhu Pavan
PowerPC pseries based VMs do not support a floppy disk controller.
This test case fails if libvirt allows a floppy disk on pseries VMs.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
 .../qemuxml2argv-disk-floppy-pseries.args  |6 +++
 .../qemuxml2argv-disk-floppy-pseries.xml   |   41 
 tests/qemuxml2argvtest.c   |1 
 3 files changed, 48 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args
new file mode 100644
index 000..6f68121
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args
@@ -0,0 +1,6 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-ppc64 -S -M pseries -m 214 -smp 1 -nographic -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \
+file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \
+file=/dev/fd0,if=floppy,unit=0 -drive file=/tmp/firmware.img,if=floppy,unit=1 \
+-net none -serial none -parallel none
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml
new file mode 100644
index 000..be0ede6
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml
@@ -0,0 +1,41 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219136/memory
+  currentMemory unit='KiB'219136/currentMemory
+  vcpu placement='static'1/vcpu
+  os
+type arch='ppc64' machine='pseries'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu-system-ppc64/emulator
+disk type='block' device='disk'
+  driver name='qemu' type='raw'/
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+disk type='block' device='floppy'
+  driver name='qemu' type='raw'/
+  source dev='/dev/fd0'/
+  target dev='fda' bus='fdc'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+disk type='file' device='floppy'
+  driver name='qemu' type='raw'/
+  source file='/tmp/firmware.img'/
+  target dev='fdb' bus='fdc'/
+  address type='drive' controller='0' bus='0' target='0' unit='1'/
+/disk
+controller type='usb' index='0'/
+controller type='fdc' index='0'/
+controller type='ide' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='none'/
+  /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f9b30d9..afd2ef3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -737,6 +737,7 @@ mymain(void)
 QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_VIRTIO_TX_ALG);
 DO_TEST(disk-cdrom-tray-no-device-cap, NONE);
 DO_TEST(disk-floppy, NONE);
+DO_TEST_FAILURE(disk-floppy-pseries, QEMU_CAPS_DRIVE);
 DO_TEST(disk-floppy-tray-no-device-cap, NONE);
 DO_TEST(disk-floppy-tray,
 QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE);

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


Re: [libvirt] [PATCH 2/2] Avoid starting a PowerPC VM with floppy disk

2015-07-30 Thread madhu pavan



On 07/28/2015 05:32 PM, Ján Tomko wrote:

On Fri, Jul 24, 2015 at 03:30:49PM -0400, Kothapally Madhu Pavan wrote:

PowerPC pseries based VMs do not support a floppy disk controller.
This prohibits libvirt from creating qemu command with floppy device.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
  src/qemu/qemu_command.c |   47 +--

Extending the qemuxml2argvtest with a test case that fails on such
config would be nice.


  1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 42906a8..93f84e2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9486,6 +9486,12 @@ qemuBuildCommandLine(virConnectPtr conn,
  boot[i] = 'd';
  break;
  case VIR_DOMAIN_BOOT_FLOPPY:
+/* PowerPC pseries based VMs do not support floppy device */
+if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}

There is no need to error out earlier for machines that do not use
deviceboot. This error can be dropped, we will hit the next one.


  boot[i] = 'a';
  break;
  case VIR_DOMAIN_BOOT_DISK:
@@ -9769,6 +9775,12 @@ qemuBuildCommandLine(virConnectPtr conn,
  bootCD = 0;
  break;
  case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+/* PowerPC pseries based VMs do not support floppy device */
+if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}

This is more appropriate place for the error message. Personally, I would move 
it
above the switch() and let the switch only deal with boot indexes.


  bootindex = bootFloppy;
  bootFloppy = 0;
  break;
@@ -9812,6 +9824,12 @@ qemuBuildCommandLine(virConnectPtr conn,
  
  if (withDeviceArg) {

  if (disk-bus == VIR_DOMAIN_DISK_BUS_FDC) {
+/* PowerPC pseries based VMs do not support floppy device 
*/
+if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}

This is dead code, the condition will never be true here, we already
matched the error above.


  if (virAsprintf(optstr, drive%c=drive-%s,
  disk-info.addr.drive.unit ? 'B' : 'A',
  disk-info.alias)  0)
@@ -9854,6 +9872,12 @@ qemuBuildCommandLine(virConnectPtr conn,
  /* Newer Q35 machine types require an explicit FDC controller */
  virBufferTrim(fdc_opts, ,, -1);
  if ((fdc_opts_str = virBufferContentAndReset(fdc_opts))) {
+/* PowerPC pseries based VMs do not support floppy device */
+if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support floppy 
device));
+goto error;
+}

Same here.


  virCommandAddArg(cmd, -device);
  virCommandAddArgFormat(cmd, isa-fdc,%s, fdc_opts_str);
  VIR_FREE(fdc_opts_str);
@@ -9918,10 +9942,17 @@ qemuBuildCommandLine(virConnectPtr conn,
 _(cannot create virtual FAT disks in 
read-write mode));
  goto error;
  }
-if (disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+if (disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+/* PowerPC pseries based VMs do not support floppy device 
*/
+if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}
  fmt = fat:floppy:%s;
-else
+} else {
  fmt = fat:%s;
+}
  

This code path can only be taken for QEMUs not having QEMU_CAPS_DRIVE,
i.e. older than at least 7 years. I do not think touching this part of
code is worth

Re: [libvirt] [PATCH 0/2] Disable floppy disk for PowerPC VMs

2015-07-30 Thread madhu pavan



On 07/29/2015 06:25 PM, Andrea Bolognani wrote:

On Fri, 2015-07-24 at 15:29 -0400, Kothapally Madhu Pavan wrote:

This is an attempt to fix:
Libvirt BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1180486

Libvirt currently assumes ISA_based floppy disks to be available
across all
architectures and machine types. However, PowerPC Book 3S compatible
('ie pseries)
virtual machines do not support Floppy disks.

This patch series prevents libvirt from launching ppc64[le] -based
'pseries'
VMs with floppy devices.

Hi,

sorry for not replying right away.

I started looking into that bug a while ago but I got sidetracked
shortly afterwards, so I don't have much to show for it. I'd like
to share my opinion on the matter anyway.

I believe you're basically following the right approach, eg. avoid
setting floppy-related capabilities and erroring out afterwards if
attempts are made to use devices that require such capabilites.

However, I think the implementation should be a little more
generic: ideally, the code would contain no references to the
ppc64 architecture and whether or not floppy disks are be allowed
would be determined by probing the QEMU binary.

Writing the code this way would allow us to handle automatically
other architectures where floppy disks do not make sense[1] and
situations where floppy support is not available[2].

If that turns out not to be possible or practical, at the very
least the check should be performed in one single spot, and not
replicated a dozen times thorough the library.So,
As, present method of capabilities parsing uses qemu -h, and the list 
received in this case is arch agnostic. So, we cannot do arch-specific 
caps parsing unless qemu undergoes a rewrite. I will be providing a new 
patch set with as minimal changes as possible.


Thanks,
Madhu Pavan.

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


[libvirt] [PATCH v2 0/3] Disable floppy disk for PowerPC VMs

2015-07-30 Thread Kothapally Madhu Pavan
This is an attempt to fix:
Libvirt BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1180486

Libvirt currently assumes ISA_based floppy disks to be available across all
architectures and machine types. However, PowerPC Book 3S compatible ('ie 
pseries)
virtual machines do not support Floppy disks.

This patch series prevents libvirt from launching ppc64[le] -based 'pseries'
VMs with floppy devices.

This version of patch attempts to fulfill the coments of Andrea Bolognani
and Jan Tomko on previous version of patch.
---

Kothapally Madhu Pavan (3):
  Caps: Disable floppy disk for PowerPC VM
  Avoid starting a PowerPC VM with floppy disk
  test: Introduce test case to disallow floppy disk on pseries


 src/qemu/qemu_capabilities.c   |   15 +--
 src/qemu/qemu_command.c|8 
 .../qemuxml2argv-disk-floppy-pseries.args  |6 +++
 .../qemuxml2argv-disk-floppy-pseries.xml   |   41 
 tests/qemuxml2argvtest.c   |1 
 5 files changed, 67 insertions(+), 4 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-floppy-pseries.xml

--
Kothapally Madhu Pavan

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


[libvirt] [PATCH 0/2] Disable floppy disk for PowerPC VMs

2015-07-24 Thread Kothapally Madhu Pavan
This is an attempt to fix:
Libvirt BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1180486

Libvirt currently assumes ISA_based floppy disks to be available across all
architectures and machine types. However, PowerPC Book 3S compatible ('ie 
pseries)
virtual machines do not support Floppy disks.

This patch series prevents libvirt from launching ppc64[le] -based 'pseries'
VMs with floppy devices.

---

Kothapally Madhu Pavan (2):
  Caps: Disable floppy disk for PowerPC Vm
  Avoid starting a PowerPC VM with floppy disk


 src/conf/domain_conf.c   |   19 +
 src/qemu/qemu_capabilities.c |   15 ++---
 src/qemu/qemu_command.c  |   47 --
 3 files changed, 71 insertions(+), 10 deletions(-)

--

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


[libvirt] [PATCH 2/2] Avoid starting a PowerPC VM with floppy disk

2015-07-24 Thread Kothapally Madhu Pavan
PowerPC pseries based VMs do not support a floppy disk controller.
This prohibits libvirt from creating qemu command with floppy device.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
 src/qemu/qemu_command.c |   47 +--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 42906a8..93f84e2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9486,6 +9486,12 @@ qemuBuildCommandLine(virConnectPtr conn,
 boot[i] = 'd';
 break;
 case VIR_DOMAIN_BOOT_FLOPPY:
+/* PowerPC pseries based VMs do not support floppy device */
+if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}
 boot[i] = 'a';
 break;
 case VIR_DOMAIN_BOOT_DISK:
@@ -9769,6 +9775,12 @@ qemuBuildCommandLine(virConnectPtr conn,
 bootCD = 0;
 break;
 case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
+/* PowerPC pseries based VMs do not support floppy device */
+if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}
 bootindex = bootFloppy;
 bootFloppy = 0;
 break;
@@ -9812,6 +9824,12 @@ qemuBuildCommandLine(virConnectPtr conn,
 
 if (withDeviceArg) {
 if (disk-bus == VIR_DOMAIN_DISK_BUS_FDC) {
+/* PowerPC pseries based VMs do not support floppy device 
*/
+if (ARCH_IS_PPC64(def-os.arch)  
STRPREFIX(def-os.machine, pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not 
support floppy device));
+goto error;
+}
 if (virAsprintf(optstr, drive%c=drive-%s,
 disk-info.addr.drive.unit ? 'B' : 'A',
 disk-info.alias)  0)
@@ -9854,6 +9872,12 @@ qemuBuildCommandLine(virConnectPtr conn,
 /* Newer Q35 machine types require an explicit FDC controller */
 virBufferTrim(fdc_opts, ,, -1);
 if ((fdc_opts_str = virBufferContentAndReset(fdc_opts))) {
+/* PowerPC pseries based VMs do not support floppy device */
+if (ARCH_IS_PPC64(def-os.arch)  STRPREFIX(def-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}
 virCommandAddArg(cmd, -device);
 virCommandAddArgFormat(cmd, isa-fdc,%s, fdc_opts_str);
 VIR_FREE(fdc_opts_str);
@@ -9918,10 +9942,17 @@ qemuBuildCommandLine(virConnectPtr conn,
_(cannot create virtual FAT disks in 
read-write mode));
 goto error;
 }
-if (disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+if (disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+/* PowerPC pseries based VMs do not support floppy device 
*/
+if (ARCH_IS_PPC64(def-os.arch)  
STRPREFIX(def-os.machine, pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not 
support floppy device));
+goto error;
+}
 fmt = fat:floppy:%s;
-else
+} else {
 fmt = fat:%s;
+}
 
 if (virAsprintf(file, fmt, disk-src)  0)
 goto error;
@@ -11674,6 +11705,12 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
 def-device = VIR_DOMAIN_DISK_DEVICE_CDROM;
 def-src-readonly = true;
 } else if (STREQ(values[i], floppy)) {
+/* PowerPC pseries based VMs do not support floppy device */
+if (ARCH_IS_PPC64(dom-os.arch)  STRPREFIX(dom-os.machine, 
pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}
 def-device = VIR_DOMAIN_DISK_DEVICE_FLOPPY;
 }
 } else

[libvirt] [PATCH 1/2] Caps: Disable floppy disk for PowerPC VM

2015-07-24 Thread Kothapally Madhu Pavan
PowerPC pseries based VMs do not support a floppy disk controller.
This prohibits libvirt from adding floppy disk for a PowerPC pseries VM.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
 src/conf/domain_conf.c   |   19 +++
 src/qemu/qemu_capabilities.c |   15 +++
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 73ac537..b9f35b4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6540,7 +6540,9 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
  virHashTablePtr bootHash,
  virSecurityLabelDefPtr* vmSeclabels,
  int nvmSeclabels,
- unsigned int flags)
+ unsigned int flags,
+ virArch arch,
+ const char *machine)
 {
 virDomainDiskDefPtr def;
 xmlNodePtr sourceNode = NULL;
@@ -7165,6 +7167,12 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 } else {
 if (def-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
+/* PowerPC pseries based VMs do not support floppy device */
+if (ARCH_IS_PPC64(arch)  STRPREFIX(machine, pseries)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+  _(PowerPC pseries machines do not support 
floppy device));
+goto error;
+}
 def-bus = VIR_DOMAIN_DISK_BUS_FDC;
 } else if (!(flags  VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) {
 if (STRPREFIX(target, hd))
@@ -12375,7 +12383,8 @@ virDomainDeviceDefParse(const char *xmlStr,
 if (!(dev-data.disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
 NULL, def-seclabels,
 def-nseclabels,
-flags)))
+flags, def-os.arch,
+def-os.machine)))
 goto error;
 break;
 case VIR_DOMAIN_DEVICE_LEASE:
@@ -12519,7 +12528,8 @@ virDomainDiskDefSourceParse(const char *xmlStr,
 if (!(disk = virDomainDiskDefParseXML(xmlopt, node, ctxt,
   NULL, def-seclabels,
   def-nseclabels,
-  flags)))
+  flags, def-os.arch,
+  def-os.machine)))
 goto cleanup;
 
 ret = disk-src;
@@ -15539,7 +15549,8 @@ virDomainDefParseXML(xmlDocPtr xml,
 bootHash,
 def-seclabels,
 def-nseclabels,
-flags);
+flags, 
def-os.arch,
+def-os.machine);
 if (!disk)
 goto error;
 
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d8cb32d..e304473 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3919,25 +3919,32 @@ virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps,
 
 static int
 virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps,
+const char *machine,
 virDomainCapsDeviceDiskPtr disk)
 {
 disk-device.supported = true;
 /* QEMU supports all of these */
 VIR_DOMAIN_CAPS_ENUM_SET(disk-diskDevice,
  VIR_DOMAIN_DISK_DEVICE_DISK,
- VIR_DOMAIN_DISK_DEVICE_CDROM,
- VIR_DOMAIN_DISK_DEVICE_FLOPPY);
+ VIR_DOMAIN_DISK_DEVICE_CDROM);
+
+/* PowerPC pseries based VMs do not support floppy device */
+if (!(ARCH_IS_PPC64(qemuCaps-arch)  STRPREFIX(machine, pseries)))
+VIR_DOMAIN_CAPS_ENUM_SET(disk-diskDevice, 
VIR_DOMAIN_DISK_DEVICE_FLOPPY);
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO))
 VIR_DOMAIN_CAPS_ENUM_SET(disk-diskDevice, VIR_DOMAIN_DISK_DEVICE_LUN);
 
 VIR_DOMAIN_CAPS_ENUM_SET(disk-bus,
  VIR_DOMAIN_DISK_BUS_IDE,
- VIR_DOMAIN_DISK_BUS_FDC,
  VIR_DOMAIN_DISK_BUS_SCSI,
  VIR_DOMAIN_DISK_BUS_VIRTIO,
  /* VIR_DOMAIN_DISK_BUS_SD */);
 
+/* PowerPC pseries based VMs do not support floppy device */
+if (!(ARCH_IS_PPC64(qemuCaps-arch)  STRPREFIX(machine, pseries)))
+VIR_DOMAIN_CAPS_ENUM_SET(disk-bus

[libvirt] [PATCH v3] nodeinfo: fix to parse present cpus rather than possible cpus

2015-06-26 Thread Kothapally Madhu Pavan
Currently we are parsing all the possible cpus to get the
nodeinfo. This fix will perform a check for present cpus
before parsing.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com


---
 src/nodeinfo.c |   11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2fafe2d..5689c9b 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -43,6 +43,7 @@
 #include c-ctype.h
 #include viralloc.h
 #include nodeinfopriv.h
+#include nodeinfo.h
 #include physmem.h
 #include virerror.h
 #include count-one-bits.h
@@ -418,6 +419,7 @@ virNodeParseNode(const char *node,
 int processors = 0;
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
+virBitmapPtr present_cpumap = NULL;
 int sock_max = 0;
 cpu_set_t sock_map;
 int sock;
@@ -438,12 +440,17 @@ virNodeParseNode(const char *node,
 goto cleanup;
 }
 
+present_cpumap = nodeGetPresentCPUBitmap();
+
 /* enumerate sockets in the node */
 CPU_ZERO(sock_map);
 while ((direrr = virDirRead(cpudir, cpudirent, node))  0) {
 if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1)
 continue;
 
+if (present_cpumap  !(virBitmapIsSet(present_cpumap, cpu)))
+continue;
+
 if ((online = virNodeGetCpuValue(node, cpu, online, 1))  0)
 goto cleanup;
 
@@ -477,6 +484,9 @@ virNodeParseNode(const char *node,
 if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1)
 continue;
 
+if (present_cpumap  !(virBitmapIsSet(present_cpumap, cpu)))
+continue;
+
 if ((online = virNodeGetCpuValue(node, cpu, online, 1))  0)
 goto cleanup;
 
@@ -537,6 +547,7 @@ virNodeParseNode(const char *node,
 ret = -1;
 }
 VIR_FREE(core_maps);
+virBitmapFree(present_cpumap);
 
 return ret;
 }

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


[libvirt] [PATCH v2] nodeinfo: fix to parse present cpus rather than possible cpus

2015-06-16 Thread Kothapally Madhu Pavan
Currently we are parsing all the possible cpus to get the
nodeinfo. This fix will perform a check for present cpus
before parsing.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
 src/nodeinfo.c |   13 +
 1 file changed, 13 insertions(+)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2fafe2d..0134aba 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -57,6 +57,7 @@
 #define VIR_FROM_THIS VIR_FROM_NONE
 
 VIR_LOG_INIT(nodeinfo);
+virBitmapPtr nodeGetPresentCPUBitmap(void);
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 static int
@@ -418,6 +419,7 @@ virNodeParseNode(const char *node,
 int processors = 0;
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
+virBitmapPtr present_cpumap = NULL;
 int sock_max = 0;
 cpu_set_t sock_map;
 int sock;
@@ -438,12 +440,18 @@ virNodeParseNode(const char *node,
 goto cleanup;
 }
 
+present_cpumap = nodeGetPresentCPUBitmap();
+
 /* enumerate sockets in the node */
 CPU_ZERO(sock_map);
 while ((direrr = virDirRead(cpudir, cpudirent, node))  0) {
 if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1)
 continue;
 
+if (present_cpumap)
+if (!(virBitmapIsBitSet(present_cpumap, cpu)))
+continue;
+
 if ((online = virNodeGetCpuValue(node, cpu, online, 1))  0)
 goto cleanup;
 
@@ -477,6 +485,10 @@ virNodeParseNode(const char *node,
 if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1)
 continue;
 
+if (present_cpumap)
+if (!(virBitmapIsBitSet(present_cpumap, cpu)))
+continue;
+
 if ((online = virNodeGetCpuValue(node, cpu, online, 1))  0)
 goto cleanup;
 
@@ -537,6 +549,7 @@ virNodeParseNode(const char *node,
 ret = -1;
 }
 VIR_FREE(core_maps);
+virBitmapFree(present_cpumap);
 
 return ret;
 }

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


[libvirt] [PATCH] nodeinfo: fix to parse present cpus rather than possible cpus

2015-06-10 Thread Kothapally Madhu Pavan
Currently we are parsing all the possible cpus to get the
nodeinfo. This fix will perform a check for present cpus
before parsing.

Signed-off-by: Kothapally Madhu Pavan k...@linux.vnet.ibm.com
---
 src/nodeinfo.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2fafe2d..9e6684f 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -57,6 +57,7 @@
 #define VIR_FROM_THIS VIR_FROM_NONE
 
 VIR_LOG_INIT(nodeinfo);
+virBitmapPtr nodeGetPresentCPUBitmap(void);
 
 #if defined(__FreeBSD__) || defined(__APPLE__)
 static int
@@ -418,6 +419,7 @@ virNodeParseNode(const char *node,
 int processors = 0;
 DIR *cpudir = NULL;
 struct dirent *cpudirent = NULL;
+virBitmapPtr present_cpumap = NULL;
 int sock_max = 0;
 cpu_set_t sock_map;
 int sock;
@@ -438,12 +440,17 @@ virNodeParseNode(const char *node,
 goto cleanup;
 }
 
+present_cpumap = nodeGetPresentCPUBitmap();
+
 /* enumerate sockets in the node */
 CPU_ZERO(sock_map);
 while ((direrr = virDirRead(cpudir, cpudirent, node))  0) {
 if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1)
 continue;
 
+if (!(virBitmapIsBitSet(present_cpumap, cpu)))
+continue;
+
 if ((online = virNodeGetCpuValue(node, cpu, online, 1))  0)
 goto cleanup;
 
@@ -477,6 +484,9 @@ virNodeParseNode(const char *node,
 if (sscanf(cpudirent-d_name, cpu%u, cpu) != 1)
 continue;
 
+if (!(virBitmapIsBitSet(present_cpumap, cpu)))
+continue;
+
 if ((online = virNodeGetCpuValue(node, cpu, online, 1))  0)
 goto cleanup;
 

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


[libvirt] [PATCH v2] virsh: Fix to list online cpus using virsh capabilities

2015-05-26 Thread Kothapally Madhu Pavan
Virsh capabilities will list offline cpus as online when
libvirt is compiled with numactl option disabled. This
fix will list correct set of online cpus.
---
 src/nodeinfo.c |   43 +++
 1 file changed, 43 insertions(+)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 22df95c..410c9de 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1651,6 +1651,47 @@ nodeCapsInitNUMAFake(virCapsPtr caps ATTRIBUTE_UNUSED)
 if (VIR_ALLOC_N(cpus, ncpus)  0)
 return -1;
 
+#ifdef __linux__
+{
+int cid = 0;
+int onlinecpus = nodeinfo.cpus;
+
+id = 0;
+for (s = 0; s  nodeinfo.sockets; s++) {
+for (c = 0; c  nodeinfo.cores; c++) {
+for (t = 0; t  nodeinfo.threads; t++) {
+if (virNodeGetCpuValue(SYSFS_CPU_PATH, id, online, 1)) {
+cpus[cid].id = id;
+cpus[cid].socket_id = s;
+cpus[cid].core_id = c;
+if (!(cpus[cid].siblings = virBitmapNew(ncpus)))
+goto error;
+ignore_value(virBitmapSetBit(cpus[cid].siblings, id));
+cid++;
+}
+
+id++;
+}
+}
+}
+
+if (virCapabilitiesAddHostNUMACell(caps, 0,
+   nodeinfo.memory,
+   onlinecpus, cpus,
+   0, NULL,
+   0, NULL)  0)
+goto error;
+
+return 0;
+
+ error:
+for (; cid = 0; cid--)
+virBitmapFree(cpus[cid].siblings);
+VIR_FREE(cpus);
+return -1;
+}
+#else
+{
 id = 0;
 for (s = 0; s  nodeinfo.sockets; s++) {
 for (c = 0; c  nodeinfo.cores; c++) {
@@ -1680,6 +1721,8 @@ nodeCapsInitNUMAFake(virCapsPtr caps ATTRIBUTE_UNUSED)
 virBitmapFree(cpus[id].siblings);
 VIR_FREE(cpus);
 return -1;
+}
+#endif
 }
 
 static int

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


  1   2   >