Re: [libvirt] [PATCH 0/2] rework the check for the numa cpus

[lhuang]

On 04/20/2015 10:39 PM, Michal Privoznik wrote:

On 02.04.2015 10:02, Luyao Huang wrote:

We introduce a check for numa cpu total count in 5f7b71,
But seems this check cannot work well. There are some scenarioes:

1. one of cpu id is out of maxvcpus, can set success(cpu count = 5  10):

vcpu placement='static'10/vcpu
cell id='0' cpus='0-3,100' memory='512000' unit='KiB'/

the cpus '100' exceed maxvcpus, this setting is not valid (although qemu
do not output error).

2. use the same cpu in 2 cell, can set success(cpu count = 8  10):
vcpu placement='static'10/vcpu
cell id='0' cpus='0-3' memory='512000' unit='KiB'/
cell id='1' cpus='0-3' memory='512000' unit='KiB'/

I guess nobody will use this setting if he really want use numa in his
vm. (qemu not output error, but we can find some interesting things in
vm, all cpus have bind in one numa node)

3. use the same cpu in 2 cell, cannot set success(cpu count = 11  10):
vcpu placement='static'10/vcpu
cell id='0' cpus='0-6' memory='512000' unit='KiB'/
cell id='1' cpus='0-3' memory='512000' unit='KiB'/

No need forbid this scenario if scenario 2 is a 'valid' setting.

However add new check during parse xml will make vm has broken settings
disappear after update libvirtd, so possible solutions:

1. add new check when parse xml, and find a way to avoid vm evaporate.
I chose this way and i don't think just drop the invalid settings is a good
idea for numa cpus, so i add a new function.

2. add new check when start vm.
I think this is a configuration issue, so no need report it so late.

3. just remove this check and do not add new check... :)

Luyao Huang (2):
   conf: introduce a new function to add check avoid losing track
   conf: rework the cpu check for vm numa settings

  src/bhyve/bhyve_driver.c |  4 ++--
  src/conf/domain_conf.c   | 21 ++---
  src/conf/domain_conf.h   |  1 +
  src/conf/numa_conf.c | 37 ++---
  src/conf/numa_conf.h |  2 +-
  src/esx/esx_driver.c |  2 +-
  src/libxl/libxl_driver.c |  4 ++--
  src/lxc/lxc_driver.c |  4 ++--
  src/openvz/openvz_driver.c   |  4 ++--
  src/parallels/parallels_driver.c |  2 +-
  src/phyp/phyp_driver.c   |  2 +-
  src/qemu/qemu_driver.c   |  4 ++--
  src/test/test_driver.c   |  4 ++--
  src/uml/uml_driver.c |  4 ++--
  src/vbox/vbox_common.c   |  2 +-
  src/vmware/vmware_driver.c   |  4 ++--
  src/xen/xen_driver.c |  4 ++--
  src/xenapi/xenapi_driver.c   |  4 ++--
  18 files changed, 70 insertions(+), 39 deletions(-)


I agree with renaming and extending the virDomainNumaGetCPUCountTotal().
Even the diff in 2/2 shows the function is utterly broken. However, I
think this function can be called unconditionally - even when the flag
is not set. Having said that, the flag is redundant.


Oh, good news to me :)

So if this function can be called unconditionally, there is no reason to 
introduce this new flag.




Moreover, when sending a patchset, the sources should compile cleanly
after each patch. This is not the case with this one.


Get it, i will pay more attention for this things next time, thanks for 
pointing out that.



Looking forward to v2.


Thanks for your review and advise, i will propose a new version these days.


Michal


Luyao

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

[libvirt] libvirt-0.10.2-46 compress failed

[饶俊明]

Hello

When compress libvirt-0.10.2-46.el6.src.rpm to support rbd, then failed;

