[libvirt] [libvirt-test-api][PATCH] Add a new test case for setUserPassword

[Luyao Huang]

Signed-off-by: Luyao Huang 
---
 cases/linux_domain.conf |  22 
 repos/domain/set_user_passwd.py | 111 
 2 files changed, 133 insertions(+)
 create mode 100644 repos/domain/set_user_passwd.py

diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf
index 19daded..9a65cf3 100644
--- a/cases/linux_domain.conf
+++ b/cases/linux_domain.conf
@@ -299,6 +299,28 @@ domain:info_iothread
 conn
 qemu:///system
 
+domain:set_user_passwd
+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+conn
+qemu:///system
+
+domain:set_user_passwd
+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+conn
+qemu:///system
+flags
+encrypted
+
 domain:destroy
 guestname
 $defaultname
diff --git a/repos/domain/set_user_passwd.py b/repos/domain/set_user_passwd.py
new file mode 100644
index 000..bf9bd06
--- /dev/null
+++ b/repos/domain/set_user_passwd.py
@@ -0,0 +1,111 @@
+#!/usr/bin/env python
+
+import libvirt
+from libvirt import libvirtError
+import lxml
+import lxml.etree
+import crypt
+from utils import utils
+
+required_params = ('guestname', 'username', 'userpassword',)
+optional_params = {'conn': 'qemu:///system', 'flags': '',}
+
+def get_guest_mac(vm):
+tree = lxml.etree.fromstring(vm.XMLDesc(0))
+set = tree.xpath("/domain/devices/interface/mac")
+
+for n in set:
+return n.attrib['address']
+
+return False
+
+
+def check_agent_status(vm):
+""" make sure agent is okay to use """
+
+tree = lxml.etree.fromstring(vm.XMLDesc(0))
+
+set = 
tree.xpath("//channel[@type='unix']/target[@name='org.qemu.guest_agent.0']")
+for n in set:
+if n.attrib['state'] == 'connected':
+return True
+
+return False
+
+def create_new_user(ipaddr, newusername, username, userpasswd, logger):
+cmd = "useradd %s" % newusername
+ret, retinfo = utils.remote_exec_pexpect(ipaddr, username, userpasswd, cmd)
+if ret == 0 or "already exists" in retinfo:
+return 0
+else:
+logger.error("Fail: cannot create a new user: %s" % retinfo)
+return 1
+
+def verify_cur_user(ipaddr, username, userpasswd):
+cmd = "whoami"
+ret, retinfo = utils.remote_exec_pexpect(ipaddr, username, userpasswd, cmd)
+
+return ret
+
+def set_user_passwd(params):
+"""
+   test API for setUserPassword in class virDomain
+"""
+
+logger = params['logger']
+guest = params['guestname']
+username = params['username']
+userpasswd = params['userpassword']
+
+if 'flags' in params:
+if params['flags'] == 'encrypted':
+flags = libvirt.VIR_DOMAIN_PASSWORD_ENCRYPTED
+else:
+flags = 0
+else:
+flags = 0
+
+try:
+if 'conn' in params:
+conn = libvirt.open(params['conn'])
+else:
+conn = libvirt.open(optional_params['conn'])
+
+logger.info("get connection to libvirtd")
+vm = conn.lookupByName(guest)
+logger.info("test guest name: %s" % guest)
+
+if not check_agent_status(vm):
+logger.error("guest agent is not connected")
+return 1
+
+mac = get_guest_mac(vm)
+if not mac:
+logger.error("cannot get guest interface mac")
+return 1
+
+ipaddr = utils.mac_to_ip(mac, 180)
+if not ipaddr:
+logger.error("cannot get guest IP")
+return 1
+
+if flags > 0:
+passwd = crypt.crypt("123456", crypt.mksalt(crypt.METHOD_SHA512))
+else:
+passwd = "123456"
+
+
+if create_new_user(ipaddr, "usertestapi", username, userpasswd, 
logger) != 0:
+return 1
+
+vm.setUserPassword("usertestapi", passwd, flags)
+
+if verify_cur_user(ipaddr, "usertestapi", "123456") != 0:
+logger.error("cannot login guest via new user")
+return 1
+
+except libvirtError, e:
+logger.error("API error message: %s" % e.message)
+return 1
+
+return 0
-- 
1.8.3.1

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

[libvirt] [PATCH] Add a new test case for setUserPassword

[Luyao Huang]

Signed-off-by: Luyao Huang 
---
 cases/linux_domain.conf |  22 
 repos/domain/set_user_passwd.py | 111 
 2 files changed, 133 insertions(+)
 create mode 100644 repos/domain/set_user_passwd.py

diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf
index 19daded..9a65cf3 100644
--- a/cases/linux_domain.conf
+++ b/cases/linux_domain.conf
@@ -299,6 +299,28 @@ domain:info_iothread
 conn
 qemu:///system
 
+domain:set_user_passwd
+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+conn
+qemu:///system
+
+domain:set_user_passwd
+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+conn
+qemu:///system
+flags
+encrypted
+
 domain:destroy
 guestname
 $defaultname
diff --git a/repos/domain/set_user_passwd.py b/repos/domain/set_user_passwd.py
new file mode 100644
index 000..bf9bd06
--- /dev/null
+++ b/repos/domain/set_user_passwd.py
@@ -0,0 +1,111 @@
+#!/usr/bin/env python
+
+import libvirt
+from libvirt import libvirtError
+import lxml
+import lxml.etree
+import crypt
+from utils import utils
+
+required_params = ('guestname', 'username', 'userpassword',)
+optional_params = {'conn': 'qemu:///system', 'flags': '',}
+
+def get_guest_mac(vm):
+tree = lxml.etree.fromstring(vm.XMLDesc(0))
+set = tree.xpath("/domain/devices/interface/mac")
+
+for n in set:
+return n.attrib['address']
+
+return False
+
+
+def check_agent_status(vm):
+""" make sure agent is okay to use """
+
+tree = lxml.etree.fromstring(vm.XMLDesc(0))
+
+set = 
tree.xpath("//channel[@type='unix']/target[@name='org.qemu.guest_agent.0']")
+for n in set:
+if n.attrib['state'] == 'connected':
+return True
+
+return False
+
+def create_new_user(ipaddr, newusername, username, userpasswd, logger):
+cmd = "useradd %s" % newusername
+ret, retinfo = utils.remote_exec_pexpect(ipaddr, username, userpasswd, cmd)
+if ret == 0 or "already exists" in retinfo:
+return 0
+else:
+logger.error("Fail: cannot create a new user: %s" % retinfo)
+return 1
+
+def verify_cur_user(ipaddr, username, userpasswd):
+cmd = "whoami"
+ret, retinfo = utils.remote_exec_pexpect(ipaddr, username, userpasswd, cmd)
+
+return ret
+
+def set_user_passwd(params):
+"""
+   test API for setUserPassword in class virDomain
+"""
+
+logger = params['logger']
+guest = params['guestname']
+username = params['username']
+userpasswd = params['userpassword']
+
+if 'flags' in params:
+if params['flags'] == 'encrypted':
+flags = libvirt.VIR_DOMAIN_PASSWORD_ENCRYPTED
+else:
+flags = 0
+else:
+flags = 0
+
+try:
+if 'conn' in params:
+conn = libvirt.open(params['conn'])
+else:
+conn = libvirt.open(optional_params['conn'])
+
+logger.info("get connection to libvirtd")
+vm = conn.lookupByName(guest)
+logger.info("test guest name: %s" % guest)
+
+if not check_agent_status(vm):
+logger.error("guest agent is not connected")
+return 1
+
+mac = get_guest_mac(vm)
+if not mac:
+logger.error("cannot get guest interface mac")
+return 1
+
+ipaddr = utils.mac_to_ip(mac, 180)
+if not ipaddr:
+logger.error("cannot get guest IP")
+return 1
+
+if flags > 0:
+passwd = crypt.crypt("123456", crypt.mksalt(crypt.METHOD_SHA512))
+else:
+passwd = "123456"
+
+
+if create_new_user(ipaddr, "usertestapi", username, userpasswd, 
logger) != 0:
+return 1
+
+vm.setUserPassword("usertestapi", passwd, flags)
+
+if verify_cur_user(ipaddr, "usertestapi", "123456") != 0:
+logger.error("cannot login guest via new user")
+return 1
+
+except libvirtError, e:
+logger.error("API error message: %s" % e.message)
+return 1
+
+return 0
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] Add a new test case for setUserPassword

[Luyao Huang]

Please ignore this one, i have forgot set the header.

- Original Message -
From: "Luyao Huang" 
To: libvir-list@redhat.com
Cc: "Luyao Huang" 
Sent: Monday, September 21, 2015 3:27:02 PM
Subject: [libvirt] [PATCH] Add a new test case for setUserPassword

Signed-off-by: Luyao Huang 
---
 cases/linux_domain.conf |  22 
 repos/domain/set_user_passwd.py | 111 
 2 files changed, 133 insertions(+)
 create mode 100644 repos/domain/set_user_passwd.py

diff --git a/cases/linux_domain.conf b/cases/linux_domain.conf
index 19daded..9a65cf3 100644
--- a/cases/linux_domain.conf
+++ b/cases/linux_domain.conf
@@ -299,6 +299,28 @@ domain:info_iothread
 conn
 qemu:///system
 
+domain:set_user_passwd
+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+conn
+qemu:///system
+
+domain:set_user_passwd
+guestname
+$defaultname
+username
+$username
+userpassword
+$password
+conn
+qemu:///system
+flags
+encrypted
+
 domain:destroy
 guestname
 $defaultname
diff --git a/repos/domain/set_user_passwd.py b/repos/domain/set_user_passwd.py
new file mode 100644
index 000..bf9bd06
--- /dev/null
+++ b/repos/domain/set_user_passwd.py
@@ -0,0 +1,111 @@
+#!/usr/bin/env python
+
+import libvirt
+from libvirt import libvirtError
+import lxml
+import lxml.etree
+import crypt
+from utils import utils
+
+required_params = ('guestname', 'username', 'userpassword',)
+optional_params = {'conn': 'qemu:///system', 'flags': '',}
+
+def get_guest_mac(vm):
+tree = lxml.etree.fromstring(vm.XMLDesc(0))
+set = tree.xpath("/domain/devices/interface/mac")
+
+for n in set:
+return n.attrib['address']
+
+return False
+
+
+def check_agent_status(vm):
+""" make sure agent is okay to use """
+
+tree = lxml.etree.fromstring(vm.XMLDesc(0))
+
+set = 
tree.xpath("//channel[@type='unix']/target[@name='org.qemu.guest_agent.0']")
+for n in set:
+if n.attrib['state'] == 'connected':
+return True
+
+return False
+
+def create_new_user(ipaddr, newusername, username, userpasswd, logger):
+cmd = "useradd %s" % newusername
+ret, retinfo = utils.remote_exec_pexpect(ipaddr, username, userpasswd, cmd)
+if ret == 0 or "already exists" in retinfo:
+return 0
+else:
+logger.error("Fail: cannot create a new user: %s" % retinfo)
+return 1
+
+def verify_cur_user(ipaddr, username, userpasswd):
+cmd = "whoami"
+ret, retinfo = utils.remote_exec_pexpect(ipaddr, username, userpasswd, cmd)
+
+return ret
+
+def set_user_passwd(params):
+"""
+   test API for setUserPassword in class virDomain
+"""
+
+logger = params['logger']
+guest = params['guestname']
+username = params['username']
+userpasswd = params['userpassword']
+
+if 'flags' in params:
+if params['flags'] == 'encrypted':
+flags = libvirt.VIR_DOMAIN_PASSWORD_ENCRYPTED
+else:
+flags = 0
+else:
+flags = 0
+
+try:
+if 'conn' in params:
+conn = libvirt.open(params['conn'])
+else:
+conn = libvirt.open(optional_params['conn'])
+
+logger.info("get connection to libvirtd")
+vm = conn.lookupByName(guest)
+logger.info("test guest name: %s" % guest)
+
+if not check_agent_status(vm):
+logger.error("guest agent is not connected")
+return 1
+
+mac = get_guest_mac(vm)
+if not mac:
+logger.error("cannot get guest interface mac")
+return 1
+
+ipaddr = utils.mac_to_ip(mac, 180)
+if not ipaddr:
+logger.error("cannot get guest IP")
+return 1
+
+if flags > 0:
+passwd = crypt.crypt("123456", crypt.mksalt(crypt.METHOD_SHA512))
+else:
+passwd = "123456"
+
+
+if create_new_user(ipaddr, "usertestapi", username, userpasswd, 
logger) != 0:
+return 1
+
+vm.setUserPassword("usertestapi", passwd, flags)
+
+if verify_cur_user(ipaddr, "usertestapi", "123456") != 0:
+logger.error("cannot login guest via new user")
+return 1
+
+except libvirtError, e:
+logger.error("API error message: %s" % e.message)
+return 1
+
+return 0
-- 
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

Re: [libvirt] crash in virDomainNumaGetMemorySize

[Peter Krempa]

On Fri, Sep 18, 2015 at 17:10:20 +0200, Olaf Hering wrote:
> With current master (56945e1), while toying around with a WS2008R2 Hyper-V 
> host:
> 
> root@probook:~ # gdb --quiet -ex 'r -c hv dumpxml all-sles12-dev' -ex bt -ex 
> detach -ex quit virsh
> Reading symbols from virsh...Reading symbols from 
> /usr/lib/debug/usr/bin/virsh.debug...done.
> done.
> Starting program: /usr/bin/virsh -c hv dumpxml all-sles12-dev
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> Enter Administrator's password for optiplex.fritz.box:
> [New Thread 0x7fffee601700 (LWP 26921)]
> 
> Program received signal SIGSEGV, Segmentation fault.
> virDomainNumaGetMemorySize (numa=0x0) at conf/numa_conf.c:971
> 971 for (i = 0; i < numa->nmem_nodes; i++)
> #0  virDomainNumaGetMemorySize (numa=0x0) at conf/numa_conf.c:971
> #1  0x7793bce0 in virDomainDefGetMemoryActual 
> (def=def@entry=0x5582ead0) at conf/domain_conf.c:7857
> #2  0x77948978 in virDomainDefFormatInternal (def=0x5582ead0, 
> flags=0, buf=buf@entry=0x7fffdde0) at conf/domain_conf.c:21677
> #3  0x7794b8bc in virDomainDefFormat (def=, 
> flags=) at conf/domain_conf.c:22507
> #4  0x77a94c33 in hypervDomainGetXMLDesc (domain=, 
> flags=0) at hyperv/hyperv_driver.c:882
> #5  0x779b5351 in virDomainGetXMLDesc 
> (domain=domain@entry=0x558237a0, flags=0) at libvirt-domain.c:2591
> #6  0x5558b978 in cmdDumpXML (ctl=0x7fffe1c0, cmd= out>) at virsh-domain.c:9634
> #7  0x5557d04f in vshCommandRun (ctl=0x7fffe1c0, 
> cmd=0x55822580) at vsh.c:1212
> #8  0x55579a4d in main (argc=5, argv=0x7fffe3c8) at virsh.c:921
> Detaching from program: /usr/bin/virsh, process 26917
> root@probook:~ # rpm -qf `which virsh`
> libvirt-client-20150917T085913.56945e1-3.xen_unstable.1.x86_64

Hmm, looks like hyperv isn't initializing the "def->numa" field, but
virDomainNumaGetMemorySize is expecting it. I actually have a patchset
almost ready that should fix this along with other things.

I hope to send the patches soon.

Peter


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

Re: [libvirt] [PATCH] vz: remove error logging from prlsdkUUIDParse

[Dmitry Guryanov]

On 09/21/2015 02:08 PM, Maxim Nestratov wrote:

From: Maxim Nestratov 

As far as not every call of prlsdkUUIDParse assume correct UUID
supplied, there is no use to complain about wrong format in it.
Otherwise our log is flooded with false error messages.
For instance, calling prlsdkUUIDParse from prlsdkEventsHandler
works as a filter and in case of uuid absence for event issuer,
we simply know that we shouldn't continue further processing.
What does uuid string contain in this case? Maybe it's better to 
explicitly check for null for example?



Instead of error logging for all calls we should explicitly take
into accaunt where it is called from.
Signed-off-by: Maxim Nestratov 
---
  src/vz/vz_sdk.c |   11 +++
  1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 744b58a..32ca1ef 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -329,8 +329,6 @@ prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid)
  
  /* trim curly braces */

  if (virUUIDParse(tmp + 1, uuid) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("UUID in config file malformed"));
  ret = -1;
  goto error;
  }
@@ -365,8 +363,11 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
  PrlVmCfg_GetUuid(sdkdom, uuidstr, );
  prlsdkCheckRetGoto(pret, error);
  
-if (prlsdkUUIDParse(uuidstr, uuid) < 0)

+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Domain UUID is malformed or empty"));
  goto error;
+}
  
  return 0;
  
@@ -1724,8 +1725,10 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque)

  pret = PrlEvent_GetType(prlEvent, );
  prlsdkCheckRetGoto(pret, cleanup);
  
-if (prlsdkUUIDParse(uuidstr, uuid) < 0)

+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+VIR_DEBUG("Skipping event type %d", prlEventType);
  goto cleanup;