Error log:
cc1: warnings being treated as errors
storage/storage_backend_rbd.c: In function 'virStorageBackendRBDRefreshPool':
storage/storage_backend_rbd.c:284: error: declaration of 'stat' shadows a 
global declaration [-Wshadow]
/usr/include/sys/stat.h:455: error: shadowed declaration is here [-Wshadow]
At top level:
cc1: error: unrecognized command line option -Wno-suggest-attribute=const
cc1: error: unrecognized command line option -Wno-suggest-attribute=pure
make[3]: *** [libvirt_driver_storage_impl_la-storage_backend_rbd.lo] Error 1
make[3]: *** Waiting for unfinished jobs
make[3]: Leaving directory `/home/ada/rpmbuild/BUILD/libvirt-0.10.2/src'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/home/ada/rpmbuild/BUILD/libvirt-0.10.2/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/ada/rpmbuild/BUILD/libvirt-0.10.2'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.g0IuFq (%build)
RPM build errors:
Bad exit status from /var/tmp/rpm-tmp.g0IuFq (%build)

Thanks!!!





The information in this email is confidential and may be legally privileged. If 
you have received this email in error or are not the intended recipient, please 
immediately notify the sender and delete this message from your computer. Any 
use, distribution, or copying of this email other than by the intended 
recipient is strictly prohibited. All messages sent to and from us may be 
monitored to ensure compliance with internal policies and to protect our 
business. 
Emails are not secure and cannot be guaranteed to be error free as they can be 
intercepted, amended, lost or destroyed, or contain viruses. Anyone who 
communicates with us by email is taken to accept these risks. 

��跺�浠惰��璇锋敞���锛�
��浠跺��淇�瀵�淇℃��锛���ヨ舵�浠讹��璇峰�″���ュ�浜哄苟��存�ュ伙��涓�寰�浣跨�ㄣ��浼���澶���舵�浠躲��
杩���洪��浠跺���版���稿��瑙��с�浠跺藉�琚��琚�淇���广��涓㈠け���琚���村���璁＄虹��姣�绛�涓�瀹���ㄦ点��
 


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

Re: [libvirt] [PATCH v4 2/4] scsi: Adjust return value for virStorageBackendSCSINewLun

[Ján Tomko]

On Mon, Apr 20, 2015 at 12:50:07PM -0400, John Ferlan wrote:
 
 ...
 
  +/* Check if the pool is using a stable target path. The call to
  + * virStorageBackendStablePath will fail if the pool target path
  + * isn't stable and just return the strdup'd 'devpath' anyway.
  + * This would be indistinguishable to failing to find the stable
  + * path to the device if the virDirRead loop to search the
  + * target pool path for our devpath had failed.
  + */
  +if (!virStorageBackendPoolPathIsStable(pool-def-target.path)) {
  +virReportError(VIR_ERR_INVALID_ARG,
  +   _(unable to use target path '%s' for dev '%s'),
  +   NULLSTR(pool-def-target.path), dev);
  +goto cleanup;
  +}
  
  /dev is a valid non-stable pool target path.
  
  +
   if (VIR_ALLOC(vol)  0)
   goto cleanup;
   
  @@ -187,13 +211,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr 
  pool,
   true)) == NULL)
   goto cleanup;
   
  -if (STREQ(devpath, vol-target.path) 
  -!(STREQ(pool-def-target.path, /dev) ||
  -  STREQ(pool-def-target.path, /dev/))) {
  +if (STREQ(devpath, vol-target.path)) {
   
  
  Before, when virStorageBackendStablePath returned the same devpath because
  the pool path was /dev, we continued with processing the volume.
  
  After this patch, we won't even get here because of the first check.
  
  Failure to stabilize the path should be expected here, if the pool
  target path is not stable.
  
 
 OK, but because virStorageBackendStablePath won't process the
 pool-target.path of /dev, we'll return the duplicated 'devpath' and
 return -2 for every volume in the pool thus making it look empty.
 
 What good is that?
 

None. We should process the volume instead of returning -2.

Jan

 Wouldn't it be better to tell the user that /dev is not a valid stable
 path name... The path really needs to be more specific... I suppose one
 could change virStorageBackendStablePath to accept /dev and do the
 search, but that could take a while depending on the size of the
 /dev file system.
 
 John
 
 --
 libvir-list mailing list
 libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [PATCH v4 1/4] storage: Split out the valid path check

[Ján Tomko]

On Mon, Apr 20, 2015 at 12:54:46PM -0400, John Ferlan wrote:
 
 
 On 04/20/2015 12:23 PM, Eric Blake wrote:
  On 04/19/2015 06:38 PM, John Ferlan wrote:
  For virStorageBackendStablePath, in order to make decisions in other code
  split out the checks regarding whether the pool's target is empty, using 
  /dev,
  using /dev/, or doesn't start with /dev
 
  Signed-off-by: John Ferlan jfer...@redhat.com
  ---
   src/storage/storage_backend.c | 26 +-
   src/storage/storage_backend.h |  1 +
   2 files changed, 14 insertions(+), 13 deletions(-)
 
  diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
  index 0435983..b07e0d9 100644
  --- a/src/storage/storage_backend.c
  +++ b/src/storage/storage_backend.c
  @@ -1674,6 +1674,17 @@ 
  virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
   return 0;
   }
   
  +bool
  +virStorageBackendPoolPathIsStable(const char *path)
  +{
  +if (path == NULL || STREQ(path, /dev) || STREQ(path, /dev/))
  +return false;
  +
  +if (!STRPREFIX(path, /dev))
  +return false;
  
  I think you want /dev/ here as the prefix to be required; otherwise,
  /device would match the prefix.  (This also means that someone using
  //dev/... would fail the check, but that's probably something we don't
  need to worry about).
  
 Hmm... Sure I see that... I can make that adjustment. I'll wait a bit
 before pushing just so see if there's other feedback...
 

I think that change should be separate from this code motion.

Jan


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

Re: [libvirt] [PATCH v4 4/4] scsi: Adjust return values from processLU

[Ján Tomko]

On Sun, Apr 19, 2015 at 08:38:36PM -0400, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1171933
 
 Adjust the processLU error returns to be a bit more logical. Currently,
 the calling code cannot determine the difference between a non disk/lun
 volume and a processed/found disk/lun. It can also not differentiate
 between perhaps real/fatal error and one that won't necessarily stop
 the code from finding other volumes.
 
 After this patch virStorageBackendSCSIFindLUsInternal will stop processing
 as soon as a fatal message occurs rather than continuting processing
 for no apparent reason. It will also only set the *found value when
 at least one of the processLU's was successful.
 
 With the failed return, if the reason for the stop was that the pool
 target path did not exist, was /dev, was /dev/, or did not start with
 /dev, then iSCSI pool startup and refresh will fail.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/storage/storage_backend_scsi.c | 38 
 +-
  1 file changed, 25 insertions(+), 13 deletions(-)
 

ACK

Jan


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

Re: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads

[John Ferlan]

On 04/21/2015 09:45 AM, Peter Krempa wrote:
 On Tue, Apr 14, 2015 at 21:18:25 -0400, John Ferlan wrote:
 Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or
 remove an IOThread to/from the host either for live or config optoins

 The implementation for the 'live' option will use the iothreadpids list
 in order to make decision, while the 'config' option will use the
 iothreadids list.  Additionally, for deletion each may have to adjust
 the iothreadpin list.

 IOThreads are implemented by qmp objects, the code makes use of the existing
 qemuMonitorAddObject or qemuMonitorDelObject APIs.

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/conf/domain_audit.c  |   9 +
  src/conf/domain_audit.h  |   6 +
  src/libvirt_private.syms |   1 +
  src/qemu/qemu_driver.c   | 431 
 +++
  4 files changed, 447 insertions(+)

 
 ...
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 008258f..f42d4fb 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -6179,6 +6179,435 @@ qemuDomainPinIOThread(virDomainPtr dom,
  return ret;
  }
  
 +static int
 +qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
 + virDomainObjPtr vm,
 + unsigned int iothread_id)
 +{
 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +char *alias = NULL;
 +size_t idx;
 +int rc = -1;
 +int ret = -1;
 +unsigned int orig_niothreads = vm-def-iothreads;
 +unsigned int exp_niothreads = vm-def-iothreads;
 +int new_niothreads = 0;
 +qemuMonitorIOThreadInfoPtr *new_iothreads = NULL;
 +unsigned int idval = 0;
 +virCgroupPtr cgroup_iothread = NULL;
 +char *mem_mask = NULL;
 +virDomainIOThreadIDDefPtr iothrid;
 +
 +if (virDomainIOThreadIDFind(vm-def, iothread_id)) {
 +virReportError(VIR_ERR_INVALID_ARG,
 +   _(an IOThread is already using iothread_id '%u'),
 +   iothread_id);
 +goto cleanup;
 +}
 +
 +if (virAsprintf(alias, iothread%u, iothread_id)  0)
 +return -1;
 +
 +qemuDomainObjEnterMonitor(driver, vm);
 +
 +rc = qemuMonitorAddObject(priv-mon, iothread, alias, NULL);
 +exp_niothreads++;
 +if (rc  0)
 +goto exit_monitor;
 +
 +/* After hotplugging the IOThreads we need to re-detect the
 + * IOThreads thread_id's, adjust the cgroups, thread affinity,
 + * and add the thread_id to the vm-def-iothreadids list.
 + */
 +if ((new_niothreads = qemuMonitorGetIOThreads(priv-mon,
 +  new_iothreads))  0)
 
 Since we are not doing any fancy iothread naming, this function can
 parse the iothread IDs from the alias right away ... [1]
 
 
 +goto exit_monitor;
 +
 +if (qemuDomainObjExitMonitor(driver, vm)  0)
 +goto cleanup;
 +
 +if (new_niothreads != exp_niothreads) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(got wrong number of IOThread ids from QEMU 
 monitor. 
 + got %d, wanted %d),
 +   new_niothreads, exp_niothreads);
 +vm-def-iothreads = new_niothreads;
 +goto cleanup;
 +}
 +vm-def-iothreads = exp_niothreads;
 +
 +if (virDomainNumatuneGetMode(vm-def-numa, -1) ==
 +VIR_DOMAIN_NUMATUNE_MEM_STRICT 
 +virDomainNumatuneMaybeFormatNodeset(vm-def-numa,
 +priv-autoNodeset,
 +mem_mask, -1)  0)
 +goto cleanup;
 +
 +
 +/*
 + * If we've successfully added an IOThread, find out where we added it
 + * in the QEMU IOThread list, so we can add it to our iothreadids list
 + */
 
 The message seems obvious when looking at the code.
 
 +for (idx = 0; idx  new_niothreads; idx++) {
 +if (qemuDomainParseIOThreadAlias(new_iothreads[idx]-name, idval) 
  0)
 
 ... [1] so that you don't have to do it manually.
 

IOW: 
if (STREQ(alias, new_iothreads[idx]-name))
break


 +goto cleanup;
 +if (iothread_id == idval)
 +break;
 +}
 +
 +if (idval != iothread_id) {

And of course idx != new_niothreads, plus removing 'idval'

 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(cannot find new IOThread '%u' in QEMU monitor.),
 +   iothread_id);
 +goto cleanup;
 +}
 +
 +if (virDomainIOThreadIDAdd(vm-def, iothread_id)  0)
 
 virDomainIOThreadIDAdd could return the pointer to the created item ...
 

OK, now we have

if (!(iothrid = virDomainIOThreadIDAdd(vm-def, iothread_id)))
goto cleanup;

iothrid-thread_id = new_iothreads[idx]-thread_id;

 +goto cleanup;
 +
 +if (!(iothrid = virDomainIOThreadIDFind(vm-def, iothread_id))) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(cannot find 

Re: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads

[John Ferlan]

...

 This is the reason why I've requested in the previous review that the
 pinning information would be merged into the iothread data structure.
 You then would not have to synchronise two data structures.

 
 Understood ...  I started down that path, but then got bogged down in cputune
 information inside iothreadid's and the difference with the vCPU code.  So I
 kept it this way to reduce the number of changes.
 
 I think it's worthy of being done, but I hope a follow-up patch will be 
 acceptable. 
 

ug... starting looking at it again with the existing view of
iothreadid's fresh in mind and it seems I'll have to jump in during this
series because there's a check in virDomainIOThreadPinDefParseXML for
pin-id  iothreads, which won't be true once Add hits, so I'll work
it in now sigh

John

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

[libvirt] [PATCH] vircommand: fix polling in virCommandProcessIO

[Roman Bogorodskiy]

When running on FreeBSD, there's a bug in virCommandProcessIO
polling that is triggered by the commandtest.

A test that triggers EPIPE in commandtest (named test20) hungs
forever on FreeBSD.

Apparently, this happens because FreeBSD sets POLLHUP flag on revents
when stdin in closed. And as the current implementation only checks for
POLLOUT and POLLERR, it ends up looping forever inside
virCommandProcessIO and not trying to do one more write() that would
trigger EPIPE.

To fix that check for the POLLHUP flag along with POLLOUT and POLLERR.
---
 src/util/vircommand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 648f5ed..f9b3c3f 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -2097,7 +2097,7 @@ virCommandProcessIO(virCommandPtr cmd)
 }
 }
 
-if (fds[i].revents  (POLLOUT | POLLERR) 
+if (fds[i].revents  (POLLOUT | POLLHUP | POLLERR) 
 fds[i].fd == cmd-inpipe) {
 int done;
 
-- 
2.3.5

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

Re: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads

[John Ferlan]

 +goto cleanup;
 +}
 +
 +iothrid-thread_id = new_iothreads[idx]-thread_id;
 
 You are also not marking the thread structure as 'defined' and thus
 wouldn't format it later ...
 
 

Changed to 'undefined' now, so this is unnecessary

(although perhaps 'autofill' is a better term

John

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

Re: [libvirt] [PATCH 1/7] virutil: Introduce virIsValidHostname

[Peter Krempa]

On Mon, Apr 20, 2015 at 15:25:29 -0400, John Ferlan wrote:
 
 
  As a hindsight from reviewing 6/7. This function should also be in
  virsocketaddr.c
 
 
  
  
  hmmm.. yes I see.. Guess I got hung up on virSocketAddr... and didn't
  focus as closely on the implementation where virSocketAddrParse can take
  NULL as the first parameter... Guess, that means patches 2-5 can just be
  called as:
  
  if (virSocketAddrParse(NULL, pool-def-source.hosts[0].name,
  AF_UNSPEC)  0)
  
  
 
 Without actually trying it - seemed like a good idea; however, the 
 virSocketAddrParse uses/sets hints.ai_flags = AI_NUMERICHOST; thus
 it requires a numeric value and not one that could be a name or a
 number, so it seems this particular code cannot use it.

Well, you still can add a new helper that will allow to use your
approach.

 
 I really see those virSocketAddr* API's as different, very specific
 to supporting the network socket's and socket address formats; whereas,
 this code will take a string representation of either the name or
 the number as provided in the XML and validate it.

We still can make it a more universal helper for address conversions.

 
 I don't think this set of API's belongs there as it's not manipulating
 virSocketAddr's.

The general utils file used to be a dumping place for all the various
helpers, but now we are usually trying to put them together with the
semanticaly similar helpers, or making a group of them to make them
semantically similar. I think it makes sense in this case and the
general utils file is not the right place for this helper.

 So, I'll change the function intro to:
 
  * Unlike virGetHostname, this variant of the code receives a hostname and
  * retrieves the getaddrinfo. If the passed hostname can be successfully
  * resolved via getaddrinfo, then return true; otherwise, if the hostname
  * cannot be resolved for any reason, return false.
 
 and 
 
 remove the localhost specific checking
 
 and adjust the commit message to remove the 'getnameinfo' reference.
 
 John

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 4/7] gluster: Check for validity of pool source hostname

[Peter Krempa]

On Mon, Apr 20, 2015 at 15:39:23 -0400, John Ferlan wrote:
 
 
 On 04/20/2015 09:45 AM, Peter Krempa wrote:
  On Sun, Apr 19, 2015 at 20:49:09 -0400, John Ferlan wrote:
  Prior to attempting to open the gluster connection, let's make sure we
  can resolve the source pool hostname.
 
  Signed-off-by: John Ferlan jfer...@redhat.com
  ---
   src/storage/storage_backend_gluster.c | 3 +++
   1 file changed, 3 insertions(+)
 
  diff --git a/src/storage/storage_backend_gluster.c 
  b/src/storage/storage_backend_gluster.c
  index d2e79bc..e3a8c21 100644
  --- a/src/storage/storage_backend_gluster.c
  +++ b/src/storage/storage_backend_gluster.c
  @@ -96,6 +96,9 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool)
   trailing_slash = false;
   }
   
  +if (!virIsValidHostname(pool-def-source.hosts[0].name))
  +return NULL;
  +
  
  Okay, it might be marginally worth at this point, since gluster is/was
  leaking some memory and threads after it was initialized.
  
  On the other hand you'll be resolving the hostname again a few lines
  below.
  
 
 Which call, glfs_set_volfile_server?  It wasn't intuitively obvious...

glfs_init does all the magic (and leaking)


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

Re: [libvirt] [PATCH] xend: Remove a couple of unused function prototypes.

[Peter Krempa]

On Mon, Apr 20, 2015 at 17:32:21 +0100, Richard W.M. Jones wrote:
 Commit 70f446631f142ae92b4d4eb349fcf11408171556 (from 2008) introduced
 some functions for testing whether xend was returning correct sound
 models.  Those functions have long gone, but the function prototypes
 remain.  This commit removes the unused prototypes.
 
 Signed-off-by: Richard W.M. Jones rjo...@redhat.com
 ---
  src/xen/xend_internal.h | 4 
  1 file changed, 4 deletions(-)

ACK,

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 03/11] caps: Use an enum internally for ostype value

[Christophe Fergeau]

Hey,

On Fri, Apr 17, 2015 at 09:45:13PM -0400, Cole Robinson wrote:
 +VIR_ENUM_IMPL(virDomainOS, VIR_DOMAIN_OSTYPE_LAST,
 +  hvm,
 +  xen,
 +  linux,
 +  exe,
 +  uml,
 +  aix)
 +

This is preexisting before your patch, but is more obvious now.
Shouldn't 'aix' be listed in docs/schemas/capability.rng? Currently it's

  define name='ostype'
element name='os_type'
  choice
valuexen/value !-- Xen 3.0 pv --
valuelinux/value !-- same as 'xen' - legacy --
valuehvm/value !-- unmodified OS --
valueexe/value !-- For container based virt --
valueuml/value !-- user mode linux --
  /choice
/element
  /define

Christophe


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

Re: [libvirt] [PATCH 7/7] storage: Check for duplicate host for incoming pool def

[Ján Tomko]

On Mon, Apr 20, 2015 at 12:23:25PM -0400, John Ferlan wrote:
 
 
 On 04/20/2015 11:11 AM, Ján Tomko wrote:
  On Sun, Apr 19, 2015 at 08:49:12PM -0400, John Ferlan wrote:
  https://bugzilla.redhat.com/show_bug.cgi?id=1171984
 
  Use the new virIsSameHostnameInfo API to determine whether the proposed
  storage pool definition matches the existing storage pool definition
 
  Signed-off-by: John Ferlan jfer...@redhat.com
  ---
   src/conf/storage_conf.c | 11 ++-
   1 file changed, 10 insertions(+), 1 deletion(-)
 
  diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
  index 4852dfb..c1bc242 100644
  --- a/src/conf/storage_conf.c
  +++ b/src/conf/storage_conf.c
  @@ -2415,7 +2415,16 @@ 
  virStoragePoolSourceMatchSingleHost(virStoragePoolSourcePtr poolsrc,
   if (poolsrc-hosts[0].port != defsrc-hosts[0].port)
   return false;
   
  
  This function is called when parsing the configuration, which should not
  depend on host state.
  
  For example, if libvirt is started really early at boot time and the
  hostnames cannot be resolved by the DNS yet, they will pass the check
  but they will disappear on libvirtd restart.
 
 Hmm... the downside of unreliable dependencies.
 
  
  The hostname-ip pairings are not stable either, so if we do this, I
  think it should be done on pool startup, not config parsing.
  
 
 Right, but by the time we get to pool startup we'd be at the same point
 as referenced above - if done early enough at boot time, then it's not
 going to fail, but perhaps better than nothing.
 

Randomly failing to parse a config is worse than nothing.

 
 I suppose at least moving to startup allows for better error paths since
 currently errors can be overwritten or ignored.
 
 
  -return STREQ(poolsrc-hosts[0].name, defsrc-hosts[0].name);
  +if (STRNEQ(poolsrc-hosts[0].name, defsrc-hosts[0].name)) {
  +/* Matching just a name isn't reliable as someone could provide
  + * the name for one pool and the IP Address for another pool, so
  
  Resolving them is IMHO just as unreliable.
  
  Re: the original bug - is it possible to check that we have connected to
  a session with a different hostname than what we requested?
 
 What does the connected session hostname have to do with the original
 bug?  The bug requests checking that an iSCSI pool doesn't use a host
 name='some IP Address'... for one pool:
 
 host name='10.66.6.12' port='3260'/
 
 and a resolved name for another pool on the same host:
 
 host name='test1' port='3260'/
 
 Where :
 
 # cat /etc/hosts
 10.66.6.12 test1
 
 The bug pointed out iSCSI in particular, but since other pools use the
 source... host name='%s'.../... / XML formatting it seemed logical
 to have them all use the same checks.
 

The bug requests that this should be forbidden when defining the pool
(which is NOTABUG), because we then allow starting both of them, but
stopping one of them also stops the other one, while libvirt still
thinks it's up (this is the real bug).

We definitely shouldn't resolve the hostnames when we parse the XML, the
XML parser/formatter should not depend on the host state.

Resolving the hostname on pool startup would at least remove the
roulette from XML parser, but still won't be foolproof -
multiple IP addresses can point to the same host.
More importantly, it won't solve the underlying issue:

We don't care what host we connect to.

As long as virStorageBackendISCSISession finds the sourcedevice path
in the output of iscsiadm --mode session, the pool is marked as started.

While it might make sense to have the pool accessible via different
networks (or different address families), our iscsi backend is not ready
for that.

I don't know how to allow that, (without guessing what iscsiadm resolved
the hostname to), but marking the pool as started, if the session was
created by another pool is wrong.


  
  Or just disallow starting two pools with the same targetname, since they
  are supposed to be unique?
  
 
 'targetname' as in ?

The sourcedevice path='' attribute of the iscsi pool, which we
feed to iscsiadm via --targetname.

Jan


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

Re: [libvirt] [PATCH 7/7] storage: Check for duplicate host for incoming pool def

[Peter Krempa]

On Mon, Apr 20, 2015 at 12:23:25 -0400, John Ferlan wrote:
 
 
 On 04/20/2015 11:11 AM, Ján Tomko wrote:
  On Sun, Apr 19, 2015 at 08:49:12PM -0400, John Ferlan wrote:
  https://bugzilla.redhat.com/show_bug.cgi?id=1171984
 
  Use the new virIsSameHostnameInfo API to determine whether the proposed
  storage pool definition matches the existing storage pool definition
 
  Signed-off-by: John Ferlan jfer...@redhat.com
  ---
   src/conf/storage_conf.c | 11 ++-
   1 file changed, 10 insertions(+), 1 deletion(-)
 
  diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
  index 4852dfb..c1bc242 100644
  --- a/src/conf/storage_conf.c
  +++ b/src/conf/storage_conf.c
  @@ -2415,7 +2415,16 @@ 
  virStoragePoolSourceMatchSingleHost(virStoragePoolSourcePtr poolsrc,
   if (poolsrc-hosts[0].port != defsrc-hosts[0].port)
   return false;
   
  
  This function is called when parsing the configuration, which should not
  depend on host state.
  
  For example, if libvirt is started really early at boot time and the
  hostnames cannot be resolved by the DNS yet, they will pass the check
  but they will disappear on libvirtd restart.
 
 Hmm... the downside of unreliable dependencies.

Not only unreliable dependencies. The issue might also happen with
factors external to the host. For example by adding a new DNS name for
the duplicate host.

 
  
  The hostname-ip pairings are not stable either, so if we do this, I
  think it should be done on pool startup, not config parsing.
  
 
 Right, but by the time we get to pool startup we'd be at the same point
 as referenced above - if done early enough at boot time, then it's not
 going to fail, but perhaps better than nothing.

Well, that's actually not true. If the second (duplicate) pool is
starting at one point, the address either can be resolved and deemed
duplicate, or the resolution would fail anyways and the pool would not
be started.

When compared with the approach when defining the pool (or reading back
XML files):
1) if the hostname is not duplicate or not resolvable at the point when
you define the pool but changes to be duplicate later you wouldn't catch
the issue
2) if the pool source becomes duplicate in the life of the host it would
vanish after libvirt restart

Peter


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

[libvirt] [PATCH] vircapstest: fix build without LXC

[Roman Bogorodskiy]

When building without lxc support enabled, build fails with:

CLD vircapstest
vircapstest.o: In function `test_virCapsDomainDataLookupLXC':
vircapstest.c:(.text+0x9ef): undefined reference to `testLXCCapsInit'

Fix that by hiding LXC tests under appropriate #ifdef.
---
 tests/vircapstest.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/vircapstest.c b/tests/vircapstest.c
index 3e5038b..b2c321e 100644
--- a/tests/vircapstest.c
+++ b/tests/vircapstest.c
@@ -289,6 +289,7 @@ test_virCapsDomainDataLookupXen(const void *data 
ATTRIBUTE_UNUSED)
 return ret;
 }
 
+#ifdef WITH_LXC
 static int
 test_virCapsDomainDataLookupLXC(const void *data ATTRIBUTE_UNUSED)
 {
@@ -311,6 +312,7 @@ test_virCapsDomainDataLookupLXC(const void *data 
ATTRIBUTE_UNUSED)
 virObjectUnref(caps);
 return ret;
 }
+#endif /* WITH_LXC */
 
 static int
 mymain(void)
@@ -326,9 +328,11 @@ mymain(void)
 if (virtTestRun(virCapsDomainDataLookupXen,
 test_virCapsDomainDataLookupXen, NULL)  0)
 ret = -1;
+#ifdef WITH_LXC
 if (virtTestRun(virCapsDomainDataLookupLXC,
 test_virCapsDomainDataLookupLXC, NULL)  0)
 ret = -1;
+#endif /* WITH_LXC */
 
 return ret;
 }
-- 
2.3.5

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

Re: [libvirt] [PATCH 1/3] locking: relax PID requirement

[Daniel P. Berrange]

On Fri, Apr 17, 2015 at 03:36:20PM -0600, Jim Fehlig wrote:
 Some hypervisors like Xen do not have PIDs associated with domains.
 Relax the requirement for PID != 0 in the locking code so it can
 be used by hypervisors that do not represent domains as a process
 running on the host.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/locking/lock_daemon.c  |  2 +-
  src/locking/lock_daemon_dispatch.c | 49 
 +++---
  src/locking/lock_driver_lockd.c|  7 ++
  3 files changed, 17 insertions(+), 41 deletions(-)
 
 diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
 index bb165c0..042ff94 100644
 --- a/src/locking/lock_daemon.c
 +++ b/src/locking/lock_daemon.c
 @@ -678,7 +678,7 @@ virLockDaemonClientFree(void *opaque)
  signum = SIGKILL;
  else
  signum = 0;
 -if (virProcessKill(priv-clientPid, signum)  0) {
 +if (priv-clientPid != 0  virProcessKill(priv-clientPid, 
 signum)  0) {
  if (errno == ESRCH)
  break;
  
 diff --git a/src/locking/lock_daemon_dispatch.c 
 b/src/locking/lock_daemon_dispatch.c
 index a7cee9d..f2704ec 100644
 --- a/src/locking/lock_daemon_dispatch.c
 +++ b/src/locking/lock_daemon_dispatch.c
 @@ -62,11 +62,8 @@ 
 virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server 
 ATTRIBUTE_UNU
  goto cleanup;
  }
  
 -if (!priv-ownerPid) {
 -virReportError(VIR_ERR_OPERATION_INVALID, %s,
 -   _(lock owner details have not been registered));
 -goto cleanup;
 -}
 +if (!priv-ownerPid)
 +VIR_WARN(lock owner PID has not been registered);

In this file, all these ownerPid checks are really there to validate that
the virLockSpaceProtocolDispatchRegister() method has been called first.

I think we need to check that check, so rather than removing all these
error reports, lets change them all to check priv-ownerId instead of
priv-ownerPid.

  
  if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args-path))) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 @@ -120,11 +117,8 @@ 
 virLockSpaceProtocolDispatchCreateResource(virNetServerPtr server 
 ATTRIBUTE_UNUS
  goto cleanup;
  }
  
 -if (!priv-ownerPid) {
 -virReportError(VIR_ERR_OPERATION_INVALID, %s,
 -   _(lock owner details have not been registered));
 -goto cleanup;
 -}
 +if (!priv-ownerPid)
 +VIR_WARN(lock owner PID has not been registered);
  
  if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args-path))) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 @@ -169,11 +163,8 @@ 
 virLockSpaceProtocolDispatchDeleteResource(virNetServerPtr server 
 ATTRIBUTE_UNUS
  goto cleanup;
  }
  
 -if (!priv-ownerPid) {
 -virReportError(VIR_ERR_OPERATION_INVALID, %s,
 -   _(lock owner details have not been registered));
 -goto cleanup;
 -}
 +if (!priv-ownerPid)
 +VIR_WARN(lock owner PID has not been registered);
  
  if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args-path))) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 @@ -218,11 +209,8 @@ virLockSpaceProtocolDispatchNew(virNetServerPtr server 
 ATTRIBUTE_UNUSED,
  goto cleanup;
  }
  
 -if (!priv-ownerPid) {
 -virReportError(VIR_ERR_OPERATION_INVALID, %s,
 -   _(lock owner details have not been registered));
 -goto cleanup;
 -}
 +if (!priv-ownerPid)
 +VIR_WARN(lock owner PID has not been registered);
  
  if (!args-path || STREQ(args-path, )) {
  virReportError(VIR_ERR_OPERATION_INVALID, %s,
 @@ -273,11 +261,8 @@ virLockSpaceProtocolDispatchRegister(virNetServerPtr 
 server ATTRIBUTE_UNUSED,
  goto cleanup;
  }
  
 -if (priv-ownerPid) {
 -virReportError(VIR_ERR_OPERATION_INVALID, %s,
 -   _(lock owner details have already been registered));
 -goto cleanup;
 -}
 +if (priv-ownerPid)
 +VIR_WARN(lock owner PID has not been registered);
  
  if (VIR_STRDUP(priv-ownerName, args-owner.name)  0)
  goto cleanup;
 @@ -320,11 +305,8 @@ 
 virLockSpaceProtocolDispatchReleaseResource(virNetServerPtr server 
 ATTRIBUTE_UNU
  goto cleanup;
  }
  
 -if (!priv-ownerPid) {
 -virReportError(VIR_ERR_OPERATION_INVALID, %s,
 -   _(lock owner details have not been registered));
 -goto cleanup;
 -}
 +if (!priv-ownerPid)
 +VIR_WARN(lock owner PID has not been registered);
  
  if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args-path))) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 @@ -370,11 +352,8 @@ virLockSpaceProtocolDispatchRestrict(virNetServerPtr 
 server ATTRIBUTE_UNUSED,
  goto cleanup;
  }
  
 -if (!priv-ownerPid) {

Re: [libvirt] [PATCH 3/3] libxl: provide integration with lock manager

[Daniel P. Berrange]

On Fri, Apr 17, 2015 at 03:36:22PM -0600, Jim Fehlig wrote:
 Provide integration with libvirt's lock manager in the libxl driver.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/Makefile.am  | 12 +
  src/libxl/libvirtd_libxl.aug |  2 ++
  src/libxl/libxl.conf | 10 
  src/libxl/libxl_conf.c   | 14 +++
  src/libxl/libxl_conf.h   |  6 +
  src/libxl/libxl_domain.c | 47 
 ++--
  src/libxl/libxl_domain.h |  1 +
  src/libxl/libxl_driver.c | 25 +++
  src/libxl/libxl_migration.c  |  6 +
  src/libxl/test_libvirtd_libxl.aug.in |  1 +
  10 files changed, 122 insertions(+), 2 deletions(-)
 
 diff --git a/src/Makefile.am b/src/Makefile.am
 index 9a5f16c..1438174 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -2246,6 +2246,12 @@ BUILT_SOURCES += locking/qemu-lockd.conf
  DISTCLEANFILES += locking/qemu-lockd.conf
  endif WITH_QEMU
  
 +if WITH_LIBXL
 +nodist_conf_DATA += locking/libxl-lockd.conf
 +BUILT_SOURCES += locking/libxl-lockd.conf
 +DISTCLEANFILES += locking/libxl-lockd.conf
 +endif WITH_LIBXL
 +
  locking/%-lockd.conf: $(srcdir)/locking/lockd.conf
   $(AM_V_GEN)$(MKDIR_P) locking ; \
   cp $ $@
 @@ -2431,6 +2437,12 @@ nodist_conf_DATA += locking/qemu-sanlock.conf
  BUILT_SOURCES += locking/qemu-sanlock.conf
  DISTCLEANFILES += locking/qemu-sanlock.conf
  endif WITH_QEMU
 +
 +if WITH_LIBXL
 +nodist_conf_DATA += locking/libxl-sanlock.conf
 +BUILT_SOURCES += locking/libxl-sanlock.conf
 +DISTCLEANFILES += locking/libxl-sanlock.conf
 +endif WITH_LIBXL
  else ! WITH_SANLOCK
  EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES)
  endif ! WITH_SANLOCK
 diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug
 index f225954..d5aa150 100644
 --- a/src/libxl/libvirtd_libxl.aug
 +++ b/src/libxl/libvirtd_libxl.aug
 @@ -25,9 +25,11 @@ module Libvirtd_libxl =
  
 (* Config entry grouped by function - same order as example config *)
 let autoballoon_entry = bool_entry autoballoon
 +   let lock_entry = str_entry lock_manager
  
 (* Each entry in the config is one of the following ... *)
 let entry = autoballoon_entry
 + | lock_entry
  
 let comment = [ label #comment . del /#[ \t]*/ #  .  store /([^ 
 \t\n][^\n]*)?/ . del /\n/ \n ]
 let empty = [ label #empty . eol ]
 diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf
 index c104d40..ba3de7a 100644
 --- a/src/libxl/libxl.conf
 +++ b/src/libxl/libxl.conf
 @@ -10,3 +10,13 @@
  # autoballoon setting.
  #
  #autoballoon = 1
 +
 +
 +# In order to prevent accidentally starting two domains that
 +# share one writable disk, libvirt offers two approaches for
 +# locking files: sanlock and virtlockd.  sanlock is an external
 +# project which libvirt integrates with via the libvirt-lock-sanlock
 +# package.  virtlockd is a libvirt implementation that is enabled with
 +# lockd.  Accepted values are sanlock and lockd.
 +#
 +#lock_manager = lockd
 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index 6ea2889..503e8a4 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -102,6 +102,7 @@ libxlDriverConfigDispose(void *obj)
  VIR_FREE(cfg-libDir);
  VIR_FREE(cfg-saveDir);
  VIR_FREE(cfg-autoDumpDir);
 +VIR_FREE(cfg-lockManagerName);
  }
  
  
 @@ -1495,6 +1496,7 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
const char *filename)
  {
  virConfPtr conf = NULL;
 +virConfValuePtr p;
  int ret = -1;
  
  /* Check the file is readable before opening it, otherwise
 @@ -1512,6 +1514,18 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
  if (libxlGetAutoballoonConf(cfg, conf)  0)
  goto cleanup;
  
 +if ((p = virConfGetValue(conf, lock_manager))) {
 +if (p-type != VIR_CONF_STRING) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   %s,
 +   _(Unexpected type for 'lock_manager' setting));
 +goto cleanup;
 +}
 +
 +if (VIR_STRDUP(cfg-lockManagerName, p-str)  0)
 +goto cleanup;
 +}
 +
  ret = 0;
  
   cleanup:
 diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
 index 5ba1a71..0a1c0db 100644
 --- a/src/libxl/libxl_conf.h
 +++ b/src/libxl/libxl_conf.h
 @@ -38,6 +38,7 @@
  # include virobject.h
  # include virchrdev.h
  # include virhostdev.h
 +# include locking/lock_manager.h
  
  # define LIBXL_DRIVER_NAME xenlight
  # define LIBXL_VNC_PORT_MIN  5900
 @@ -98,6 +99,8 @@ struct _libxlDriverConfig {
   * memory for new domains from domain0. */
  bool autoballoon;
  
 +char *lockManagerName;
 +
  /* Once created, caps are immutable */
  virCapsPtr caps;
  
 @@ -144,6 +147,9 @@ struct _libxlDriverPrivate {
  
  /* Immutable pointer, lockless APIs*/
  virSysinfoDefPtr 

Re: [libvirt] [PATCH v4 1/4] storage: Split out the valid path check

[John Ferlan]

}
  
 +bool
 +virStorageBackendPoolPathIsStable(const char *path)
 +{
 +if (path == NULL || STREQ(path, /dev) || STREQ(path, /dev/))
 +return false;
 +
 +if (!STRPREFIX(path, /dev))
 +return false;

 I think you want /dev/ here as the prefix to be required; otherwise,
 /device would match the prefix.  (This also means that someone using
 //dev/... would fail the check, but that's probably something we don't
 need to worry about).

 Hmm... Sure I see that... I can make that adjustment. I'll wait a bit
 before pushing just so see if there's other feedback...

 
 I think that change should be separate from this code motion.
 

OK, so consider patch 1.5/4:


Fix the if (!STRPREFIX(path, /dev)) to be if (!STRPREFIX(path, /dev/))
to ensure a path such as /device isn't declared stable.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/storage/storage_backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index b07e0d9..e0311e1 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1680,7 +1680,7 @@ virStorageBackendPoolPathIsStable(const char *path)
 if (path == NULL || STREQ(path, /dev) || STREQ(path, /dev/))
 return false;

-if (!STRPREFIX(path, /dev))
+if (!STRPREFIX(path, /dev/))
 return false;

 return true;
-- 
2.1.0

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

Re: [libvirt] [PATCH 2/3] libxl: Introduce configuration file for libxl driver

[Daniel P. Berrange]

On Fri, Apr 17, 2015 at 03:36:21PM -0600, Jim Fehlig wrote:
 Introduce libxl.conf configuration file, adding the 'autoballoon'
 setting as the first knob for controlling the libxl driver.
 
 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  libvirt.spec.in  |  8 +
  src/Makefile.am  | 24 +-
  src/libxl/libvirtd_libxl.aug | 42 +
  src/libxl/libxl.conf | 12 +++
  src/libxl/libxl_conf.c   | 61 
 
  src/libxl/libxl_conf.h   |  5 +++
  src/libxl/libxl_driver.c |  9 ++
  src/libxl/test_libvirtd_libxl.aug.in |  5 +++
  8 files changed, 159 insertions(+), 7 deletions(-)

ACK


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] spec: If installing default network, reload libvirtd (bz 867546)

[Laine Stump]

On 04/21/2015 10:34 AM, Laine Stump wrote:
 On 04/21/2015 09:48 AM, Michal Privoznik wrote:
 On 16.04.2015 21:42, Cole Robinson wrote:
 If libvirt-daemon-config-network is installed while libvirtd is already
 running, the daemon needs to be restarted to pick up the change.

 Instead let's trigger a daemon reload when the package is first installed.
 Then the default network is available immediately if libvirtd was already
 running.

 https://bugzilla.redhat.com/show_bug.cgi?id=867546
 ---
  libvirt.spec.in | 8 
  1 file changed, 8 insertions(+)

 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index e08c9e7..ada0257 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -1770,6 +1770,14 @@ if test $1 -eq 1  test ! -f 
 %{_sysconfdir}/libvirt/qemu/networks/default.xml ;
%{_datadir}/libvirt/networks/default.xml \
%{_sysconfdir}/libvirt/qemu/networks/default.xml
  ln -s ../default.xml 
 %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
 +
 +# Make sure libvirt picks up the new network defininiton
 +  %if %{with_systemd}
 +/bin/systemctl reload libvirtd.service /dev/null 21 ||:
 +  %else
 +/sbin/service libvirtd reload  /dev/null 21 || :
 +  %endif
 +
  fi
  %endif
  

 There's already a 'systemctl try-restart libvirtd.service' call just a
 few lines below. They were added in 4789fb2e. I think we can use them -
 also, I'm not sure why it doesn't work since we are restarting daemon
 even now (without this patch).
 I think the issue may be that the default network is in a different
 sub-package than libvirtd, so libvirtd is already started/restart by the
 time the file is installed. Or something like that. I do recall that
 this is a real problem though.

Heh. And if I had taken the time to go read the BZ that Cole referenced
in his commit log message, I would have seen that I came to that same
conclusion all the way back in 2012 :-/ It's amazing just how much I
forget...

So I think this patch will be okay if we can just verify that it doesn't
experience the same problem that I experienced when performing the
reload manually. (Or, what about the idea I posted in the BZ about
checking if libvirtd is running, and sending the file to virsh
net-define instead of copying it when that's the case?)

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

Re: [libvirt] [PATCH] vircapstest: fix build without LXC

[Michal Privoznik]

On 21.04.2015 10:14, Roman Bogorodskiy wrote:
 When building without lxc support enabled, build fails with:
 
 CLD vircapstest
 vircapstest.o: In function `test_virCapsDomainDataLookupLXC':
 vircapstest.c:(.text+0x9ef): undefined reference to `testLXCCapsInit'
 
 Fix that by hiding LXC tests under appropriate #ifdef.
 ---
  tests/vircapstest.c | 4 
  1 file changed, 4 insertions(+)
 
 diff --git a/tests/vircapstest.c b/tests/vircapstest.c
 index 3e5038b..b2c321e 100644
 --- a/tests/vircapstest.c
 +++ b/tests/vircapstest.c
 @@ -289,6 +289,7 @@ test_virCapsDomainDataLookupXen(const void *data 
 ATTRIBUTE_UNUSED)
  return ret;
  }
  
 +#ifdef WITH_LXC
  static int
  test_virCapsDomainDataLookupLXC(const void *data ATTRIBUTE_UNUSED)
  {
 @@ -311,6 +312,7 @@ test_virCapsDomainDataLookupLXC(const void *data 
 ATTRIBUTE_UNUSED)
  virObjectUnref(caps);
  return ret;
  }
 +#endif /* WITH_LXC */
  
  static int
  mymain(void)
 @@ -326,9 +328,11 @@ mymain(void)
  if (virtTestRun(virCapsDomainDataLookupXen,
  test_virCapsDomainDataLookupXen, NULL)  0)
  ret = -1;
 +#ifdef WITH_LXC
  if (virtTestRun(virCapsDomainDataLookupLXC,
  test_virCapsDomainDataLookupLXC, NULL)  0)
  ret = -1;
 +#endif /* WITH_LXC */
  
  return ret;
  }
 

This fixes the problem only for LXC, while other drivers (qemu and xen)
may have been disabled too. I'm squashing in the obvious fix, ACKing and
pushing.

Michal

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

Re: [libvirt] [PATCH v3 1/6] conf: Add new domain XML element 'iothreadids'

[Peter Krempa]

On Tue, Apr 14, 2015 at 21:18:21 -0400, John Ferlan wrote:
 Adding a new XML element 'iothreadids' in order to allow defining
 specific IOThread ID's rather than relying on the algorithm to assign
 IOThread ID's starting at 1 and incrementing to iothreads count.
 
 This will allow future patches to be able to add new IOThreads by
 a specific iothread_id and of course delete any exisiting IOThread.
 
 Each iothreadids element will have 'n' iothread children elements
 which will have attribute id.  The id will allow for definition
 of any valid (eg  0) iothread_id value.
 
 On input, if any iothreadids iothread's are provided, they will
 be marked so that we only print out what we read in.
 
 On input, if no iothreadids are provided, the PostParse code will
 self generate a list of ID's starting at 1 and going to the number
 of iothreads defined for the domain (just like the current algorithm
 numbering scheme).  A future patch will rework the existing algorithm
 to make use of the iothreadids list.
 
 On output, only print out the iothreadids if they were read in.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  docs/formatdomain.html.in |  30 +++
  docs/schemas/domaincommon.rng |  12 +++
  src/conf/domain_conf.c| 190 
 +-
  src/conf/domain_conf.h|  15 
  src/libvirt_private.syms  |   3 +
  5 files changed, 248 insertions(+), 2 deletions(-)
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index e921749..518f7c5 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -521,6 +521,18 @@
...
  lt;/domaingt;
  /pre
 +pre
 +lt;domaingt;
 +  ...
 +  lt;iothreadidsgt;
 +lt;iothread id=2/gt;
 +lt;iothread id=4/gt;
 +lt;iothread id=6/gt;
 +lt;iothread id=8/gt;
 +  lt;/iothreadidsgt;
 +  ...
 +lt;/domaingt;
 +/pre
  
  dl
dtcodeiothreads/code/dt
 @@ -530,7 +542,25 @@
  virtio-blk-pci and virtio-blk-ccw target storage devices. There
  should be only 1 or 2 IOThreads per host CPU. There may be more
  than one supported device assigned to each IOThread.
 +span class=sinceSince 1.2.8/span
/dd
 +  dtcodeiothreadids/code/dt
 +  dd
 +The optional codeiothreadids/code element provides the capability
 +to specifically define the IOThread ID's for the domain.  By default,
 +IOThread ID's are sequentially numbered starting from 1 through the
 +number of codeiothreads/code defined for the domain. The
 +codeid/code attribute is used to define the IOThread ID. The
 +codeid/code attribute must be a positive integer greater than 0.
 +If there are less codeiothreadids/code defined than
 +codeiothreads/code defined for the domain, then libvirt will
 +sequentially fill codeiothreadids/code starting at 1 avoiding
 +any predefined codeid/code. If there are more
 +codeiothreadids/code defined than codeiothreads/code
 +defined for the domain, then the codeiothreads/code value
 +will be adjusted accordingly.
 +span class=sinceSince 1.2.15/span
 +   /dd
  /dl
  
  h3a name=elementsCPUTuningCPU Tuning/a/h3
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 03fd541..4bd32bd 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -675,6 +675,18 @@
/optional
  
optional
 +element name=iothreadids
 +  zeroOrMore
 +element name=iothread
 +  attribute name=id
 +ref name=unsignedInt/
 +  /attribute
 +/element
 +  /zeroOrMore
 +/element
 +  /optional
 +
 +  optional
  ref name=blkiotune/
/optional
  
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 4d7e3c9..e86b7e2 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -2113,6 +2113,23 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
  return NULL;
  }
  
 +
 +static void
 +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
 +int nids)
 +{
 +size_t i;
 +
 +if (!def)
 +return;
 +
 +for (i = 0; i  nids; i++)
 +VIR_FREE(def[i]);
 +
 +VIR_FREE(def);
 +}
 +
 +
  void
  virDomainPinDefFree(virDomainPinDefPtr def)
  {
 @@ -2310,6 +2327,8 @@ void virDomainDefFree(virDomainDefPtr def)
  
  virCPUDefFree(def-cpu);
  
 +virDomainIOThreadIDDefArrayFree(def-iothreadids, def-niothreadids);
 +
  virDomainPinDefArrayFree(def-cputune.vcpupin, def-cputune.nvcpupin);
  
  virDomainPinDefFree(def-cputune.emulatorpin);
 @@ -3304,6 +3323,18 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
  return -1;
  }
  
 +/* Fully populate the IOThread ID list */
 +if (def-iothreads  def-iothreads != def-niothreadids) {
 +unsigned int iothread_id = 

Re: [libvirt] [PATCH] qemu: Always refresh capabilities if no guests found (bz 1000116)

[Cole Robinson]

On 04/21/2015 10:13 AM, Michal Privoznik wrote:
 On 16.04.2015 21:08, Cole Robinson wrote:
 - Remove all qemu emulators
 - Restart libvirtd
 - Install qemu emulators
 - Call 'virsh version' - errors

 The only thing that will force the qemu driver to refresh it's cached
 capablities info is an explict API call to GetCapabilities.

 However in the case when the initial caps lookup at driver connect didn't
 find a single qemu emulator to poll, the driver is effectively useless
 and really can't do anything until it's populated some qemu capabilities
 info.

 With the above steps, the user would have to either know about the
 magic refresh capabilities call, or restart libvirtd to pick up the
 changes.

 Instead, this patch changes things so that every time a part of th
 driver requests access to capabilities info, check to see if
 we've previously seen any emulators. If not, force a refresh.

 In the case of 'still no emulators found', this is still very quick, so
 I can't think of a downside.

 https://bugzilla.redhat.com/show_bug.cgi?id=1000116
 ---
  src/qemu/qemu_conf.c | 7 +++
  1 file changed, 7 insertions(+)

 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index 2cf3905..b662b69 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -965,6 +965,13 @@ virCapsPtr 
 virQEMUDriverGetCapabilities(virQEMUDriverPtr driver,
  qemuDriverLock(driver);
  }
  
 +if (driver-caps-nguests == 0  !refresh) {
 +VIR_DEBUG(Capabilities didn't detect any guests. Forcing a 
 +refresh.);
 +qemuDriverUnlock(driver);
 +return virQEMUDriverGetCapabilities(driver, true);
 +}
 +
  ret = virObjectRef(driver-caps);
  qemuDriverUnlock(driver);
  return ret;

 
 ACK, although I'd remove (bz ...) from the $subj and keep the BZ link
 in the commit message.
 

Thanks, pushed now with that tweak

- Cole

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

Re: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads

[Peter Krempa]

On Tue, Apr 21, 2015 at 15:45:48 +0200, Peter Krempa wrote:
 On Tue, Apr 14, 2015 at 21:18:25 -0400, John Ferlan wrote:
  Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or
  remove an IOThread to/from the host either for live or config optoins
  
  The implementation for the 'live' option will use the iothreadpids list
  in order to make decision, while the 'config' option will use the
  iothreadids list.  Additionally, for deletion each may have to adjust
  the iothreadpin list.
  
  IOThreads are implemented by qmp objects, the code makes use of the existing
  qemuMonitorAddObject or qemuMonitorDelObject APIs.
  
  Signed-off-by: John Ferlan jfer...@redhat.com
  ---
   src/conf/domain_audit.c  |   9 +
   src/conf/domain_audit.h  |   6 +
   src/libvirt_private.syms |   1 +
   src/qemu/qemu_driver.c   | 431 
  +++
   4 files changed, 447 insertions(+)
  
 
 ...
 
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
  index 008258f..f42d4fb 100644
  --- a/src/qemu/qemu_driver.c
  +++ b/src/qemu/qemu_driver.c
  @@ -6179,6 +6179,435 @@ qemuDomainPinIOThread(virDomainPtr dom,

...

  +
  +if (virDomainIOThreadIDAdd(vm-def, iothread_id)  0)
 
 virDomainIOThreadIDAdd could return the pointer to the created item ...
 
  +goto cleanup;
  +
  +if (!(iothrid = virDomainIOThreadIDFind(vm-def, iothread_id))) {
  +virReportError(VIR_ERR_INTERNAL_ERROR,
  +   _(cannot find just added IOThread '%u'),
  +   iothread_id);
 
 So that you don't have to look it up right after adding it.
 
  +goto cleanup;
  +}
  +
  +iothrid-thread_id = new_iothreads[idx]-thread_id;

You are also not marking the thread structure as 'defined' and thus
wouldn't format it later ...


  +
  +/* Add IOThread to cgroup if present */
  +if (priv-cgroup) {
  +cgroup_iothread =
  +qemuDomainAddCgroupForThread(priv-cgroup,
  + VIR_CGROUP_THREAD_IOTHREAD,
  + iothread_id, mem_mask,
  + iothrid-thread_id);
  +if (!cgroup_iothread)
  +goto cleanup;
  +}
  +
  +/* Inherit def-cpuset */
  +if (vm-def-cpumask) {
 
 Automatic NUMA placement(priv-autoCpuset) needs to be taken into account too.
 
  +if (qemuDomainHotplugAddPin(vm-def-cpumask, iothread_id,
  +vm-def-cputune.iothreadspin,
  +vm-def-cputune.niothreadspin)  0)
  +
  +goto cleanup;
  +
  +if (qemuDomainHotplugPinThread(vm-def-cpumask, iothread_id,
  +   iothrid-thread_id, 
  cgroup_iothread)  0)
  +goto cleanup;
  +
  +if (qemuProcessSetSchedParams(iothread_id, iothrid-thread_id,
  +  vm-def-cputune.niothreadsched,
  +  vm-def-cputune.iothreadsched)  0)
 
 qemuProcessSetSchedParams won't do anything since the new thread doesn't
 have any scheduler assigned.
 
  +goto cleanup;
  +}
  +
  +ret = 0;
  +
  + cleanup:
  +if (new_iothreads) {
  +for (idx = 0; idx  new_niothreads; idx++)
  +qemuMonitorIOThreadInfoFree(new_iothreads[idx]);
  +VIR_FREE(new_iothreads);
  +}
  +VIR_FREE(mem_mask);
  +virDomainAuditIOThread(vm, orig_niothreads, new_niothreads,
  +   update, rc == 0);
  +if (cgroup_iothread)
  +virCgroupFree(cgroup_iothread);
 
 virCgroupFree() handles NULL just fine.
 
  +VIR_FREE(alias);
  +return ret;
  +
  + exit_monitor:
  +ignore_value(qemuDomainObjExitMonitor(driver, vm));
  +goto cleanup;
  +}

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] spec: If installing default network, reload libvirtd (bz 867546)

[Laine Stump]

On 04/21/2015 09:48 AM, Michal Privoznik wrote:
 On 16.04.2015 21:42, Cole Robinson wrote:
 If libvirt-daemon-config-network is installed while libvirtd is already
 running, the daemon needs to be restarted to pick up the change.

 Instead let's trigger a daemon reload when the package is first installed.
 Then the default network is available immediately if libvirtd was already
 running.

 https://bugzilla.redhat.com/show_bug.cgi?id=867546
 ---
  libvirt.spec.in | 8 
  1 file changed, 8 insertions(+)

 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index e08c9e7..ada0257 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -1770,6 +1770,14 @@ if test $1 -eq 1  test ! -f 
 %{_sysconfdir}/libvirt/qemu/networks/default.xml ;
%{_datadir}/libvirt/networks/default.xml \
%{_sysconfdir}/libvirt/qemu/networks/default.xml
  ln -s ../default.xml 
 %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
 +
 +# Make sure libvirt picks up the new network defininiton
 +  %if %{with_systemd}
 +/bin/systemctl reload libvirtd.service /dev/null 21 ||:
 +  %else
 +/sbin/service libvirtd reload  /dev/null 21 || :
 +  %endif
 +
  fi
  %endif
  

 There's already a 'systemctl try-restart libvirtd.service' call just a
 few lines below. They were added in 4789fb2e. I think we can use them -
 also, I'm not sure why it doesn't work since we are restarting daemon
 even now (without this patch).

I think the issue may be that the default network is in a different
sub-package than libvirtd, so libvirtd is already started/restart by the
time the file is installed. Or something like that. I do recall that
this is a real problem though.


I also meant to respond to this patch and again got distracted - when I
tried to manually do that this patch supposedly does, I got an error:

I copied a new XML file into /etc/libvirt/qemu/networks and a link for
it into /etc/libvirt/qemu/networks/autostart, then called systemctl
reload libvirtd.service, I ended up with a bridge device created with
the correct IP address, but libvirt showed the new network as inactive
and this error log:

networkCheckRouteCollision:121 : internal error: Network is already in
use by interface virbr3

(virbr3 is the bridge created for this network; it didn't exist prior to
telling libvirtd to reload, and there is no other route in conflict with
it.)

I didn't have time to investigate, but am wondering if the patch is
really doing what is intended.


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

Re: [libvirt] [PATCH v3 5/6] qemu: Add support to Add/Delete IOThreads

[Peter Krempa]

On Tue, Apr 14, 2015 at 21:18:25 -0400, John Ferlan wrote:
 Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or
 remove an IOThread to/from the host either for live or config optoins
 
 The implementation for the 'live' option will use the iothreadpids list
 in order to make decision, while the 'config' option will use the
 iothreadids list.  Additionally, for deletion each may have to adjust
 the iothreadpin list.
 
 IOThreads are implemented by qmp objects, the code makes use of the existing
 qemuMonitorAddObject or qemuMonitorDelObject APIs.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/conf/domain_audit.c  |   9 +
  src/conf/domain_audit.h  |   6 +
  src/libvirt_private.syms |   1 +
  src/qemu/qemu_driver.c   | 431 
 +++
  4 files changed, 447 insertions(+)
 

...

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 008258f..f42d4fb 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -6179,6 +6179,435 @@ qemuDomainPinIOThread(virDomainPtr dom,
  return ret;
  }
  
 +static int
 +qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
 + virDomainObjPtr vm,
 + unsigned int iothread_id)
 +{
 +qemuDomainObjPrivatePtr priv = vm-privateData;
 +char *alias = NULL;
 +size_t idx;
 +int rc = -1;
 +int ret = -1;
 +unsigned int orig_niothreads = vm-def-iothreads;
 +unsigned int exp_niothreads = vm-def-iothreads;
 +int new_niothreads = 0;
 +qemuMonitorIOThreadInfoPtr *new_iothreads = NULL;
 +unsigned int idval = 0;
 +virCgroupPtr cgroup_iothread = NULL;
 +char *mem_mask = NULL;
 +virDomainIOThreadIDDefPtr iothrid;
 +
 +if (virDomainIOThreadIDFind(vm-def, iothread_id)) {
 +virReportError(VIR_ERR_INVALID_ARG,
 +   _(an IOThread is already using iothread_id '%u'),
 +   iothread_id);
 +goto cleanup;
 +}
 +
 +if (virAsprintf(alias, iothread%u, iothread_id)  0)
 +return -1;
 +
 +qemuDomainObjEnterMonitor(driver, vm);
 +
 +rc = qemuMonitorAddObject(priv-mon, iothread, alias, NULL);
 +exp_niothreads++;
 +if (rc  0)
 +goto exit_monitor;
 +
 +/* After hotplugging the IOThreads we need to re-detect the
 + * IOThreads thread_id's, adjust the cgroups, thread affinity,
 + * and add the thread_id to the vm-def-iothreadids list.
 + */
 +if ((new_niothreads = qemuMonitorGetIOThreads(priv-mon,
 +  new_iothreads))  0)

Since we are not doing any fancy iothread naming, this function can
parse the iothread IDs from the alias right away ... [1]


 +goto exit_monitor;
 +
 +if (qemuDomainObjExitMonitor(driver, vm)  0)
 +goto cleanup;
 +
 +if (new_niothreads != exp_niothreads) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(got wrong number of IOThread ids from QEMU 
 monitor. 
 + got %d, wanted %d),
 +   new_niothreads, exp_niothreads);
 +vm-def-iothreads = new_niothreads;
 +goto cleanup;
 +}
 +vm-def-iothreads = exp_niothreads;
 +
 +if (virDomainNumatuneGetMode(vm-def-numa, -1) ==
 +VIR_DOMAIN_NUMATUNE_MEM_STRICT 
 +virDomainNumatuneMaybeFormatNodeset(vm-def-numa,
 +priv-autoNodeset,
 +mem_mask, -1)  0)
 +goto cleanup;
 +
 +
 +/*
 + * If we've successfully added an IOThread, find out where we added it
 + * in the QEMU IOThread list, so we can add it to our iothreadids list
 + */

The message seems obvious when looking at the code.

 +for (idx = 0; idx  new_niothreads; idx++) {
 +if (qemuDomainParseIOThreadAlias(new_iothreads[idx]-name, idval)  
 0)

... [1] so that you don't have to do it manually.

 +goto cleanup;
 +if (iothread_id == idval)
 +break;
 +}
 +
 +if (idval != iothread_id) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(cannot find new IOThread '%u' in QEMU monitor.),
 +   iothread_id);
 +goto cleanup;
 +}
 +
 +if (virDomainIOThreadIDAdd(vm-def, iothread_id)  0)

virDomainIOThreadIDAdd could return the pointer to the created item ...

 +goto cleanup;
 +
 +if (!(iothrid = virDomainIOThreadIDFind(vm-def, iothread_id))) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(cannot find just added IOThread '%u'),
 +   iothread_id);

So that you don't have to look it up right after adding it.

 +goto cleanup;
 +}
 +
 +iothrid-thread_id = new_iothreads[idx]-thread_id;
 +
 +/* Add IOThread to cgroup if present */
 +if (priv-cgroup) {
 +cgroup_iothread =
 +

Re: [libvirt] [PATCH] spec: If installing default network, reload libvirtd (bz 867546)

[Michal Privoznik]

On 16.04.2015 21:42, Cole Robinson wrote:
 If libvirt-daemon-config-network is installed while libvirtd is already
 running, the daemon needs to be restarted to pick up the change.
 
 Instead let's trigger a daemon reload when the package is first installed.
 Then the default network is available immediately if libvirtd was already
 running.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=867546
 ---
  libvirt.spec.in | 8 
  1 file changed, 8 insertions(+)
 
 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index e08c9e7..ada0257 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -1770,6 +1770,14 @@ if test $1 -eq 1  test ! -f 
 %{_sysconfdir}/libvirt/qemu/networks/default.xml ;
%{_datadir}/libvirt/networks/default.xml \
%{_sysconfdir}/libvirt/qemu/networks/default.xml
  ln -s ../default.xml 
 %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
 +
 +# Make sure libvirt picks up the new network defininiton
 +  %if %{with_systemd}
 +/bin/systemctl reload libvirtd.service /dev/null 21 ||:
 +  %else
 +/sbin/service libvirtd reload  /dev/null 21 || :
 +  %endif
 +
  fi
  %endif
  
 

There's already a 'systemctl try-restart libvirtd.service' call just a
few lines below. They were added in 4789fb2e. I think we can use them -
also, I'm not sure why it doesn't work since we are restarting daemon
even now (without this patch).

Michal

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

Re: [libvirt] [PATCH] qemu: Always refresh capabilities if no guests found (bz 1000116)

[Michal Privoznik]

On 16.04.2015 21:08, Cole Robinson wrote:
 - Remove all qemu emulators
 - Restart libvirtd
 - Install qemu emulators
 - Call 'virsh version' - errors
 
 The only thing that will force the qemu driver to refresh it's cached
 capablities info is an explict API call to GetCapabilities.
 
 However in the case when the initial caps lookup at driver connect didn't
 find a single qemu emulator to poll, the driver is effectively useless
 and really can't do anything until it's populated some qemu capabilities
 info.
 
 With the above steps, the user would have to either know about the
 magic refresh capabilities call, or restart libvirtd to pick up the
 changes.
 
 Instead, this patch changes things so that every time a part of th
 driver requests access to capabilities info, check to see if
 we've previously seen any emulators. If not, force a refresh.
 
 In the case of 'still no emulators found', this is still very quick, so
 I can't think of a downside.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1000116
 ---
  src/qemu/qemu_conf.c | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index 2cf3905..b662b69 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -965,6 +965,13 @@ virCapsPtr virQEMUDriverGetCapabilities(virQEMUDriverPtr 
 driver,
  qemuDriverLock(driver);
  }
  
 +if (driver-caps-nguests == 0  !refresh) {
 +VIR_DEBUG(Capabilities didn't detect any guests. Forcing a 
 +refresh.);
 +qemuDriverUnlock(driver);
 +return virQEMUDriverGetCapabilities(driver, true);
 +}
 +
  ret = virObjectRef(driver-caps);
  qemuDriverUnlock(driver);
  return ret;
 

ACK, although I'd remove (bz ...) from the $subj and keep the BZ link
in the commit message.

Michal

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

[libvirt] [PATCH v4 6/6] tests: add machine-vmport-opt qemu test

[Marc-André Lureau]

Check that the vmport feature is correctly used in qemu commande line.
---
 .../qemuxml2argv-machine-vmport-opt.args   |  6 +
 .../qemuxml2argv-machine-vmport-opt.xml| 28 ++
 tests/qemuxml2argvtest.c   |  2 ++
 3 files changed, 36 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args 
b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args
new file mode 100644
index 000..ea1a11f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args
@@ -0,0 +1,6 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-S -machine pc,accel=tcg,vmport=off -m 214 -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
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml
new file mode 100644
index 000..671d3a9
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml
@@ -0,0 +1,28 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219100/memory
+  currentMemory unit='KiB'219100/currentMemory
+  vcpu placement='static'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  features
+vmport state='off'/
+  /features
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='ide' index='0'/
+memballoon model='virtio'/
+  /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index acd6126..299d0af 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -607,6 +607,8 @@ mymain(void)
 DO_TEST_FAILURE(machine-core-on, QEMU_CAPS_MACHINE_OPT);
 DO_TEST(machine-usb-opt, QEMU_CAPS_MACHINE_OPT,
 QEMU_CAPS_MACHINE_USB_OPT);
+DO_TEST(machine-vmport-opt, QEMU_CAPS_MACHINE_OPT,
+QEMU_CAPS_MACHINE_VMPORT_OPT);
 DO_TEST(kvm, QEMU_CAPS_MACHINE_OPT);
 DO_TEST(boot-cdrom, NONE);
 DO_TEST(boot-network, NONE);
-- 
2.1.0

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

[libvirt] [PATCH v4 4/6] qemu: add virQEMUCapsSupportsVmport

[Marc-André Lureau]

The vmport machine argument works with pc machine kind, not with xen for
example.
---
 src/qemu/qemu_capabilities.c | 13 +
 src/qemu/qemu_capabilities.h |  3 +++
 src/qemu/qemu_command.c  | 19 ---
 src/qemu/qemu_domain.c   | 19 +++
 src/qemu/qemu_domain.h   |  3 +++
 5 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ca26855..607b6d5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3725,6 +3725,19 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def,
 
 
 bool
+virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
+  const virDomainDef *def)
+{
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT))
+return false;
+
+return qemuDomainMachineIsI440FX(def) ||
+qemuDomainMachineIsQ35(def) ||
+STREQ(def-os.machine, isapc);
+}
+
+
+bool
 virQEMUCapsIsMachineSupported(virQEMUCapsPtr qemuCaps,
   const char *canonical_machine)
 {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 48c8f96..81557b7 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -263,6 +263,9 @@ bool virQEMUCapsGet(virQEMUCapsPtr qemuCaps,
 bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps,
virDomainDefPtr def);
 
+bool virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps,
+   const virDomainDef *def);
+
 char *virQEMUCapsFlagsString(virQEMUCapsPtr qemuCaps);
 
 const char *virQEMUCapsGetBinary(virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 29b876e..f25a75f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1954,25 +1954,6 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
 }
 
 
-static bool
-qemuDomainMachineIsQ35(virDomainDefPtr def)
-{
-return (STRPREFIX(def-os.machine, pc-q35) ||
-STREQ(def-os.machine, q35));
-}
-
-
-static bool
-qemuDomainMachineIsI440FX(virDomainDefPtr def)
-{
-return (STREQ(def-os.machine, pc) ||
-STRPREFIX(def-os.machine, pc-0.) ||
-STRPREFIX(def-os.machine, pc-1.) ||
-STRPREFIX(def-os.machine, pc-i440) ||
-STRPREFIX(def-os.machine, rhel));
-}
-
-
 static int
 qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
 virQEMUCapsPtr qemuCaps,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1368386..506c0af 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3082,3 +3082,22 @@ qemuDomainSupportsBlockJobs(virDomainObjPtr vm,
 
 return 0;
 }
+
+
+bool
+qemuDomainMachineIsQ35(const virDomainDef *def)
+{
+return (STRPREFIX(def-os.machine, pc-q35) ||
+STREQ(def-os.machine, q35));
+}
+
+
+bool
+qemuDomainMachineIsI440FX(const virDomainDef *def)
+{
+return (STREQ(def-os.machine, pc) ||
+STRPREFIX(def-os.machine, pc-0.) ||
+STRPREFIX(def-os.machine, pc-1.) ||
+STRPREFIX(def-os.machine, pc-i440) ||
+STRPREFIX(def-os.machine, rhel));
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 6bea7c7..d68e41b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -431,4 +431,7 @@ void qemuDomObjEndAPI(virDomainObjPtr *vm);
 int qemuDomainAlignMemorySizes(virDomainDefPtr def);
 void qemuDomainMemoryDeviceAlignSize(virDomainMemoryDefPtr mem);
 
+bool qemuDomainMachineIsQ35(const virDomainDef *def);
+bool qemuDomainMachineIsI440FX(const virDomainDef *def);
+
 #endif /* __QEMU_DOMAIN_H__ */
-- 
2.1.0

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

[libvirt] [PATCH v4 5/6] qemu: add machine vmport argument

[Marc-André Lureau]

Fill qemu command line vmport argument as required.
---
 src/qemu/qemu_command.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f25a75f..7304d90 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7294,6 +7294,7 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
 obsoleteAccel = true;
 } else {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
+virTristateSwitch vmport = def-features[VIR_DOMAIN_FEATURE_VMPORT];
 
 virCommandAddArg(cmd, -machine);
 virBufferAdd(buf, def-os.machine, -1);
@@ -7311,6 +7312,19 @@ qemuBuildMachineArgStr(virCommandPtr cmd,
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT))
 virBufferAddLit(buf, ,usb=off);
 
+if (vmport) {
+if (!virQEMUCapsSupportsVmport(qemuCaps, def)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(vmport is not available 
+ with this QEMU binary));
+virBufferFreeAndReset(buf);
+return -1;
+}
+
+virBufferAsprintf(buf, ,vmport=%s,
+  virTristateSwitchTypeToString(vmport));
+}
+
 if (def-mem.dump_core) {
 if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-- 
2.1.0

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

[libvirt] [PATCH v4 2/6] domain/conf: add VIR_DOMAIN_FEATURE_VMPORT

[Marc-André Lureau]

---
 src/conf/domain_conf.c | 5 -
 src/conf/domain_conf.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 479b4c2..1467cd3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -140,7 +140,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
   kvm,
   pvspinlock,
   capabilities,
-  pmu)
+  pmu,
+  vmport)
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
   default,
@@ -14323,6 +14324,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 
 case VIR_DOMAIN_FEATURE_PMU:
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
+case VIR_DOMAIN_FEATURE_VMPORT:
 node = ctxt-node;
 ctxt-node = nodes[i];
 if ((tmp = virXPathString(string(./@state), ctxt))) {
@@ -20863,6 +20865,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 
 case VIR_DOMAIN_FEATURE_PMU:
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
+case VIR_DOMAIN_FEATURE_VMPORT:
 switch ((virTristateSwitch) def-features[i]) {
 case VIR_TRISTATE_SWITCH_LAST:
 case VIR_TRISTATE_SWITCH_ABSENT:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 25d3ee6..21cbd0f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1649,6 +1649,7 @@ typedef enum {
 VIR_DOMAIN_FEATURE_PVSPINLOCK,
 VIR_DOMAIN_FEATURE_CAPABILITIES,
 VIR_DOMAIN_FEATURE_PMU,
+VIR_DOMAIN_FEATURE_VMPORT,
 
 VIR_DOMAIN_FEATURE_LAST
 } virDomainFeature;
-- 
2.1.0

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

[libvirt] [PATCH v4 3/6] qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT

[Marc-André Lureau]

Set the capability based on qmp query, or qemu version.  The qmp query
includes vmport with 2.2, but no longer with 2.3. It lists only
non-machine specific capabilities, so check the qemu version too until a
machine-specific query is supported.
---
 src/qemu/qemu_capabilities.c | 7 +++
 src/qemu/qemu_capabilities.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 01ed1e2..ca26855 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -279,6 +279,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   qxl.vgamem_mb,
   qxl-vga.vgamem_mb,
   pc-dimm,
+
+  machine-vmport-opt, /* 185 */
 );
 
 
@@ -2509,6 +2511,7 @@ struct virQEMUCapsCommandLineProps {
 
 static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = {
 { machine, mem-merge, QEMU_CAPS_MEM_MERGE },
+{ machine, vmport, QEMU_CAPS_MACHINE_VMPORT_OPT },
 { drive, discard, QEMU_CAPS_DRIVE_DISCARD },
 { realtime, mlock, QEMU_CAPS_MLOCK },
 { boot-opts, strict, QEMU_CAPS_BOOT_STRICT },
@@ -3255,6 +3258,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 if (qemuCaps-version = 1006000)
 virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
 
+/* vmport option is supported v2.2.0 onwards */
+if (qemuCaps-version = 2002000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);
+
 if (virQEMUCapsProbeQMPCommands(qemuCaps, mon)  0)
 goto cleanup;
 if (virQEMUCapsProbeQMPEvents(qemuCaps, mon)  0)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index c7b1ac7..48c8f96 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -224,6 +224,7 @@ typedef enum {
 QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */
 QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */
 QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */
+QEMU_CAPS_MACHINE_VMPORT_OPT = 185, /* -machine xxx,vmport=on/off/auto */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
-- 
2.1.0

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

[libvirt] [PATCH v4 0/6] Add vmport feature

[Marc-André Lureau]

Hi,

The QEMU machine vmport option allows to set the VMWare IO port
emulation. This emulation is useful for absolute pointer input when the
guest has vmware input drivers, and is enabled by default for kvm.

However it is unnecessary for Spice-enabled VM, since the agent already
handles absolute pointer and multi-monitors. Furthermore, it prevents
Spice from switching to relative input since the regular ps/2 pointer
driver is replaced by the vmware driver. It is thus advised to disable
vmport when using a Spice VM. This will permit the Spice client to
switch from absolute to relative pointer, as it may be required for
certain games or applications.

The following patch series allows to configure the vmport feature.

v2:
 - use a feature instead of a domain attribute, suggested by D. Berrange
v3:
 - use qmp query for caps instead of version
v4:
 - use vesion check  qmp, add runtime check for machine kind

Marc-André Lureau (6):
  docs: add domain vmport feature
  domain/conf: add VIR_DOMAIN_FEATURE_VMPORT
  qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT
  qemu: add virQEMUCapsSupportsVmport
  qemu: add machine vmport argument
  tests: add machine-vmport-opt qemu test

 docs/formatdomain.html.in  |  6 
 docs/schemas/domaincommon.rng  |  9 ++
 src/conf/domain_conf.c |  5 +++-
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_capabilities.c   | 20 +
 src/qemu/qemu_capabilities.h   |  4 +++
 src/qemu/qemu_command.c| 33 +-
 src/qemu/qemu_domain.c | 19 +
 src/qemu/qemu_domain.h |  3 ++
 .../qemuxml2argv-machine-vmport-opt.args   |  6 
 .../qemuxml2argv-machine-vmport-opt.xml| 28 ++
 tests/qemuxml2argvtest.c   |  2 ++
 12 files changed, 116 insertions(+), 20 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml

-- 
2.1.0

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

[libvirt] [PATCH v4 1/6] docs: add domain vmport feature

[Marc-André Lureau]

A new feature that can be turned on or off.

The QEMU machine vmport option allows to set the VMWare IO port
emulation. This emulation is useful for absolute pointer input when the
guest has vmware input drivers, and is enabled by default for kvm.

However it is unnecessary for Spice-enabled VM, since the agent already
handles absolute pointer and multi-monitors. Furthermore, it prevents
Spice from switching to relative input since the regular ps/2 pointer
driver is replaced by the vmware driver. It is thus advised to disable
vmport when using a Spice VM. This will permit the Spice client to
switch from absolute to relative pointer, as it may be required for
certain games or applications.
---
 docs/formatdomain.html.in | 6 ++
 docs/schemas/domaincommon.rng | 9 +
 2 files changed, 15 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e921749..ca5aa25 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1501,6 +1501,12 @@
 performance monitoring unit for the guest.
 span class=sinceSince 1.2.12/span
   /dd
+  dtcodevmport/code/dt
+  ddDepending on the codestate/code attribute (values 
codeon/code,
+codeoff/code, default codeon/code) enable or disable
+the emulation of VMWare IO port, for vmmouse etc.
+span class=sinceSince 1.2.15/span
+  /dd
 /dl
 
 h3a name=elementsTimeTime keeping/a/h3
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 19461f5..439323d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4014,6 +4014,15 @@
   optional
 ref name=pmu/
   /optional
+  optional
+element name=vmport
+  optional
+attribute name=state
+  ref name=virOnOff/
+/attribute
+  /optional
+/element
+  /optional
 /interleave
   /element
 /optional
-- 
2.1.0

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

Re: [libvirt] libvirt-0.10.2-46 compress failed

[Michal Privoznik]

On 21.04.2015 08:48, 饶俊明 wrote:
 Hello
 
   When compress libvirt-0.10.2-46.el6.src.rpm to support rbd, then failed;
 
 Error log:
 cc1: warnings being treated as errors
 storage/storage_backend_rbd.c: In function 'virStorageBackendRBDRefreshPool':
 storage/storage_backend_rbd.c:284: error: declaration of 'stat' shadows a 
 global declaration [-Wshadow]
 /usr/include/sys/stat.h:455: error: shadowed declaration is here [-Wshadow]
 At top level:
 cc1: error: unrecognized command line option -Wno-suggest-attribute=const
 cc1: error: unrecognized command line option -Wno-suggest-attribute=pure
 make[3]: *** [libvirt_driver_storage_impl_la-storage_backend_rbd.lo] Error 1
 make[3]: *** Waiting for unfinished jobs
 make[3]: Leaving directory `/home/ada/rpmbuild/BUILD/libvirt-0.10.2/src'
 make[2]: *** [all] Error 2
 make[2]: Leaving directory `/home/ada/rpmbuild/BUILD/libvirt-0.10.2/src'
 make[1]: *** [all-recursive] Error 1
 make[1]: Leaving directory `/home/ada/rpmbuild/BUILD/libvirt-0.10.2'
 make: *** [all] Error 2
 error: Bad exit status from /var/tmp/rpm-tmp.g0IuFq (%build)
 RPM build errors:
 Bad exit status from /var/tmp/rpm-tmp.g0IuFq (%build)
 
 Thanks!!!
 

Seems like this patch needs to be backported:

commit 6100cd985c9958f5590875e80597e26118e58bd9
Author: Michael Chapman m...@very.puzzling.org
AuthorDate: Wed Dec 11 19:14:51 2013 +1100
Commit: Michal Privoznik mpriv...@redhat.com
CommitDate: Wed Dec 11 10:18:15 2013 +0100

storage_backend_rbd: rename stat variable

This variable shadows the stat(2) function, which only became visible in
this scope as of commit 9cac8639. Rename the variable so it doesn't
conflict.

Signed-off-by: Michael Chapman m...@very.puzzling.org


v1.2.0-117-g6100cd9

Michal

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

[libvirt] [PATCH v2 2/2] processNicRxFilterChangedEvent: Take appropriate actions for NET_TYPE_NETWORK too

[Michal Privoznik]

Because packets going through the egress from a bridge (where our
bandwidth limiting takes place) have no information about which
interface they came from, the QoS rules that we create instead
use the source MAC address of the packets to make their decisions
about which QDisc the packet should be in.

One flaw in this is that when a guest changed the MAC address it
used, packets from the guest would no longer be put into the
correct QDisc, but would instead be put in an unprivileged
class, resulting in the bandwidth floor (minimum guaranteed)
being no longer honored.

Now that libvirt has infrastructure to capture and respond to
RX_FILTER_CHANGE events from qemu (sent whenever a guest
interface modifies its MAC address, among other things), we can
notice when a guest MAC address changes, and update the QoS rules
accordingly, so that bandwidth floor is honored even after a
guest MAC address change.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/qemu/qemu_driver.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6fc9696..6209754b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4377,6 +4377,18 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
 syncNicRxFilterDeviceOptions(def-ifname, guestFilter, hostFilter);
 }
 