+}
  
  switch (prlEventType) {

  case PET_DSP_EVT_VM_STATE_CHANGED:


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

Re: [libvirt] [PATCH v2 1/4] virfile: Rename virFileUnlink to virFileRemove

[Michal Privoznik]

On 21.09.2015 14:08, John Ferlan wrote:
> Similar to commit id '35847860', it's possible to attempt to create
> a 'netfs' directory in an NFS root-squash environment which will cause
> the 'vol-delete' command to fail.  It's also possible error paths from
> the 'vol-create' would result in an error to remove a created directory
> if the permissions were incorrect (and disallowed root access).
> 
> Thus rename the virFileUnlink to be virFileRemove to match the C API
> functionality, adjust the code to following using rmdir or unlink
> depending on the path type, and then use/call it for the VIR_STORAGE_VOL_DIR
> 
> Signed-off-by: John Ferlan 
> ---
>  src/libvirt_private.syms |  2 +-
>  src/storage/storage_backend_fs.c | 22 ++
>  src/util/virfile.c   | 29 -
>  src/util/virfile.h   |  2 +-
>  4 files changed, 32 insertions(+), 23 deletions(-)

ACK

Michal

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

[libvirt] [PATCH] vz: remove error logging from prlsdkUUIDParse

[Maxim Nestratov]

From: Maxim Nestratov 

As far as not every call of prlsdkUUIDParse assume correct UUID
supplied, there is no use to complain about wrong format in it.
Otherwise our log is flooded with false error messages.
For instance, calling prlsdkUUIDParse from prlsdkEventsHandler
works as a filter and in case of uuid absence for event issuer,
we simply know that we shouldn't continue further processing.
Instead of error logging for all calls we should explicitly take
into accaunt where it is called from.

Signed-off-by: Maxim Nestratov 
---
 src/vz/vz_sdk.c |   11 +++
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 744b58a..32ca1ef 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -329,8 +329,6 @@ prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid)
 
 /* trim curly braces */
 if (virUUIDParse(tmp + 1, uuid) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("UUID in config file malformed"));
 ret = -1;
 goto error;
 }
@@ -365,8 +363,11 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
 PrlVmCfg_GetUuid(sdkdom, uuidstr, );
 prlsdkCheckRetGoto(pret, error);
 
-if (prlsdkUUIDParse(uuidstr, uuid) < 0)
+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Domain UUID is malformed or empty"));
 goto error;
+}
 
 return 0;
 
@@ -1724,8 +1725,10 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
opaque)
 pret = PrlEvent_GetType(prlEvent, );
 prlsdkCheckRetGoto(pret, cleanup);
 
-if (prlsdkUUIDParse(uuidstr, uuid) < 0)
+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+VIR_DEBUG("Skipping event type %d", prlEventType);
 goto cleanup;
+}
 
 switch (prlEventType) {
 case PET_DSP_EVT_VM_STATE_CHANGED:
-- 
1.7.1

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

[libvirt] [PATCH] qemu: Update balloon state after migration finishes

[Peter Krempa]

Since qemu doesn't know at the beginning of migration what the actual
balloon size is until the migration stream transfers the appropriate
fields we also need to update the balloon size unconditionally in the
finish phase of the migration.
---
 src/qemu/qemu_migration.c | 4 
 src/qemu/qemu_process.c   | 2 +-
 src/qemu/qemu_process.h   | 4 
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 903612b..38649ed 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5701,6 +5701,10 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
 if (qemuMigrationStopNBDServer(driver, vm, mig) < 0)
 goto endjob;

+if (qemuProcessRefreshBalloonState(driver, vm,
+   QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
+goto endjob;
+
 if (flags & VIR_MIGRATE_PERSIST_DEST) {
 if (qemuMigrationPersist(driver, vm, mig, !v3proto) < 0) {
 /* Hmpf.  Migration was successful, but making it persistent
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7187dc1..f14582b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2091,7 +2091,7 @@ 
qemuProcessReconnectRefreshChannelVirtioState(virQEMUDriverPtr driver,
 }


-static int
+int
 qemuProcessRefreshBalloonState(virQEMUDriverPtr driver,
virDomainObjPtr vm,
int asyncJob)
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index d40f68d..b198c2b 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -115,4 +115,8 @@ virDomainDiskDefPtr 
qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm,

 int qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm);

+int qemuProcessRefreshBalloonState(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   int asyncJob);
+
 #endif /* __QEMU_PROCESS_H__ */
-- 
2.4.5

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

Re: [libvirt] [PATCH] vz: remove error logging from prlsdkUUIDParse

[Michal Privoznik]

On 21.09.2015 13:08, Maxim Nestratov wrote:
> From: Maxim Nestratov 
> 
> As far as not every call of prlsdkUUIDParse assume correct UUID
> supplied, there is no use to complain about wrong format in it.
> Otherwise our log is flooded with false error messages.
> For instance, calling prlsdkUUIDParse from prlsdkEventsHandler
> works as a filter and in case of uuid absence for event issuer,
> we simply know that we shouldn't continue further processing.
> Instead of error logging for all calls we should explicitly take
> into accaunt where it is called from.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/vz/vz_sdk.c |   11 +++
>  1 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 744b58a..32ca1ef 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -329,8 +329,6 @@ prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid)
>  
>  /* trim curly braces */
>  if (virUUIDParse(tmp + 1, uuid) < 0) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("UUID in config file malformed"));
>  ret = -1;

This line is redundant too. I mean, @ret is already initialized to -1.

>  goto error;
>  }
> @@ -365,8 +363,11 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
>  PrlVmCfg_GetUuid(sdkdom, uuidstr, );
>  prlsdkCheckRetGoto(pret, error);
>  
> -if (prlsdkUUIDParse(uuidstr, uuid) < 0)
> +if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("Domain UUID is malformed or empty"));
>  goto error;
> +}

Hm. This will mask the original error. If, for instance, there's been an
OOM in prlsdkUUIDParse(), the error is overwritten with this generic
error message. Well, in either cases we don't want to continue anyway ...

>  
>  return 0;
>  
> @@ -1724,8 +1725,10 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR 
> opaque)
>  pret = PrlEvent_GetType(prlEvent, );
>  prlsdkCheckRetGoto(pret, cleanup);
>  
> -if (prlsdkUUIDParse(uuidstr, uuid) < 0)
> +if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
> +VIR_DEBUG("Skipping event type %d", prlEventType);
>  goto cleanup;
> +}
>  
>  switch (prlEventType) {
>  case PET_DSP_EVT_VM_STATE_CHANGED:
> 


ACKed with the obvious fix and pushed.

Michal

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

Re: [libvirt] [PATCH 1/4] storage: Use virFileUnlink instead of rmdir

[John Ferlan]

On 09/21/2015 06:43 AM, Michal Privoznik wrote:
> On 18.09.2015 20:20, John Ferlan wrote:
>> Similar to commit id '35847860', it's possible to attempt to create
>> a 'netfs' directory in an NFS root-squash environment which will cause
>> the 'vol-delete' command to fail.  It's also possible error paths from
>> the 'vol-create' would result in an error to remove a created directory
>> if the permissions were incorrect (and disallowed root access).
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  src/storage/storage_backend_fs.c | 20 +---
>>  src/util/virfile.c   | 23 +--
>>  2 files changed, 26 insertions(+), 17 deletions(-)
>>
> 
[...]
>>
> 
> Since this function is now able to work with dirs too, maybe it should
> be renamed to something like virFileDirRemove? But that can be saved for
> a follow up patch. Not a show stopper to me.
> 

OK, but rather than go through more churn - I've sent a v2 of patch 1.

I've followed the C API then and named it 'virFileRemove'

John

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

[libvirt] [PATCH v2 1/4] virfile: Rename virFileUnlink to virFileRemove

[John Ferlan]

Similar to commit id '35847860', it's possible to attempt to create
a 'netfs' directory in an NFS root-squash environment which will cause
the 'vol-delete' command to fail.  It's also possible error paths from
the 'vol-create' would result in an error to remove a created directory
if the permissions were incorrect (and disallowed root access).

Thus rename the virFileUnlink to be virFileRemove to match the C API
functionality, adjust the code to following using rmdir or unlink
depending on the path type, and then use/call it for the VIR_STORAGE_VOL_DIR

Signed-off-by: John Ferlan 
---
 src/libvirt_private.syms |  2 +-
 src/storage/storage_backend_fs.c | 22 ++
 src/util/virfile.c   | 29 -
 src/util/virfile.h   |  2 +-
 4 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1bb41f7..8c1f388 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1454,6 +1454,7 @@ virFileReadAllQuiet;
 virFileReadHeaderFD;
 virFileReadLimFD;
 virFileRelLinkPointsTo;
+virFileRemove;
 virFileResolveAllLinks;
 virFileResolveLink;
 virFileRewrite;
@@ -1461,7 +1462,6 @@ virFileSanitizePath;
 virFileSkipRoot;
 virFileStripSuffix;
 virFileTouch;
-virFileUnlink;
 virFileUnlock;
 virFileUpdatePerm;
 virFileWaitForDevices;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index f41a41e..99ea394 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1203,25 +1203,23 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 switch ((virStorageVolType) vol->type) {
 case VIR_STORAGE_VOL_FILE:
-if (virFileUnlink(vol->target.path, vol->target.perms->uid,
+case VIR_STORAGE_VOL_DIR:
+if (virFileRemove(vol->target.path, vol->target.perms->uid,
   vol->target.perms->gid) < 0) {
 /* Silently ignore failures where the vol has already gone away */
 if (errno != ENOENT) {
-virReportSystemError(errno,
- _("cannot unlink file '%s'"),
- vol->target.path);
+if (vol->type == VIR_STORAGE_VOL_FILE)
+virReportSystemError(errno,
+ _("cannot unlink file '%s'"),
+ vol->target.path);
+else
+virReportSystemError(errno,
+ _("cannot remove directory '%s'"),
+ vol->target.path);
 return -1;
 }
 }
 break;
-case VIR_STORAGE_VOL_DIR:
-if (rmdir(vol->target.path) < 0) {
-virReportSystemError(errno,
- _("cannot remove directory '%s'"),
- vol->target.path);
-return -1;
-}
-break;
 case VIR_STORAGE_VOL_BLOCK:
 case VIR_STORAGE_VOL_NETWORK:
 case VIR_STORAGE_VOL_NETDIR:
diff --git a/src/util/virfile.c b/src/util/virfile.c
index fe9f8d4..668daf8 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2315,8 +2315,8 @@ virFileOpenAs(const char *path, int openflags, mode_t 
mode,
 }
 
 
-/* virFileUnlink:
- * @path: file to unlink
+/* virFileRemove:
+ * @path: file to unlink or directory to remove
  * @uid: uid that was used to create the file (not required)
  * @gid: gid that was used to create the file (not required)
  *
@@ -2327,7 +2327,7 @@ virFileOpenAs(const char *path, int openflags, mode_t 
mode,
  * from the child.
  */
 int
-virFileUnlink(const char *path,
+virFileRemove(const char *path,
   uid_t uid,
   gid_t gid)
 {
@@ -2341,8 +2341,12 @@ virFileUnlink(const char *path,
  * the file/volume, then use unlink directly
  */
 if ((geteuid() != 0) ||
-((uid == (uid_t) -1) && (gid == (gid_t) -1)))
-return unlink(path);
+((uid == (uid_t) -1) && (gid == (gid_t) -1))) {
+if (virFileIsDir(path))
+return rmdir(path);
+else
+return unlink(path);
+}
 
 /* Otherwise, we have to deal with the NFS root-squash craziness
  * to run under the uid/gid that created the volume in order to
@@ -2406,9 +2410,16 @@ virFileUnlink(const char *path,
 goto childerror;
 }
 
-if (unlink(path) < 0) {
-ret = errno;
-goto childerror;
+if (virFileIsDir(path)) {
+if (rmdir(path) < 0) {
+ret = errno;
+goto childerror;
+}
+} else {
+if (unlink(path) < 0) {
+ret = errno;
+goto childerror;
+}
 }
 
  childerror:
@@ -2643,7 +2654,7 @@ virDirCreate(const char *path ATTRIBUTE_UNUSED,
 }
 
 int
-virFileUnlink(const char *path,

Re: [libvirt] [PATCH 0/4] Remove open coding virProcessWait for root-squash

[Michal Privoznik]

On 18.09.2015 20:20, John Ferlan wrote:
> A followup of sorts to recently pushed patches regarding NFS root-squash.
> During libvirt-security list review it was pointed out that the new code
> was essentially open coding what virProcessWait does. However, since the
> model being used also was open coded and there was a time element, the
> change was allowed as is with the expectation that a cleanup patch would
> follow.  Which is what leads into this series
> 
> The series started out purely as removing the open code and replacing
> with the call to virProcessWait, but during that exercise I also realized
> that it was possible to create a 'netdir' in a NFS root-squash environment
> (eg, virDirCreate); however, the corrollary to remove the directory using
> a fork/exec didn't exist - in fact all that was called was rmdir, which
> failed to delete in the NFS root-squash environment.  Rather than having
> a whole new interface, the first patch reworks virFileUnlink to check
> whether the target is a directory or a file and either call rmdir or
> unlink appropriately.
> 
> The one common thread amongst the 3 API's changed here is they each looked
> to return an errno value, while typically virProcessWait consumers only
> return -1 and errno. Determining which failure in virProcessWait returns
> -1 is possible because one exit path uses virReportSystemError to report
> the error that caused the waitpid() to fail, while the other error path
> either receives the errno from the child process or if not present had
> already "assumed" EACCES, so these changes follow that model, except that
> if it's determined the waitpid failed, EINTR is returned similar to how
> virFileAccessibleAs sets errno and returns -1.
> 
> John Ferlan (4):
>   storage: Use virFileUnlink instead of rmdir
>   virfile: Use virProcessWait in virFileOpenForked
>   virfile: Use virProcessWait in virFileUnlink
>   virfile: Use virProcessWait in virDirCreate
> 
>  src/storage/storage_backend_fs.c |  20 +++--
>  src/util/virfile.c   | 153 
> ---
>  2 files changed, 71 insertions(+), 102 deletions(-)
> 

ACK series. Looking forward to the follow up patch.

Michal

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

Re: [libvirt] [PATCH 2/4] Implement infrastracture for mocking up QEMU capabilities cache

[Michal Privoznik]

On 15.09.2015 10:05, Ján Tomko wrote:
> From: Pavel Fedin 
> 
> The main purpose of this patch is to introduce test mode to
> virQEMUCapsCacheLookup(). This is done by adding a global variable, which
> effectively overrides binary name. This variable is supposed to be set by
> test suite.
> 
> The second addition is qemuTestCapsCacheInsert() function which allows the
> test suite to actually populate the cache.
> 
> Signed-off-by: Pavel Fedin 
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_capabilities.c | 16 +++-
>  src/qemu/qemu_capspriv.h | 36 
>  tests/testutilsqemu.c| 40 
>  tests/testutilsqemu.h|  5 +
>  4 files changed, 88 insertions(+), 9 deletions(-)
>  create mode 100644 src/qemu/qemu_capspriv.h
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 4ad1bdb..9a819d2 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -42,6 +42,7 @@
>  #include "virstring.h"
>  #include "qemu_hostdev.h"
>  #include "qemu_domain.h"
> +#include "qemu_capspriv.h"
>  
>  #include 
>  #include 
> @@ -331,15 +332,6 @@ struct _virQEMUCaps {
>  unsigned int *machineMaxCpus;
>  };
>  
> -struct _virQEMUCapsCache {
> -virMutex lock;
> -virHashTablePtr binaries;
> -char *libDir;
> -char *cacheDir;
> -uid_t runUid;
> -gid_t runGid;
> -};
> -
>  struct virQEMUCapsSearchData {
>  virArch arch;
>  };
> @@ -3718,11 +3710,17 @@ virQEMUCapsCacheNew(const char *libDir,
>  return NULL;
>  }
>  
> +const char *qemuTestCapsName;
>  
>  virQEMUCapsPtr
>  virQEMUCapsCacheLookup(virQEMUCapsCachePtr cache, const char *binary)
>  {
>  virQEMUCapsPtr ret = NULL;
> +
> +/* This is used only by test suite!!! */
> +if (qemuTestCapsName)
> +binary = qemuTestCapsName;
> +
>  virMutexLock(>lock);
>  ret = virHashLookup(cache->binaries, binary);
>  if (ret &&
> diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
> new file mode 100644
> index 000..9288dbd
> --- /dev/null
> +++ b/src/qemu/qemu_capspriv.h
> @@ -0,0 +1,36 @@
> +/*
> + * qemu_capspriv.h: private declarations for QEMU capabilities generation
> + *
> + * Copyright (C) 2015 Samsung Electronics Co. Ltd
> + * Copyright (C) 2015 Pavel Fedin
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * .
> + *
> + * Author: Pavel Fedin 
> + */
> +

Maybe we want to guard this file so that it cannot be included from a
random .c file? Something like we do in ./src/util/vircommandpriv.h?

> +#ifndef __QEMU_CAPSPRIV_H__
> +# define __QEMU_CAPSPRIV_H__
> +
> +struct _virQEMUCapsCache {
> +virMutex lock;
> +virHashTablePtr binaries;
> +char *libDir;
> +char *cacheDir;
> +uid_t runUid;
> +gid_t runGid;
> +};
> +
> +#endif
> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
> index 84dfa75..275d22a 100644
> --- a/tests/testutilsqemu.c
> +++ b/tests/testutilsqemu.c
> @@ -8,6 +8,7 @@
>  # include "cpu_conf.h"
>  # include "qemu/qemu_driver.h"
>  # include "qemu/qemu_domain.h"
> +# include "qemu/qemu_capspriv.h"
>  # include "virstring.h"
>  
>  # define VIR_FROM_THIS VIR_FROM_QEMU
> @@ -527,6 +528,32 @@ qemuTestParseCapabilities(const char *capsFile)
>  return NULL;
>  }
>  
> +int qemuTestCapsCacheInsert(virQEMUCapsCachePtr cache, const char *binary,
> +virQEMUCapsPtr caps)
> +{
> +int ret;
> +
> +if (caps) {
> +/* Our caps were created artificially, so we don't want
> + * virQEMUCapsCacheFree() to attempt to deallocate them */
> +virObjectRef(caps);
> +} else {
> +caps = virQEMUCapsNew();
> +if (!caps)
> +return -ENOMEM;
> +}
> +
> +/* We can have repeating names for our test data sets,
> + * so make sure there's no old copy */
> +virHashRemoveEntry(cache->binaries, binary);
> +
> +ret = virHashAddEntry(cache->binaries, binary, caps);
> +if (ret < 0)
> +virObjectUnref(caps);

I think we want to set @qemuTestCapsName here:

if (ret < 0)
  virObjectUnref(caps)
else
  qemuTestCapsName = binary;

> +
> +return ret;
> +}
> +
>  int 

Re: [libvirt] [PATCH 1/4] tests: split out common qemu driver initialization

[Michal Privoznik]

On 15.09.2015 10:05, Ján Tomko wrote:
> From: Pavel Fedin 
> 
> Two utility functions are introduced for proper initialization and
> cleanup of the driver.
> 
> Signed-off-by: Pavel Fedin 
> Signed-off-by: Ján Tomko 
> ---
>  tests/domainsnapshotxml2xmltest.c | 10 +++---
>  tests/qemuagenttest.c | 11 ++-
>  tests/qemuargv2xmltest.c  | 12 ++--
>  tests/qemuhotplugtest.c   |  9 ++---
>  tests/qemuxml2argvtest.c  | 11 ++-
>  tests/qemuxml2xmltest.c   |  8 +++-
>  tests/qemuxmlnstest.c | 11 +++
>  tests/testutilsqemu.c | 30 ++
>  tests/testutilsqemu.h |  2 ++
>  9 files changed, 53 insertions(+), 51 deletions(-)
> 
> diff --git a/tests/domainsnapshotxml2xmltest.c 
> b/tests/domainsnapshotxml2xmltest.c
> index 3955a19..b66af3e 100644
> --- a/tests/domainsnapshotxml2xmltest.c
> +++ b/tests/domainsnapshotxml2xmltest.c
> @@ -152,13 +152,10 @@ mymain(void)
>  {
>  int ret = 0;
>  
> -if ((driver.caps = testQemuCapsInit()) == NULL)
> +if (qemuTestDriverInit() < 0)
>  return EXIT_FAILURE;
>  
> -if (!(driver.xmlopt = virQEMUDriverCreateXMLConf())) {
> -virObjectUnref(driver.caps);
> -return EXIT_FAILURE;
> -}
> +driver.config->allowDiskFormatProbing = true;

Apart from what Martin already pointed out ...

>  
>  if (VIR_ALLOC(testSnapshotXMLVariableLineRegex) < 0)
>  goto cleanup;
> @@ -227,8 +224,7 @@ mymain(void)
>  if (testSnapshotXMLVariableLineRegex)
>  regfree(testSnapshotXMLVariableLineRegex);
>  VIR_FREE(testSnapshotXMLVariableLineRegex);
> -virObjectUnref(driver.caps);
> -virObjectUnref(driver.xmlopt);
> +qemuTestDriverFree();
>  
>  return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>  }
> diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
> index 52cc834..1ebc030 100644
> --- a/tests/qemuagenttest.c
> +++ b/tests/qemuagenttest.c
> @@ -31,6 +31,8 @@
>  
>  #define VIR_FROM_THIS VIR_FROM_NONE
>  
> +static virQEMUDriver driver;
> +

There's no need for this variable to be global. Put it into mymain() please.

>  static int
>  testQemuAgentFSFreeze(const void *data)
>  {
> @@ -909,7 +911,6 @@ static int
>  mymain(void)
>  {
>  int ret = 0;
> -virDomainXMLOptionPtr xmlopt;
>  
>  #if !WITH_YAJL
>  fputs("libvirt not compiled with yajl, skipping this test\n", stderr);
> @@ -917,13 +918,13 @@ mymain(void)
>  #endif
>  
>  if (virThreadInitialize() < 0 ||
> -!(xmlopt = virQEMUDriverCreateXMLConf(NULL)))
> +qemuTestDriverInit() < 0)
>  return EXIT_FAILURE;
>  
>  virEventRegisterDefaultImpl();
>  
> -#define DO_TEST(name)   \
> -if (virtTestRun(# name, testQemuAgent ## name, xmlopt) < 0) \
> +#define DO_TEST(name)  \
> +if (virtTestRun(# name, testQemuAgent ## name, driver.xmlopt) < 0) \
>  ret = -1
>  
>  DO_TEST(FSFreeze);
> @@ -938,7 +939,7 @@ mymain(void)
>  
>  DO_TEST(Timeout); /* Timeout should always be called last */
>  
> -virObjectUnref(xmlopt);
> +qemuTestDriverFree();
>  
>  return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
>  }

Michal

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

Re: [libvirt] [PATCH 3/4] Use mockup cache

[Michal Privoznik]

On 15.09.2015 10:05, Ján Tomko wrote:
> From: Pavel Fedin 
> 
> Use the new API in order to correctly add capability sets to the cache
> before parsing XML files
> 
> Signed-off-by: Pavel Fedin 

s/^/tests: / in $SUBJ.

> ---
>  tests/qemuhotplugtest.c  | 23 +++
>  tests/qemuxml2argvtest.c |  6 ++
>  tests/qemuxmlnstest.c|  6 ++
>  3 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
> index 3cf7f36..109d820 100644
> --- a/tests/qemuhotplugtest.c
> +++ b/tests/qemuhotplugtest.c
> @@ -57,7 +57,7 @@ static int
>  qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
>   virDomainObjPtr *vm,
>   const char *domxml,
> - bool event)
> + bool event, const char *testname)
>  {
>  int ret = -1;
>  qemuDomainObjPrivatePtr priv = NULL;
> @@ -65,12 +65,6 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
>  if (!(*vm = virDomainObjNew(xmlopt)))
>  goto cleanup;
>  
> -if (!((*vm)->def = virDomainDefParseString(domxml,
> -   driver.caps,
> -   driver.xmlopt,
> -   
> VIR_DOMAIN_DEF_PARSE_INACTIVE)))
> -goto cleanup;
> -
>  priv = (*vm)->privateData;
>  
>  if (!(priv->qemuCaps = virQEMUCapsNew()))
> @@ -85,6 +79,18 @@ qemuHotplugCreateObjects(virDomainXMLOptionPtr xmlopt,
>  if (event)
>  virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_DEVICE_DEL_EVENT);
>  
> +qemuTestCapsName = testname;
> +ret = qemuTestCapsCacheInsert(driver.qemuCapsCache, testname,
> +  priv->qemuCaps);


I think that @qemuTestCapsName should be set in
qemuTestCapsCacheInsert(). On its successful return.

> +if (ret < 0)
> +goto cleanup;
> +
> +if (!((*vm)->def = virDomainDefParseString(domxml,
> +   driver.caps,
> +   driver.xmlopt,
> +   
> VIR_DOMAIN_DEF_PARSE_INACTIVE)))
> +goto cleanup;
> +
>  if (qemuDomainAssignAddresses((*vm)->def, priv->qemuCaps, *vm) < 0)
>  goto cleanup;
>  
> @@ -243,7 +249,8 @@ testQemuHotplug(const void *data)
>  vm = test->vm;
>  } else {
>  if (qemuHotplugCreateObjects(driver.xmlopt, , domain_xml,
> - test->deviceDeletedEvent) < 0)
> + test->deviceDeletedEvent,
> + test->domain_filename) < 0)
>  goto cleanup;
>  }
>  
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index cd12356..1fc767e 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -423,6 +423,12 @@ testCompareXMLToArgvHelper(const void *data)
>  if (virQEMUCapsGet(info->extraFlags, QEMU_CAPS_ENABLE_FIPS))
>  flags |= FLAG_FIPS;
>  
> +qemuTestCapsName = info->name;
> +result = qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name,
> + info->extraFlags);
> +if (result < 0)
> +goto cleanup;
> +
>  result = testCompareXMLToArgvFiles(xml, args, info->extraFlags,
> info->migrateFrom, info->migrateFd,
> flags);
> diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c
> index 40e32dc..65bf1d3 100644
> --- a/tests/qemuxmlnstest.c
> +++ b/tests/qemuxmlnstest.c
> @@ -179,6 +179,12 @@ testCompareXMLToArgvHelper(const void *data)
>  abs_srcdir, info->name) < 0)
>  goto cleanup;
>  
> +qemuTestCapsName = info->name;
> +result = qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name,
> + info->extraFlags);
> +if (result < 0)
> +goto cleanup;
> +
>  result = testCompareXMLToArgvFiles(xml, args, info->extraFlags,
> info->migrateFrom, info->migrateFd,
> info->json, info->expectError);
> 

Otherwise looking good.

Michal

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

Re: [libvirt] [PATCH 4/4] Removed unneeded check

[Michal Privoznik]

On 15.09.2015 10:05, Ján Tomko wrote:
> From: Pavel Fedin 
> 
> Since test suite now correctly creates capabilities cache, the hack is not
> needed any more.
> 
> Signed-off-by: Pavel Fedin 
> Signed-off-by: Ján Tomko 
> ---
>  src/qemu/qemu_domain.c | 10 ++
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index c8b0ccd..26ca12d 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1039,10 +1039,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>  return ret;
>  
>  
> -/* This condition is actually a (temporary) hack for test suite which
> - * does not create capabilities cache */
> -if (driver && driver->qemuCapsCache)
> -qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
> def->emulator);
> +qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
>  
>  /* Add implicit PCI root controller if the machine has one */
>  switch (def->os.arch) {
> @@ -1241,10 +1238,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>  if (driver)
>  cfg = virQEMUDriverGetConfig(driver);
>  
> -/* This condition is actually a (temporary) hack for test suite which
> - * does not create capabilities cache */
> -if (driver && driver->qemuCapsCache)
> -qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, 
> def->emulator);
> +qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, def->emulator);
>  
>  if (dev->type == VIR_DOMAIN_DEVICE_NET &&
>  dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
> 

ACK

Michal

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

Re: [libvirt] [PATCH] vz: remove error logging from prlsdkUUIDParse

[Maxim Nestratov]

21.09.2015 14:59, Dmitry Guryanov пишет:

On 09/21/2015 02:08 PM, Maxim Nestratov wrote:

From: Maxim Nestratov 

As far as not every call of prlsdkUUIDParse assume correct UUID
supplied, there is no use to complain about wrong format in it.
Otherwise our log is flooded with false error messages.
For instance, calling prlsdkUUIDParse from prlsdkEventsHandler
works as a filter and in case of uuid absence for event issuer,
we simply know that we shouldn't continue further processing.
What does uuid string contain in this case? Maybe it's better to 
explicitly check for null for example?

Usually an empty string. But I can't bet it is always so.




Instead of error logging for all calls we should explicitly take
into accaunt where it is called from.
Signed-off-by: Maxim Nestratov 
---
  src/vz/vz_sdk.c |   11 +++
  1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 744b58a..32ca1ef 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -329,8 +329,6 @@ prlsdkUUIDParse(const char *uuidstr, unsigned 
char *uuid)

/* trim curly braces */
  if (virUUIDParse(tmp + 1, uuid) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("UUID in config file malformed"));
  ret = -1;
  goto error;
  }
@@ -365,8 +363,11 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
  PrlVmCfg_GetUuid(sdkdom, uuidstr, );
  prlsdkCheckRetGoto(pret, error);
  -if (prlsdkUUIDParse(uuidstr, uuid) < 0)
+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Domain UUID is malformed or empty"));
  goto error;
+}
return 0;
  @@ -1724,8 +1725,10 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, 
PRL_VOID_PTR opaque)

  pret = PrlEvent_GetType(prlEvent, );
  prlsdkCheckRetGoto(pret, cleanup);
  -if (prlsdkUUIDParse(uuidstr, uuid) < 0)
+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+VIR_DEBUG("Skipping event type %d", prlEventType);
  goto cleanup;