+if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_NETWORK) {
+const char *brname = virDomainNetGetActualBridgeName(def);
+
+/* For libivrt network connections, set the following TUN/TAP network
+ * device attributes to match those of the guest network device:
+ * - QoS filters (which are based on MAC address)
+ */
+if (virNetDevBandwidthUpdateFilter(brname, guestFilter-mac,
+   def-data.network.actual-class_id) 
 0)
+goto endjob;
+}
+
  endjob:
 qemuDomainObjEndJob(driver, vm);
 
-- 
2.0.5

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

[libvirt] [PATCH v2 1/2] virDomainActualNetDefContentsFormat: Format class_id only for status XML

[Michal Privoznik]

In one of my previous patches (b68a56bcfe) I made class_id to
format more frequently. Well, now it's formatting way too
frequent - even for regular active XML. Users don't need to see
it, so lets format it only for the status XML where it's really
needed.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/domain_conf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 479b4c2..9aad782 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18522,7 +18522,8 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
 
 virBufferAddLit(buf, /\n);
 }
-if (def-data.network.actual  def-data.network.actual-class_id) {
+if (flags  VIR_DOMAIN_DEF_FORMAT_STATUS 
+def-data.network.actual  def-data.network.actual-class_id) {
 virBufferAsprintf(buf, class id='%u'/\n,
   def-data.network.actual-class_id);
 }
-- 
2.0.5

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

[libvirt] [PATCH v2 0/2] QoS: Adapt to changing MAC

[Michal Privoznik]

Diff to v1:
- Laine's comments worked in

Michal Privoznik (2):
  virDomainActualNetDefContentsFormat: Format class_id only for status
XML
  processNicRxFilterChangedEvent: Take appropriate actions for
NET_TYPE_NETWORK too

 src/conf/domain_conf.c |  3 ++-
 src/qemu/qemu_driver.c | 12 
 2 files changed, 14 insertions(+), 1 deletion(-)

-- 
2.0.5

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

[libvirt] [PATCH 1/3] util: storage: Fix possible crash when source path is NULL

[Peter Krempa]

Some storage protocols allow to have the @path field in struct
virStorageSource set to NULL. Add NULLSTR() wrappers to handle this
possibility until I finish the storage source error formatter.
---
 src/util/virstoragefile.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 824cf5d..46aff92 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1391,20 +1391,21 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
 if (idx) {
 virReportError(VIR_ERR_INVALID_ARG,
_(could not find backing store %u in chain for '%s'),
-   idx, start);
+   idx, NULLSTR(start));
 } else if (name) {
 if (startFrom)
 virReportError(VIR_ERR_INVALID_ARG,
_(could not find image '%s' beneath '%s' in 
- chain for '%s'), name, startFrom-path, start);
+ chain for '%s'), name, NULLSTR(startFrom-path),
+   NULLSTR(start));
 else
 virReportError(VIR_ERR_INVALID_ARG,
_(could not find image '%s' in chain for '%s'),
-   name, start);
+   name, NULLSTR(start));
 } else {
 virReportError(VIR_ERR_INVALID_ARG,
_(could not find base image in chain for '%s'),
-   start);
+   NULLSTR(start));
 }
 *parent = NULL;
 return NULL;