+}
switch (prlEventType) {
  case PET_DSP_EVT_VM_STATE_CHANGED:




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

Re: [libvirt] [PATCH 1/4] storage: Use virFileUnlink instead of rmdir

[Michal Privoznik]

On 18.09.2015 20:20, John Ferlan wrote:
> Similar to commit id '35847860', it's possible to attempt to create
> a 'netfs' directory in an NFS root-squash environment which will cause
> the 'vol-delete' command to fail.  It's also possible error paths from
> the 'vol-create' would result in an error to remove a created directory
> if the permissions were incorrect (and disallowed root access).
> 
> Signed-off-by: John Ferlan 
> ---
>  src/storage/storage_backend_fs.c | 20 +---
>  src/util/virfile.c   | 23 +--
>  2 files changed, 26 insertions(+), 17 deletions(-)
> 


> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index fe9f8d4..7c285d9 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -2316,7 +2316,7 @@ virFileOpenAs(const char *path, int openflags, mode_t 
> mode,
>  
>  
>  /* virFileUnlink:
> - * @path: file to unlink
> + * @path: file to unlink or directory to remove
>   * @uid: uid that was used to create the file (not required)
>   * @gid: gid that was used to create the file (not required)
>   *
> @@ -2341,8 +2341,12 @@ virFileUnlink(const char *path,
>   * the file/volume, then use unlink directly
>   */
>  if ((geteuid() != 0) ||
> -((uid == (uid_t) -1) && (gid == (gid_t) -1)))
> -return unlink(path);
> +((uid == (uid_t) -1) && (gid == (gid_t) -1))) {
> +if (virFileIsDir(path))
> +return rmdir(path);
> +else
> +return unlink(path);
> +}
>  
>  /* Otherwise, we have to deal with the NFS root-squash craziness
>   * to run under the uid/gid that created the volume in order to
> @@ -2406,9 +2410,16 @@ virFileUnlink(const char *path,
>  goto childerror;
>  }
>  
> -if (unlink(path) < 0) {
> -ret = errno;
> -goto childerror;
> +if (virFileIsDir(path)) {
> +if (rmdir(path) < 0) {
> +ret = errno;
> +goto childerror;
> +}
> +} else {
> +if (unlink(path) < 0) {
> +ret = errno;
> +goto childerror;
> +}
>  }
>  
>   childerror:
> 

Since this function is now able to work with dirs too, maybe it should
be renamed to something like virFileDirRemove? But that can be saved for
a follow up patch. Not a show stopper to me.

Michal

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

Re: [libvirt] [PATCH] qemu: Update balloon state after migration finishes

[Michal Privoznik]

On 21.09.2015 13:43, Peter Krempa wrote:
> Since qemu doesn't know at the beginning of migration what the actual
> balloon size is until the migration stream transfers the appropriate
> fields we also need to update the balloon size unconditionally in the
> finish phase of the migration.
> ---

Okay, I did not understand this at first glance, but my later discussion
with Peter showed, that if we have the balloon event, we no longer use
monitor command to update the balloon size when needed. Therefore we
must refresh it upon successful migration. Mind mentioning that in the
commit message?

>  src/qemu/qemu_migration.c | 4 
>  src/qemu/qemu_process.c   | 2 +-
>  src/qemu/qemu_process.h   | 4 
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 903612b..38649ed 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -5701,6 +5701,10 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>  if (qemuMigrationStopNBDServer(driver, vm, mig) < 0)
>  goto endjob;
> 
> +if (qemuProcessRefreshBalloonState(driver, vm,
> +   QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
> +goto endjob;
> +
>  if (flags & VIR_MIGRATE_PERSIST_DEST) {
>  if (qemuMigrationPersist(driver, vm, mig, !v3proto) < 0) {
>  /* Hmpf.  Migration was successful, but making it persistent
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 7187dc1..f14582b 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -2091,7 +2091,7 @@ 
> qemuProcessReconnectRefreshChannelVirtioState(virQEMUDriverPtr driver,
>  }
> 
> 
> -static int
> +int
>  qemuProcessRefreshBalloonState(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> int asyncJob)
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index d40f68d..b198c2b 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -115,4 +115,8 @@ virDomainDiskDefPtr 
> qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm,
> 
>  int qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm);
> 
> +int qemuProcessRefreshBalloonState(virQEMUDriverPtr driver,
> +   virDomainObjPtr vm,
> +   int asyncJob);
> +
>  #endif /* __QEMU_PROCESS_H__ */
> 

ACK

Michal

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

[libvirt] [libvirt-test-api][PATCHv2] Add new test case for allocPages API

[Luyao Huang]

Signed-off-by: Luyao Huang 
---
 cases/test_connection.conf | 10 
 repos/virconn/connection_allocPages.py | 84 ++
 2 files changed, 94 insertions(+)
 create mode 100644 repos/virconn/connection_allocPages.py

diff --git a/cases/test_connection.conf b/cases/test_connection.conf
index 336b1ad..a4406bf 100644
--- a/cases/test_connection.conf
+++ b/cases/test_connection.conf
@@ -77,3 +77,13 @@ virconn:connection_getMemoryParameters
 virconn:connection_getMemoryStats
 conn
 qemu:///system
+
+virconn:connection_allocPages
+conn
+qemu:///system
+
+virconn:connection_allocPages
+conn
+qemu:///system
+flags
+pageset
diff --git a/repos/virconn/connection_allocPages.py 
b/repos/virconn/connection_allocPages.py
new file mode 100644
index 000..de3c071
--- /dev/null
+++ b/repos/virconn/connection_allocPages.py
@@ -0,0 +1,84 @@
+#!/usr/bin/env python
+
+import libvirt
+from libvirt import libvirtError
+import lxml
+import lxml.etree
+
+required_params = ()
+optional_params = {'conn': '', 'flags': ''}
+
+HOST_HUGEPAGE = 
'/sys/devices/system/node/node%d/hugepages/hugepages-%dkB/nr_hugepages'
+
+def get_host_pagesize(conn):
+ret = []
+tree = lxml.etree.fromstring(conn.getCapabilities())
+
+set = tree.xpath("/capabilities/host/cpu/pages")
+for n in set:
+ret.append(int(n.attrib['size']))
+
+return ret
+
+def get_host_pagecount(pagesize):
+try:
+return int(open(HOST_HUGEPAGE % (0, pagesize)).read())
+except IOError:
+return -1
+
+def connection_allocPages(params):
+"""
+   test API for allocPages in class virConnect
+"""
+logger = params['logger']
+fail=0
+
+if 'flags' in params:
+if params['flags'] == 'pageset':
+flags = libvirt.VIR_NODE_ALLOC_PAGES_SET
+else:
+logger.error("Unknown flags name: %s" % params['flags'])
+return 1
+else:
+flags = 0
+
+try:
+if 'conn' in params:
+conn=libvirt.open(params['conn'])
+else:
+conn=libvirt.open(optional_params['conn'])
+logger.info("get connection to libvirtd")
+list1 = get_host_pagesize(conn)
+
+except libvirtError, e:
+logger.error("API error message: %s" % e.message)
+return 1
+
+for i in list1:
+logger.info("test hugepage size %d" % i)
+
+if get_host_pagecount(i) == -1:
+logger.info("Skip system page size %d" % i)
+continue
+
+try:
+cur_count = get_host_pagecount(i)
+if flags == libvirt.VIR_NODE_ALLOC_PAGES_SET:
+conn.allocPages({i : cur_count + 1}, 0, 1, flags)
+else:
+conn.allocPages({i : 1}, 0, 1, flags)
+if get_host_pagecount(i) != cur_count + 1:
+logger.error("libvirt set a wrong page count to %dKiB 
hugepage" % i)
+fail = 1
+except libvirtError, e:
+if "Allocated only" in e.message:
+tmp_count = int(e.message.split()[-1])
+
+if tmp_count != get_host_pagecount(i):
+logger.error("libvirt output %dKiB hugepage count is not 
right" % i)
+fail = 1
+else:
+logger.error("API error message: %s" % e.message)
+return 1
+
+return fail
-- 
1.8.3.1

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

[libvirt] [PATCH v2 sandbox] virt-sandbox-image: switch to use URI to identify templates

[Daniel P. Berrange]

Currently the CLI syntax is somewhat docker specific requiring
inclusion of --registry arg to identify the docker download
server. Other app containers have a notion of download server,
but don't separate it from the template name.

This patch removes that docker-ism by changing to use a URI
for identifying the template image. So instead of

  virt-sandbox-image download \
  --source docker --registry index.docker.io
  --username dan --password 123456 ubuntu:15.04

You can use

  virt-sandbox-image download 
docker://dan:123...@index.docker.io/ubuntu?tag=15.04

The only mandatory part is the source prefix and image name, so
that can shorten to just

  virt-sandbox-image download docker:///ubuntu

to pull down the latest ubuntu image, from the default registry
using no authentication.
---

Changed in v2:

 - Rebase against master, instead of (unpushed) docker volume patch

 libvirt-sandbox/image/cli.py  |  71 +
 libvirt-sandbox/image/sources/DockerSource.py | 142 ++
 libvirt-sandbox/image/sources/Source.py   |  29 +++---
 libvirt-sandbox/image/template.py | 110 
 4 files changed, 228 insertions(+), 124 deletions(-)
 create mode 100644 libvirt-sandbox/image/template.py

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index 1718cc5..4d02289 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -3,7 +3,7 @@
 # Authors: Daniel P. Berrange 
 #  Eren Yagdiran 
 #
-# Copyright (C) 2013 Red Hat, Inc.
+# Copyright (C) 2013-2015 Red Hat, Inc.
 # Copyright (C) 2015 Universitat Politècnica de Catalunya.
 #
 # This program is free software; you can redistribute it and/or modify
@@ -34,6 +34,8 @@ import subprocess
 import random
 import string
 
+from libvirt_sandbox.image import template
+
 if os.geteuid() == 0:
 default_template_dir = "/var/lib/libvirt/templates"
 default_image_dir = "/var/lib/libvirt/images"
@@ -44,15 +46,6 @@ else:
 debug = False
 verbose = False
 
-import importlib
-def dynamic_source_loader(name):
-name = name[0].upper() + name[1:]
-modname = "libvirt_sandbox.image.sources." + name + "Source"
-mod = importlib.import_module(modname)
-classname = name + "Source"
-classimpl = getattr(mod, classname)
-return classimpl()
-
 gettext.bindtextdomain("libvirt-sandbox", "/usr/share/locale")
 gettext.textdomain("libvirt-sandbox")
 try:
@@ -73,11 +66,10 @@ def info(msg):
 
 def download(args):
 try:
-
dynamic_source_loader(args.source).download_template(templatename=args.template,
- 
templatedir=args.template_dir,
- 
registry=args.registry,
- 
username=args.username,
- 
password=args.password)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+source.download_template(template=tmpl,
+ templatedir=args.template_dir)
 except IOError,e:
 print "Source %s cannot be found in given path" %args.source
 except Exception,e:
@@ -85,17 +77,21 @@ def download(args):
 
 def delete(args):
 try:
-
dynamic_source_loader(args.source).delete_template(templatename=args.template,
-   
templatedir=args.template_dir)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+source.delete_template(template=tmpl,
+   templatedir=args.template_dir)
 except Exception,e:
 print "Delete Error %s", str(e)
 
 def create(args):
 try:
-
dynamic_source_loader(args.source).create_template(templatename=args.template,
-   
templatedir=args.template_dir,
-   
connect=args.connect,
-   format=args.format)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+source.create_template(template=tmpl,
+   templatedir=args.template_dir,
+   connect=args.connect,
+   format=args.format)
 except Exception,e:
 print "Create Error %s" % str(e)
 
@@ -103,19 +99,22 @@ def run(args):
 try:
 if args.connect is not None:
 check_connect(args.connect)
-source = dynamic_source_loader(args.source)
+
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+
 name = args.name
 if name is None:
 randomid = 

[libvirt] [PATCH 01/13] libxl: vz: Use accessor instead of direct access for max_balloon

[Peter Krempa]

Commits 45697fe5 and f863ac80 used direct access to the variable instead
of the preferred accessor method.
---
 src/libxl/libxl_driver.c | 2 +-
 src/vz/vz_driver.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8087c27..019f04f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -553,7 +553,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
 vm->def->vcpus = d_info.vcpu_online;
 vm->def->maxvcpus = d_info.vcpu_max_id + 1;
 vm->def->mem.cur_balloon = d_info.current_memkb;
-vm->def->mem.max_balloon = d_info.max_memkb;
+virDomainDefSetMemoryInitial(vm->def, d_info.max_memkb);

 ret = 0;

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 8fa7957..ba5d7e4 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1200,7 +1200,7 @@ vzDomainGetMaxMemory(virDomainPtr domain)
 if (!(dom = vzDomObjFromDomain(domain)))
 return -1;

-ret = dom->def->mem.max_balloon;
+ret = virDomainDefGetMemoryActual(dom->def->mem);
 virObjectUnlock(dom);
 return ret;
 }
-- 
2.4.5

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

[libvirt] [PATCH 02/13] conf: Add helper to determine whether memory hotplug is enabled for a vm

[Peter Krempa]

Add a simple helper so that the code doesn't have to rewrite the same
condition multiple times.
---
 src/conf/domain_conf.c| 9 -
 src/conf/domain_conf.h| 1 +
 src/libvirt_private.syms  | 1 +
 src/qemu/qemu_command.c   | 2 +-
 src/qemu/qemu_domain.c| 2 +-
 src/qemu/qemu_migration.c | 5 ++---
 6 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a3b3ccb..fa2e331 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1154,7 +1154,7 @@ int
 virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def)
 {
 /* memory hotplug tunables are not supported by this driver */
-if (def->mem.max_memory > 0 || def->mem.memory_slots > 0) {
+if (virDomainDefHasMemoryHotplug(def)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("memory hotplug tunables  are not "
  "supported by this hypervisor driver"));
@@ -7671,6 +7671,13 @@ virDomainParseMemoryLimit(const char *xpath,
 }


+bool
+virDomainDefHasMemoryHotplug(const virDomainDef *def)
+{
+return def->mem.memory_slots > 0 || def->mem.max_memory > 0;
+}
+
+
 /**
  * virDomainDefGetMemoryInitial:
  * @def: domain definition
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8be390b..cfd2600 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2324,6 +2324,7 @@ struct _virDomainDef {
 unsigned long long virDomainDefGetMemoryInitial(virDomainDefPtr def);
 void virDomainDefSetMemoryInitial(virDomainDefPtr def, unsigned long long 
size);
 unsigned long long virDomainDefGetMemoryActual(virDomainDefPtr def);
+bool virDomainDefHasMemoryHotplug(const virDomainDef *def);

 typedef enum {
 VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_AES,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8c1f388..1a92422 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -217,6 +217,7 @@ virDomainDefGetMemoryActual;
 virDomainDefGetMemoryInitial;
 virDomainDefGetSecurityLabelDef;
 virDomainDefHasDeviceAddress;
+virDomainDefHasMemoryHotplug;
 virDomainDefMaybeAddController;
 virDomainDefMaybeAddInput;
 virDomainDefNeedsPlacementAdvice;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 25f57f2..e1f199c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9323,7 +9323,7 @@ qemuBuildCommandLine(virConnectPtr conn,

 virCommandAddArg(cmd, "-m");

-if (def->mem.max_memory) {
+if (virDomainDefHasMemoryHotplug(def)) {
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PC_DIMM)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("memory hotplug isn't supported by this QEMU 
binary"));
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e4a88cd..f840b0d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1367,7 +1367,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 }

 if (dev->type == VIR_DOMAIN_DEVICE_MEMORY &&
-def->mem.max_memory == 0) {
+!virDomainDefHasMemoryHotplug(def)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("maxMemory has to be specified when using memory "
  "devices "));
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 903612b..948ad3b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3000,10 +3000,9 @@ qemuMigrationBeginPhase(virQEMUDriverPtr driver,
 }
 }

-if (vm->def->mem.max_memory ||
+if (virDomainDefHasMemoryHotplug(vm->def) ||
 ((flags & VIR_MIGRATE_PERSIST_DEST) &&
- vm->newDef &&
- vm->newDef->mem.max_memory))
+ vm->newDef && virDomainDefHasMemoryHotplug(vm->newDef)))
 cookieFlags |= QEMU_MIGRATION_COOKIE_MEMORY_HOTPLUG;

 if (!(mig = qemuMigrationEatCookie(driver, vm, NULL, 0, 0)))
-- 
2.4.5

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

[libvirt] [PATCH 06/13] conf: Add XML parser flag that will allow us to do incompatible updates

[Peter Krempa]

Add a new parser flag that will mark code paths that parse XML files
wich will not be used with existing VM state so that post parse
callbacks can possibly do ABI incompatible changes if needed.
---
 src/conf/domain_conf.h|  2 ++
 src/qemu/qemu_driver.c| 12 
 src/qemu/qemu_migration.c |  3 ++-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 15a8576..a1221ba 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2652,6 +2652,8 @@ typedef enum {
 /* don't validate os.type and arch against capabilities. Prevents
  * VMs from disappearing when qemu is removed and libvirtd is restarted */
 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,
 } virDomainDefParseFlags;

 typedef enum {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fc3b60d..3c959f6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1705,7 +1705,8 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr 
conn,
 unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD;
 virQEMUCapsPtr qemuCaps = NULL;
 virCapsPtr caps = NULL;
-unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+   VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;

 virCheckFlags(VIR_DOMAIN_START_PAUSED |
   VIR_DOMAIN_START_AUTODESTROY |
@@ -7146,7 +7147,8 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr 
conn,
 goto cleanup;

 def = virDomainDefParseString(xmlData, caps, driver->xmlopt,
-  VIR_DOMAIN_DEF_PARSE_INACTIVE);
+  VIR_DOMAIN_DEF_PARSE_INACTIVE |
+  VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);
 if (!def)
 goto cleanup;

@@ -7460,7 +7462,8 @@ static virDomainPtr 
qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml
 virQEMUCapsPtr qemuCaps = NULL;
 virQEMUDriverConfigPtr cfg;
 virCapsPtr caps = NULL;
-unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+   VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;

 virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);

@@ -8430,7 +8433,8 @@ static int qemuDomainAttachDeviceFlags(virDomainPtr dom, 
const char *xml,
 virDomainDefPtr vmdef = NULL;
 virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
 int ret = -1;
-unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+   VIR_DOMAIN_DEF_PARSE_ABI_UPDATE;
 virQEMUCapsPtr qemuCaps = NULL;
 qemuDomainObjPrivatePtr priv;
 virQEMUDriverConfigPtr cfg = NULL;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 948ad3b..3dd3718 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1270,7 +1270,8 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
 }
 mig->persistent = virDomainDefParseNode(doc, nodes[0],
 caps, driver->xmlopt,
-VIR_DOMAIN_DEF_PARSE_INACTIVE);
+VIR_DOMAIN_DEF_PARSE_INACTIVE |
+
VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);
 if (!mig->persistent) {
 /* virDomainDefParseNode already reported
  * an error for us */
-- 
2.4.5

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

[libvirt] [PATCH 03/13] qemu: Make memory alignment helper more universal

[Peter Krempa]

Extract the size determination into a separate function and reuse it
across the memory device alignment functions. Since later we will need
to decide the alignment size according to architecture let's pass def to
the functions.
---
 src/qemu/qemu_domain.c  | 26 ++
 src/qemu/qemu_domain.h  |  3 ++-
 src/qemu/qemu_hotplug.c |  4 ++--
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f840b0d..a5e9b75 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3367,30 +3367,39 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,
 }


+static unsigned long long
+qemuDomainGetMemorySizeAlignment(virDomainDefPtr def ATTRIBUTE_UNUSED)
+{
+/* Align memory size. QEMU requires rounding to next 4KiB block.
+ * We'll take the "traditional" path and round it to 1MiB*/
+
+return 1024;
+}
+
+
 int
 qemuDomainAlignMemorySizes(virDomainDefPtr def)
 {
 unsigned long long mem;
+unsigned long long align = qemuDomainGetMemorySizeAlignment(def);
 size_t ncells = virDomainNumaGetNodeCount(def->numa);
 size_t i;

 /* align NUMA cell sizes if relevant */
 for (i = 0; i < ncells; i++) {
 mem = virDomainNumaGetNodeMemorySize(def->numa, i);
-virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(mem, 1024));
+virDomainNumaSetNodeMemorySize(def->numa, i, VIR_ROUND_UP(mem, align));
 }

 /* align initial memory size */
 mem = virDomainDefGetMemoryInitial(def);
-virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, 1024));
+virDomainDefSetMemoryInitial(def, VIR_ROUND_UP(mem, align));

-/* Align maximum memory size. QEMU requires rounding to next 4KiB block.
- * We'll take the "traditional" path and round it to 1MiB*/
-def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, 1024);
+def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, align);

 /* Align memory module sizes */
 for (i = 0; i < def->nmems; i++)
-qemuDomainMemoryDeviceAlignSize(def->mems[i]);
+def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align);

 return 0;
 }
@@ -3405,9 +3414,10 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
  * size so this should be safe).
  */
 void
-qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem)
+qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def,
+virDomainMemoryDefPtr mem)
 {
-mem->size = VIR_ROUND_UP(mem->size, 1024);
+mem->size = VIR_ROUND_UP(mem->size, qemuDomainGetMemorySizeAlignment(def));
 }


diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 8cf535f..64cd7e1 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -469,7 +469,8 @@ bool qemuDomainHasBlockjob(virDomainObjPtr vm, bool 
copy_only)
 ATTRIBUTE_NONNULL(1);

 int qemuDomainAlignMemorySizes(virDomainDefPtr def);
-void qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem);
+void qemuDomainMemoryDeviceAlignSize(virDomainDefPtr def,
+ virDomainMemoryDefPtr mem);

 virDomainChrSourceDefPtr qemuFindAgentConfig(virDomainDefPtr def);

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4797836..afc5408 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1802,7 +1802,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
 if (!(devstr = qemuBuildMemoryDeviceStr(mem, vm->def, priv->qemuCaps)))
 goto cleanup;

-qemuDomainMemoryDeviceAlignSize(mem);
+qemuDomainMemoryDeviceAlignSize(vm->def, mem);

 if (qemuBuildMemoryBackendStr(mem->size, mem->pagesize,
   mem->targetNode, mem->sourceNodes, NULL,
@@ -4273,7 +4273,7 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver,
 return -1;
 }

-qemuDomainMemoryDeviceAlignSize(memdef);
+qemuDomainMemoryDeviceAlignSize(vm->def, memdef);

 if ((idx = virDomainMemoryFindByDef(vm->def, memdef)) < 0) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
-- 
2.4.5

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

Re: [libvirt] [PATCH v2] test driver: don't unlock pool after freeing it

[Peter Krempa]

On Thu, Sep 17, 2015 at 09:12:31 -0400, David Mansfield wrote:
> 
> 
> On 09/17/2015 01:58 AM, Peter Krempa wrote:
> > On Wed, Sep 16, 2015 at 17:14:28 -0400, David Mansfield wrote:
> 

...

> From 08927d6ff222603e1be2a032c5fed68a5df8c68f Mon Sep 17 00:00:00 2001
> From: David Mansfield 
> Date: Thu, 17 Sep 2015 08:59:24 -0400
> Subject: [PATCH v2] test driver: don't unlock pool after freeing it
> 
> ---
>  src/test/test_driver.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 213a9a1..90ab09f 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4321,6 +4321,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
>  }
>  
>  virStoragePoolObjRemove(>pools, privpool);
> +privpool = NULL;
>  ret = 0;
>  
>   cleanup:
> -- 
> 2.4.3
> 

ACK and pushed now. I've added a valgrind trace to the commit message.

Thanks for taking time and fixing this.

Peter

(Btw, we usually prefer patch postings via "git send-email" but for
single patches that isn't a big issue and the patch applied just fine).



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

[libvirt] [PATCH v2] virsh: Fix job status indicator for 0 length block jobs

[Peter Krempa]

Although 0 length block jobs aren't entirely useful, the output of virsh
blockjob is empty due to the condition that suppresses the output for
migration jobs that did not start. Since the only place that actually
uses the condition that suppresses the output is in migration, let's
move the check there and thus add support for 0 of 0 equaling to 100%.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1196711
---
 tools/virsh-domain.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73c476d..fb138d5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1700,10 +1700,6 @@ virshPrintJobProgress(const char *label, unsigned long 
long remaining,
 {
 int progress;

-if (total == 0)
-/* migration has not been started */
-return;
-
 if (remaining == 0) {
 /* migration has completed */
 progress = 100;
@@ -4401,7 +4397,7 @@ virshWatchJob(vshControl *ctl,
 ret = virDomainGetJobInfo(dom, );
 pthread_sigmask(SIG_SETMASK, , NULL);
 if (ret == 0) {
-if (verbose)
+if (verbose && jobinfo.dataTotal > 0)
 virshPrintJobProgress(label, jobinfo.dataRemaining,
   jobinfo.dataTotal);

-- 
2.4.5

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

[libvirt] [PATCH 10/13] conf: Don't always recalculate initial memory size from NUMA size totals

[Peter Krempa]

When implementing memory hotplug I've opted to recalculate the initial
memory size (contents of the  element) as a sum of the sizes of
NUMA nodes when NUMA was enabled. This was based on an assumption that
qemu did not allow starting when the NUMA node size total didn't equal
to the initial memory size. Unfortunately the check was introduced to
qemu just lately.

This patch uses the new XML parser flag to decide whether it's safe to
update the memory size total from the NUMA cell sizes or not.

As an additional improvement we now report an error in case when the
size of hotplug memory would exceed the total memory size.

The rest of the changes assures that the function is called with correct
flags.
---
 src/conf/domain_conf.c  | 37 ++---
 src/conf/domain_conf.h  |  1 +
 src/qemu/qemu_command.c |  3 ++-
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d2a13ca..64cfd8c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3726,15 +3726,36 @@ virDomainDefRemoveDuplicateMetadata(virDomainDefPtr def)


 static int
-virDomainDefPostParseMemory(virDomainDefPtr def)
+virDomainDefPostParseMemory(virDomainDefPtr def,
+unsigned int parseFlags)
 {
 size_t i;
+unsigned long long numaMemory = 0;
+unsigned long long hotplugMemory = 0;

-if ((def->mem.initial_memory = virDomainNumaGetMemorySize(def->numa)) == 
0) {
+/* Attempt to infer the initial memory size from the sum NUMA memory sizes
+ * in case ABI updates are allowed or the  element wasn't 
specified */
+if (def->mem.total_memory == 0 ||
+parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE)
+numaMemory = virDomainNumaGetMemorySize(def->numa);
+
+if (numaMemory) {
+def->mem.initial_memory = numaMemory;
+} else {
 def->mem.initial_memory = def->mem.total_memory;

+/* calculate the sizes of hotplug memory */
 for (i = 0; i < def->nmems; i++)
-def->mem.initial_memory -= def->mems[i]->size;
+hotplugMemory += def->mems[i]->size;
+
+if (hotplugMemory > def->mem.initial_memory) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Total size of memory devices exceeds the total "
+ "memory size"));
+return -1;
+}
+
+def->mem.initial_memory -= hotplugMemory;
 }

 if (virDomainDefGetMemoryInitial(def) == 0) {
@@ -3770,7 +3791,8 @@ virDomainDefPostParseMemory(virDomainDefPtr def)

 static int
 virDomainDefPostParseInternal(virDomainDefPtr def,
-  virCapsPtr caps ATTRIBUTE_UNUSED)
+  virCapsPtr caps ATTRIBUTE_UNUSED,
+  unsigned int parseFlags)
 {
 size_t i;

@@ -3781,7 +3803,7 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 return -1;
 }

-if (virDomainDefPostParseMemory(def) < 0)
+if (virDomainDefPostParseMemory(def, parseFlags) < 0)
 return -1;

 /*
@@ -4274,6 +4296,7 @@ virDomainDefPostParseDeviceIterator(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 int
 virDomainDefPostParse(virDomainDefPtr def,
   virCapsPtr caps,
+  unsigned int parseFlags,
   virDomainXMLOptionPtr xmlopt)
 {
 int ret;
@@ -4299,7 +4322,7 @@ virDomainDefPostParse(virDomainDefPtr def,
 return ret;


-if ((ret = virDomainDefPostParseInternal(def, caps)) < 0)
+if ((ret = virDomainDefPostParseInternal(def, caps, parseFlags)) < 0)
 return ret;

 return 0;
@@ -16426,7 +16449,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;

 /* callback to fill driver specific domain aspects */
-if (virDomainDefPostParse(def, caps, xmlopt) < 0)
+if (virDomainDefPostParse(def, caps, flags, xmlopt) < 0)
 goto error;

 /* Auto-add any implied controllers which aren't present */
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ab250bd..25914b4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2459,6 +2459,7 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr 
xmlopt)
 int
 virDomainDefPostParse(virDomainDefPtr def,
   virCapsPtr caps,
+  unsigned int parseFlags,
   virDomainXMLOptionPtr xmlopt);

 static inline bool
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3044b11..7a1c9fe 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -13958,7 +13958,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
 if (virDomainDefAddImplicitControllers(def) < 0)
 goto error;

-if (virDomainDefPostParse(def, qemuCaps, xmlopt) < 0)
+if (virDomainDefPostParse(def, qemuCaps, VIR_DOMAIN_DEF_PARSE_ABI_UPDATE,
+  xmlopt) < 0)
 goto error;

 if (cmd->num_args || 

[libvirt] [PATCH 11/13] qemu: command: Align memory sizes only on fresh starts

[Peter Krempa]

When we are starting a qemu process for an incomming migration or
snapshot reloading we should not modify the memory sizes in the domain
since we could potentially change the guest ABI that was tediously
checked before. Additionally the function now updates the initial memory
size according to the NUMA node size, which should not happen if we are
restoring state.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1252685
---
 src/qemu/qemu_command.c | 3 ++-
 tests/qemuxml2argvdata/qemuxml2argv-restore-v1.args | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7a1c9fe..bb1835c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9318,7 +9318,8 @@ qemuBuildCommandLine(virConnectPtr conn,
 if (qemuBuildDomainLoaderCommandLine(cmd, def, qemuCaps) < 0)
 goto error;

-if (qemuDomainAlignMemorySizes(def) < 0)
+if (!migrateFrom && !snapshot &&
+qemuDomainAlignMemorySizes(def) < 0)
 goto error;

 virCommandAddArg(cmd, "-m");
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-restore-v1.args 
b/tests/qemuxml2argvdata/qemuxml2argv-restore-v1.args
index 5c67702..458c015 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-restore-v1.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-restore-v1.args
@@ -1,5 +1,5 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M \
-pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
+pc -m 213 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \
 -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 -net none -serial none 
-parallel \
 none -incoming stdio
-- 
2.4.5

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

[libvirt] [PATCH 13/13] test: Add test to validate that memory sizes don't get updated on migration

[Peter Krempa]

---
 .../qemuxml2argv-migrate-numa-unaligned.args   | 13 +
 .../qemuxml2argv-migrate-numa-unaligned.xml| 33 ++
 tests/qemuxml2argvtest.c   | 13 +++--
 3 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.args 
b/tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.args
new file mode 100644
index 000..4659a65
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.args
@@ -0,0 +1,13 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/kvm -S -M pc -m 14338 -smp 32 \
+-object memory-backend-ram,id=ram-node0,size=20482048,host-nodes=3,\
+policy=preferred \
+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
+-object memory-backend-ram,id=ram-node1,size=675907584,host-nodes=0-7,\
+policy=bind \
+-numa node,nodeid=1,cpus=1-27,cpus=29,memdev=ram-node1 \
+-object memory-backend-ram,id=ram-node2,size=24578457600,host-nodes=1-2,\
+host-nodes=5,host-nodes=7,policy=bind \
+-numa node,nodeid=2,cpus=28,cpus=30-31,memdev=ram-node2 \
+-nographic -monitor unix:/tmp/test-monitor,server,nowait \
+-no-acpi -boot c -usb -net none -serial none -parallel none -incoming stdio
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.xml
new file mode 100644
index 000..4fbb210
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.xml
@@ -0,0 +1,33 @@
+
+  QEMUGuest
+  9f4b6512-e73a-4a25-93e8-5307802821ce
+  14682468
+  14682468
+  32
+  
+
+
+
+  
+  
+hvm
+
+  
+  
+
+  
+  
+  
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/kvm
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index d4432df..2a423df 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -553,16 +553,19 @@ mymain(void)
  FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_ERROR,   \
  __VA_ARGS__)

+# define DO_TEST_LINUX(name, ...)   \
+DO_TEST_LINUX_FULL(name, NULL, -1, 0, __VA_ARGS__)
+
 # ifdef __linux__
 /* This is a macro that invokes test only on Linux. It's
  * meant to be called in those cases where qemuxml2argvmock
  * cooperation is expected (e.g. we need a fixed time,
  * predictable NUMA topology and so on). On non-Linux
  * platforms the macro just consume its argument. */
-#  define DO_TEST_LINUX(name, ...)  \
-DO_TEST_FULL(name, NULL, -1, 0, __VA_ARGS__)
+#  define DO_TEST_LINUX_FULL(name, ...) \
+DO_TEST_FULL(name, __VA_ARGS__)
 # else  /* __linux__ */
-#  define DO_TEST_LINUX(name, ...)  \
+#  define DO_TEST_LINUX_FULL(name, ...) \
 do {\
 const char *tmp ATTRIBUTE_UNUSED = name;\
 } while (0)
@@ -1271,6 +1274,10 @@ mymain(void)
 DO_TEST_FULL("migrate", "tcp:10.0.0.1:5000", -1, 0,
 QEMU_CAPS_MIGRATE_QEMU_TCP);

+DO_TEST_LINUX_FULL("migrate-numa-unaligned", "stdio", 7, 0,
+   QEMU_CAPS_MIGRATE_KVM_STDIO, QEMU_CAPS_NUMA,
+   QEMU_CAPS_OBJECT_MEMORY_RAM);
+
 DO_TEST("qemu-ns", NONE);

 DO_TEST("smp", QEMU_CAPS_SMP_TOPOLOGY);
-- 
2.4.5

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

[libvirt] [PATCH 04/13] conf: Drop VIR_DOMAIN_DEF_PARSE_CLOCK_ADJUST flag

[Peter Krempa]

The flag was used only for formatting the XML and once the parser and
formatter flags were split in 0ecd6851093945dd5ddc78266c61b577c65394ae
it doesn't make sense any more to have it.
---
 src/conf/domain_conf.c  | 1 -
 src/conf/domain_conf.h  | 7 +++
 tests/qemuxml2xmltest.c | 3 +--
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fa2e331..63dcecd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22716,7 +22716,6 @@ virDomainObjListLoadStatus(virDomainObjListPtr doms,
   VIR_DOMAIN_DEF_PARSE_STATUS |
   VIR_DOMAIN_DEF_PARSE_ACTUAL_NET |
   VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES |
-  VIR_DOMAIN_DEF_PARSE_CLOCK_ADJUST |
   
VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS)))
 goto error;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cfd2600..fcb6854 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2639,14 +2639,13 @@ typedef enum {
 VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES = 1 << 3,
 VIR_DOMAIN_DEF_PARSE_ALLOW_ROM   = 1 << 4,
 VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT  = 1 << 5,
-VIR_DOMAIN_DEF_PARSE_CLOCK_ADJUST= 1 << 6,
 /* parse only source half of  */
-VIR_DOMAIN_DEF_PARSE_DISK_SOURCE = 1 << 7,
-VIR_DOMAIN_DEF_PARSE_VALIDATE= 1 << 8,
+VIR_DOMAIN_DEF_PARSE_DISK_SOURCE = 1 << 6,
+VIR_DOMAIN_DEF_PARSE_VALIDATE= 1 << 7,
 /* don't validate os.type and arch against capabilities. Prevents
  * VMs from disappearing when qemu is removed and libvirtd is restarted
  */
-VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS = 1 << 9,
+VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS = 1 << 8,
 } virDomainDefParseFlags;

 typedef enum {
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 5a20ebc..d7464e8 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -180,8 +180,7 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
   driver.caps, driver.xmlopt,
   VIR_DOMAIN_DEF_PARSE_STATUS |
   VIR_DOMAIN_DEF_PARSE_ACTUAL_NET |
-  VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES |
-  VIR_DOMAIN_DEF_PARSE_CLOCK_ADJUST))) {
+  VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES))) {
 fprintf(stderr, "Failed to parse domain status XML:\n%s", source);
 goto cleanup;
 }
-- 
2.4.5

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

[libvirt] [PATCH 05/13] conf: Document all VIR_DOMAIN_DEF_PARSE_* flags

[Peter Krempa]