-- 
2.3.5

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

Re: [libvirt] [PATCH] spec: If installing default network, reload libvirtd (bz 867546)

[Cole Robinson]

On 04/21/2015 10:43 AM, Laine Stump wrote:
 On 04/21/2015 10:34 AM, Laine Stump wrote:
 On 04/21/2015 09:48 AM, Michal Privoznik wrote:
 On 16.04.2015 21:42, Cole Robinson wrote:
 If libvirt-daemon-config-network is installed while libvirtd is already
 running, the daemon needs to be restarted to pick up the change.

 Instead let's trigger a daemon reload when the package is first installed.
 Then the default network is available immediately if libvirtd was already
 running.

 https://bugzilla.redhat.com/show_bug.cgi?id=867546
 ---
  libvirt.spec.in | 8 
  1 file changed, 8 insertions(+)

 diff --git a/libvirt.spec.in b/libvirt.spec.in
 index e08c9e7..ada0257 100644
 --- a/libvirt.spec.in
 +++ b/libvirt.spec.in
 @@ -1770,6 +1770,14 @@ if test $1 -eq 1  test ! -f 
 %{_sysconfdir}/libvirt/qemu/networks/default.xml ;
%{_datadir}/libvirt/networks/default.xml \
%{_sysconfdir}/libvirt/qemu/networks/default.xml
  ln -s ../default.xml 
 %{_sysconfdir}/libvirt/qemu/networks/autostart/default.xml
 +
 +# Make sure libvirt picks up the new network defininiton
 +  %if %{with_systemd}
 +/bin/systemctl reload libvirtd.service /dev/null 21 ||:
 +  %else
 +/sbin/service libvirtd reload  /dev/null 21 || :
 +  %endif
 +
  fi
  %endif
  

 There's already a 'systemctl try-restart libvirtd.service' call just a
 few lines below. They were added in 4789fb2e. I think we can use them -
 also, I'm not sure why it doesn't work since we are restarting daemon
 even now (without this patch).
 I think the issue may be that the default network is in a different
 sub-package than libvirtd, so libvirtd is already started/restart by the
 time the file is installed. Or something like that. I do recall that
 this is a real problem though.
 
 Heh. And if I had taken the time to go read the BZ that Cole referenced
 in his commit log message, I would have seen that I came to that same
 conclusion all the way back in 2012 :-/ It's amazing just how much I
 forget...
 
 So I think this patch will be okay if we can just verify that it doesn't
 experience the same problem that I experienced when performing the
 reload manually.

I'll see if I can reproduce, I didn't hit that issue in my testing. Maybe
try-restart is fine, but not sure if people will care

 (Or, what about the idea I posted in the BZ about
 checking if libvirtd is running, and sending the file to virsh
 net-define instead of copying it when that's the case?)
 

Interacting with virsh during RPM install scares me. Would need some testing
to make sure there isn't polkit weirdness, how we deal with
LIBVIRT_DEFAULT_URI, possibly other strange issues.

- Cole

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

[libvirt] [PATCH 3/3] util: storage: Improve error message when requesting image above 'start'

[Peter Krempa]

When a user would specify a backing chain index that is above the start
point libvirt would report a rather unhelpful error:

invalid argument: could not find backing store 1 in chain for 'sub/link2'

This patch adds an explicit check that the index is below start point in
the backing store and reports the following error if not:

invalid argument: requested backing store index 1 is above 'sub/../qcow2' in 
chain for 'sub/link2'

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177062
---
 src/util/virstoragefile.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index c9d3977..2a2f238 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1341,6 +1341,15 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
 *parent = startFrom;
 }

+if (idx  idx  i) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _(requested backing store index %u is above '%s' 
+ in chain for '%s'),
+   idx, NULLSTR(startFrom-path), NULLSTR(start));
+*parent = NULL;
+return NULL;
+}
+
 while (chain) {
 if (!name  !idx) {
 if (!chain-backingStore)
-- 
2.3.5

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

[libvirt] [PATCH 0/3] util: storage: fix indexed access to backing store

[Peter Krempa]

Improve error reporting when an index can't be found in the backing chain.

Peter Krempa (3):
  util: storage: Fix possible crash when source path is NULL
  util: storage: Add hint to error message that indexed access was used
  util: storage: Improve error message when requesting image above
'start'

 src/util/virstoragefile.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

-- 
2.3.5

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

[libvirt] [PATCH 2/3] util: storage: Add hint to error message that indexed access was used

[Peter Krempa]

---
 src/util/virstoragefile.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 46aff92..c9d3977 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1390,7 +1390,8 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
  error:
 if (idx) {
 virReportError(VIR_ERR_INVALID_ARG,
-   _(could not find backing store %u in chain for '%s'),
+   _(could not find backing store index %u in chain 
+ for '%s'),
idx, NULLSTR(start));
 } else if (name) {
 if (startFrom)
-- 
2.3.5

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

[libvirt] [PATCH V2 1/2] locking: relax PID requirement

[Jim Fehlig]

Some hypervisors like Xen do not have PIDs associated with domains.
Relax the requirement for PID != 0 in the locking code so it can
be used by hypervisors that do not represent domains as a process
running on the host.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---

V2:
Change check for ownerPid to ownerId in lock daemon dispatcher

Change instance of VIR_WARN to VIR_DEBUG to prevent polluting the
logs with warnings when creating Xen domains.

 src/locking/lock_daemon.c  |  2 +-
 src/locking/lock_daemon_dispatch.c | 16 
 src/locking/lock_driver_lockd.c|  7 ++-
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index bb165c0..042ff94 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -678,7 +678,7 @@ virLockDaemonClientFree(void *opaque)
 signum = SIGKILL;
 else
 signum = 0;
-if (virProcessKill(priv-clientPid, signum)  0) {
+if (priv-clientPid != 0  virProcessKill(priv-clientPid, 
signum)  0) {
 if (errno == ESRCH)
 break;
 
diff --git a/src/locking/lock_daemon_dispatch.c 
b/src/locking/lock_daemon_dispatch.c
index a7cee9d..bad646c 100644
--- a/src/locking/lock_daemon_dispatch.c
+++ b/src/locking/lock_daemon_dispatch.c
@@ -62,7 +62,7 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr 
server ATTRIBUTE_UNU
 goto cleanup;
 }
 
-if (!priv-ownerPid) {
+if (!priv-ownerId) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(lock owner details have not been registered));
 goto cleanup;
@@ -120,7 +120,7 @@ virLockSpaceProtocolDispatchCreateResource(virNetServerPtr 
server ATTRIBUTE_UNUS
 goto cleanup;
 }
 
-if (!priv-ownerPid) {
+if (!priv-ownerId) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(lock owner details have not been registered));
 goto cleanup;
@@ -169,7 +169,7 @@ virLockSpaceProtocolDispatchDeleteResource(virNetServerPtr 
server ATTRIBUTE_UNUS
 goto cleanup;
 }
 
-if (!priv-ownerPid) {
+if (!priv-ownerId) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(lock owner details have not been registered));
 goto cleanup;
@@ -218,7 +218,7 @@ virLockSpaceProtocolDispatchNew(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 
-if (!priv-ownerPid) {
+if (!priv-ownerId) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(lock owner details have not been registered));
 goto cleanup;
@@ -273,9 +273,9 @@ virLockSpaceProtocolDispatchRegister(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 
-if (priv-ownerPid) {
+if (!args-owner.id) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
-   _(lock owner details have already been registered));
+   _(lock owner details have not been registered));
 goto cleanup;
 }
 
@@ -320,7 +320,7 @@ virLockSpaceProtocolDispatchReleaseResource(virNetServerPtr 
server ATTRIBUTE_UNU
 goto cleanup;
 }
 
-if (!priv-ownerPid) {
+if (!priv-ownerId) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(lock owner details have not been registered));
 goto cleanup;
@@ -370,7 +370,7 @@ virLockSpaceProtocolDispatchRestrict(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 
-if (!priv-ownerPid) {
+if (!priv-ownerId) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(lock owner details have not been registered));
 goto cleanup;
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 3a48a6a..c974d60 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -466,11 +466,8 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr 
lock,
_(Missing ID parameter for domain object));
 return -1;
 }
-if (priv-pid == 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(Missing PID parameter for domain object));
-return -1;
-}
+if (priv-pid == 0)
+VIR_DEBUG(Missing PID parameter for domain object);
 if (!priv-name) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Missing name parameter for domain object));
-- 
1.8.4.5

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

[libvirt] [PATCH V2 2/2] libxl: provide integration with lock manager

[Jim Fehlig]

Provide integration with libvirt's lock manager in the libxl driver.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---

ATM, no change from V1.

 src/Makefile.am  | 12 +
 src/libxl/libvirtd_libxl.aug |  2 ++
 src/libxl/libxl.conf | 10 
 src/libxl/libxl_conf.c   | 14 +++
 src/libxl/libxl_conf.h   |  6 +
 src/libxl/libxl_domain.c | 47 ++--
 src/libxl/libxl_domain.h |  1 +
 src/libxl/libxl_driver.c | 25 +++
 src/libxl/libxl_migration.c  |  6 +
 src/libxl/test_libvirtd_libxl.aug.in |  1 +
 10 files changed, 122 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 9a5f16c..1438174 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2246,6 +2246,12 @@ BUILT_SOURCES += locking/qemu-lockd.conf
 DISTCLEANFILES += locking/qemu-lockd.conf
 endif WITH_QEMU
 
+if WITH_LIBXL
+nodist_conf_DATA += locking/libxl-lockd.conf
+BUILT_SOURCES += locking/libxl-lockd.conf
+DISTCLEANFILES += locking/libxl-lockd.conf
+endif WITH_LIBXL
+
 locking/%-lockd.conf: $(srcdir)/locking/lockd.conf
$(AM_V_GEN)$(MKDIR_P) locking ; \
cp $ $@
@@ -2431,6 +2437,12 @@ nodist_conf_DATA += locking/qemu-sanlock.conf
 BUILT_SOURCES += locking/qemu-sanlock.conf
 DISTCLEANFILES += locking/qemu-sanlock.conf
 endif WITH_QEMU
+
+if WITH_LIBXL
+nodist_conf_DATA += locking/libxl-sanlock.conf
+BUILT_SOURCES += locking/libxl-sanlock.conf
+DISTCLEANFILES += locking/libxl-sanlock.conf
+endif WITH_LIBXL
 else ! WITH_SANLOCK
 EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES)
 endif ! WITH_SANLOCK
diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug
index f225954..d5aa150 100644
--- a/src/libxl/libvirtd_libxl.aug
+++ b/src/libxl/libvirtd_libxl.aug
@@ -25,9 +25,11 @@ module Libvirtd_libxl =
 
(* Config entry grouped by function - same order as example config *)
let autoballoon_entry = bool_entry autoballoon
+   let lock_entry = str_entry lock_manager
 
(* Each entry in the config is one of the following ... *)
let entry = autoballoon_entry
+ | lock_entry
 
let comment = [ label #comment . del /#[ \t]*/ #  .  store /([^ 
\t\n][^\n]*)?/ . del /\n/ \n ]
let empty = [ label #empty . eol ]
diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf
index c104d40..ba3de7a 100644
--- a/src/libxl/libxl.conf
+++ b/src/libxl/libxl.conf
@@ -10,3 +10,13 @@
 # autoballoon setting.
 #
 #autoballoon = 1
+
+
+# In order to prevent accidentally starting two domains that
+# share one writable disk, libvirt offers two approaches for
+# locking files: sanlock and virtlockd.  sanlock is an external
+# project which libvirt integrates with via the libvirt-lock-sanlock
+# package.  virtlockd is a libvirt implementation that is enabled with
+# lockd.  Accepted values are sanlock and lockd.
+#
+#lock_manager = lockd
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 1b504fa..29498d5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -102,6 +102,7 @@ libxlDriverConfigDispose(void *obj)
 VIR_FREE(cfg-libDir);
 VIR_FREE(cfg-saveDir);
 VIR_FREE(cfg-autoDumpDir);
+VIR_FREE(cfg-lockManagerName);
 }
 
 
@@ -1495,6 +1496,7 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
   const char *filename)
 {
 virConfPtr conf = NULL;
+virConfValuePtr p;
 int ret = -1;
 
 /* Check the file is readable before opening it, otherwise
@@ -1512,6 +1514,18 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
 if (libxlGetAutoballoonConf(cfg, conf)  0)
 goto cleanup;
 
+if ((p = virConfGetValue(conf, lock_manager))) {
+if (p-type != VIR_CONF_STRING) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s,
+   _(Unexpected type for 'lock_manager' setting));
+goto cleanup;
+}
+
+if (VIR_STRDUP(cfg-lockManagerName, p-str)  0)
+goto cleanup;
+}
+
 ret = 0;
 
  cleanup:
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 5ba1a71..0a1c0db 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -38,6 +38,7 @@
 # include virobject.h
 # include virchrdev.h
 # include virhostdev.h
+# include locking/lock_manager.h
 
 # define LIBXL_DRIVER_NAME xenlight
 # define LIBXL_VNC_PORT_MIN  5900
@@ -98,6 +99,8 @@ struct _libxlDriverConfig {
  * memory for new domains from domain0. */
 bool autoballoon;
 
+char *lockManagerName;
+
 /* Once created, caps are immutable */
 virCapsPtr caps;
 
@@ -144,6 +147,9 @@ struct _libxlDriverPrivate {
 
 /* Immutable pointer, lockless APIs*/
 virSysinfoDefPtr hostsysinfo;
+
+/* Immutable pointer. lockless access */
+virLockManagerPluginPtr lockManager;
 };
 
 # define LIBXL_SAVE_MAGIC libvirt-xml\n \0 \r
diff --git 

Re: [libvirt] [PATCH 3/3] libxl: provide integration with lock manager

[Jim Fehlig]

Daniel P. Berrange wrote:
 On Fri, Apr 17, 2015 at 03:36:22PM -0600, Jim Fehlig wrote:
   
 Provide integration with libvirt's lock manager in the libxl driver.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  src/Makefile.am  | 12 +
  src/libxl/libvirtd_libxl.aug |  2 ++
  src/libxl/libxl.conf | 10 
  src/libxl/libxl_conf.c   | 14 +++
  src/libxl/libxl_conf.h   |  6 +
  src/libxl/libxl_domain.c | 47 
 ++--
  src/libxl/libxl_domain.h |  1 +
  src/libxl/libxl_driver.c | 25 +++
  src/libxl/libxl_migration.c  |  6 +
  src/libxl/test_libvirtd_libxl.aug.in |  1 +
  10 files changed, 122 insertions(+), 2 deletions(-)

 diff --git a/src/Makefile.am b/src/Makefile.am
 index 9a5f16c..1438174 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -2246,6 +2246,12 @@ BUILT_SOURCES += locking/qemu-lockd.conf
  DISTCLEANFILES += locking/qemu-lockd.conf
  endif WITH_QEMU
  
 +if WITH_LIBXL
 +nodist_conf_DATA += locking/libxl-lockd.conf
 +BUILT_SOURCES += locking/libxl-lockd.conf
 +DISTCLEANFILES += locking/libxl-lockd.conf
 +endif WITH_LIBXL
 +
  locking/%-lockd.conf: $(srcdir)/locking/lockd.conf
  $(AM_V_GEN)$(MKDIR_P) locking ; \
  cp $ $@
 @@ -2431,6 +2437,12 @@ nodist_conf_DATA += locking/qemu-sanlock.conf
  BUILT_SOURCES += locking/qemu-sanlock.conf
  DISTCLEANFILES += locking/qemu-sanlock.conf
  endif WITH_QEMU
 +
 +if WITH_LIBXL
 +nodist_conf_DATA += locking/libxl-sanlock.conf
 +BUILT_SOURCES += locking/libxl-sanlock.conf
 +DISTCLEANFILES += locking/libxl-sanlock.conf
 +endif WITH_LIBXL
  else ! WITH_SANLOCK
  EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES)
  endif ! WITH_SANLOCK
 diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug
 index f225954..d5aa150 100644
 --- a/src/libxl/libvirtd_libxl.aug
 +++ b/src/libxl/libvirtd_libxl.aug
 @@ -25,9 +25,11 @@ module Libvirtd_libxl =
  
 (* Config entry grouped by function - same order as example config *)
 let autoballoon_entry = bool_entry autoballoon
 +   let lock_entry = str_entry lock_manager
  
 (* Each entry in the config is one of the following ... *)
 let entry = autoballoon_entry
 + | lock_entry
  
 let comment = [ label #comment . del /#[ \t]*/ #  .  store /([^ 
 \t\n][^\n]*)?/ . del /\n/ \n ]
 let empty = [ label #empty . eol ]
 diff --git a/src/libxl/libxl.conf b/src/libxl/libxl.conf
 index c104d40..ba3de7a 100644
 --- a/src/libxl/libxl.conf
 +++ b/src/libxl/libxl.conf
 @@ -10,3 +10,13 @@
  # autoballoon setting.
  #
  #autoballoon = 1
 +
 +
 +# In order to prevent accidentally starting two domains that
 +# share one writable disk, libvirt offers two approaches for
 +# locking files: sanlock and virtlockd.  sanlock is an external
 +# project which libvirt integrates with via the libvirt-lock-sanlock
 +# package.  virtlockd is a libvirt implementation that is enabled with
 +# lockd.  Accepted values are sanlock and lockd.
 +#
 +#lock_manager = lockd
 diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
 index 6ea2889..503e8a4 100644
 --- a/src/libxl/libxl_conf.c
 +++ b/src/libxl/libxl_conf.c
 @@ -102,6 +102,7 @@ libxlDriverConfigDispose(void *obj)
  VIR_FREE(cfg-libDir);
  VIR_FREE(cfg-saveDir);
  VIR_FREE(cfg-autoDumpDir);
 +VIR_FREE(cfg-lockManagerName);
  }
  
  
 @@ -1495,6 +1496,7 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