---
 src/conf/domain_conf.h | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index fcb6854..15a8576 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2632,19 +2632,25 @@ void virDomainObjListRemoveLocked(virDomainObjListPtr 
doms,
 typedef enum {
 /* parse internal domain status information */
 VIR_DOMAIN_DEF_PARSE_STATUS  = 1 << 0,
+/* Parse only parts of the XML that would be present in an inactive libvirt
+ * XML. Note that the flag does not imply that ABI incompatible
+ * transformations can be used, since it's used to strip runtime info when
+ * restoring save images/migration. */
 VIR_DOMAIN_DEF_PARSE_INACTIVE= 1 << 1,
 /* parse  element */
 VIR_DOMAIN_DEF_PARSE_ACTUAL_NET  = 1 << 2,
 /* parse original states of host PCI device */
 VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES = 1 << 3,
+/* internal flag passed to device info sub-parser to allow using  */
 VIR_DOMAIN_DEF_PARSE_ALLOW_ROM   = 1 << 4,
+/* internal flag passed to device info sub-parser to allow specifying boot 
order */
 VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT  = 1 << 5,
 /* parse only source half of  */
 VIR_DOMAIN_DEF_PARSE_DISK_SOURCE = 1 << 6,
+/* perform RNG schema validation on the passed XML document */
 VIR_DOMAIN_DEF_PARSE_VALIDATE= 1 << 7,
 /* don't validate os.type and arch against capabilities. Prevents
- * VMs from disappearing when qemu is removed and libvirtd is restarted
- */
+ * VMs from disappearing when qemu is removed and libvirtd is restarted */
 VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS = 1 << 8,
 } virDomainDefParseFlags;

-- 
2.4.5

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

[libvirt] [PATCH 00/13] Memory alignment vs. migration fixes

[Peter Krempa]

The refactorings that I've done in preparation for memory hotplug had the right
idea in regards to handling of the various memory size fields in libvirt, but
the execution of the refactors was suboptimal for migration compatibilty.

This patchset fixes problems when migrating from older libvirt versions that
allowed specifying a config where contents  were not equal to the total
size of all NUMA nodes. Additionally a few

Patches 01-05/13 are small cleanups to various parts of the code.

Patch 06/13 adds a XML parser flag that will be used later on.

Patches 07-08 do a few more cleanups in the memory parsing code.

Patch 12/13 in this series isn't entirely relevant to this series, but depends
on 09-11/13 so I've included it here. The patch modifies the alignment for PPC
machines that started requiring alignment to 256MiB in some cases. As we now
have a way to keep migrations intact we can switch the alignment always.

Patch 13/13 adds a test case based on an existing test to check that the sizing
code did not regress (again ...).

Peter Krempa (13):
  libxl: vz: Use accessor instead of direct access for max_balloon
  conf: Add helper to determine whether memory hotplug is enabled for a
vm
  qemu: Make memory alignment helper more universal
  conf: Drop VIR_DOMAIN_DEF_PARSE_CLOCK_ADJUST flag
  conf: Document all VIR_DOMAIN_DEF_PARSE_* flags
  conf: Add XML parser flag that will allow us to do incompatible
updates
  conf: Split memory related post parse stuff into separate function
  conf: Rename max_balloon to total_memory
  conf: Pre-calculate initial memory size instead of always calculating
it
  conf: Don't always recalculate initial memory size from NUMA size
totals
  qemu: command: Align memory sizes only on fresh starts
  qemu: ppc64: Align memory sizes to 256MiB blocks
  test: Add test to validate that memory sizes don't get updated on
migration

 src/conf/domain_conf.c | 113 +++--
 src/conf/domain_conf.h |  29 --
 src/hyperv/hyperv_driver.c |   2 +-
 src/libvirt_private.syms   |   2 +
 src/libxl/libxl_driver.c   |   4 +-
 src/lxc/lxc_driver.c   |   2 +-
 src/lxc/lxc_native.c   |   4 +-
 src/phyp/phyp_driver.c |   2 +-
 src/qemu/qemu_command.c|  12 ++-
 src/qemu/qemu_domain.c |  44 +---
 src/qemu/qemu_domain.h |   3 +-
 src/qemu/qemu_driver.c |  14 ++-
 src/qemu/qemu_hotplug.c|   4 +-
 src/qemu/qemu_migration.c  |   8 +-
 src/test/test_driver.c |   2 +-
 src/uml/uml_driver.c   |   2 +-
 src/vbox/vbox_common.c |   4 +-
 src/vmx/vmx.c  |   2 +-
 src/vz/vz_driver.c |   2 +-
 src/vz/vz_sdk.c|   2 +-
 src/xen/xm_internal.c  |   2 +-
 src/xenapi/xenapi_driver.c |   2 +-
 src/xenconfig/xen_common.c |   2 +-
 src/xenconfig/xen_sxpr.c   |   2 +-
 .../qemuxml2argv-migrate-numa-unaligned.args   |  13 +++
 .../qemuxml2argv-migrate-numa-unaligned.xml|  33 ++
 .../qemuxml2argv-pseries-cpu-compat.args   |   2 +-
 .../qemuxml2argvdata/qemuxml2argv-restore-v1.args  |   2 +-
 tests/qemuxml2argvtest.c   |  13 ++-
 tests/qemuxml2xmltest.c|   3 +-
 30 files changed, 238 insertions(+), 93 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-migrate-numa-unaligned.xml

-- 
2.4.5

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

[libvirt] [PATCH 09/13] conf: Pre-calculate initial memory size instead of always calculating it

[Peter Krempa]

Add 'initial_memory' member to struct virDomainMemtune so that the
memory size can be pre-calculated once instead of inferring it always
again and again.

Separating of the fields will also allow finer granularity of decisions
in later patches where it will allow to keep the old initial memory
value in cases where we are handling incomming migration from older
versions that did not always update the size from NUMA as the code did
previously.

The change also requires modification of the qemu memory alignment
function since at the point where we are modifying the size of NUMA
nodes the total size needs to be recalculated too.

The refactoring done in this patch also fixes a crash in the hyperv
driver that did not properly initialize def->numa and thus
virDomainNumaGetMemorySize(def->numa) crashed.

In summary this patch should have no functional impact at this point.
---
 src/conf/domain_conf.c   | 52 +---
 src/conf/domain_conf.h   |  3 +++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_domain.c   | 15 +-
 4 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a20ff2c..d2a13ca 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3728,6 +3728,15 @@ virDomainDefRemoveDuplicateMetadata(virDomainDefPtr def)
 static int
 virDomainDefPostParseMemory(virDomainDefPtr def)
 {
+size_t i;
+
+if ((def->mem.initial_memory = virDomainNumaGetMemorySize(def->numa)) == 
0) {
+def->mem.initial_memory = def->mem.total_memory;
+
+for (i = 0; i < def->nmems; i++)
+def->mem.initial_memory -= def->mems[i]->size;
+}
+
 if (virDomainDefGetMemoryInitial(def) == 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Memory size must be specified via  or in the 
"
@@ -7699,19 +7708,7 @@ virDomainDefHasMemoryHotplug(const virDomainDef *def)
 unsigned long long
 virDomainDefGetMemoryInitial(virDomainDefPtr def)
 {
-unsigned long long ret;
-size_t i;
-
-/* return NUMA memory size total in case numa is enabled */
-if ((ret = virDomainNumaGetMemorySize(def->numa)) > 0) {
-return ret;
-} else {
-ret = def->mem.total_memory;
-for (i = 0; i < def->nmems; i++)
-ret -= def->mems[i]->size;
-}
-
-return def->mem.total_memory;
+return def->mem.initial_memory;
 }


@@ -7720,13 +7717,30 @@ virDomainDefGetMemoryInitial(virDomainDefPtr def)
  * @def: domain definition
  * @size: size to set
  *
- * Sets the total memory size in @def.
+ * Sets the total memory size in @def. This function should be used only by
+ * hypervisors that don't support memory hotplug.
  */
 void
 virDomainDefSetMemoryTotal(virDomainDefPtr def,
unsigned long long size)
 {
 def->mem.total_memory = size;
+def->mem.initial_memory = size;
+}
+
+
+/**
+ * virDomainDefSetMemoryInitial:
+ * @def: domain definition
+ * @size: size to set
+ *
+ * Sets the initial memory size (without memory modules) in @def.
+ */
+void
+virDomainDefSetMemoryInitial(virDomainDefPtr def,
+ unsigned long long size)
+{
+def->mem.initial_memory = size;
 }


@@ -7744,12 +7758,10 @@ virDomainDefGetMemoryActual(virDomainDefPtr def)
 unsigned long long ret;
 size_t i;

-if ((ret = virDomainNumaGetMemorySize(def->numa)) > 0) {
-for (i = 0; i < def->nmems; i++)
-ret += def->mems[i]->size;
-} else {
-ret = def->mem.total_memory;
-}
+ret = def->mem.initial_memory;
+
+for (i = 0; i < def->nmems; i++)
+ret += def->mems[i]->size;

 return ret;
 }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 74c29bd..ab250bd 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2145,6 +2145,8 @@ struct _virDomainMemtune {
 /* total memory size including memory modules in kibibytes, this field
  * should be accessed only via accessors */
 unsigned long long total_memory;
+/* initial memory size in kibibytes = total_memory excluding memory 
modules*/
+unsigned long long initial_memory;
 unsigned long long cur_balloon; /* in kibibytes, capped at ulong thanks
to virDomainGetInfo */

@@ -2324,6 +2326,7 @@ struct _virDomainDef {

 unsigned long long virDomainDefGetMemoryInitial(virDomainDefPtr def);
 void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size);
+void virDomainDefSetMemoryInitial(virDomainDefPtr def, unsigned long long 
size);
 unsigned long long virDomainDefGetMemoryActual(virDomainDefPtr def);
 bool virDomainDefHasMemoryHotplug(const virDomainDef *def);

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d31687d..c87efa1 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -227,6 +227,7 @@ virDomainDefParseFile;
 virDomainDefParseNode;
 virDomainDefParseString;
 

[libvirt] [PATCH 08/13] conf: Rename max_balloon to total_memory

[Peter Krempa]

The name of the variable was misleading. Rename it and it's setting
accessor before other fixes.
---
 src/conf/domain_conf.c | 18 +-
 src/conf/domain_conf.h |  7 ---
 src/hyperv/hyperv_driver.c |  2 +-
 src/libvirt_private.syms   |  2 +-
 src/libxl/libxl_driver.c   |  4 ++--
 src/lxc/lxc_driver.c   |  2 +-
 src/lxc/lxc_native.c   |  4 ++--
 src/phyp/phyp_driver.c |  2 +-
 src/qemu/qemu_command.c|  4 ++--
 src/qemu/qemu_domain.c |  2 +-
 src/qemu/qemu_driver.c |  2 +-
 src/test/test_driver.c |  2 +-
 src/uml/uml_driver.c   |  2 +-
 src/vbox/vbox_common.c |  4 ++--
 src/vmx/vmx.c  |  2 +-
 src/vz/vz_sdk.c|  2 +-
 src/xen/xm_internal.c  |  2 +-
 src/xenapi/xenapi_driver.c |  2 +-
 src/xenconfig/xen_common.c |  2 +-
 src/xenconfig/xen_sxpr.c   |  2 +-
 20 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca60e60..a20ff2c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7706,27 +7706,27 @@ virDomainDefGetMemoryInitial(virDomainDefPtr def)
 if ((ret = virDomainNumaGetMemorySize(def->numa)) > 0) {
 return ret;
 } else {
-ret = def->mem.max_balloon;
+ret = def->mem.total_memory;
 for (i = 0; i < def->nmems; i++)
 ret -= def->mems[i]->size;
 }

-return def->mem.max_balloon;
+return def->mem.total_memory;
 }


 /**
- * virDomainDefSetMemoryInitial:
+ * virDomainDefSetMemoryTotal:
  * @def: domain definition
  * @size: size to set
  *
- * Sets the initial memory size in @def.
+ * Sets the total memory size in @def.
  */
 void
-virDomainDefSetMemoryInitial(virDomainDefPtr def,
- unsigned long long size)
+virDomainDefSetMemoryTotal(virDomainDefPtr def,
+   unsigned long long size)
 {
-def->mem.max_balloon = size;
+def->mem.total_memory = size;
 }


@@ -7748,7 +7748,7 @@ virDomainDefGetMemoryActual(virDomainDefPtr def)
 for (i = 0; i < def->nmems; i++)
 ret += def->mems[i]->size;
 } else {
-ret = def->mem.max_balloon;
+ret = def->mem.total_memory;
 }

 return ret;
@@ -14763,7 +14763,7 @@ virDomainDefParseXML(xmlDocPtr xml,

 /* Extract domain memory */
 if (virDomainParseMemory("./memory[1]", NULL, ctxt,
- >mem.max_balloon, false, true) < 0)
+ >mem.total_memory, false, true) < 0)
 goto error;

 if (virDomainParseMemory("./currentMemory[1]", NULL, ctxt,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a1221ba..74c29bd 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2142,8 +2142,9 @@ typedef struct _virDomainMemtune virDomainMemtune;
 typedef virDomainMemtune *virDomainMemtunePtr;

 struct _virDomainMemtune {
-unsigned long long max_balloon; /* in kibibytes, capped at ulong thanks
-   to virDomainGetMaxMemory */
+/* total memory size including memory modules in kibibytes, this field
+ * should be accessed only via accessors */
+unsigned long long total_memory;
 unsigned long long cur_balloon; /* in kibibytes, capped at ulong thanks
to virDomainGetInfo */

@@ -2322,7 +2323,7 @@ struct _virDomainDef {
 };

 unsigned long long virDomainDefGetMemoryInitial(virDomainDefPtr def);
-void virDomainDefSetMemoryInitial(virDomainDefPtr def, unsigned long long 
size);
+void virDomainDefSetMemoryTotal(virDomainDefPtr def, unsigned long long size);
 unsigned long long virDomainDefGetMemoryActual(virDomainDefPtr def);
 bool virDomainDefHasMemoryHotplug(const virDomainDef *def);

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index b539541..1958bbe 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -870,7 +870,7 @@ hypervDomainGetXMLDesc(virDomainPtr domain, unsigned int 
flags)
 if (VIR_STRDUP(def->description, virtualSystemSettingData->data->Notes) < 
0)
 goto cleanup;

-virDomainDefSetMemoryInitial(def, memorySettingData->data->Limit * 1024); 
/* megabyte to kilobyte */
+virDomainDefSetMemoryTotal(def, memorySettingData->data->Limit * 1024); /* 
megabyte to kilobyte */
 def->mem.cur_balloon = memorySettingData->data->VirtualQuantity * 1024; /* 
megabyte to kilobyte */

 def->vcpus = processorSettingData->data->VirtualQuantity;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1a92422..d31687d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -227,7 +227,7 @@ virDomainDefParseFile;
 virDomainDefParseNode;
 virDomainDefParseString;
 virDomainDefPostParse;
-virDomainDefSetMemoryInitial;
+virDomainDefSetMemoryTotal;
 virDomainDeleteConfig;
 virDomainDeviceAddressIsValid;
 virDomainDeviceAddressTypeToString;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c

Re: [libvirt] [PATCH] vz: remove error logging from prlsdkUUIDParse

[Maxim Nestratov]

21.09.2015 14:56, Michal Privoznik пишет:

On 21.09.2015 13:08, Maxim Nestratov wrote:

From: Maxim Nestratov 

As far as not every call of prlsdkUUIDParse assume correct UUID
supplied, there is no use to complain about wrong format in it.
Otherwise our log is flooded with false error messages.
For instance, calling prlsdkUUIDParse from prlsdkEventsHandler
works as a filter and in case of uuid absence for event issuer,
we simply know that we shouldn't continue further processing.
Instead of error logging for all calls we should explicitly take
into accaunt where it is called from.

Signed-off-by: Maxim Nestratov 
---
  src/vz/vz_sdk.c |   11 +++
  1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 744b58a..32ca1ef 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -329,8 +329,6 @@ prlsdkUUIDParse(const char *uuidstr, unsigned char *uuid)
  
  /* trim curly braces */

  if (virUUIDParse(tmp + 1, uuid) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("UUID in config file malformed"));
  ret = -1;

This line is redundant too. I mean, @ret is already initialized to -1.


  goto error;
  }
@@ -365,8 +363,11 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
  PrlVmCfg_GetUuid(sdkdom, uuidstr, );
  prlsdkCheckRetGoto(pret, error);
  
-if (prlsdkUUIDParse(uuidstr, uuid) < 0)

+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Domain UUID is malformed or empty"));
  goto error;
+}

Hm. This will mask the original error. If, for instance, there's been an
OOM in prlsdkUUIDParse(), the error is overwritten with this generic
error message. Well, in either cases we don't want to continue anyway ...

  
  return 0;
  
@@ -1724,8 +1725,10 @@ prlsdkEventsHandler(PRL_HANDLE prlEvent, PRL_VOID_PTR opaque)

  pret = PrlEvent_GetType(prlEvent, );
  prlsdkCheckRetGoto(pret, cleanup);
  
-if (prlsdkUUIDParse(uuidstr, uuid) < 0)

+if (prlsdkUUIDParse(uuidstr, uuid) < 0) {
+VIR_DEBUG("Skipping event type %d", prlEventType);
  goto cleanup;
+}
  
  switch (prlEventType) {

  case PET_DSP_EVT_VM_STATE_CHANGED:



ACKed with the obvious fix and pushed.

Michal


Thanks!

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

[libvirt] [PATCH 07/13] conf: Split memory related post parse stuff into separate function

[Peter Krempa]

The post parse func is growing rather large. Since later patches will
introduce more logic in the memory post parse code, split it into a
separate handler.
---
 src/conf/domain_conf.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 63dcecd..ca60e60 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3726,18 +3726,8 @@ virDomainDefRemoveDuplicateMetadata(virDomainDefPtr def)


 static int
-virDomainDefPostParseInternal(virDomainDefPtr def,
-  virCapsPtr caps ATTRIBUTE_UNUSED)
+virDomainDefPostParseMemory(virDomainDefPtr def)
 {
-size_t i;
-
-/* verify init path for container based domains */
-if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("init binary must be specified"));
-return -1;
-}
-
 if (virDomainDefGetMemoryInitial(def) == 0) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("Memory size must be specified via  or in the 
"
@@ -3765,6 +3755,26 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 return -1;
 }

+return 0;
+}
+
+
+static int
+virDomainDefPostParseInternal(virDomainDefPtr def,
+  virCapsPtr caps ATTRIBUTE_UNUSED)
+{
+size_t i;
+
+/* verify init path for container based domains */
+if (def->os.type == VIR_DOMAIN_OSTYPE_EXE && !def->os.init) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("init binary must be specified"));
+return -1;
+}
+
+if (virDomainDefPostParseMemory(def) < 0)
+return -1;
+
 /*
  * Some really crazy backcompat stuff for consoles
  *
-- 
2.4.5

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

[libvirt] [PATCH 12/13] qemu: ppc64: Align memory sizes to 256MiB blocks

[Peter Krempa]

For some machine types ppc64 machines now require that memory sizes are
aligned to 256MiB increments (due to the dynamically reconfigurable
memory). As now we treat existing configs reasonably in regards to
migration, we can round all the sizes unconditionally. The only drawback
will be that the memory size of a VM can potentially increase by
(256MiB - 1byte) * number_of_NUMA_nodes.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1249006
---
CC: David Gibson 

 src/qemu/qemu_domain.c  | 7 ++-
 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f92e1d3..2065fc4 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3368,8 +3368,13 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,


 static unsigned long long
-qemuDomainGetMemorySizeAlignment(virDomainDefPtr def ATTRIBUTE_UNUSED)
+qemuDomainGetMemorySizeAlignment(virDomainDefPtr def)
 {
+/* PPC requires the memory sizes to be rounded to 256MiB increments, so
+ * round them to the size always. */
+if (ARCH_IS_PPC64(def->os.arch))
+return 256 * 1024;
+
 /* Align memory size. QEMU requires rounding to next 4KiB block.
  * We'll take the "traditional" path and round it to 1MiB*/

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args 
b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args
index 64df406..305e924 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat.args
@@ -1,7 +1,7 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
 QEMU_AUDIO_DRV=none /usr/bin/qemu-system-ppc64 -S -M pseries \
 -cpu host,compat=power7 \
--m 214 -smp 4 -nographic -nodefconfig -nodefaults \
+-m 256 -smp 4 -nographic -nodefconfig -nodefaults \
 -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \
 -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb \
 -chardev pty,id=charserial0 \
-- 
2.4.5

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

[libvirt] [PATCH sandbox] docker: don't assume X-Docker-Token is set

[Daniel P. Berrange]

The Red Hat docker registry (registry.access.redhat.com) does
not set any X-Docker-Token HTTP header in its responses. Change
the code so it only passes around this header if it is actually
present.

Signed-off-by: Daniel P. Berrange 
---
 libvirt-sandbox/image/sources/DockerSource.py | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
b/libvirt-sandbox/image/sources/DockerSource.py
index f367c8f..78b2a53 100644
--- a/libvirt-sandbox/image/sources/DockerSource.py
+++ b/libvirt-sandbox/image/sources/DockerSource.py
@@ -83,10 +83,14 @@ class DockerSource(Source):
 checksums = {}
 for layer in data:
 pass
+
+headers = {}
+if token is not None:
+headers["Authorization"] = "Token" + token
 (data, res) = self._get_json(template,
  registryendpoint,
  "/v1/repositories" + template.path + 
"/tags",
- { "Authorization": "Token " + token })
+ headers)
 
 cookie = res.info().getheader('Set-Cookie')
 
@@ -98,7 +102,7 @@ class DockerSource(Source):
 (data, res) = self._get_json(template,
  registryendpoint,
  "/v1/images/" + imagetagid + "/ancestry",
- { "Authorization": "Token "+ token })
+ headers)
 
 if data[0] != imagetagid:
 raise ValueError(["Expected first layer id '%s' to match image id 
'%s'",
@@ -121,7 +125,7 @@ class DockerSource(Source):
 res = self._save_data(template,
   registryendpoint,
   "/v1/images/" + layerid + "/json",
-  { "Authorization": "Token " + token 
},
+  headers,
   jsonfile)
 createdFiles.append(jsonfile)
 
@@ -134,7 +138,7 @@ class DockerSource(Source):
 self._save_data(template,
 registryendpoint,
 "/v1/images/" + layerid + "/layer",
-{ "Authorization": "Token "+token },
+headers,
 datafile, datacsum, layersize)
 createdFiles.append(datafile)
 
-- 
2.4.3

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

Re: [libvirt] [PATCH v2] Close the source fd if the destination qemu exits during tunnelled migration

[John Ferlan]

On 09/21/2015 05:09 AM, Shivaprasad bhat wrote:
> Thanks John for the comments.
> 
> 
> On Fri, Sep 18, 2015 at 10:34 PM, John Ferlan  wrote:
>>
>>
>> On 09/14/2015 10:44 AM, Shivaprasad G Bhat wrote:
>>> Tunnelled migration can hang if the destination qemu exits despite all the
>>> ABI checks. This happens whenever the destination qemu exits before the
>>> complete transfer is noticed by source qemu. The savevm state checks at
>>> runtime can fail at destination and cause qemu to error out.
>>> The source qemu cant notice it as the EPIPE is not propogated to it.
>>> The qemuMigrationIOFunc() notices the stream being broken from 
>>> virStreamSend()
>>> and it cleans up the stream alone. The qemuMigrationWaitForCompletion() 
>>> would
>>> never get to 100% transfer completion.
>>> The qemuMigrationWaitForCompletion() never breaks out as well since
>>> the ssh connection to destination is healthy, and the source qemu also 
>>> thinks
>>> the migration is ongoing as the Fd to which it transfers, is never
>>> closed or broken. So, the migration will hang forever. Even Ctrl-C on the
>>> virsh migrate wouldn't be honoured. Close the source side FD when there is
>>> an error in the stream. That way, the source qemu updates itself and
>>> qemuMigrationWaitForCompletion() notices the failure.
>>>
>>> Close the FD for all kinds of errors to be sure. The error message is not
>>> copied for EPIPE so that the destination error is copied instead later.
>>>
>>> Note:
>>> Reproducible with repeated migrations between Power hosts running in 
>>> different
>>> subcores-per-core modes.
>>>
>>> Changes from v1 -> v2:
>>>VIR_FORCE_CLOSE() was called twice for this use case which would log
>>>unneccessary warnings. So, move the fd close to qemuMigrationIOFunc
>>>so that there are no unnecessary duplicate attempts.(Would this trigger
>>>a Coverity error? I don't have a setup to check.)
>>>
>>> Signed-off-by: Shivaprasad G Bhat 
>>> ---
>>>  src/qemu/qemu_migration.c |8 ++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>>> index ff89ab5..9602fb2 100644
>>> --- a/src/qemu/qemu_migration.c
>>> +++ b/src/qemu/qemu_migration.c
>>> @@ -4012,6 +4012,7 @@ static void qemuMigrationIOFunc(void *arg)
>>>  if (virStreamFinish(data->st) < 0)
>>>  goto error;
>>>
>>> +VIR_FORCE_CLOSE(data->sock);
>>>  VIR_FREE(buffer);
>>>
>>>  return;
>>> @@ -4029,7 +4030,11 @@ static void qemuMigrationIOFunc(void *arg)
>>>  }
>>>
>>>   error:
>>> -virCopyLastError(>err);
>>> +/* Let the source qemu know that the transfer cant continue anymore.
>>> + * Don't copy the error for EPIPE as destination has the actual error. 
>>> */
>>> +VIR_FORCE_CLOSE(data->sock);
>>> +if (!virLastErrorIsSystemErrno(EPIPE))
>>> +virCopyLastError(>err);
>>>  virResetLastError();
>>>  VIR_FREE(buffer);
>>>  }
>>> @@ -4397,7 +4402,6 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>>>  if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0)
>>>  ret = -1;
>>>  }
>>> -VIR_FORCE_CLOSE(fd);
>>
>> ^^^
>>
>> This causes Coverity to claim a RESOURCE_LEAK
>>
>> Feels like this was a mistake edit...
>>
> 
> The VIR_FORCE_CLOSE() inside qemuMigrationIOFunc() alone is sufficient.
> Having this again here would lead to Warning in the logs. I too thought 
> coverity
> would complain. Is there a way to force ignore a coverity warning?
> 

Typically a marker of sorts, such as

/* coverity[leaked_handle]  */

Although I'm not sure that's the best way to handle this either.

The problem I see though is this is an "error path" issue and when
perhaps it's safe/required to close fd/io->sock/data->sock.

Your commit comment notes that the issue is seen on a fairly specific
event (virStreamSend failure) for a specific type of migration. As I
read the code, that failure jumps to error (as does virStreamFinish). So
now you have a fairly specific set of instances which perhaps would
cause qemuMigrationWaitForCompletion to need to fail. The fix you have
proposed is to close the data->sock (io->sock, fd). However, your
proposal is a larger hammer. I assume closure of data->sock causes
WaitForCompletion to fail (perhaps differently) and that's why you chose it.

Going back to the beginning of qemuMigrationRun, setting the 'fd' and
using StartTunnel seems to rely on MIGRATION_FWD_DIRECT, but if a tunnel
is created an (ugh) 'iothread' variable is NON-NULL. What if 'iothread'
were passed to qemuMigrationWaitForCompletion? Then again passed to
qemuMigrationCompleted which would check if iothread non-NULL and for
some new flag that could be created in _qemuMigrationIOThread, being
true. If it were true, then that would cause failure so that
WaitForCompletion would return error status. The only way the flag is
set to true is in qemuMigrationIOFunc when the code 

[libvirt] [PATCH sandbox] virt-sandbox-image: switch to use URI to identify templates

[Daniel P. Berrange]

Currently the CLI syntax is somewhat docker specific requiring
inclusion of --registry arg to identify the docker download
server. Other app containers have a notion of download server,
but don't separate it from the template name.

This patch removes that docker-ism by changing to use a URI
for identifying the template image. So instead of

  virt-sandbox-image download \
  --source docker --registry index.docker.io
  --username dan --password 123456 ubuntu:15.04

You can use

  virt-sandbox-image download 
docker://dan@123456:index.docker.io/ubuntu?tag=15.04

The only mandatory part is the source prefix and image name, so
that can shorten to just

  virt-sandbox-image download docker:///ubuntu

to pull down the latest ubuntu image, from the default registry
using no authentication.
---
 libvirt-sandbox/image/cli.py  |  73 +
 libvirt-sandbox/image/sources/DockerSource.py | 146 ++
 libvirt-sandbox/image/sources/Source.py   |  33 +++---
 libvirt-sandbox/image/template.py | 109 +++
 4 files changed, 232 insertions(+), 129 deletions(-)
 create mode 100644 libvirt-sandbox/image/template.py

diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
index c8b5ba5..dd109a8 100755
--- a/libvirt-sandbox/image/cli.py
+++ b/libvirt-sandbox/image/cli.py
@@ -3,7 +3,7 @@
 # Authors: Daniel P. Berrange 
 #  Eren Yagdiran 
 #
-# Copyright (C) 2013 Red Hat, Inc.
+# Copyright (C) 2013-2015 Red Hat, Inc.
 # Copyright (C) 2015 Universitat Politècnica de Catalunya.
 #
 # This program is free software; you can redistribute it and/or modify
@@ -34,6 +34,8 @@ import subprocess
 import random
 import string
 
+from libvirt_sandbox.image import template
+
 if os.geteuid() == 0:
 default_template_dir = "/var/lib/libvirt/templates"
 default_image_dir = "/var/lib/libvirt/images"
@@ -44,15 +46,6 @@ else:
 debug = False
 verbose = False
 
-import importlib
-def dynamic_source_loader(name):
-name = name[0].upper() + name[1:]
-modname = "libvirt_sandbox.image.sources." + name + "Source"
-mod = importlib.import_module(modname)
-classname = name + "Source"
-classimpl = getattr(mod, classname)
-return classimpl()
-
 gettext.bindtextdomain("libvirt-sandbox", "/usr/share/locale")
 gettext.textdomain("libvirt-sandbox")
 try:
@@ -73,11 +66,10 @@ def info(msg):
 
 def download(args):
 try:
-
dynamic_source_loader(args.source).download_template(templatename=args.template,
- 
templatedir=args.template_dir,
- 
registry=args.registry,
- 
username=args.username,
- 
password=args.password)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+source.download_template(template=tmpl,
+ templatedir=args.template_dir)
 except IOError,e:
 print "Source %s cannot be found in given path" %args.source
 except Exception,e:
@@ -85,17 +77,21 @@ def download(args):
 
 def delete(args):
 try:
-
dynamic_source_loader(args.source).delete_template(templatename=args.template,
-   
templatedir=args.template_dir)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+source.delete_template(template=tmpl,
+   templatedir=args.template_dir)
 except Exception,e:
 print "Delete Error %s", str(e)
 
 def create(args):
 try:
-
dynamic_source_loader(args.source).create_template(templatename=args.template,
-   
templatedir=args.template_dir,
-   
connect=args.connect,
-   format=args.format)
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+source.create_template(template=tmpl,
+   templatedir=args.template_dir,
+   connect=args.connect,
+   format=args.format)
 except Exception,e:
 print "Create Error %s" % str(e)
 
@@ -104,19 +100,22 @@ def run(args):
 global image_dir
 if args.connect is not None:
 check_connect(args.connect)
-source = dynamic_source_loader(args.source)
+
+tmpl = template.Template.from_uri(args.template)
+source = tmpl.get_source_impl()
+
 name = args.name
 if name is None:
 randomid = ''.join(random.choice(string.lowercase) for i in 
range(10))
-

Re: [libvirt] [PATCH] vz: set mount point for container image-based disks

[Maxim Nestratov]

21.09.2015 12:23, Daniel P. Berrange пишет:

On Mon, Sep 21, 2015 at 12:14:57PM +0300, Maxim Nestratov wrote:

21.09.2015 11:44, Daniel P. Berrange пишет:

On Sun, Sep 20, 2015 at 10:17:51PM +0300, Maxim Nestratov wrote:

From: Maxim Nestratov 

In order to support not only root disks with type=file for containers,
we need to specify mount points for them.
For instance, if a secondary disk is added by the following record in
xml:

 
   
   
   
 

we are going to add it to container mounted to '/mnt/sdb' path.

That's not what the  element is for.   is about exposing
block device nodes to the container. It shouldn't try todo anything
with this device nodes. They might be used as raw data storage by
an application, so we can't assume they should be mounted.

If you want to mount things then you should be using 
instead.


Hm. It actually means that any disks with type file shouldn't work in
containers. Right?

No, the disk source on the host is not correlated to how it is
exposed to the guest.

With  it means you have to setup some kind of
loop device in the host OS, and then expose the block device to
the guest with that. The same if you have 
then you have to use host kernel or qemu-nbd, for example, to
setup a block device that you can then expose to the guest.


And working root disks like this is a mistake? But why?

Yes, that would be a mistake too. The root filesystem should
be exposed using  with a  of /


In vz, any images plugged into containers are also treated as disks. The
only difference between 'filesystem' and 'disk' is whether we should mount
it or not. That's all. While from point of view of a container user it is
just another storage. Why not  just mount it automatically?

The  element is intended to expose raw block devices to the guest.

The  is intended to expose mounted volumed to the guest.

A  can support both type=block and type=file as sources on the
host side, as well as others like type=network.

A  can support both type=block and type=file as sources
on the host, as well as a few others.

If you make  automatically mount volumes, then you've just
discarded the only semantic difference between  and
, which is not only wrong but also pointless. If you want
to mount it, you should just use , and not try to
make  do the same as .

It is key that  does *not* try to interpret what todo with
the storage - it is entirely upto the guest to decide what todo
with it. For example, an oracle database might decide to use the
block device directly as data storage. Or you might be running an
application that wants to manipulate the filesystem (eg a fsck
tool) inside the block dev, so again it would be inappropriate to
mount it.

Regards,
Daniel

Ok. I see. Thank you for the explanation.
Please, disregard the patch then.

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

Re: [libvirt] [PATCH v2 sandbox] virt-sandbox-image: switch to use URI to identify templates

[Cedric Bosdonnat]

On Mon, 2015-09-21 at 15:45 +0100, Daniel P. Berrange wrote:
> Currently the CLI syntax is somewhat docker specific requiring
> inclusion of --registry arg to identify the docker download
> server. Other app containers have a notion of download server,
> but don't separate it from the template name.
> 
> This patch removes that docker-ism by changing to use a URI
> for identifying the template image. So instead of
> 
>   virt-sandbox-image download \
>   --source docker --registry index.docker.io
>   --username dan --password 123456 ubuntu:15.04
> 
> You can use
> 
>   virt-sandbox-image download 
> docker://dan:123...@index.docker.io/ubuntu?tag=15.04
> 
> The only mandatory part is the source prefix and image name, so
> that can shorten to just
> 
>   virt-sandbox-image download docker:///ubuntu
> 
> to pull down the latest ubuntu image, from the default registry
> using no authentication.
> ---
> 
> Changed in v2:
> 
>  - Rebase against master, instead of (unpushed) docker volume patch
> 
>  libvirt-sandbox/image/cli.py  |  71 +
>  libvirt-sandbox/image/sources/DockerSource.py | 142 
> ++
>  libvirt-sandbox/image/sources/Source.py   |  29 +++---
>  libvirt-sandbox/image/template.py | 110 

Missing change in libvirt-sandbox/image/Makefile.am to add template.py.
As is that file isn't installed.

I'm also just realizing that we didn't add Eren't commit for the
virt-sandbox-image man page. Adding it later is fine, but we need to
keep that on our radar.