const char *filename)
  {
  virConfPtr conf = NULL;
 +virConfValuePtr p;
  int ret = -1;
  
  /* Check the file is readable before opening it, otherwise
 @@ -1512,6 +1514,18 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr 
 cfg,
  if (libxlGetAutoballoonConf(cfg, conf)  0)
  goto cleanup;
  
 +if ((p = virConfGetValue(conf, lock_manager))) {
 +if (p-type != VIR_CONF_STRING) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   %s,
 +   _(Unexpected type for 'lock_manager' setting));
 +goto cleanup;
 +}
 +
 +if (VIR_STRDUP(cfg-lockManagerName, p-str)  0)
 +goto cleanup;
 +}
 +
  ret = 0;
  
   cleanup:
 diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
 index 5ba1a71..0a1c0db 100644
 --- a/src/libxl/libxl_conf.h
 +++ b/src/libxl/libxl_conf.h
 @@ -38,6 +38,7 @@
  # include virobject.h
  # include virchrdev.h
  # include virhostdev.h
 +# include locking/lock_manager.h
  
  # define LIBXL_DRIVER_NAME xenlight
  # define LIBXL_VNC_PORT_MIN  5900
 @@ -98,6 +99,8 @@ struct _libxlDriverConfig {
   * memory for new domains from domain0. */
  bool autoballoon;
  
 +char *lockManagerName;
 +
  /* Once created, caps are immutable */
  virCapsPtr caps;
  
 @@ -144,6 +147,9 @@ struct _libxlDriverPrivate {
  
  /* Immutable pointer, lockless 

Re: [libvirt] [PATCH 03/11] caps: Use an enum internally for ostype value

[Cole Robinson]

On 04/21/2015 05:22 AM, Christophe Fergeau wrote:
 Hey,
 
 On Fri, Apr 17, 2015 at 09:45:13PM -0400, Cole Robinson wrote:
 +VIR_ENUM_IMPL(virDomainOS, VIR_DOMAIN_OSTYPE_LAST,
 +  hvm,
 +  xen,
 +  linux,
 +  exe,
 +  uml,
 +  aix)
 +
 
 This is preexisting before your patch, but is more obvious now.
 Shouldn't 'aix' be listed in docs/schemas/capability.rng? Currently it's
 
   define name='ostype'
 element name='os_type'
   choice
 valuexen/value !-- Xen 3.0 pv --
 valuelinux/value !-- same as 'xen' - legacy --
 valuehvm/value !-- unmodified OS --
 valueexe/value !-- For container based virt --
 valueuml/value !-- user mode linux --
   /choice
 /element
   /define
 
 Christophe
 

Good point, I just sent a patch

Thanks,
Cole

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

[libvirt] [PATCH] schemas: capability: Add ostype 'aix'

[Cole Robinson]

---
 docs/schemas/capability.rng | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 5f3ec70..3868ee2 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -266,6 +266,7 @@
 valuehvm/value !-- unmodified OS --
 valueexe/value !-- For container based virt --
 valueuml/value !-- user mode linux --
+valueaix/value !-- used by phyp driver --
   /choice
 /element
   /define
-- 
2.3.5

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

Re: [libvirt] [PATCH 2/3] libxl: Introduce configuration file for libxl driver

[Jim Fehlig]

Daniel P. Berrange wrote:
 On Fri, Apr 17, 2015 at 03:36:21PM -0600, Jim Fehlig wrote:
   
 Introduce libxl.conf configuration file, adding the 'autoballoon'
 setting as the first knob for controlling the libxl driver.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
  libvirt.spec.in  |  8 +
  src/Makefile.am  | 24 +-
  src/libxl/libvirtd_libxl.aug | 42 +
  src/libxl/libxl.conf | 12 +++
  src/libxl/libxl_conf.c   | 61 
 
  src/libxl/libxl_conf.h   |  5 +++
  src/libxl/libxl_driver.c |  9 ++
  src/libxl/test_libvirtd_libxl.aug.in |  5 +++
  8 files changed, 159 insertions(+), 7 deletions(-)
 

 ACK
   

Thanks!  I've pushed this patch since it is independent of the other two.

Regards,
Jim

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

Re: [libvirt] [PATCH] schemas: capability: Add ostype 'aix'

[Eric Blake]

On 04/21/2015 02:17 PM, Cole Robinson wrote:
 ---
  docs/schemas/capability.rng | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
 index 5f3ec70..3868ee2 100644
 --- a/docs/schemas/capability.rng
 +++ b/docs/schemas/capability.rng
 @@ -266,6 +266,7 @@
  valuehvm/value !-- unmodified OS --
  valueexe/value !-- For container based virt --
  valueuml/value !-- user mode linux --
 +valueaix/value !-- used by phyp driver --

ACK

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

[libvirt] [PATCH V2 0/2] Support libvirt's lock manager in the libxl driver

[Jim Fehlig]

V2 of

https://www.redhat.com/archives/libvir-list/2015-April/msg00845.html

Patch 2 has been ACKed and pushed since it is independent of the
lock manager integration.  Patch 1 has been changed to address
Daniel's comments.  Patch 2 (was patch 3 in V1) is unchanged for
the time being.

Jim Fehlig (2):
  locking: relax PID requirement
  libxl: provide integration with lock manager

 src/Makefile.am  | 12 +
 src/libxl/libvirtd_libxl.aug |  2 ++
 src/libxl/libxl.conf | 10 
 src/libxl/libxl_conf.c   | 14 +++
 src/libxl/libxl_conf.h   |  6 +
 src/libxl/libxl_domain.c | 47 ++--
 src/libxl/libxl_domain.h |  1 +
 src/libxl/libxl_driver.c | 25 +++
 src/libxl/libxl_migration.c  |  6 +
 src/libxl/test_libvirtd_libxl.aug.in |  1 +
 src/locking/lock_daemon.c|  2 +-
 src/locking/lock_daemon_dispatch.c   | 16 ++--
 src/locking/lock_driver_lockd.c  |  7 ++
 13 files changed, 133 insertions(+), 16 deletions(-)

-- 
1.8.4.5

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

Re: [libvirt] [RFC 0/7] Live Migration with Pass-through Devices proposal

[Chen Fan]

Hi Laine,

Thanks for your review for my patches.

and do you know that solarflare's patches have made some update version
since

https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html

?

if not, I hope to go on to complete this work. ;)

Thanks,
Chen


On 04/20/2015 06:29 AM, Laine Stump wrote:

On 04/17/2015 04:53 AM, Chen Fan wrote:

backgrond:
Live migration is one of the most important features of virtualization 
technology.
With regard to recent virtualization techniques, performance of network I/O is 
critical.
Current network I/O virtualization (e.g. Para-virtualized I/O, VMDq) has a 
significant
performance gap with native network I/O. Pass-through network devices have near
native performance, however, they have thus far prevented live migration. No 
existing
methods solve the problem of live migration with pass-through devices perfectly.

There was an idea to solve the problem in website:
https://www.kernel.org/doc/ols/2008/ols2008v2-pages-261-267.pdf
Please refer to above document for detailed information.

This functionality has been on my mind/bug list for a long time, but I
haven't been able to pursue it much. See this BZ, along with the
original patches submitted by Shradha Shah from SolarFlare:

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

(I was a bit optimistic in my initial review of the patches - there are
actually a lot of issues that weren't handled by those patches.)


So I think this problem maybe could be solved by using the combination of 
existing
technology. and the following steps are we considering to implement:

-  before boot VM, we anticipate to specify two NICs for creating bonding device
(one plugged and one virtual NIC) in XML. here we can specify the NIC's mac 
addresses
in XML, which could facilitate qemu-guest-agent to find the network 
interfaces in guest.

An interesting idea, but I think that is a 2nd level enhancement, not
necessary initially (and maybe not ever, due to the high possibility of
it being extremely difficult to get right in 100% of the cases).


-  when qemu-guest-agent startup in guest it would send a notification to 
libvirt,
then libvirt will call the previous registered initialize callbacks. so 
through
the callback functions, we can create the bonding device according to the 
XML
configuration. and here we use netcf tool which can facilitate to create 
bonding device
easily.

This isn't quite making sense - the bond will be on the guest, which may
not have netcf installed. Anyway, I think it should be up to the guest's
own system network config to have the bond already setup. If you try to
impose it from outside that infrastructure, you run too much risk of
running afoul of something on the guest (e.g. NetworkManager)



-  during migration, unplug the passthroughed NIC. then do native migration.

Correct. This is the most important part. But not just unplugging it,
you also need to wait until the unplug operation completes (it is
asynchronous). (After this point, the emulated NIC that is part of the
bond would get all of the traffic).


-  on destination side, check whether need to hotplug new NIC according to 
specified XML.
usually, we use migrate --xml command option to specify the destination 
host NIC mac
address to hotplug a new NIC, because source side passthrough NIC mac 
address is different,
then hotplug the deivce according to the destination XML configuration.

Why does the MAC address need to be different? Are you suggesting doing
this with passed-through non-SRIOV NICs? An SRIOV virtual function gets
its MAC address from the libvirt config, so it's very simple to use the
same MAC address across the migration. Any network card that would be
able to do this on any sort of useful scale will be SRIOV-capable (or
should be replaced with one that is - some of them are not that expensive).



TODO:
   1.  when hot add a new NIC in destination side after migration finished, the 
NIC device
   need to re-enslave on bonding device in guest. otherwise, it is offline. 
maybe
   we should consider bonding driver to support add interfaces dynamically.

I never looked at the details of how SolarFlare's code handled the guest
side (they have/had their own patchset they maintained for some older
version of libvirt which integrated with some sort of enhanced bonding
driver on the guests). I assumed the bond driver could handle this
already, but have to say I never investigated.



This is an example on how this might work, so I want to hear some voices about 
this scenario.

Thanks,
Chen

Chen Fan (7):
   qemu-agent: add agent init callback when detecting guest setup
   qemu: add guest init event callback to do the initialize work for
 guest
   hostdev: add a 'bond' type element in hostdev element


Putting this into hostdev is the wrong approach, for two reasons: 1)
it doesn't account for the device to be used being in a different
address on the source and destination 

Re: [libvirt] [Qemu-devel] [RFC 3/3] qemu-agent: add notify for qemu-ga boot

[Eric Blake]

On 04/17/2015 02:53 AM, Chen Fan wrote:
 Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
 ---
  qga/main.c | 13 +
  1 file changed, 13 insertions(+)

I'm not sure that qga should be sending asynchronous messages (so far,
it only every replies synchronously).  As it is, we already wired up a
qemu event that fires any time the guest opens or closes the virtio
connection powering the agent; libvirt can already use those events to
know when the agent has opened the connection, and is presumably ready
to listen to commands after first booting.  So I don't think this patch
is needed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

[libvirt] [PATCH v4 1/9] conf: Add new domain XML element 'iothreadids'

[John Ferlan]

Adding a new XML element 'iothreadids' in order to allow defining
specific IOThread ID's rather than relying on the algorithm to assign
IOThread ID's starting at 1 and incrementing to iothreads count.

This will allow future patches to be able to add new IOThreads by
a specific iothread_id and of course delete any exisiting IOThread.

Each iothreadids element will have 'n' iothread children elements
which will have attribute id.  The id will allow for definition
of any valid (eg  0) iothread_id value.

On input, if any iothreadids iothread's are provided, they will
be marked so that we only print out what we read in.

On input, if no iothreadids are provided, the PostParse code will
self generate a list of ID's starting at 1 and going to the number
of iothreads defined for the domain (just like the current algorithm
numbering scheme).  A future patch will rework the existing algorithm
to make use of the iothreadids list.

On output, only print out the iothreadids if they were read in.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatdomain.html.in |  30 +++
 docs/schemas/domaincommon.rng |  12 +++
 src/conf/domain_conf.c| 200 +-
 src/conf/domain_conf.h|  18 
 src/libvirt_private.syms  |   4 +
 5 files changed, 262 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e921749..518f7c5 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -521,6 +521,18 @@
   ...
 lt;/domaingt;
 /pre
+pre
+lt;domaingt;
+  ...
+  lt;iothreadidsgt;
+lt;iothread id=2/gt;
+lt;iothread id=4/gt;
+lt;iothread id=6/gt;
+lt;iothread id=8/gt;
+  lt;/iothreadidsgt;
+  ...
+lt;/domaingt;
+/pre
 
 dl
   dtcodeiothreads/code/dt
@@ -530,7 +542,25 @@
 virtio-blk-pci and virtio-blk-ccw target storage devices. There
 should be only 1 or 2 IOThreads per host CPU. There may be more
 than one supported device assigned to each IOThread.
+span class=sinceSince 1.2.8/span
   /dd
+  dtcodeiothreadids/code/dt
+  dd
+The optional codeiothreadids/code element provides the capability
+to specifically define the IOThread ID's for the domain.  By default,
+IOThread ID's are sequentially numbered starting from 1 through the
+number of codeiothreads/code defined for the domain. The
+codeid/code attribute is used to define the IOThread ID. The
+codeid/code attribute must be a positive integer greater than 0.
+If there are less codeiothreadids/code defined than
+codeiothreads/code defined for the domain, then libvirt will
+sequentially fill codeiothreadids/code starting at 1 avoiding
+any predefined codeid/code. If there are more
+codeiothreadids/code defined than codeiothreads/code
+defined for the domain, then the codeiothreads/code value
+will be adjusted accordingly.
+span class=sinceSince 1.2.15/span
+   /dd
 /dl
 
 h3a name=elementsCPUTuningCPU Tuning/a/h3
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 19461f5..7072954 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -539,6 +539,18 @@
   /optional
 
   optional
+element name=iothreadids
+  zeroOrMore
+element name=iothread
+  attribute name=id
+ref name=unsignedInt/
+  /attribute
+/element
+  /zeroOrMore
+/element
+  /optional
+
+  optional
 ref name=blkiotune/
   /optional
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 479b4c2..da1bb7e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2114,6 +2114,31 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
 return NULL;
 }
 
+
+void
+virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def)
+{
+if (def)
+VIR_FREE(def);
+}
+
+
+static void
+virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
+int nids)
+{
+size_t i;
+
+if (!def)
+return;
+
+for (i = 0; i  nids; i++)
+virDomainIOThreadIDDefFree(def[i]);
+
+VIR_FREE(def);
+}
+
+
 void
 virDomainPinDefFree(virDomainPinDefPtr def)
 {
@@ -2310,6 +2335,8 @@ void virDomainDefFree(virDomainDefPtr def)
 
 virCPUDefFree(def-cpu);
 
+virDomainIOThreadIDDefArrayFree(def-iothreadids, def-niothreadids);
+
 virDomainPinDefArrayFree(def-cputune.vcpupin, def-cputune.nvcpupin);
 
 virDomainPinDefFree(def-cputune.emulatorpin);
@@ -3298,6 +3325,21 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
 return -1;
 }
 
+/* Fully populate the IOThread ID list */
+if (def-iothreads  def-iothreads != def-niothreadids) {
+unsigned int iothread_id = 1;
+while (def-niothreadids != def-iothreads) {
+if 

[libvirt] [PATCH v4 0/9] Implement Add/Del IOThreads

[John Ferlan]

v3 here:
http://www.redhat.com/archives/libvir-list/2015-April/msg00621.html

Changes since v3

Patch 1:
 * Add back in and use virDomainIOThreadIDDefFree
 * Change _virDomainIOThreadIDDef 'undefined' to 'autofill'
 * Adjust virDomainDefParseXML to only allocate what's defined
   allowing virDomainDefPostParseInternal to Add in IOThreads
   which weren't defined and set the 'autofill' boolean
 * Have virDomainIOThreadIDAdd return a virDomainIOThreadIDDefPtr

Patch 2: No changes

Patch 3: (NEW)
 * Discussed in prior review regarding making virDomainPinIsDuplicate
   a static function (needed to move it)

Patch 4: (NEW)
 * Move the iothreadspin data (e.g., the cpumask) into _virDomainIOThreadIDDef
 * Kept the 'niothreadspin' and manipulated as necessary since there was code
   to write out cputune data that I didn't want to reinvent/rototill just
   to search through iothreadids for a cpumask
 * Adjusted virDomainIOThreadPinDefParseXML to handle storing the cpumask
   in the right iothreadsid[] entry.  If not found (it may not be since the
   virDomainDefPostParseInternal hasn't run), then create an autofill version
   of an iothreadids entry.
 * Remove/adjust a lot of code that used to handle iothreadspin

Patch 5: (NEW)
 * Slight adjustment for iothreadsched to allow for any id value. This
   code stores iothread id's as bitmap entries, so it didn't have the same
   issues as the iothreadspin code

Patch 6-7: Unchanged

Patch 8:
 * Adjusted the search for the new thread code to use existing alias
 * Use the virDomainIOThreadIDAdd returned pointer
 * Comment adjustments from code review
 * Removal of erroneously cut-n-pasted code in Delete path
 * Use the -dst for the message
 * Changes based on having cpumask in the iothrid data

Patch 9: No changes

John Ferlan (9):
  conf: Add new domain XML element 'iothreadids'
  qemu: Use domain iothreadids to IOThread's 'thread_id'
  conf: Move virDomainPinIsDuplicate and make static
  Move iothreadspin information into iothreadids
  conf: Adjust the iothreadsched expectations
  Implement virDomainAddIOThread and virDomainDelIOThread
  remote: Add support for AddIOThread and DelIOThread
  qemu: Add support to Add/Delete IOThreads
  virsh: Add iothreadadd and iothreaddel commands

 docs/formatdomain.html.in  |  46 +-
 docs/schemas/domaincommon.rng  |  12 +
 include/libvirt/libvirt-domain.h   |   6 +
 src/conf/domain_audit.c|   9 +
 src/conf/domain_audit.h|   6 +
 src/conf/domain_conf.c | 340 ++
 src/conf/domain_conf.h |  25 +-
 src/driver-hypervisor.h|  12 +
 src/libvirt-domain.c   | 118 +
 src/libvirt_private.syms   |   6 +-
 src/libvirt_public.syms|   6 +
 src/qemu/qemu_cgroup.c |  31 +-
 src/qemu/qemu_command.c|  38 +-
 src/qemu/qemu_command.h|   3 +
 src/qemu/qemu_domain.c |  36 --
 src/qemu/qemu_domain.h |   3 -
 src/qemu/qemu_driver.c | 511 ++---
 src/qemu/qemu_process.c|  40 +-
 src/remote/remote_driver.c |   2 +
 src/remote/remote_protocol.x   |  30 +-
 src/remote_protocol-structs|  12 +
 .../qemuxml2argv-cputune-iothreadsched-toomuch.xml |   1 +
 .../qemuxml2argv-iothreads-ids-partial.args|  10 +
 .../qemuxml2argv-iothreads-ids-partial.xml |  33 ++
 .../qemuxml2argv-iothreads-ids.args|   8 +
 .../qemuxml2argv-iothreads-ids.xml |  33 ++
 tests/qemuxml2argvtest.c   |   2 +
 tests/qemuxml2xmltest.c|   2 +
 tools/virsh-domain.c   | 164 +++
 tools/virsh.pod|  31 ++
 30 files changed, 1320 insertions(+), 256 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml

-- 
2.1.0

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

[libvirt] [PATCH v4 2/9] qemu: Use domain iothreadids to IOThread's 'thread_id'

[John Ferlan]

Add 'thread_id' to the virDomainIOThreadIDDef as a means to store the
'thread_id' as returned from the live qemu monitor data.

Remove the iothreadpids list from _qemuDomainObjPrivate and replace with
the new iothreadids 'thread_id' element.

Rather than use the default numbering scheme of 1..number of iothreads
defined for the domain, use the iothreadid's list for the iothread_id

Since iothreadids list keeps track of the iothread_id's, these are
now used in place of the many places where a for loop would know
that the ID was + 1 from the array element.

The new tests ensure usage of the iothreadid values for an exact number
of iothreads and the usage of a smaller number of iothreadid values than
iothreads that exist (and usage of the default numbering scheme).

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_cgroup.c | 22 ++---
 src/qemu/qemu_command.c| 38 +-
 src/qemu/qemu_command.h|  3 ++
 src/qemu/qemu_domain.c | 36 
 src/qemu/qemu_domain.h |  3 --
 src/qemu/qemu_driver.c | 35 +---
 src/qemu/qemu_process.c| 37 ++---
 .../qemuxml2argv-iothreads-ids-partial.args| 10 ++
 .../qemuxml2argv-iothreads-ids-partial.xml | 33 +++
 .../qemuxml2argv-iothreads-ids.args|  8 +
 .../qemuxml2argv-iothreads-ids.xml | 33 +++
 tests/qemuxml2argvtest.c   |  2 ++
 tests/qemuxml2xmltest.c|  2 ++
 14 files changed, 164 insertions(+), 99 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids-partial.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-iothreads-ids.xml

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d4229eb..2d53a8d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2060,6 +2060,7 @@ typedef virDomainIOThreadIDDef *virDomainIOThreadIDDefPtr;
 struct _virDomainIOThreadIDDef {
 bool autofill;
 unsigned int iothread_id;
+int thread_id;
 };
 
 void virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def);
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e83342d..cdf0aaf 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -802,8 +802,9 @@ qemuRestoreCgroupState(virDomainObjPtr vm)
 virCgroupFree(cgroup_temp);
 }
 
-for (i = 0; i  priv-niothreadpids; i++) {
-if (virCgroupNewThread(priv-cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1,
+for (i = 0; i  vm-def-niothreadids; i++) {
+if (virCgroupNewThread(priv-cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+   vm-def-iothreadids[i]-iothread_id,
false, cgroup_temp)  0 ||
 virCgroupSetCpusetMemoryMigrate(cgroup_temp, true)  0 ||
 virCgroupGetCpusetMems(cgroup_temp, nodeset)  0 ||
@@ -1175,11 +1176,6 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
 if (priv-cgroup == NULL)
 return 0;
 
-if (def-iothreads  priv-niothreadpids == 0) {
-VIR_WARN(Unable to get iothreads' pids.);
-return 0;
-}
-
 if (virDomainNumatuneGetMode(vm-def-numa, -1) ==
 VIR_DOMAIN_NUMATUNE_MEM_STRICT 
 virDomainNumatuneMaybeFormatNodeset(vm-def-numa,
@@ -1187,16 +1183,18 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
 mem_mask, -1)  0)
 goto cleanup;
 
-for (i = 0; i  priv-niothreadpids; i++) {
+for (i = 0; i  def-niothreadids; i++) {
 /* IOThreads are numbered 1..n, although the array is 0..n-1,
  * so we will account for that here
  */
-if (virCgroupNewThread(priv-cgroup, VIR_CGROUP_THREAD_IOTHREAD, i + 1,
+if (virCgroupNewThread(priv-cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+   def-iothreadids[i]-iothread_id,
true, cgroup_iothread)  0)
 goto cleanup;
 
 /* move the thread for iothread to sub dir */
-if (virCgroupAddTask(cgroup_iothread, priv-iothreadpids[i])  0)
+if (virCgroupAddTask(cgroup_iothread,
+ def-iothreadids[i]-thread_id)  0)
 goto cleanup;
 
 if (period || quota) {
@@ -1221,8 +1219,8 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
 
 /* specific cpu mask */
 for (j = 0; j  def-cputune.niothreadspin; j++) {
-/* IOThreads are numbered/named 1..n */
-if 

[libvirt] [PATCH v4 9/9] virsh: Add iothreadadd and iothreaddel commands

[John Ferlan]

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

Add command to allow adding and removing IOThreads from the domain including
the configuration and live domain.

$ virsh iothreadadd --help
  NAME
iothreadadd - add an IOThread to the guest domain

  SYNOPSIS
iothreadadd domain id [--config] [--live] [--current]

  DESCRIPTION
Add an IOThread to the guest domain.

  OPTIONS
[--domain] string  domain name, id or uuid
[--id] number  iothread for the new IOThread
--config affect next boot
--live   affect running domain
--currentaffect current domain

$ virsh iothreaddel --help
  NAME
iothreaddel - delete an IOThread from the guest domain

  SYNOPSIS
iothreaddel domain id [--config] [--live] [--current]

  DESCRIPTION
Delete an IOThread from the guest domain.

  OPTIONS
[--domain] string  domain name, id or uuid
[--id] number  iothread_id for the IOThread to delete
--config affect next boot
--live   affect running domain
--currentaffect current domain

Assuming a running $dom with multiple IOThreads assigned and that
that the $dom has disks assigned to IOThread 1 and IOThread 2:

$ virsh iothreadinfo $dom
 IOThread ID CPU Affinity
 ---
  1   2
  2   3
  3   0-1

$ virsh iothreadadd $dom 1
error: invalid argument: an IOThread is already using iothread_id '1' in 
iothreadpids

$ virsh iothreadadd $dom 1 --config
error: invalid argument: an IOThread is already using iothread_id '1' in 
persistent iothreadids

$ virsh iothreadadd $dom 4
$ virsh iothreadinfo $dom
 IOThread ID CPU Affinity
 ---
  1   2
  2   3
  3   0-1
  4   0-3

$ virsh iothreadinfo $dom --config
 IOThread ID CPU Affinity
 ---
  1   2
  2   3
  3   0-1

$ virsh iothreadadd $dom 4 --config
$ virsh iothreadinfo $dom --config
 IOThread ID CPU Affinity
  ---
1   2
2   3
3   0-1
4   0-3

Assuming the same original configuration

$ virsh iothreaddel $dom 1
error: invalid argument: cannot remove IOThread 1 since it is being used by 
disk path '/home/vm-images/f18'

$ virsh iothreaddel $dom 3

$ virsh iothreadinfo $dom
 IOThread ID CPU Affinity
 ---
  1   2
  2   3

$ virsh iothreadinfo $dom --config
 IOThread ID CPU Affinity
 ---
  1   2
  2   3
  3   0-1

Signed-off-by: John Ferlan jfer...@redhat.com
---
 tools/virsh-domain.c | 164 +++
 tools/virsh.pod  |  31 ++
 2 files changed, 195 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4f58f07..3c9cd64 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6880,6 +6880,158 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd)
 }
 
 /*
+ * iothreadadd command
+ */
+static const vshCmdInfo info_iothreadadd[] = {
+{.name = help,
+ .data = N_(add an IOThread to the guest domain)
+},
+{.name = desc,
+ .data = N_(Add an IOThread to the guest domain.)
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_iothreadadd[] = {
+{.name = domain,
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_(domain name, id or uuid)
+},
+{.name = id,
+ .type = VSH_OT_INT,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_(iothread for the new IOThread)
+},
+{.name = config,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect next boot)
+},
+{.name = live,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect running domain)
+},
+{.name = current,
+ .type = VSH_OT_BOOL,
+ .help = N_(affect current domain)
+},
+{.name = NULL}
+};
+
+static bool
+cmdIOThreadAdd(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+int iothread_id = 0;
+bool ret = false;
+bool config = vshCommandOptBool(cmd, config);
+bool live = vshCommandOptBool(cmd, live);
+bool current = vshCommandOptBool(cmd, current);
+unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
+
+VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+if (config)
+flags |= VIR_DOMAIN_AFFECT_CONFIG;
+if (live)
+flags |= VIR_DOMAIN_AFFECT_LIVE;
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (vshCommandOptInt(cmd, id, iothread_id)  0 || iothread_id = 0) {
+vshError(ctl, _(Invalid IOThread id value: '%d'), iothread_id);
+goto cleanup;
+}
+
+if (virDomainAddIOThread(dom, 

[libvirt] [PATCH v4 8/9] qemu: Add support to Add/Delete IOThreads

[John Ferlan]

Add qemuDomainAddIOThread and qemuDomainDelIOThread in order to add or
remove an IOThread to/from the host either for live or config optoins

The implementation for the 'live' option will use the iothreadpids list
in order to make decision, while the 'config' option will use the
iothreadids list.  Additionally, for deletion each may have to adjust
the iothreadpin list.

IOThreads are implemented by qmp objects, the code makes use of the existing
qemuMonitorAddObject or qemuMonitorDelObject APIs.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_audit.c  |   9 ++
 src/conf/domain_audit.h  |   6 +
 src/libvirt_private.syms |   1 +
 src/qemu/qemu_driver.c   | 401 +++
 4 files changed, 417 insertions(+)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 3e93d97..4ea10d2 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -790,6 +790,15 @@ virDomainAuditVcpu(virDomainObjPtr vm,
 return virDomainAuditResource(vm, vcpu, oldvcpu, newvcpu, reason, 
success);
 }
 
+void
+virDomainAuditIOThread(virDomainObjPtr vm,
+   unsigned int oldiothread, unsigned int newiothread,
+   const char *reason, bool success)
+{
+return virDomainAuditResource(vm, iothread, oldiothread, newiothread,
+  reason, success);
+}
+
 static void
 virDomainAuditLifecycle(virDomainObjPtr vm, const char *op,
 const char *reason, bool success)
diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h
index 4c1ef90..97dadca 100644
--- a/src/conf/domain_audit.h
+++ b/src/conf/domain_audit.h
@@ -102,6 +102,12 @@ void virDomainAuditVcpu(virDomainObjPtr vm,
 const char *reason,
 bool success)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
+void virDomainAuditIOThread(virDomainObjPtr vm,
+unsigned int oldiothread,
+unsigned int newiothread,
+const char *reason,
+bool success)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
 void virDomainAuditSecurityLabel(virDomainObjPtr vm,
  bool success)
 ATTRIBUTE_NONNULL(1);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a0f4152..6f2cf40 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -127,6 +127,7 @@ virDomainAuditDisk;
 virDomainAuditFS;
 virDomainAuditHostdev;
 virDomainAuditInit;
+virDomainAuditIOThread;
 virDomainAuditMemory;
 virDomainAuditNet;
 virDomainAuditNetDevice;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ee13d08..c34abc9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6137,6 +6137,405 @@ qemuDomainPinIOThread(virDomainPtr dom,
 return ret;
 }
 
+static int
+qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ unsigned int iothread_id)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+char *alias = NULL;
+size_t idx;
+int rc = -1;
+int ret = -1;
+unsigned int orig_niothreads = vm-def-iothreads;
+unsigned int exp_niothreads = vm-def-iothreads;
+int new_niothreads = 0;
+qemuMonitorIOThreadInfoPtr *new_iothreads = NULL;
+virCgroupPtr cgroup_iothread = NULL;
+char *mem_mask = NULL;
+virDomainIOThreadIDDefPtr iothrid;
+
+if (virDomainIOThreadIDFind(vm-def, iothread_id)) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _(an IOThread is already using iothread_id '%u'),
+   iothread_id);
+goto cleanup;
+}
+
+if (virAsprintf(alias, iothread%u, iothread_id)  0)
+return -1;
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+rc = qemuMonitorAddObject(priv-mon, iothread, alias, NULL);
+exp_niothreads++;
+if (rc  0)
+goto exit_monitor;
+
+/* After hotplugging the IOThreads we need to re-detect the
+ * IOThreads thread_id's, adjust the cgroups, thread affinity,
+ * and add the thread_id to the vm-def-iothreadids list.
+ */
+if ((new_niothreads = qemuMonitorGetIOThreads(priv-mon,
+  new_iothreads))  0)
+goto exit_monitor;
+
+if (qemuDomainObjExitMonitor(driver, vm)  0)
+goto cleanup;
+
+if (new_niothreads != exp_niothreads) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(got wrong number of IOThread ids from QEMU monitor. 
+ got %d, wanted %d),
+   new_niothreads, exp_niothreads);
+vm-def-iothreads = new_niothreads;
+goto cleanup;
+}
+vm-def-iothreads = exp_niothreads;
+
+if (virDomainNumatuneGetMode(vm-def-numa, -1) ==
+VIR_DOMAIN_NUMATUNE_MEM_STRICT 
+virDomainNumatuneMaybeFormatNodeset(vm-def-numa,
+ 

[libvirt] [PATCH v4 7/9] remote: Add support for AddIOThread and DelIOThread

[John Ferlan]

Add remote support for the add/delete IOThread API's

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/remote/remote_driver.c   |  2 ++
 src/remote/remote_protocol.x | 30 +-
 src/remote_protocol-structs  | 12 
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9c3b53f..31417e8 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8239,6 +8239,8 @@ static virHypervisorDriver hypervisor_driver = {
 .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */
 .domainGetIOThreadInfo = remoteDomainGetIOThreadInfo, /* 1.2.14 */
 .domainPinIOThread = remoteDomainPinIOThread, /* 1.2.14 */
+.domainAddIOThread = remoteDomainAddIOThread, /* 1.2.15 */
+.domainDelIOThread = remoteDomainDelIOThread, /* 1.2.15 */
 .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */
 .domainGetSecurityLabelList = remoteDomainGetSecurityLabelList, /* 0.10.0 
*/
 .nodeGetSecurityModel = remoteNodeGetSecurityModel, /* 0.6.1 */
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index b02e58c..49b7ddd 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1212,6 +1212,18 @@ struct remote_domain_pin_iothread_args {
 unsigned int flags;
 };
 
+struct remote_domain_add_iothread_args {
+remote_nonnull_domain dom;
+unsigned int iothread_id;
+unsigned int flags;
+};
+
+struct remote_domain_del_iothread_args {
+remote_nonnull_domain dom;
+unsigned int iothread_id;
+unsigned int flags;
+};
+
 struct remote_domain_get_security_label_args {
 remote_nonnull_domain dom;
 };
@@ -5655,5 +5667,21 @@ enum remote_procedure {
  * @generate: both
  * @acl: none
  */
-REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED = 354
+REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED = 354,
+
+/**
+ * @generate:both
+ * @acl: domain:write
+ * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE
+ * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG
+ */
+REMOTE_PROC_DOMAIN_ADD_IOTHREAD = 355,
+
+/**
+ * @generate:both
+ * @acl: domain:write
+ * @acl: domain:save:!VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE
+ * @acl: domain:save:VIR_DOMAIN_AFFECT_CONFIG
+ */
+REMOTE_PROC_DOMAIN_DEL_IOTHREAD = 356
 };
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 2b6b47a..116b572 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@ -834,6 +834,16 @@ struct remote_domain_pin_iothread_args {
 } cpumap;
 u_int  flags;
 };
+struct remote_domain_add_iothread_args {
+remote_nonnull_domain  dom;
+u_int  iothread_id;
+u_int  flags;
+};
+struct remote_domain_del_iothread_args {
+remote_nonnull_domain  dom;
+u_int  iothread_id;
+u_int  flags;
+};
 struct remote_domain_get_security_label_args {
 remote_nonnull_domain  dom;
 };
@@ -3023,4 +3033,6 @@ enum remote_procedure {
 REMOTE_PROC_DOMAIN_PIN_IOTHREAD = 352,
 REMOTE_PROC_DOMAIN_INTERFACE_ADDRESSES = 353,
 REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_ADDED = 354,
+REMOTE_PROC_DOMAIN_ADD_IOTHREAD = 355,
+REMOTE_PROC_DOMAIN_DEL_IOTHREAD = 356,
 };
-- 
2.1.0

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

[libvirt] [PATCH v4 5/9] conf: Adjust the iothreadsched expectations

[John Ferlan]

With iothreadid's allowing any 'id' value for an iothread_id, the
iothreadsched code needs a slight adjustment to allow for any
unsigned int value in order to create the bitmap of ids that will
have scheduler adjustments. Adjusted the doc description as well.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatdomain.html.in   | 6 +-
 src/conf/domain_conf.c  | 2 +-
 .../qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml | 1 +
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7af6bd7..99ce298 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -691,7 +691,11 @@
 type (values codebatch/code, codeidle/code, codefifo/code,
 coderr/code) for particular vCPU/IOThread threads (based on
 codevcpus/code and codeiothreads/code, leaving out
-codevcpus/code/codeiothreads/code sets the default).  For
+codevcpus/code/codeiothreads/code sets the default). Valid
+codevcpus/code values start at 0 through one less than the
+number of vCPU's defined for the domain. Valid codeiothreads/code
+values are described in the codeiothreadids/code
+a href=#elementsIOThreadsAllocationcodedescription/code/a. 
For
 real-time schedulers (codefifo/code, coderr/code), priority 
must
 be specified as well (and is ignored for non-real-time ones). The value
 range for the priority depends on the host kernel (usually 1-99).
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 969e56f..3e8551b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14313,7 +14313,7 @@ virDomainDefParseXML(xmlDocPtr xml,
 
 for (i = 0; i  def-cputune.niothreadsched; i++) {
 if (virDomainThreadSchedParse(nodes[i],
-  1, def-iothreads,
+  1, UINT_MAX,
   iothreads,
   def-cputune.iothreadsched[i])  0)
 goto error;
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml
index 1540969..97a5cde 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreadsched-toomuch.xml
@@ -13,6 +13,7 @@
 vcpupin vcpu='1' cpuset='1'/
 emulatorpin cpuset='1'/
 vcpusched vcpus='0-1' scheduler='fifo' priority='1'/
+iothreadsched iothreads='1' scheduler='batch'/
 iothreadsched iothreads='2' scheduler='batch'/
   /cputune
   os
-- 
2.1.0

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

[libvirt] [PATCH v4 6/9] Implement virDomainAddIOThread and virDomainDelIOThread

[John Ferlan]

Add libvirt API's to manage adding and deleting IOThreads to/from the
domain

Signed-off-by: John Ferlan jfer...@redhat.com
---
 include/libvirt/libvirt-domain.h |   6 ++
 src/driver-hypervisor.h  |  12 
 src/libvirt-domain.c | 118 +++
 src/libvirt_public.syms  |   6 ++
 4 files changed, 142 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 8a4fe53..528cfae 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1615,6 +1615,12 @@ int  virDomainPinIOThread(virDomainPtr 
domain,
   unsigned char *cpumap,
   int maplen,
   unsigned int flags);
+int  virDomainAddIOThread(virDomainPtr domain,
+  unsigned int iothread_id,
+  unsigned int flags);
+int  virDomainDelIOThread(virDomainPtr domain,
+  unsigned int iothread_id,
+  unsigned int flags);
 
 /**
  * VIR_USE_CPU:
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 1b92460..8b8d031 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -393,6 +393,16 @@ typedef int
unsigned int flags);
 
 typedef int
+(*virDrvDomainAddIOThread)(virDomainPtr domain,
+   unsigned int iothread_id,
+   unsigned int flags);
+
+typedef int
+(*virDrvDomainDelIOThread)(virDomainPtr domain,
+   unsigned int iothread_id,
+   unsigned int flags);
+
+typedef int
 (*virDrvDomainGetSecurityLabel)(virDomainPtr domain,
 virSecurityLabelPtr seclabel);
 
@@ -1273,6 +1283,8 @@ struct _virHypervisorDriver {
 virDrvDomainGetMaxVcpus domainGetMaxVcpus;
 virDrvDomainGetIOThreadInfo domainGetIOThreadInfo;
 virDrvDomainPinIOThread domainPinIOThread;
+virDrvDomainAddIOThread domainAddIOThread;
+virDrvDomainDelIOThread domainDelIOThread;
 virDrvDomainGetSecurityLabel domainGetSecurityLabel;
 virDrvDomainGetSecurityLabelList domainGetSecurityLabelList;
 virDrvNodeGetSecurityModel nodeGetSecurityModel;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index ccacdb4..ec5e2ff 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8020,6 +8020,124 @@ virDomainPinIOThread(virDomainPtr domain,
 
 
 /**
+ * virDomainAddIOThread:
+ * @domain: a domain object
+ * @iothread_id: the specific IOThread ID value to add
+ * @flags: bitwise-OR of virDomainModificationImpact
+ *
+ * Dynamically add an IOThread to the domain. If @iothread_id is a positive
+ * non-zero value, then attempt to add the specific IOThread ID and error
+ * out if the iothread id already exists.
+ *
+ * Note that this call can fail if the underlying virtualization hypervisor
+ * does not support it or if growing the number is arbitrarily limited.
+ * This function requires privileged access to the hypervisor.
+ *
+ * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG.
+ * Both flags may be set.
+ * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain
+ * and may fail if domain is not alive.
+ * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state,
+ * and will fail for transient domains. If neither flag is specified (that is,
+ * @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain modifies
+ * persistent setup, while an active domain is hypervisor-dependent on whether
+ * just live or both live and persistent state is changed.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+int
+virDomainAddIOThread(virDomainPtr domain,
+ unsigned int iothread_id,
+ unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, iothread_id=%u, flags=%x,
+ iothread_id, flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+virCheckReadOnlyGoto(domain-conn-flags, error);
+
+conn = domain-conn;
+
+if (conn-driver-domainAddIOThread) {
+int ret;
+ret = conn-driver-domainAddIOThread(domain, iothread_id, flags);
+if (ret  0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+
+ error:
+virDispatchError(domain-conn);
+return -1;
+}
+
+
+/**
+ * virDomainDelIOThread:
+ * @domain: a domain object
+ * @iothread_id: the specific IOThread ID value to delete
+ * @flags: bitwise-OR of virDomainModificationImpact
+ *
+ * Dynamically delete an IOThread from the domain. The @iothread_id to be
+ * deleted must not have a resource associated with it and can be any of
+ * the currently valid IOThread ID's.
+ *
+ * 

[libvirt] [PATCH v4 4/9] Move iothreadspin information into iothreadids

[John Ferlan]

Remove the iothreadspin array from cputune and replace with a cpumask
to be stored in the iothreadids list

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatdomain.html.in |  10 ++--
 src/conf/domain_conf.c| 118 +-
 src/conf/domain_conf.h|   2 +-
 src/qemu/qemu_cgroup.c|  13 ++---
 src/qemu/qemu_driver.c|  79 +--
 src/qemu/qemu_process.c   |   7 +--
 6 files changed, 86 insertions(+), 143 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 518f7c5..7af6bd7 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -624,11 +624,11 @@
  and attribute codecpuset/code of element codevcpu/code is
  not specified, the IOThreads are pinned to all the physical CPUs
  by default. There are two required attributes, the attribute
- codeiothread/code specifies the IOThread id and the attribute
- codecpuset/code specifying which physical CPUs to pin to. The
- codeiothread/code value begins at 1 through the number of
-  a href=#elementsIOThreadsAllocationcodeiothreads/code/a
- allocated to the domain. A value of 0 is not permitted.
+ codeiothread/code specifies the IOThread ID and the attribute
+ codecpuset/code specifying which physical CPUs to pin to. See
+ the codeiothreadids/code
+ a href=#elementsIOThreadsAllocationcodedescription/code/a
+ for valid codeiothread/code values.
 span class=sinceSince 1.2.9/span
/dd
   dtcodeshares/code/dt
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bd25d52..969e56f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2118,8 +2118,10 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
 void
 virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def)
 {
-if (def)
+if (def) {
+virBitmapFree(def-cpumask);
 VIR_FREE(def);
+}
 }
 
 
@@ -2341,9 +2343,6 @@ void virDomainDefFree(virDomainDefPtr def)
 
 virDomainPinDefFree(def-cputune.emulatorpin);
 
-virDomainPinDefArrayFree(def-cputune.iothreadspin,
- def-cputune.niothreadspin);
-
 for (i = 0; i  def-cputune.nvcpusched; i++)
 virBitmapFree(def-cputune.vcpusched[i].ids);
 VIR_FREE(def-cputune.vcpusched);
@@ -13316,74 +13315,77 @@ virDomainVcpuPinDefParseXML(xmlNodePtr node,
  * and an iothreadspin has the form
  *   iothreadpin iothread='1' cpuset='2'/
  */
-static virDomainPinDefPtr
+static int
 virDomainIOThreadPinDefParseXML(xmlNodePtr node,
 xmlXPathContextPtr ctxt,
-int iothreads)
+virDomainDefPtr def)
 {
-virDomainPinDefPtr def;
+int ret = -1;
+virDomainIOThreadIDDefPtr iothrid;
+virBitmapPtr cpumask;
 xmlNodePtr oldnode = ctxt-node;
 unsigned int iothreadid;
 char *tmp = NULL;
 
-if (VIR_ALLOC(def)  0)
-return NULL;
-
 ctxt-node = node;
 
 if (!(tmp = virXPathString(string(./@iothread), ctxt))) {
 virReportError(VIR_ERR_XML_ERROR, %s,
_(missing iothread id in iothreadpin));
-goto error;
+goto cleanup;
 }
 
 if (virStrToLong_uip(tmp, NULL, 10, iothreadid)  0) {
 virReportError(VIR_ERR_XML_ERROR,
_(invalid setting for iothread '%s'), tmp);
-goto error;
+goto cleanup;
 }
 VIR_FREE(tmp);
 
 if (iothreadid == 0) {
 virReportError(VIR_ERR_XML_ERROR, %s,
_(zero is an invalid iothread id value));
-goto error;
-}
-
-/* IOThreads are numbered iothread1...iothreadn, where
- * n is the iothreads value */
-if (iothreadid  iothreads) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(iothread id must not exceed iothreads));
-goto error;
+goto cleanup;
 }
 
-def-id = iothreadid;
-
 if (!(tmp = virXMLPropString(node, cpuset))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(missing cpuset for iothreadpin));
-goto error;
+goto cleanup;
 }
 
-if (virBitmapParse(tmp, 0, def-cpumask, VIR_DOMAIN_CPUMASK_LEN)  0)
-goto error;
+if (virBitmapParse(tmp, 0, cpumask, VIR_DOMAIN_CPUMASK_LEN)  0)
+goto cleanup;
 
-if (virBitmapIsAllClear(def-cpumask)) {
+if (virBitmapIsAllClear(cpumask)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Invalid value of 'cpuset': %s),
tmp);
-goto error;
+goto cleanup;
+}
+
+if (!(iothrid = virDomainIOThreadIDFind(def, iothreadid))) {
+if (!(iothrid = virDomainIOThreadIDAdd(def, iothreadid)))
+goto cleanup;
+iothrid-autofill = true;
+}
+
+if (iothrid-cpumask) {
+

[libvirt] [PATCH v4 3/9] conf: Move virDomainPinIsDuplicate and make static

[John Ferlan]

Since it's only ever referenced in domain_conf.c, make the function
static, but also will need to move it to somewhere before it's referenced
rather than forward referencing it.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_conf.c   | 38 +++---
 src/conf/domain_conf.h   |  4 
 src/libvirt_private.syms |  1 -
 3 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index da1bb7e..bd25d52 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13238,6 +13238,25 @@ virDomainIOThreadIDDefParseXML(xmlNodePtr node,
 }
 
 
+/* Check if pin with same id already exists. */
+static bool
+virDomainPinIsDuplicate(virDomainPinDefPtr *def,
+int npin,
+int id)
+{
+size_t i;
+
+if (!def || !npin)
+return false;
+
+for (i = 0; i  npin; i++) {
+if (def[i]-id == id)
+return true;
+}
+
+return false;
+}
+
 /* Parse the XML definition for a vcpupin
  *
  * vcpupin has the form of
@@ -17415,25 +17434,6 @@ virDomainIOThreadIDDel(virDomainDefPtr def,
 }
 }
 
-/* Check if vcpupin with same id already exists. */
-bool
-virDomainPinIsDuplicate(virDomainPinDefPtr *def,
-int npin,
-int id)
-{
-size_t i;
-
-if (!def || !npin)
-return false;
-
-for (i = 0; i  npin; i++) {
-if (def[i]-id == id)
-return true;
-}
-
-return false;
-}
-
 virDomainPinDefPtr
 virDomainPinFind(virDomainPinDefPtr *def,
  int npin,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2d53a8d..cc98f3d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1947,10 +1947,6 @@ void virDomainPinDefArrayFree(virDomainPinDefPtr *def, 
int npin);
 virDomainPinDefPtr *virDomainPinDefCopy(virDomainPinDefPtr *src,
 int npin);
 
-bool virDomainPinIsDuplicate(virDomainPinDefPtr *def,
- int npin,
- int id);
-
 virDomainPinDefPtr virDomainPinFind(virDomainPinDefPtr *def,
 int npin,
 int id);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 77f6797..a0f4152 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -413,7 +413,6 @@ virDomainPinDefCopy;
 virDomainPinDefFree;
 virDomainPinDel;
 virDomainPinFind;
-virDomainPinIsDuplicate;
 virDomainPMSuspendedReasonTypeFromString;
 virDomainPMSuspendedReasonTypeToString;
 virDomainRedirdevBusTypeFromString;
-- 
2.1.0

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

Re: [libvirt] [PATCH v4 2/4] scsi: Adjust return value for virStorageBackendSCSINewLun

[John Ferlan]

On 04/21/2015 03:13 AM, Ján Tomko wrote:
 On Mon, Apr 20, 2015 at 12:50:07PM -0400, John Ferlan wrote:

 ...

 +/* Check if the pool is using a stable target path. The call to
 + * virStorageBackendStablePath will fail if the pool target path
 + * isn't stable and just return the strdup'd 'devpath' anyway.
 + * This would be indistinguishable to failing to find the stable
 + * path to the device if the virDirRead loop to search the
 + * target pool path for our devpath had failed.
 + */
 +if (!virStorageBackendPoolPathIsStable(pool-def-target.path)) {
 +virReportError(VIR_ERR_INVALID_ARG,
 +   _(unable to use target path '%s' for dev '%s'),
 +   NULLSTR(pool-def-target.path), dev);
 +goto cleanup;
 +}

 /dev is a valid non-stable pool target path.

 +
  if (VIR_ALLOC(vol)  0)
  goto cleanup;
  
 @@ -187,13 +211,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr 
 pool,
  true)) == NULL)
  goto cleanup;
  
 -if (STREQ(devpath, vol-target.path) 
 -!(STREQ(pool-def-target.path, /dev) ||
 -  STREQ(pool-def-target.path, /dev/))) {
 +if (STREQ(devpath, vol-target.path)) {
  

 Before, when virStorageBackendStablePath returned the same devpath because
 the pool path was /dev, we continued with processing the volume.

 After this patch, we won't even get here because of the first check.

 Failure to stabilize the path should be expected here, if the pool
 target path is not stable.


 OK, but because virStorageBackendStablePath won't process the
 pool-target.path of /dev, we'll return the duplicated 'devpath' and
 return -2 for every volume in the pool thus making it look empty.

 What good is that?

 
 None. We should process the volume instead of returning -2.
 

Does the following squashed in work for you?  Essentially restoring the
/dev || /dev/ check after virStorageBackendStablePath and adding it to
the virStorageBackendPoolPathIsStable ?

iff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index ae3cd9a..b97b2b0 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -175,7 +175,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
  * path to the device if the virDirRead loop to search the
  * target pool path for our devpath had failed.
  */
-if (!virStorageBackendPoolPathIsStable(pool-def-target.path)) {
+if (!virStorageBackendPoolPathIsStable(pool-def-target.path) 
+!(STREQ(pool-def-target.path, /dev) ||
+  STREQ(pool-def-target.path, /dev/))) {
 virReportError(VIR_ERR_INVALID_ARG,
_(unable to use target path '%s' for dev '%s'),
NULLSTR(pool-def-target.path), dev);
@@ -211,7 +213,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
 true)) == NULL)
 goto cleanup;

-if (STREQ(devpath, vol-target.path)) {
+if (STREQ(devpath, vol-target.path) 
+!(STREQ(pool-def-target.path, /dev) ||
+  STREQ(pool-def-target.path, /dev/))) {

 VIR_DEBUG(No stable path found for '%s' in '%s',
   devpath, pool-def-target.path);

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

Re: [libvirt] [PATCH v4 2/4] scsi: Adjust return value for virStorageBackendSCSINewLun

[Ján Tomko]

On Tue, Apr 21, 2015 at 06:18:56AM -0400, John Ferlan wrote:
 
 
 On 04/21/2015 03:13 AM, Ján Tomko wrote:
  On Mon, Apr 20, 2015 at 12:50:07PM -0400, John Ferlan wrote:
 
  ...
 
  +/* Check if the pool is using a stable target path. The call to
  + * virStorageBackendStablePath will fail if the pool target path
  + * isn't stable and just return the strdup'd 'devpath' anyway.
  + * This would be indistinguishable to failing to find the stable
  + * path to the device if the virDirRead loop to search the
  + * target pool path for our devpath had failed.
  + */
  +if (!virStorageBackendPoolPathIsStable(pool-def-target.path)) {
  +virReportError(VIR_ERR_INVALID_ARG,
  +   _(unable to use target path '%s' for dev '%s'),
  +   NULLSTR(pool-def-target.path), dev);
  +goto cleanup;
  +}
 
  /dev is a valid non-stable pool target path.
 
  +
   if (VIR_ALLOC(vol)  0)
   goto cleanup;
   
  @@ -187,13 +211,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr 
  pool,
   true)) == NULL)
   goto cleanup;
   
  -if (STREQ(devpath, vol-target.path) 
  -!(STREQ(pool-def-target.path, /dev) ||
  -  STREQ(pool-def-target.path, /dev/))) {
  +if (STREQ(devpath, vol-target.path)) {
   
 
  Before, when virStorageBackendStablePath returned the same devpath because
  the pool path was /dev, we continued with processing the volume.
 
  After this patch, we won't even get here because of the first check.
 
  Failure to stabilize the path should be expected here, if the pool
  target path is not stable.
 
 
  OK, but because virStorageBackendStablePath won't process the
  pool-target.path of /dev, we'll return the duplicated 'devpath' and
  return -2 for every volume in the pool thus making it look empty.
 
  What good is that?
 
  
  None. We should process the volume instead of returning -2.
  
 
 Does the following squashed in work for you?  Essentially restoring the
 /dev || /dev/ check after virStorageBackendStablePath and adding it to
 the virStorageBackendPoolPathIsStable ?
 
 iff --git a/src/storage/storage_backend_scsi.c
 b/src/storage/storage_backend_scsi.c
 index ae3cd9a..b97b2b0 100644
 --- a/src/storage/storage_backend_scsi.c
 +++ b/src/storage/storage_backend_scsi.c
 @@ -175,7 +175,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
   * path to the device if the virDirRead loop to search the
   * target pool path for our devpath had failed.
   */
 -if (!virStorageBackendPoolPathIsStable(pool-def-target.path)) {
 +if (!virStorageBackendPoolPathIsStable(pool-def-target.path) 
 +!(STREQ(pool-def-target.path, /dev) ||
 +  STREQ(pool-def-target.path, /dev/))) {
  virReportError(VIR_ERR_INVALID_ARG,
 _(unable to use target path '%s' for dev '%s'),
 NULLSTR(pool-def-target.path), dev);
 @@ -211,7 +213,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
  true)) == NULL)
  goto cleanup;
 
 -if (STREQ(devpath, vol-target.path)) {
 +if (STREQ(devpath, vol-target.path) 
 +!(STREQ(pool-def-target.path, /dev) ||
 +  STREQ(pool-def-target.path, /dev/))) {
 
  VIR_DEBUG(No stable path found for '%s' in '%s',
devpath, pool-def-target.path);
 

ACK

Jan


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

Re: [libvirt] [PATCH v2 1/2] virDomainActualNetDefContentsFormat: Format class_id only for status XML

[Laine Stump]

On 04/21/2015 08:22 AM, Michal Privoznik wrote:
 In one of my previous patches (b68a56bcfe) I made class_id to
 format more frequently. Well, now it's formatting way too
 frequent - even for regular active XML. Users don't need to see
 it, so lets format it only for the status XML where it's really
 needed.

 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/domain_conf.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 479b4c2..9aad782 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -18522,7 +18522,8 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
  
  virBufferAddLit(buf, /\n);
  }
 -if (def-data.network.actual  def-data.network.actual-class_id) {
 +if (flags  VIR_DOMAIN_DEF_FORMAT_STATUS 
 +def-data.network.actual  def-data.network.actual-class_id) {
  virBufferAsprintf(buf, class id='%u'/\n,
def-data.network.actual-class_id);
  }

ACK.

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

Re: [libvirt] [PATCH 02/15] util: Add virabstracts file for keeping abstract classes

[Martin Kletzander]

On Tue, Apr 21, 2015 at 06:41:36PM +0200, Michal Privoznik wrote:

On 16.04.2015 16:46, Martin Kletzander wrote:

The first class in this file is going to be an abstract connection class
that holds a per-connection error inside.  virConnect will be the first
child class inheriting from this one.

This is a separate file because virerror.c is going to depend on it and
putting it into datatypes along with other connect classes would create
a dependency on datatypes from the util library.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/Makefile.am  |   3 +-
 src/libvirt_private.syms |   5 +++
 src/util/virabstracts.c  | 100 +++
 src/util/virabstracts.h  |  57 +++
 4 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 src/util/virabstracts.c
 create mode 100644 src/util/virabstracts.h



You need to squash this in:



Not really, this patch is removed from the series as there is no need
for it.  But thanks a lot for having a look ;)


diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index cd1e2df..9592de7 100644
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -410,7 +410,7 @@ xenInotifyOpen(virConnectPtr conn,
VIR_DEBUG(Building initial config cache);
if (priv-useXenConfigCache 
xenXMConfigCacheRefresh(conn)  0) {
-VIR_DEBUG(Failed to enable XM config cache %s, conn-err.message);
+VIR_DEBUG(Failed to enable XM config cache %s, 
conn-parent.err.message);
return -1;
}

Michal


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

Re: [libvirt] [PATCH v2 2/2] processNicRxFilterChangedEvent: Take appropriate actions for NET_TYPE_NETWORK too

[Laine Stump]

On 04/21/2015 08:22 AM, Michal Privoznik wrote:
 Because packets going through the egress from a bridge (where our
 bandwidth limiting takes place) have no information about which
 interface they came from, the QoS rules that we create instead
 use the source MAC address of the packets to make their decisions
 about which QDisc the packet should be in.

 One flaw in this is that when a guest changed the MAC address it
 used, packets from the guest would no longer be put into the
 correct QDisc, but would instead be put in an unprivileged
 class, resulting in the bandwidth floor (minimum guaranteed)
 being no longer honored.

 Now that libvirt has infrastructure to capture and respond to
 RX_FILTER_CHANGE events from qemu (sent whenever a guest
 interface modifies its MAC address, among other things), we can
 notice when a guest MAC address changes, and update the QoS rules
 accordingly, so that bandwidth floor is honored even after a
 guest MAC address change.

 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/qemu/qemu_driver.c | 12 
  1 file changed, 12 insertions(+)

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 6fc9696..6209754b 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -4377,6 +4377,18 @@ processNicRxFilterChangedEvent(virQEMUDriverPtr driver,
  syncNicRxFilterDeviceOptions(def-ifname, guestFilter, hostFilter);
  }
  
 +if (virDomainNetGetActualType(def) == VIR_DOMAIN_NET_TYPE_NETWORK) {
 +const char *brname = virDomainNetGetActualBridgeName(def);
 +
 +/* For libivrt network connections, set the following TUN/TAP network
 + * device attributes to match those of the guest network device:
 + * - QoS filters (which are based on MAC address)
 + */


Don't you want to check to make sure this interface actually has
bandwidth settings before trying to update them? Maybe by prepending if
(virDomainNetGetActualBandwidth(def)) to this following if clause.

 +if (virNetDevBandwidthUpdateFilter(brname, guestFilter-mac,
 +   
 def-data.network.actual-class_id)  0)

Just to be defensive, you should check that def-data.network.actual !=
NULL too (it *should* always point to something when the domain is
active, but just in case)

 +goto endjob;
 +}
 +
   endjob:
  qemuDomainObjEndJob(driver, vm);
  

ACK with those two changes.

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

Re: [libvirt] [PATCH v3 1/6] conf: Add new domain XML element 'iothreadids'

[John Ferlan]

On 04/21/2015 10:08 AM, Peter Krempa wrote:
 On Tue, Apr 14, 2015 at 21:18:21 -0400, John Ferlan wrote:
 Adding a new XML element 'iothreadids' in order to allow defining
 specific IOThread ID's rather than relying on the algorithm to assign
 IOThread ID's starting at 1 and incrementing to iothreads count.

 This will allow future patches to be able to add new IOThreads by
 a specific iothread_id and of course delete any exisiting IOThread.

 Each iothreadids element will have 'n' iothread children elements
 which will have attribute id.  The id will allow for definition
 of any valid (eg  0) iothread_id value.

 On input, if any iothreadids iothread's are provided, they will
 be marked so that we only print out what we read in.

 On input, if no iothreadids are provided, the PostParse code will
 self generate a list of ID's starting at 1 and going to the number
 of iothreads defined for the domain (just like the current algorithm
 numbering scheme).  A future patch will rework the existing algorithm
 to make use of the iothreadids list.

 On output, only print out the iothreadids if they were read in.

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  docs/formatdomain.html.in |  30 +++
  docs/schemas/domaincommon.rng |  12 +++
  src/conf/domain_conf.c| 190 
 +-
  src/conf/domain_conf.h|  15 
  src/libvirt_private.syms  |   3 +
  5 files changed, 248 insertions(+), 2 deletions(-)

 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index e921749..518f7c5 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -521,6 +521,18 @@
...
  lt;/domaingt;
  /pre
 +pre
 +lt;domaingt;
 +  ...
 +  lt;iothreadidsgt;
 +lt;iothread id=2/gt;
 +lt;iothread id=4/gt;
 +lt;iothread id=6/gt;
 +lt;iothread id=8/gt;
 +  lt;/iothreadidsgt;
 +  ...
 +lt;/domaingt;
 +/pre
  
  dl
dtcodeiothreads/code/dt
 @@ -530,7 +542,25 @@
  virtio-blk-pci and virtio-blk-ccw target storage devices. There
  should be only 1 or 2 IOThreads per host CPU. There may be more
  than one supported device assigned to each IOThread.
 +span class=sinceSince 1.2.8/span
/dd
 +  dtcodeiothreadids/code/dt
 +  dd
 +The optional codeiothreadids/code element provides the 
 capability
 +to specifically define the IOThread ID's for the domain.  By 
 default,
 +IOThread ID's are sequentially numbered starting from 1 through the
 +number of codeiothreads/code defined for the domain. The
 +codeid/code attribute is used to define the IOThread ID. The
 +codeid/code attribute must be a positive integer greater than 0.
 +If there are less codeiothreadids/code defined than
 +codeiothreads/code defined for the domain, then libvirt will
 +sequentially fill codeiothreadids/code starting at 1 avoiding
 +any predefined codeid/code. If there are more
 +codeiothreadids/code defined than codeiothreads/code
 +defined for the domain, then the codeiothreads/code value
 +will be adjusted accordingly.
 +span class=sinceSince 1.2.15/span
 +   /dd
  /dl
  
  h3a name=elementsCPUTuningCPU Tuning/a/h3
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 03fd541..4bd32bd 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -675,6 +675,18 @@
/optional
  
optional
 +element name=iothreadids
 +  zeroOrMore
 +element name=iothread
 +  attribute name=id
 +ref name=unsignedInt/
 +  /attribute
 +/element
 +  /zeroOrMore
 +/element
 +  /optional
 +
 +  optional
  ref name=blkiotune/
/optional
  
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 4d7e3c9..e86b7e2 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -2113,6 +2113,23 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
  return NULL;
  }
  
 +
 +static void
 +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
 +int nids)
 +{
 +size_t i;
 +
 +if (!def)
 +return;
 +
 +for (i = 0; i  nids; i++)
 +VIR_FREE(def[i]);
 +
 +VIR_FREE(def);
 +}
 +
 +
  void
  virDomainPinDefFree(virDomainPinDefPtr def)
  {
 @@ -2310,6 +2327,8 @@ void virDomainDefFree(virDomainDefPtr def)
  
  virCPUDefFree(def-cpu);
  
 +virDomainIOThreadIDDefArrayFree(def-iothreadids, def-niothreadids);
 +
  virDomainPinDefArrayFree(def-cputune.vcpupin, def-cputune.nvcpupin);
  
  virDomainPinDefFree(def-cputune.emulatorpin);
 @@ -3304,6 +3323,18 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
  return -1;
  }
  
 +/* Fully populate the IOThread ID list */
 +if (def-iothreads  def-iothreads != 

Re: [libvirt] [PATCH 02/15] util: Add virabstracts file for keeping abstract classes

[Michal Privoznik]

On 16.04.2015 16:46, Martin Kletzander wrote:
 The first class in this file is going to be an abstract connection class
 that holds a per-connection error inside.  virConnect will be the first
 child class inheriting from this one.
 
 This is a separate file because virerror.c is going to depend on it and
 putting it into datatypes along with other connect classes would create
 a dependency on datatypes from the util library.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
  src/Makefile.am  |   3 +-
  src/libvirt_private.syms |   5 +++
  src/util/virabstracts.c  | 100 
 +++
  src/util/virabstracts.h  |  57 +++
  4 files changed, 164 insertions(+), 1 deletion(-)
  create mode 100644 src/util/virabstracts.c
  create mode 100644 src/util/virabstracts.h
 

You need to squash this in:

diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index cd1e2df..9592de7 100644
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -410,7 +410,7 @@ xenInotifyOpen(virConnectPtr conn,
 VIR_DEBUG(Building initial config cache);
 if (priv-useXenConfigCache 
 xenXMConfigCacheRefresh(conn)  0) {
-VIR_DEBUG(Failed to enable XM config cache %s, conn-err.message);
+VIR_DEBUG(Failed to enable XM config cache %s, 
conn-parent.err.message);
 return -1;
 }

Michal

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