>  4 files changed, 228 insertions(+), 124 deletions(-)
>  create mode 100644 libvirt-sandbox/image/template.py
> 
> diff --git a/libvirt-sandbox/image/cli.py b/libvirt-sandbox/image/cli.py
> index 1718cc5..4d02289 100755
> --- a/libvirt-sandbox/image/cli.py
> +++ b/libvirt-sandbox/image/cli.py
> @@ -3,7 +3,7 @@
>  # Authors: Daniel P. Berrange 
>  #  Eren Yagdiran 
>  #
> -# Copyright (C) 2013 Red Hat, Inc.
> +# Copyright (C) 2013-2015 Red Hat, Inc.
>  # Copyright (C) 2015 Universitat Politècnica de Catalunya.
>  #
>  # This program is free software; you can redistribute it and/or modify
> @@ -34,6 +34,8 @@ import subprocess
>  import random
>  import string
>  
> +from libvirt_sandbox.image import template
> +
>  if os.geteuid() == 0:
>  default_template_dir = "/var/lib/libvirt/templates"
>  default_image_dir = "/var/lib/libvirt/images"
> @@ -44,15 +46,6 @@ else:
>  debug = False
>  verbose = False
>  
> -import importlib
> -def dynamic_source_loader(name):
> -name = name[0].upper() + name[1:]
> -modname = "libvirt_sandbox.image.sources." + name + "Source"
> -mod = importlib.import_module(modname)
> -classname = name + "Source"
> -classimpl = getattr(mod, classname)
> -return classimpl()
> -
>  gettext.bindtextdomain("libvirt-sandbox", "/usr/share/locale")
>  gettext.textdomain("libvirt-sandbox")
>  try:
> @@ -73,11 +66,10 @@ def info(msg):
>  
>  def download(args):
>  try:
> -
> dynamic_source_loader(args.source).download_template(templatename=args.template,
> - 
> templatedir=args.template_dir,
> - 
> registry=args.registry,
> - 
> username=args.username,
> - 
> password=args.password)
> +tmpl = template.Template.from_uri(args.template)
> +source = tmpl.get_source_impl()
> +source.download_template(template=tmpl,
> + templatedir=args.template_dir)
>  except IOError,e:
>  print "Source %s cannot be found in given path" %args.source
>  except Exception,e:
> @@ -85,17 +77,21 @@ def download(args):
>  
>  def delete(args):
>  try:
> -
> dynamic_source_loader(args.source).delete_template(templatename=args.template,
> -   
> templatedir=args.template_dir)
> +tmpl = template.Template.from_uri(args.template)
> +source = tmpl.get_source_impl()
> +source.delete_template(template=tmpl,
> +   templatedir=args.template_dir)
>  except Exception,e:
>  print "Delete Error %s", str(e)
>  
>  def create(args):
>  try:
> -
> dynamic_source_loader(args.source).create_template(templatename=args.template,
> -   
> templatedir=args.template_dir,
> -   
> connect=args.connect,
> -   
> format=args.format)
> +tmpl = template.Template.from_uri(args.template)
> +source = tmpl.get_source_impl()
> +source.create_template(template=tmpl,
> +   

Re: [libvirt] [PATCH sandbox] docker: don't assume X-Docker-Token is set

[Cedric Bosdonnat]

On Mon, 2015-09-21 at 15:12 +0100, Daniel P. Berrange wrote:
> The Red Hat docker registry (registry.access.redhat.com) does
> not set any X-Docker-Token HTTP header in its responses. Change
> the code so it only passes around this header if it is actually
> present.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  libvirt-sandbox/image/sources/DockerSource.py | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/libvirt-sandbox/image/sources/DockerSource.py 
> b/libvirt-sandbox/image/sources/DockerSource.py
> index f367c8f..78b2a53 100644
> --- a/libvirt-sandbox/image/sources/DockerSource.py
> +++ b/libvirt-sandbox/image/sources/DockerSource.py
> @@ -83,10 +83,14 @@ class DockerSource(Source):
>  checksums = {}
>  for layer in data:
>  pass
> +
> +headers = {}
> +if token is not None:
> +headers["Authorization"] = "Token" + token
>  (data, res) = self._get_json(template,
>   registryendpoint,
>   "/v1/repositories" + template.path + 
> "/tags",
> - { "Authorization": "Token " + token })
> + headers)
>  
>  cookie = res.info().getheader('Set-Cookie')
>  
> @@ -98,7 +102,7 @@ class DockerSource(Source):
>  (data, res) = self._get_json(template,
>   registryendpoint,
>   "/v1/images/" + imagetagid + 
> "/ancestry",
> - { "Authorization": "Token "+ token })
> + headers)
>  
>  if data[0] != imagetagid:
>  raise ValueError(["Expected first layer id '%s' to match image 
> id '%s'",
> @@ -121,7 +125,7 @@ class DockerSource(Source):
>  res = self._save_data(template,
>registryendpoint,
>"/v1/images/" + layerid + "/json",
> -  { "Authorization": "Token " + 
> token },
> +  headers,
>jsonfile)
>  createdFiles.append(jsonfile)
>  
> @@ -134,7 +138,7 @@ class DockerSource(Source):
>  self._save_data(template,
>  registryendpoint,
>  "/v1/images/" + layerid + "/layer",
> -{ "Authorization": "Token "+token },
> +headers,
>  datafile, datacsum, layersize)
>  createdFiles.append(datafile)
>  

ACK

--
Cedric

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

[libvirt] New software based on libvirt

[Gustav Fransson Nyvell]

Hello,

I'm introducing to you the decentralized cloud Cherrypop.

Combining libvirt and LizardFS (as of now) it becomes a cloud completely 
without masters. Thus, any node is sufficient for the cloud to be up

and therefore no wasted resources and no single point of failure.

It's still pretty crude software but will work with some tinkering. Hope 
you try it and like it!


For more information, source and binary: 
https://github.com/gustavfranssonnyvell/cherrypop


//Gustav

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

Re: [libvirt] [libvirt-python][PATCH] generator: fix build fail with old xml lib

[Michal Privoznik]

On 02.09.2015 07:58, Luyao Huang wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1222795#c6
> 
> if build libvirt-python with some old xml lib (python-pyxml),
> build will fail and error like this:
> 
>   File "generator.py", line 139, in start
> if "string" in attrs:
>   File "/usr/local/lib/python2.7/site-packages/_xmlplus/sax/xmlreader.py" 
> \
> , line 316, in __getitem__
> return self._attrs[name]
> KeyError: 0
> 
> This is an old issue and have been mentioned in commit 3ae0a76d.
> There is no __contains__ in class AttributesImpl, python will use
> __getitem__ in this place, so we will get error.
> Let's use 'YYY in XXX.keys()' to avoid this issue.
> 
> Signed-off-by: Luyao Huang 
> ---
>  generator.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/generator.py b/generator.py
> index 2fc838c..d9ae17e 100755
> --- a/generator.py
> +++ b/generator.py
> @@ -136,7 +136,7 @@ class docParser(xml.sax.handler.ContentHandler):
>  elif attrs['file'] == "libvirt-qemu":
>  qemu_enum(attrs['type'],attrs['name'],attrs['value'])
>  elif tag == "macro":
> -if "string" in attrs:
> +if "string" in attrs.keys():
>  params.append((attrs['name'], attrs['string']))
>  
>  def end(self, tag):
> 

ACKed and pushed.

Michal

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

Re: [libvirt] [PATCH 3/3] virsh: Notify users about disconnects

[John Ferlan]

On 09/17/2015 08:23 AM, Jiri Denemark wrote:
> After my "client rpc: Report proper error for keepalive disconnections"
> patch, virsh would no long print a warning when it closes a connection
> to a daemon after a keepalive timeout. Although the warning
> 
> virsh # 2015-09-15 10:59:26.729+: 642080: info :
> libvirt version: 1.2.19
> 2015-09-15 10:59:26.729+: 642080: warning :
> virKeepAliveTimerInternal:143 : No response from client
> 0x7efdc0a46730 after 1 keepalive messages in 2 seconds
> 
> was pretty ugly, it was still useful. This patch brings the useful part
> back while making it much nicer:
> 
> virsh # error: Disconnected from qemu:///system due to keepalive timeout
> 
> Signed-off-by: Jiri Denemark 
> ---
>  tools/virsh.c | 35 ---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 

Patch series seems reasonable to me, except for one minor thing
Found by my coverity checker...  Naturally

> diff --git a/tools/virsh.c b/tools/virsh.c
> index bb12dec..23436ea 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -95,12 +95,41 @@ static int disconnected; /* we may have been disconnected 
> */
>   * handler, just save the fact it was raised.
>   */
>  static void
> -virshCatchDisconnect(virConnectPtr conn ATTRIBUTE_UNUSED,
> +virshCatchDisconnect(virConnectPtr conn,
>   int reason,
> - void *opaque ATTRIBUTE_UNUSED)
> + void *opaque)
>  {
> -if (reason != VIR_CONNECT_CLOSE_REASON_CLIENT)
> +if (reason != VIR_CONNECT_CLOSE_REASON_CLIENT) {

[1]

> +vshControl *ctl = opaque;
> +const char *str = "unknown reason";
> +virErrorPtr error;
> +char *uri;
> +
> +error = virSaveLastError();
> +uri = virConnectGetURI(conn);
> +
> +switch ((virConnectCloseReason) reason) {
> +case VIR_CONNECT_CLOSE_REASON_ERROR:
> +str = N_("Disconnected from %s due to I/O error");
> +break;
> +case VIR_CONNECT_CLOSE_REASON_EOF:
> +str = N_("Disconnected from %s due to end of file");
> +break;
> +case VIR_CONNECT_CLOSE_REASON_KEEPALIVE:
> +str = N_("Disconnected from %s due to keepalive timeout");
> +break;
> +case VIR_CONNECT_CLOSE_REASON_CLIENT:

[1]^^^ Coverity complains about DEADCODE

Other than setting str = NULL and testing for it later, I'm not exactly
sure of the 'best' course of action.  e.g., if (str) { vshError();
disconnected++; }

I think if this is handled, then the series is ACK-able.

John

> +case VIR_CONNECT_CLOSE_REASON_LAST:
> +break;
> +}
> +vshError(ctl, _(str), NULLSTR(uri));
> +
> +if (error) {
> +virSetError(error);
> +virFreeError(error);
> +}
>  disconnected++;
> +}
>  }
>  
>  /* Main Function which should be used for connecting.
> 

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

Re: [libvirt] [PATCH v2] Close the source fd if the destination qemu exits during tunnelled migration

[Shivaprasad bhat]

Thanks John for the comments.


On Fri, Sep 18, 2015 at 10:34 PM, John Ferlan  wrote:
>
>
> On 09/14/2015 10:44 AM, Shivaprasad G Bhat wrote:
>> Tunnelled migration can hang if the destination qemu exits despite all the
>> ABI checks. This happens whenever the destination qemu exits before the
>> complete transfer is noticed by source qemu. The savevm state checks at
>> runtime can fail at destination and cause qemu to error out.
>> The source qemu cant notice it as the EPIPE is not propogated to it.
>> The qemuMigrationIOFunc() notices the stream being broken from 
>> virStreamSend()
>> and it cleans up the stream alone. The qemuMigrationWaitForCompletion() would
>> never get to 100% transfer completion.
>> The qemuMigrationWaitForCompletion() never breaks out as well since
>> the ssh connection to destination is healthy, and the source qemu also thinks
>> the migration is ongoing as the Fd to which it transfers, is never
>> closed or broken. So, the migration will hang forever. Even Ctrl-C on the
>> virsh migrate wouldn't be honoured. Close the source side FD when there is
>> an error in the stream. That way, the source qemu updates itself and
>> qemuMigrationWaitForCompletion() notices the failure.
>>
>> Close the FD for all kinds of errors to be sure. The error message is not
>> copied for EPIPE so that the destination error is copied instead later.
>>
>> Note:
>> Reproducible with repeated migrations between Power hosts running in 
>> different
>> subcores-per-core modes.
>>
>> Changes from v1 -> v2:
>>VIR_FORCE_CLOSE() was called twice for this use case which would log
>>unneccessary warnings. So, move the fd close to qemuMigrationIOFunc
>>so that there are no unnecessary duplicate attempts.(Would this trigger
>>a Coverity error? I don't have a setup to check.)
>>
>> Signed-off-by: Shivaprasad G Bhat 
>> ---
>>  src/qemu/qemu_migration.c |8 ++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>> index ff89ab5..9602fb2 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -4012,6 +4012,7 @@ static void qemuMigrationIOFunc(void *arg)
>>  if (virStreamFinish(data->st) < 0)
>>  goto error;
>>
>> +VIR_FORCE_CLOSE(data->sock);
>>  VIR_FREE(buffer);
>>
>>  return;
>> @@ -4029,7 +4030,11 @@ static void qemuMigrationIOFunc(void *arg)
>>  }
>>
>>   error:
>> -virCopyLastError(>err);
>> +/* Let the source qemu know that the transfer cant continue anymore.
>> + * Don't copy the error for EPIPE as destination has the actual error. 
>> */
>> +VIR_FORCE_CLOSE(data->sock);
>> +if (!virLastErrorIsSystemErrno(EPIPE))
>> +virCopyLastError(>err);
>>  virResetLastError();
>>  VIR_FREE(buffer);
>>  }
>> @@ -4397,7 +4402,6 @@ qemuMigrationRun(virQEMUDriverPtr driver,
>>  if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0)
>>  ret = -1;
>>  }
>> -VIR_FORCE_CLOSE(fd);
>
> ^^^
>
> This causes Coverity to claim a RESOURCE_LEAK
>
> Feels like this was a mistake edit...
>

The VIR_FORCE_CLOSE() inside qemuMigrationIOFunc() alone is sufficient.
Having this again here would lead to Warning in the logs. I too thought coverity
would complain. Is there a way to force ignore a coverity warning?

Thanks and Regards,
Shivaprasad

> The rest of the patch looks reasonable; however, I'm far from the expert
> in these matters.
>
> John
>>
>>  if (priv->job.completed) {
>>  qemuDomainJobInfoUpdateTime(priv->job.completed);
>>
>> --
>> 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

Re: [libvirt] [PATCH] vz: set mount point for container image-based disks

[Daniel P. Berrange]

On Mon, Sep 21, 2015 at 12:14:57PM +0300, Maxim Nestratov wrote:
> 21.09.2015 11:44, Daniel P. Berrange пишет:
> >On Sun, Sep 20, 2015 at 10:17:51PM +0300, Maxim Nestratov wrote:
> >>From: Maxim Nestratov 
> >>
> >>In order to support not only root disks with type=file for containers,
> >>we need to specify mount points for them.
> >>For instance, if a secondary disk is added by the following record in
> >>xml:
> >>
> >> 
> >>   
> >>   
> >>   
> >> 
> >>
> >>we are going to add it to container mounted to '/mnt/sdb' path.
> >That's not what the  element is for.   is about exposing
> >block device nodes to the container. It shouldn't try todo anything
> >with this device nodes. They might be used as raw data storage by
> >an application, so we can't assume they should be mounted.
> >
> >If you want to mount things then you should be using 
> >instead.
> >
> Hm. It actually means that any disks with type file shouldn't work in
> containers. Right?

No, the disk source on the host is not correlated to how it is
exposed to the guest.

With  it means you have to setup some kind of
loop device in the host OS, and then expose the block device to
the guest with that. The same if you have 
then you have to use host kernel or qemu-nbd, for example, to
setup a block device that you can then expose to the guest.

> And working root disks like this is a mistake? But why?

Yes, that would be a mistake too. The root filesystem should
be exposed using  with a  of /

> In vz, any images plugged into containers are also treated as disks. The
> only difference between 'filesystem' and 'disk' is whether we should mount
> it or not. That's all. While from point of view of a container user it is
> just another storage. Why not  just mount it automatically?

The  element is intended to expose raw block devices to the guest.

The  is intended to expose mounted volumed to the guest.

A  can support both type=block and type=file as sources on the
host side, as well as others like type=network.

A  can support both type=block and type=file as sources
on the host, as well as a few others.

If you make  automatically mount volumes, then you've just
discarded the only semantic difference between  and
, which is not only wrong but also pointless. If you want
to mount it, you should just use , and not try to
make  do the same as .

It is key that  does *not* try to interpret what todo with
the storage - it is entirely upto the guest to decide what todo
with it. For example, an oracle database might decide to use the
block device directly as data storage. Or you might be running an
application that wants to manipulate the filesystem (eg a fsck
tool) inside the block dev, so again it would be inappropriate to
mount it.

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

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

Re: [libvirt] [PATCH] vz: set mount point for container image-based disks

[Daniel P. Berrange]

On Sun, Sep 20, 2015 at 10:17:51PM +0300, Maxim Nestratov wrote:
> From: Maxim Nestratov 
> 
> In order to support not only root disks with type=file for containers,
> we need to specify mount points for them.
> For instance, if a secondary disk is added by the following record in
> xml:
> 
> 
>   
>   
>   
> 
> 
> we are going to add it to container mounted to '/mnt/sdb' path.

That's not what the  element is for.   is about exposing
block device nodes to the container. It shouldn't try todo anything
with this device nodes. They might be used as raw data storage by
an application, so we can't assume they should be mounted.

If you want to mount things then you should be using 
instead.

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

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

Re: [libvirt] [PATCH] vz: set mount point for container image-based disks

[Maxim Nestratov]

21.09.2015 11:44, Daniel P. Berrange пишет:

On Sun, Sep 20, 2015 at 10:17:51PM +0300, Maxim Nestratov wrote:

From: Maxim Nestratov 

In order to support not only root disks with type=file for containers,
we need to specify mount points for them.
For instance, if a secondary disk is added by the following record in
xml:

 
   
   
   
 

we are going to add it to container mounted to '/mnt/sdb' path.

That's not what the  element is for.   is about exposing
block device nodes to the container. It shouldn't try todo anything
with this device nodes. They might be used as raw data storage by
an application, so we can't assume they should be mounted.

If you want to mount things then you should be using 
instead.

Regards,
Daniel
Hm. It actually means that any disks with type file shouldn't work in 
containers. Right?

And working root disks like this is a mistake? But why?
In vz, any images plugged into containers are also treated as disks. The 
only
difference between 'filesystem' and 'disk' is whether we should mount it 
or not. That's
all. While from point of view of a container user it is just another 
storage. Why not

just mount it automatically?

Best,
Maxim

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