[libvirt] [PATCHv2] Renamed internal __mon_yday into mon_yday to avoid conflicts

[Cédric Bosdonnat]

libc has another constant with the same name, which leads to
redefinition error when building against static libvirt
---
 src/util/virtime.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/virtime.c b/src/util/virtime.c
index acbec41..9d365d5 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -105,7 +105,7 @@ int virTimeFieldsNowRaw(struct tm *fields)
 #define DIV(a, b) ((a) / (b) - ((a) % (b)  0))
 #define LEAPS_THRU_END_OF(y) (DIV (y, 4) - DIV (y, 100) + DIV (y, 400))
 
-const unsigned short int __mon_yday[2][13] = {
+static const unsigned short int mon_yday[2][13] = {
 /* Normal years.  */
 { 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 },
 /* Leap years.  */
@@ -160,7 +160,7 @@ void virTimeFieldsThen(unsigned long long when, struct tm 
*fields)
 fields-tm_year = y - 1900;
 
 fields-tm_yday = days;
-ip = __mon_yday[is_leap_year(y)];
+ip = mon_yday[is_leap_year(y)];
 for (y = 11; days  (long int) ip[y]; --y)
 continue;
 days -= ip[y];
-- 
1.8.4.5

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

Re: [libvirt] [PATCH v2] Move the FIPS detection from capabilities

[Pavel Hrdina]

On 09/18/2014 09:57 PM, Eric Blake wrote:

On 09/18/2014 12:01 PM, Pavel Hrdina wrote:

We are not detecting the presence of FIPS from QEMU, but from procfs and
that means it's not QEMU capability. It was decided that we will pass
this flag to QEMU even if it's not supported by old QEMU binaries.

This patch also reverts changes done by commit a21cfb0f to
qemucapabilitestest and implements a new test case in qemuxml2argvtest.

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

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---



@@ -183,19 +176,16 @@ mymain(void)

  data.xmlopt = xmlopt;

-#define DO_TEST_FULL(name, use_fips) \
-data.base = name;\
-data.fips = use_fips;\
-if (virtTestRun(name, testQemuCaps, data)  0)  \
+#define DO_TEST(name) \
+data.base = name; \
+if (virtTestRun(name, testQemuCaps, data)  0) \


We are not very consistent on whether multiline macros should align the
\ to the same column.


That's true, I've changed the align of '\' to the same column.




  ret = -1


Eww - we really did that in a multiline macro?  I'd much rather fix
things to use:

do {
   data.base = name;
   if (virtTestRun(name, testQemuCaps, data)  0)
 ret = -1;
} while (0)


Thanks, pushed with this change.

Pavel



as long as we are touching the code.

ACK.



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

Re: [libvirt] [PATCH] libxl: Drop driver lock in libxlDomainDefineXML

[Michal Privoznik]

On 18.09.2014 23:17, Jim Fehlig wrote:

There is no need to acquire the driver-wide lock in
libxlDomainDefineXML.  When switching to jobs in the libxl
driver, most driver-wide locks were removed.  The locking here
was preserved since I mistakenly thought virDomainObjListAdd
needed protection.  This is not the case, so remove the
unnecessary locking.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
  src/libxl/libxl_driver.c | 13 +++--
  1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 2f2c590..d4ecd62 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2381,25 +2381,22 @@ libxlDomainDefineXML(virConnectPtr conn, const char 
*xml)
  virObjectEventPtr event = NULL;
  virDomainDefPtr oldDef = NULL;

-/* Lock the driver until the virDomainObj is created and locked */
-libxlDriverLock(driver);
  if (!(def = virDomainDefParseString(xml, cfg-caps, driver-xmlopt,
  1  VIR_DOMAIN_VIRT_XEN,
  VIR_DOMAIN_XML_INACTIVE)))
-goto cleanup_unlock;
+goto cleanup;

  if (virDomainDefineXMLEnsureACL(conn, def)  0)
-goto cleanup_unlock;
+goto cleanup;

  if (!(vm = virDomainObjListAdd(driver-domains, def,
 driver-xmlopt,
 0,
 oldDef)))
-goto cleanup_unlock;
+goto cleanup;

  def = NULL;
  vm-persistent = 1;
-libxlDriverUnlock(driver);

  if (virDomainSaveConfig(cfg-configDir,
  vm-newDef ? vm-newDef : vm-def)  0) {
@@ -2426,10 +2423,6 @@ libxlDomainDefineXML(virConnectPtr conn, const char *xml)
  libxlDomainEventQueue(driver, event);
  virObjectUnref(cfg);
  return dom;
-
- cleanup_unlock:
-libxlDriverUnlock(driver);
-goto cleanup;
  }

  static int



ACK

Michal

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

Re: [libvirt] [PATCH] LXC: add HOME environment variable docs

[Michal Privoznik]

On 19.09.2014 04:01, Chen Hanxiao wrote:

commit
3020594ac57c5e06e79f3db8c765f6bb18c40802
add HOME environment variable.
Add a doc for this.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
  docs/drvlxc.html.in | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in
index 403ce24..31da37c 100644
--- a/docs/drvlxc.html.in
+++ b/docs/drvlxc.html.in
@@ -88,6 +88,8 @@ to be provided by all container technologies on Linux.
  ddThe fixed string code/bin:/usr/bin/code/dd
  dtTERM/dt
  ddThe fixed string codelinux/code/dd
+dtHOME/dt
+ddThe fixed string code//code/dd
  /dl

  p



ACKed  pushed.

Michal

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

Re: [libvirt] [PATCH v1 00/10] Keep original security label

[Michal Privoznik]

On 10.09.2014 15:26, Michal Privoznik wrote:

I know I've sent several versions like ages ago, so this should
not start with v1, but hey, this is completely new approach, so
I'm gonna start from 1.

Here, the virtlockd is misused to hold the original seclabels
(although only DAC label is implemented so far). Even more, it
does a reference counting, so that only the last label restore
does the job, not the previous ones.

Michal Privoznik (10):
   locking: Allow seclabel remembering
   locking: Implement seclabel stubs for NOP
   domain_lock: Introduce seclabel APIs
   locking: Add virLockSeclabelProtocol
   driver_lockd: Implement seclabel APIs
   lock_daemon: Implement server dispatch
   lock_daemon: Implement seclabel APIs
   security_dac: Cleanup virSecurityDACSetOwnershipInternal usage
   virSecurityManagerNew: Add virLockManagerPluginPtr
   security_dac: Keep original label

  .gitignore   |   2 +
  src/Makefile.am  |  34 ++-
  src/libvirt_private.syms |   4 +
  src/lock_seclabel_protocol-structs   |  21 ++
  src/locking/domain_lock.c|  65 ++
  src/locking/domain_lock.h|  10 +
  src/locking/lock_daemon.c| 388 ++-
  src/locking/lock_daemon.h|   8 +
  src/locking/lock_daemon_dispatch.c   |  77 +++
  src/locking/lock_daemon_dispatch.h   |   3 +
  src/locking/lock_driver.h|  43 
  src/locking/lock_driver_lockd.c  | 118 ++-
  src/locking/lock_driver_nop.c|  22 ++
  src/locking/lock_manager.c   |  26 +++
  src/locking/lock_manager.h   |   9 +
  src/locking/lock_seclabel_protocol.x |  53 +
  src/lxc/lxc_controller.c |   2 +-
  src/lxc/lxc_driver.c |   3 +-
  src/qemu/qemu_driver.c   |   7 +-
  src/security/security_dac.c  | 145 ++---
  src/security/security_manager.c  |  25 ++-
  src/security/security_manager.h  |   6 +-
  tests/Makefile.am|   1 +
  tests/qemuhotplugtest.c  |   2 +-
  tests/seclabeltest.c |   2 +-
  tests/securityselinuxlabeltest.c |   2 +-
  tests/securityselinuxtest.c  |   2 +-
  27 files changed, 1028 insertions(+), 52 deletions(-)
  create mode 100644 src/lock_seclabel_protocol-structs
  create mode 100644 src/locking/lock_seclabel_protocol.x



Ping? I'd really like to see this one in the release.

Michal

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

Re: [libvirt] [PATCH 2/3] qemu: Process the hostdev rawio setting

[Roman Bogorodskiy]

  John Ferlan wrote:

 
 
 On 09/18/2014 12:50 PM, Michal Privoznik wrote:
  On 10.09.2014 01:40, John Ferlan wrote:
  Mimic the Disk processing for 'rawio', but for a scsi_host hostdev
  lun device.
 ...snip...
 
  diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
  index b1d8a32..3544716 100644
  --- a/src/qemu/qemu_process.c
  +++ b/src/qemu/qemu_process.c
  @@ -3756,6 +3756,7 @@ int qemuProcessStart(virConnectPtr conn,
struct qemuProcessHookData hookData;
unsigned long cur_balloon;
size_t i;
  +bool rawio_set = false;
char *nodeset = NULL;
virBitmapPtr nodemask = NULL;
unsigned int stop_flags;
  @@ -4122,13 +4123,15 @@ int qemuProcessStart(virConnectPtr conn,
virDomainDeviceDef dev;
virDomainDiskDefPtr disk = vm-def-disks[i];
 
  -if (vm-def-disks[i]-rawio == 1)
  +if (vm-def-disks[i]-rawio == 1) {
#ifdef CAP_SYS_RAWIO
virCommandAllowCap(cmd, CAP_SYS_RAWIO);
#else
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
   _(Raw I/O is not supported on this 
  platform));
#endif
  +rawio_set = true;
  +}
  
  Interesting. So if rawio is requested we shout an error but don't fail 
  actually. I think we are missing 'goto cleanup' here.
  
 
 See 9a2f36ec04e0436b1ba9f0c21f9be234b25ac579
 
 I can add a goto if desired or perhaps change it to a VIR_WARN() or
 something else well.
 
 I've copied the author of that commit to get an opinion...

Hi,

Yes, I guess 'goto cleanup' is missing here indeed. VIR_WARN() will also
work, but probably it'd be better user experience to report an unsupported
configuration and fail early instead of showing a warning that could be
missed.

Roman Bogorodskiy

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

Re: [libvirt] Adding multiple graphics devices to a vm

[Christophe Fergeau]

Hey,

On Fri, Sep 19, 2014 at 04:47:37AM +, ban...@openmailbox.org wrote:
 I was trying different video devices to correct the problem of having
 1024x768 as the maximum resolution for Debian guests. I found out that
 adding VMVGA gives the wide variety of choice of resolutions I was looking
 for and adjusts the display properly, in contrast to the deformed way xconf
 file did. However I still want to make use of SPICE and QXL.
 
 Is there a way to have both vmvga and qxl attached and working on the same
 guest?
 
 How would I go about doing this in the XML?

Hmm, it's surprising that you don't get more than 1024x768 with
spice+debian. If you install the qxl driver in the guest, this should
give you higher resolutions.

Christophe


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

[libvirt] [PATCH 0/4] bulk stats: QEMU implementation polishing

[Francesco Romani]

This patchset does polishing on the QEMU bulk stats implementation.
The main issue this patchset addresses is that, unless a critical
error is found, bulk stats should be silent and ignore errors.

To do so, virResetLastError() is used in a few places, but this
is not enough since errors are logged anyway.
A better approach is to avoid to report error entirely.

The patchset is organized as follows:
- patches 1 to 3 enhances the functions used in the bulk stats
  path(s) adding a 'bool report' flag, to let the caller optionally
  suppress error reporting.
- patch 4 is a general polishing patch which reduces repetition
  of the code in the block stats collection.

Francesco Romani (4):
  qemu: make qemuDomainHelperGetVcpus silent
  make virNetInterfaceStats silent
  qemu: make qemuMonitorGetAllBlockStatsInfo silent
  qemu: json monitor: reduce duplicated code

 src/lxc/lxc_driver.c |   2 +-
 src/openvz/openvz_driver.c   |   2 +-
 src/qemu/qemu_driver.c   |  32 +-
 src/qemu/qemu_monitor.c  |  12 ++--
 src/qemu/qemu_monitor.h  |   3 +-
 src/qemu/qemu_monitor_json.c | 136 ---
 src/qemu/qemu_monitor_json.h |   3 +-
 src/util/virstats.c  |  21 ---
 src/util/virstats.h  |   3 +-
 src/xen/xen_hypervisor.c |   2 +-
 10 files changed, 111 insertions(+), 105 deletions(-)

-- 
1.9.3

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

[libvirt] [PATCH 1/4] qemu: make qemuDomainHelperGetVcpus silent

[Francesco Romani]

The commit 74c066df4d8 introduced an helper to factor a code path
which is shared between the existing API and the new bulk stats API.
In the bulk stats path errors must be silenced unless critical
(e.g. memory allocation failure).

To address this need, this patch adds an argument to disable error reporting.
---
 src/qemu/qemu_driver.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f28082f..dc8d6c3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1381,8 +1381,10 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int 
*lastCpu, long *vm_rss,
 
 
 static int
-qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo,
- unsigned char *cpumaps, int maplen)
+qemuDomainHelperGetVcpus(virDomainObjPtr vm,
+ virVcpuInfoPtr info, int maxinfo,
+ unsigned char *cpumaps, int maplen,
+ bool report)
 {
 int maxcpu, hostcpus;
 size_t i, v;
@@ -1412,8 +1414,10 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, 
virVcpuInfoPtr info, int maxinfo,
NULL,
vm-pid,
priv-vcpupids[i])  0) {
-virReportSystemError(errno, %s,
- _(cannot get vCPU placement  pCPU 
time));
+if (report)
+virReportSystemError(errno, %s,
+ _(cannot get vCPU placement 
+pCPU time));
 return -1;
 }
 }
@@ -1440,8 +1444,9 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, 
virVcpuInfoPtr info, int maxinfo,
 virBitmapFree(map);
 }
 } else {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   %s, _(cpu affinity is not available));
+if (report)
+virReportError(VIR_ERR_OPERATION_INVALID,
+   %s, _(cpu affinity is not available));
 return -1;
 }
 }
@@ -5044,7 +5049,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
 goto cleanup;
 }
 
-ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen);
+ret = qemuDomainHelperGetVcpus(vm, info, maxinfo, cpumaps, maplen, true);
 
  cleanup:
 if (vm)
@@ -17530,8 +17535,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 return -1;
 
 if (qemuDomainHelperGetVcpus(dom, cpuinfo, dom-def-vcpus,
- NULL, 0)  0) {
-virResetLastError();
+ NULL, 0, false)  0) {
 ret = 0; /* it's ok to be silent and go ahead */
 goto cleanup;
 }
-- 
1.9.3

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

[libvirt] [PATCH 3/4] qemu: make qemuMonitorGetAllBlockStatsInfo silent

[Francesco Romani]

The commit 290e3c6b07a introduced an helper to factor a code path
which is shared between the existing API and the new bulk stats API.
In the bulk stats path errors must be silenced unless critical
(e.g. memory allocation failure).

To address this need, this patch adds an argument to disable error
reporting.
---
 src/qemu/qemu_driver.c   |  4 +-
 src/qemu/qemu_monitor.c  | 12 --
 src/qemu/qemu_monitor.h  |  3 +-
 src/qemu/qemu_monitor_json.c | 87 ++--
 src/qemu/qemu_monitor_json.h |  3 +-
 5 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3ff226f..36b96e5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17696,12 +17696,12 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
 qemuDomainObjEnterMonitor(driver, dom);
 
 nstats = qemuMonitorGetAllBlockStatsInfo(priv-mon, NULL,
- stats, nstats);
+ stats, nstats,
+ false);
 
 qemuDomainObjExitMonitor(driver, dom);
 
 if (nstats  0) {
-virResetLastError();
 ret = 0; /* still ok, again go ahead silently */
 goto cleanup;
 }
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 10f51c5..cac48d7 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1765,17 +1765,21 @@ int
 qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
 const char *dev_name,
 qemuBlockStatsPtr stats,
-int nstats)
+int nstats,
+bool report)
 {
 int ret;
 VIR_DEBUG(mon=%p dev=%s, mon, dev_name);
 
 if (mon-json) {
 ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name,
-  stats, nstats);
+  stats, nstats,
+  report);
 } else {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(unable to query all block stats with this QEMU));
+if (report)
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(unable to query all block stats 
+ with this QEMU));
 return -1;
 }
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2a43a3c..1eeb7b9 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -363,7 +363,8 @@ struct _qemuBlockStats {
 int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
 const char *dev_name,
 qemuBlockStatsPtr stats,
-int nstats)
+int nstats,
+bool report)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3);
 
 int qemuMonitorGetBlockStatsParamsNumber(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 857edf1..4810bd3 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1749,7 +1749,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
 if (flush_total_times)
 *flush_total_times = -1;
 
-if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name, stats, 1) != 1)
+if (qemuMonitorJSONGetAllBlockStatsInfo(mon, dev_name,
+stats, 1, true) != 1)
 goto cleanup;
 
 *rd_req = stats.rd_req;
@@ -1777,7 +1778,8 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
 int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
 const char *dev_name,
 qemuBlockStatsPtr bstats,
-int nstats)
+int nstats,
+bool report)
 {
 int ret, count;
 size_t i;
@@ -1802,8 +1804,9 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr 
mon,
 
 devices = virJSONValueObjectGet(reply, return);
 if (!devices || devices-type != VIR_JSON_TYPE_ARRAY) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(blockstats reply was missing device list));
+if (report)
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(blockstats reply was missing device list));
 goto cleanup;
 }
 
@@ -1812,9 +1815,10 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr 
mon,
 virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
 virJSONValuePtr stats;
 if (!dev || dev-type != VIR_JSON_TYPE_OBJECT) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   

[libvirt] [PATCH 2/4] make virNetInterfaceStats silent

[Francesco Romani]

virNetInterfaceStats is now used in the bulk stats path, and
in this path errors must be silenced unless critical
(e.g. memory allocation failure).

To address this need, this patch adds an argument to disable error reporting.
---
 src/lxc/lxc_driver.c   |  2 +-
 src/openvz/openvz_driver.c |  2 +-
 src/qemu/qemu_driver.c |  6 ++
 src/util/virstats.c| 21 +
 src/util/virstats.h|  3 ++-
 src/xen/xen_hypervisor.c   |  2 +-
 6 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c3cd62c..2eaeb22 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3112,7 +3112,7 @@ lxcDomainInterfaceStats(virDomainPtr dom,
 }
 
 if (ret == 0)
-ret = virNetInterfaceStats(path, stats);
+ret = virNetInterfaceStats(path, stats, true);
 else
 virReportError(VIR_ERR_INVALID_ARG,
_(Invalid path, '%s' is not a known interface), path);
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index b62273a..2b39f36 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -2010,7 +2010,7 @@ openvzDomainInterfaceStats(virDomainPtr dom,
 }
 
 if (ret == 0)
-ret = virNetInterfaceStats(path, stats);
+ret = virNetInterfaceStats(path, stats, true);
 else
 virReportError(VIR_ERR_INVALID_ARG,
_(invalid path, '%s' is not a known interface), path);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dc8d6c3..3ff226f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9884,7 +9884,7 @@ qemuDomainInterfaceStats(virDomainPtr dom,
 }
 
 if (ret == 0)
-ret = virNetInterfaceStats(path, stats);
+ret = virNetInterfaceStats(path, stats, true);
 else
 virReportError(VIR_ERR_INVALID_ARG,
_(invalid path, '%s' is not a known interface), path);
@@ -17634,10 +17634,8 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 QEMU_ADD_NAME_PARAM(record, maxparams,
 net, i, dom-def-nets[i]-ifname);
 
-if (virNetInterfaceStats(dom-def-nets[i]-ifname, tmp)  0) {
-virResetLastError();
+if (virNetInterfaceStats(dom-def-nets[i]-ifname, tmp, false)  0)
 continue;
-}
 
 QEMU_ADD_NET_PARAM(record, maxparams, i,
rx.bytes, tmp.rx_bytes);
diff --git a/src/util/virstats.c b/src/util/virstats.c
index c4725ed..496393b 100644
--- a/src/util/virstats.c
+++ b/src/util/virstats.c
@@ -51,7 +51,8 @@
 #ifdef __linux__
 int
 virNetInterfaceStats(const char *path,
- virDomainInterfaceStatsPtr stats)
+ virDomainInterfaceStatsPtr stats,
+ bool report)
 {
 int path_len;
 FILE *fp;
@@ -115,14 +116,16 @@ virNetInterfaceStats(const char *path,
 }
 VIR_FORCE_FCLOSE(fp);
 
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(/proc/net/dev: Interface not found));
+if (report)
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(/proc/net/dev: Interface not found));
 return -1;
 }
 #elif defined(HAVE_GETIFADDRS)  defined(AF_LINK)
 int
 virNetInterfaceStats(const char *path,
- virDomainInterfaceStatsPtr stats)
+ virDomainInterfaceStatsPtr stats,
+ bool report)
 {
 struct ifaddrs *ifap, *ifa;
 struct if_data *ifd;
@@ -158,7 +161,7 @@ virNetInterfaceStats(const char *path,
 }
 }
 
-if (ret  0)
+if (ret  0  report)
 virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(Interface not found));
 
@@ -168,10 +171,12 @@ virNetInterfaceStats(const char *path,
 #else
 int
 virNetInterfaceStats(const char *path ATTRIBUTE_UNUSED,
- virDomainInterfaceStatsPtr stats ATTRIBUTE_UNUSED)
+ virDomainInterfaceStatsPtr stats ATTRIBUTE_UNUSED,
+ bool report)
 {
-virReportError(VIR_ERR_OPERATION_INVALID, %s,
-   _(interface stats not implemented on this platform));
+if (report)
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(interface stats not implemented on this platform));
 return -1;
 }
 
diff --git a/src/util/virstats.h b/src/util/virstats.h
index d2c6b64..df993cd 100644
--- a/src/util/virstats.h
+++ b/src/util/virstats.h
@@ -26,6 +26,7 @@
 # include internal.h
 
 extern int virNetInterfaceStats(const char *path,
-virDomainInterfaceStatsPtr stats);
+virDomainInterfaceStatsPtr stats,
+bool report);
 
 #endif /* __STATS_LINUX_H__ */
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index d3d4aea..8b257f7 100644
--- a/src/xen/xen_hypervisor.c
+++ 

[libvirt] [PATCH 4/4] qemu: json monitor: reduce duplicated code

[Francesco Romani]

This patch replaces repetitive blocks of code with a couple
of macros for the sake of clarity.
There are no changes in behaviour.
---
 src/qemu/qemu_monitor_json.c | 129 ++-
 1 file changed, 53 insertions(+), 76 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 4810bd3..4fa72c9 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1775,6 +1775,25 @@ int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
 }
 
 
+#define QEMU_MONITOR_JSON_MALFORMED_ENTRY(kind) do { \
+if (report) \
+virReportError(VIR_ERR_INTERNAL_ERROR, \
+   _(blockstats %s entry was not  \
+ in expected format), \
+kind); \
+goto cleanup; \
+} while (0)
+
+
+#define QEMU_MONITOR_JSON_MISSING_STAT(statistic) do { \
+if (report) \
+virReportError(VIR_ERR_INTERNAL_ERROR, \
+_(cannot read %s statistic), \
+statistic); \
+goto cleanup; \
+} while (0)
+
+
 int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
 const char *dev_name,
 qemuBlockStatsPtr bstats,
@@ -1814,26 +1833,16 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr 
mon,
 for (i = 0; i  virJSONValueArraySize(devices)  count  nstats; i++) {
 virJSONValuePtr dev = virJSONValueArrayGet(devices, i);
 virJSONValuePtr stats;
-if (!dev || dev-type != VIR_JSON_TYPE_OBJECT) {
-if (report)
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(blockstats device entry was not 
- in expected format));
-goto cleanup;
-}
+if (!dev || dev-type != VIR_JSON_TYPE_OBJECT)
+QEMU_MONITOR_JSON_MALFORMED_ENTRY(device);
 
 /* If dev_name is specified, we are looking for a specific device,
  * so we must be stricter.
  */
 if (dev_name) {
 const char *thisdev = virJSONValueObjectGetString(dev, device);
-if (!thisdev) {
-if (report)
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(blockstats device entry was not 
- in expected format));
-goto cleanup;
-}
+if (!thisdev)
+QEMU_MONITOR_JSON_MALFORMED_ENTRY(device);
 
 /* New QEMU has separate names for host  guest side of the disk
  * and libvirt gives the host side a 'drive-' prefix. The passed
@@ -1847,81 +1856,44 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr 
mon,
 }
 
 if ((stats = virJSONValueObjectGet(dev, stats)) == NULL ||
-stats-type != VIR_JSON_TYPE_OBJECT) {
-if (report)
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(blockstats stats entry was not 
- in expected format));
-goto cleanup;
-}
+stats-type != VIR_JSON_TYPE_OBJECT)
+QEMU_MONITOR_JSON_MALFORMED_ENTRY(stats);
 
 if (virJSONValueObjectGetNumberLong(stats, rd_bytes,
-bstats-rd_bytes)  0) {
-if (report)
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(cannot read %s statistic),
-   rd_bytes);
-goto cleanup;
-}
+bstats-rd_bytes)  0)
+QEMU_MONITOR_JSON_MISSING_STAT(rd_bytes);
+
 if (virJSONValueObjectGetNumberLong(stats, rd_operations,
-bstats-rd_req)  0) {
-if (report)
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(cannot read %s statistic),
-rd_operations);
-goto cleanup;
-}
+bstats-rd_req)  0)
+QEMU_MONITOR_JSON_MISSING_STAT(rd_operations);
+
 if (virJSONValueObjectHasKey(stats, rd_total_time_ns) 
 (virJSONValueObjectGetNumberLong(stats, rd_total_time_ns,
- bstats-rd_total_times)  0)) {
-if (report)
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(cannot read %s statistic),
-   rd_total_time_ns);
-goto cleanup;
-}
+ bstats-rd_total_times)  0))
+QEMU_MONITOR_JSON_MISSING_STAT(rd_total_time_ns);
+
 if (virJSONValueObjectGetNumberLong(stats, wr_bytes,
-bstats-wr_bytes)  0) {
-if (report)
-

[libvirt] [PATCH v2 0/5] Add rawio for hostdev

[John Ferlan]

v1 is here:
http://www.redhat.com/archives/libvir-list/2014-September/msg00502.html

Changes since v1 (from code review):

 * Insert patch 1 to insert a missing goto if rawio is set, but the
   build doesn't suport CAP_SYS_RAWIO.
 * Insert patch 2 to use the TristateBool for virDomainDiskDef and
   the associated changes where it's used in code
 * Patch 3, 4, 5: Use TristateBool for hostdev rawio and the virYesNo
   in the rng

John Ferlan (5):
  qemu: Add missing goto on rawio
  domain_conf: Change virDomainDiskDef 'rawio' to use virTristateBool
  hostdev: Add rawio attribute to _virDomainHostdevSubsysSCSI
  qemu: Process the hostdev rawio setting
  domain_conf: Process the rawio for a hostdev device

 docs/formatdomain.html.in  | 12 +--
 docs/schemas/domaincommon.rng  | 12 +--
 src/conf/domain_conf.c | 40 +++---
 src/conf/domain_conf.h |  4 +--
 src/qemu/qemu_domain.c | 23 +++--
 src/qemu/qemu_domain.h |  4 +++
 src/qemu/qemu_driver.c |  1 +
 src/qemu/qemu_process.c| 23 -
 .../qemuxml2argv-hostdev-scsi-rawio.xml| 35 +++
 tests/qemuxml2xmltest.c|  1 +
 10 files changed, 132 insertions(+), 23 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml

-- 
1.9.3

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

[libvirt] [PATCH v2 1/5] qemu: Add missing goto on rawio

[John Ferlan]

Commit id '9a2f36ec' added a build conditional of CAP_SYS_RAWIO
in order to determine whether or not a disk definition using rawio
should be allowed on platforms without CAP_SYS_RAWIO. If one was
found, virReportError was used but the code didn't goto cleanup.

This patch adds the goto.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_process.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 245a93c..eecef12 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4357,13 +4357,15 @@ int qemuProcessStart(virConnectPtr conn,
 virDomainDeviceDef dev;
 virDomainDiskDefPtr disk = vm-def-disks[i];
 
-if (vm-def-disks[i]-rawio == 1)
+if (vm-def-disks[i]-rawio == 1) {
 #ifdef CAP_SYS_RAWIO
 virCommandAllowCap(cmd, CAP_SYS_RAWIO);
 #else
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Raw I/O is not supported on this platform));
+goto cleanup;
 #endif
+}
 
 dev.type = VIR_DOMAIN_DEVICE_DISK;
 dev.data.disk = disk;
-- 
1.9.3

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

[libvirt] [PATCH v2 4/5] qemu: Process the hostdev rawio setting

[John Ferlan]

Mimic the Disk processing for 'rawio', but for a scsi_host hostdev
lun device.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_domain.c  | 18 ++
 src/qemu/qemu_domain.h  |  4 
 src/qemu/qemu_driver.c  |  1 +
 src/qemu/qemu_process.c | 19 +++
 4 files changed, 42 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 075406e..95b71b7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1926,6 +1926,10 @@ void qemuDomainObjCheckTaint(virQEMUDriverPtr driver,
 for (i = 0; i  obj-def-ndisks; i++)
 qemuDomainObjCheckDiskTaint(driver, obj, obj-def-disks[i], logFD);
 
+for (i = 0; i  obj-def-nhostdevs; i++)
+qemuDomainObjCheckHostdevTaint(driver, obj, obj-def-hostdevs[i],
+   logFD);
+
 for (i = 0; i  obj-def-nnets; i++)
 qemuDomainObjCheckNetTaint(driver, obj, obj-def-nets[i], logFD);
 
@@ -1953,6 +1957,20 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
 }
 
 
+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+virDomainHostdevDefPtr hostdev,
+int logFD)
+{
+virDomainHostdevSubsysSCSIPtr scsisrc = hostdev-source.subsys.u.scsi;
+
+if (hostdev-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI 
+scsisrc-rawio == VIR_TRISTATE_BOOL_YES)
+qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES,
+   logFD);
+}
+
+
 void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
 virDomainObjPtr obj,
 virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 4ae2c57..d21acd7 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -301,6 +301,10 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
  virDomainObjPtr obj,
  virDomainDiskDefPtr disk,
  int logFD);
+void qemuDomainObjCheckHostdevTaint(virQEMUDriverPtr driver,
+virDomainObjPtr obj,
+virDomainHostdevDefPtr disk,
+int logFD);
 void qemuDomainObjCheckNetTaint(virQEMUDriverPtr driver,
 virDomainObjPtr obj,
 virDomainNetDefPtr net,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f28082f..b888b09 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6581,6 +6581,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 break;
 
 case VIR_DOMAIN_DEVICE_HOSTDEV:
+qemuDomainObjCheckHostdevTaint(driver, vm, dev-data.hostdev, -1);
 ret = qemuDomainAttachHostDevice(dom-conn, driver, vm,
  dev-data.hostdev);
 if (!ret)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 43cf396..8e3ae9b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3983,6 +3983,7 @@ int qemuProcessStart(virConnectPtr conn,
 struct qemuProcessHookData hookData;
 unsigned long cur_balloon;
 size_t i;
+bool rawio_set = false;
 char *nodeset = NULL;
 virBitmapPtr nodemask = NULL;
 unsigned int stop_flags;
@@ -4360,6 +4361,7 @@ int qemuProcessStart(virConnectPtr conn,
 if (vm-def-disks[i]-rawio == VIR_TRISTATE_BOOL_YES) {
 #ifdef CAP_SYS_RAWIO
 virCommandAllowCap(cmd, CAP_SYS_RAWIO);
+rawio_set = true;
 #else
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Raw I/O is not supported on this platform));
@@ -4376,6 +4378,23 @@ int qemuProcessStart(virConnectPtr conn,
 goto cleanup;
 }
 
+/* If rawio not already set, check hostdevs as well */
+if (!rawio_set) {
+for (i = 0; i  vm-def-nhostdevs; i++) {
+virDomainHostdevSubsysSCSIPtr scsisrc =
+vm-def-hostdevs[i]-source.subsys.u.scsi;
+if (scsisrc-rawio == VIR_TRISTATE_BOOL_YES) {
+#ifdef CAP_SYS_RAWIO
+virCommandAllowCap(cmd, CAP_SYS_RAWIO);
+#else
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(Raw I/O is not supported on this platform));
+goto cleanup;
+#endif
+}
+}
+}
+
 virCommandSetPreExecHook(cmd, qemuProcessHook, hookData);
 virCommandSetMaxProcesses(cmd, cfg-maxProcesses);
 virCommandSetMaxFiles(cmd, cfg-maxFiles);
-- 
1.9.3

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

[libvirt] [PATCH v2 5/5] domain_conf: Process the rawio for a hostdev device

[John Ferlan]

Add a rawio to the hostdev XML and process it mimicing the
disk XML for a lun which supports/requires rawio

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatdomain.html.in  | 12 ++--
 docs/schemas/domaincommon.rng  | 12 ++--
 src/conf/domain_conf.c | 24 +++
 .../qemuxml2argv-hostdev-scsi-rawio.xml| 35 ++
 tests/qemuxml2xmltest.c|  1 +
 5 files changed, 78 insertions(+), 6 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8c03ebb..5e2b65a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1815,7 +1815,7 @@
   dtcoderawio/code attribute
   span class=sincesince 0.9.10/span/dt
 dd
-Indicates whether the disk is needs rawio capability; valid
+Indicates whether the disk needs rawio capability. Valid
 settings are yes or no (default is no). If any one disk
 in a domain has rawio='yes', rawio capability will be enabled
 for all disks in the domain (because, in the case of QEMU, this
@@ -2925,7 +2925,7 @@
 pre
   ...
   lt;devicesgt;
-lt;hostdev mode='subsystem' type='scsi'gt;
+lt;hostdev mode='subsystem' type='scsi' sgio='filtered' rawio='yes'gt;
   lt;sourcegt;
 lt;adapter name='scsi_host0'/gt;
 lt;address type='scsi' bus='0' target='0' unit='0'/gt;
@@ -2984,7 +2984,13 @@
 (span class=sincesince 1.0.6/span) attribute indicates
 whether the kernel will filter unprivileged SG_IO commands for
 the disk, valid settings are filtered or unfiltered.
-The default is filtered.
+The default is filtered. The optional coderawio/code
+(span class=sincesince 1.2.9/span) attribute indicates
+whether the lun needs the rawio capability. Valid settings are
+yes or no. See the rawio description within the
+a href=#elementsDisksdisk/a section.
+If a disk lun in the domain already has the rawio capability,
+then this setting not required.
   /dd
 /dl
   /dd
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 19dc82f..36bc184 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1252,9 +1252,7 @@
 /choice
   /attribute
   optional
-attribute name=rawio
-  ref name=virYesNo/
-/attribute
+ref name=rawIO/
   /optional
   optional
 attribute name=sgio
@@ -3577,6 +3575,9 @@
 /choice
   /attribute
 /optional
+optional
+  ref name=rawIO/
+/optional
 element name=source
   choice
 group  !-- scsi_host adapter --
@@ -4937,4 +4938,9 @@
   /optional
 /element
   /define
+  define name=rawIO
+attribute name=rawio
+  ref name=virYesNo/
+/attribute
+  /define
 /grammar
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c240c83..bb4a4cb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4504,6 +4504,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 xmlNodePtr sourcenode;
 char *managed = NULL;
 char *sgio = NULL;
+char *rawio = NULL;
 char *backendStr = NULL;
 int backend;
 int ret = -1;
@@ -4521,6 +4522,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 }
 
 sgio = virXMLPropString(node, sgio);
+rawio = virXMLPropString(node, rawio);
 
 /* @type is passed in from the caller rather than read from the
  * xml document, because it is specified in different places for
@@ -4572,6 +4574,21 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 }
 }
 
+if (rawio) {
+if (def-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(rawio is only supported for scsi host device));
+goto error;
+}
+
+if ((scsisrc-rawio = virTristateBoolTypeFromString(rawio)) = 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(unknown hostdev rawio setting '%s'),
+   rawio);
+goto error;
+}
+}
+
 switch (def-source.subsys.type) {
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
 if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, def, flags)  0)
@@ -4611,6 +4628,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
  error:
 VIR_FREE(managed);
 VIR_FREE(sgio);
+VIR_FREE(rawio);
 VIR_FREE(backendStr);
 return ret;
 }
@@ -17804,6 +17822,12 @@ virDomainHostdevDefFormat(virBufferPtr buf,
 scsisrc-sgio)
 virBufferAsprintf(buf,  

[libvirt] [PATCH v2 3/5] hostdev: Add rawio attribute to _virDomainHostdevSubsysSCSI

[John Ferlan]

Add the 'rawio' attribute to match _virDomainDiskDef

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_conf.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index afd9943..47a1851 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -439,6 +439,7 @@ typedef virDomainHostdevSubsysSCSI 
*virDomainHostdevSubsysSCSIPtr;
 struct _virDomainHostdevSubsysSCSI {
 int protocol; /* enum virDomainHostdevSCSIProtocolType */
 int sgio; /* enum virDomainDeviceSGIO */
+int rawio; /* enum virTristateBool */
 union {
 virDomainHostdevSubsysSCSIHost host;
 virDomainHostdevSubsysSCSIiSCSI iscsi;
-- 
1.9.3

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

[libvirt] [PATCH v2 2/5] domain_conf: Change virDomainDiskDef 'rawio' to use virTristateBool

[John Ferlan]

Adjust disk definition for 'rawio' to use the TristateBool logic

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_conf.c  | 16 
 src/conf/domain_conf.h  |  3 +--
 src/qemu/qemu_domain.c  |  5 +++--
 src/qemu/qemu_process.c |  2 +-
 4 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ccec1c..c240c83 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5954,12 +5954,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 
 if (rawio) {
-def-rawio_specified = true;
-if (STREQ(rawio, yes)) {
-def-rawio = 1;
-} else if (STREQ(rawio, no)) {
-def-rawio = 0;
-} else {
+if ((def-rawio = virTristateBoolTypeFromString(rawio)) = 0) {
 virReportError(VIR_ERR_XML_ERROR,
_(unknown disk rawio setting '%s'),
rawio);
@@ -15828,12 +15823,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf,
   disk type='%s' device='%s',
   type, device);
-if (def-rawio_specified) {
-if (def-rawio == 1) {
-virBufferAddLit(buf,  rawio='yes');
-} else if (def-rawio == 0) {
-virBufferAddLit(buf,  rawio='no');
-}
+if (def-rawio) {
+virBufferAsprintf(buf,  rawio='%s',
+  virTristateBoolTypeToString(def-rawio));
 }
 
 if (def-sgio)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 640a4c5..afd9943 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -663,8 +663,7 @@ struct _virDomainDiskDef {
 int startupPolicy; /* enum virDomainStartupPolicy */
 bool transient;
 virDomainDeviceInfo info;
-bool rawio_specified;
-int rawio; /* no = 0, yes = 1 */
+int rawio; /* enum virTristateBool */
 int sgio; /* enum virDomainDeviceSGIO */
 int discard; /* enum virDomainDiskDiscard */
 unsigned int iothread; /* unused = 0,  0 specific thread # */
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5859ba7..075406e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1945,8 +1945,9 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver,
 cfg-allowDiskFormatProbing)
 qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_DISK_PROBING, logFD);
 
-if (disk-rawio == 1)
-qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, 
logFD);
+if (disk-rawio == VIR_TRISTATE_BOOL_YES)
+qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES,
+   logFD);
 
 virObjectUnref(cfg);
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index eecef12..43cf396 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4357,7 +4357,7 @@ int qemuProcessStart(virConnectPtr conn,
 virDomainDeviceDef dev;
 virDomainDiskDefPtr disk = vm-def-disks[i];
 
-if (vm-def-disks[i]-rawio == 1) {
+if (vm-def-disks[i]-rawio == VIR_TRISTATE_BOOL_YES) {
 #ifdef CAP_SYS_RAWIO
 virCommandAllowCap(cmd, CAP_SYS_RAWIO);
 #else
-- 
1.9.3

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

Re: [libvirt] [PATCH 1/4] qemu: make qemuDomainHelperGetVcpus silent

[Daniel P. Berrange]

On Fri, Sep 19, 2014 at 11:44:20AM +0200, Francesco Romani wrote:
 The commit 74c066df4d8 introduced an helper to factor a code path
 which is shared between the existing API and the new bulk stats API.
 In the bulk stats path errors must be silenced unless critical
 (e.g. memory allocation failure).
 
 To address this need, this patch adds an argument to disable error reporting.

I'm really don't like the approach of adding 'bool reportError' flags to
internal functions as it doesn't scale. These flags end up creeping out
across more  more of the code base and different callers are going to
disagree about which errors should be ignored and which not. Having the
callers use virResetError is preferrable IMHO as it avoids polluting
countless internal functions with ill defined args.

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 1/4] qemu: make qemuDomainHelperGetVcpus silent

[Peter Krempa]

On 09/19/14 12:29, Daniel P. Berrange wrote:
 On Fri, Sep 19, 2014 at 11:44:20AM +0200, Francesco Romani wrote:
 The commit 74c066df4d8 introduced an helper to factor a code path
 which is shared between the existing API and the new bulk stats API.
 In the bulk stats path errors must be silenced unless critical
 (e.g. memory allocation failure).

 To address this need, this patch adds an argument to disable error reporting.
 
 I'm really don't like the approach of adding 'bool reportError' flags to
 internal functions as it doesn't scale. These flags end up creeping out
 across more  more of the code base and different callers are going to
 disagree about which errors should be ignored and which not. Having the
 callers use virResetError is preferrable IMHO as it avoids polluting

In Eric's last review he advised to change from virResetError to the
boolean flag so that logs are not polluted. Without this patch, the code
is in the state as you suggest, but it contradicts Eric's stance.

 countless internal functions with ill defined args.
 
 Regards,
 Daniel
 

Peter



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

Re: [libvirt] retiring v0.9.6-maint

[Ján Tomko]

On 09/18/2014 05:15 PM, Eric Blake wrote:
 On 09/18/2014 02:36 AM, Daniel P. Berrange wrote:
 On Wed, Sep 17, 2014 at 04:24:07PM -0600, Eric Blake wrote:
 Any objections to retiring the v0.9.6-maint branch?  After all, we have
 already retired the v0.9.11-maint branch
 (http://libvirt.org/git/?p=libvirt.git;a=commit;h=cd0d348ed), and the
 only activity on v0.9.6-maint since 0.9.6.4 was released in January 2013
 was the backport of a single CVE fix.  The branch no longer builds
 cleanly on Fedora 20, and while I could identify patches to backport to
 fix the build situation, it's not worth my time if we can just retire
 the branch.

 FWIW, I'm not really a fan of deleting the branches. Is there any harm
 to just leaving it there idle ?
 
 The branches aren't deleted, per se, just a new commit added on top of
 the branch that declares the intent.  For example, all you see if you
 check out v0.9.11-maint is this README file:
 
 http://libvirt.org/git/?p=libvirt.git;a=blob;f=README;h=68aeed1ae7d131661f2ba07eff1b4ae16ac4f3b8;hb=cd0d348ed
 
 The branch would still usable by checking out v0.9.11-maint^ as a
 detached head, so the history is still there.  All I'm proposing is
 documenting that we aren't going to try and port security fixes to the
 branch any longer, because no one appears to be actively using it.
 

I think we need to be clearer what and how is maintained on the website.

The Security Process [1] states:
 The libvirt community maintains one or more stable release branches at any
 given point in time. The security team will aim to publish fixes for GIT
 master (which will become the next major release) and each currently
 maintained stable release branch. The distro maintainers will be
 responsible for backporting the officially published fixes to other release
 branches where applicable.

But in practice, CVE fixes are pushed to all -maint branches, not just those
with releases.

http://libvirt.org/downloads.html mentions that supported -maint branches are
considered during CVE analysis, but it's unclear on the definition of support.

This paragraph about hourly snapshots:
 These snapshots should be usable, but we make no guarantees about their
 stability; furthermore, they should NOT be considered formal releases, and
 they may have transient security problems that will not be assigned a CVE.

may give the impressions that the CVEs are fixed in the maintenance releases,
even when they're only backported on the branches.

(The wiki [2] lists past maintenance releases, but no indication whether there
will be more releases).

Since stable releases were made out of 0.9.6, I think we should mention on the
wiki/download page, that no more releases are going to be made and they are no
longer supported (same for 0.9.11 and maybe 0.10.2 too?), in addition
to/instead of deleting the content of the branch.

(Also, maintaining 20 releases is IMHO a waste of time, personally I only
backport my important fixes to the latest Fedora release where I know it will
be picked up in the next release and the latest -maint branch. Does anyone use
the -maint branches without maintenance releases? IIRC they were created for
Gentoo, but it looks like all the current versions use the vanilla sources,
with no backport from the maint branches [3]).

Jan

[3] http://packages.gentoo.org/package/app-emulation/libvirt
[2] http://wiki.libvirt.org/page/Maintenance_Releases
[1] http://libvirt.org/securityprocess.html



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

Re: [libvirt] [PATCH v2 2/9] qemu: Add news throttle options to the structure _virDomainBlockIoTuneInfo.

[Michal Privoznik]

On 15.09.2014 19:27, Matthias Gatto wrote:

Modify the structure _virDomainBlockIoTuneInfo to support total_bytes_sec_max, 
write_bytes_sec_max,
read_bytes_sec_max, total_iops_sec_max, write_iops_sec_max, read_iops_sec_max, 
size_iops_sec options.

Add the boolean support_max_options in the structure _virDomainBlockIoTuneInfo 
to know
if the qemu binary suport the bps_max options and they friends.

Change the initialization of the variable expectedInfo in qemumonitorjsontest.c
to avoid compiling problem.

Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
  src/conf/domain_conf.h  | 8 
  tests/qemumonitorjsontest.c | 2 +-
  2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ff7d640..5423523 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -606,6 +606,14 @@ struct _virDomainBlockIoTuneInfo {
  unsigned long long total_iops_sec;
  unsigned long long read_iops_sec;
  unsigned long long write_iops_sec;
+unsigned long long total_bytes_sec_max;
+unsigned long long read_bytes_sec_max;
+unsigned long long write_bytes_sec_max;
+unsigned long long total_iops_sec_max;
+unsigned long long read_iops_sec_max;
+unsigned long long write_iops_sec_max;
+unsigned long long size_iops_sec;
+bool suport_max_options;


Spelling is not right: s/suport/support/

Why is the field needed anyway? You're setting it depending on 
virQEMUCapsGet() anyway ...



  };
  typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr;

diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index baee80a..5a2a337 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1835,7 +1835,7 @@ 
testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data)
  if (!test)
  return -1;

-expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6};
+expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 
11, 12, 13, false};

  if (qemuMonitorTestAddItem(test, query-block, queryBlockReply)  0 ||
  qemuMonitorTestAddItemParams(test, block_set_io_throttle,



Michal

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

Re: [libvirt] [PATCH v2 5/9] qemu: Add bps_max and friend to domain_conf

[Michal Privoznik]

On 15.09.2014 19:27, Matthias Gatto wrote:

Allow qemu driver to save the configuration use by bps_max and they friends.

Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
  src/conf/domain_conf.c | 89 +-
  1 file changed, 88 insertions(+), 1 deletion(-)


I'd expect this patch to be merged with 1/9.

Michal

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

Re: [libvirt] [PATCH v2 9/9] virsh: Add bps_max and friends to virsh

[Michal Privoznik]

On 15.09.2014 19:27, Matthias Gatto wrote:

Add the new throttle options to virsh, and send them to libvirt.

Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
  tools/virsh-domain.c | 119 +++
  1 file changed, 119 insertions(+)


virsh.pod needs to be updated too

Michal

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

Re: [libvirt] [PATCH v2 3/9] qemu: Add the capabilitie to detect if the qemu binary have the capability to use bps_max and friends

[Michal Privoznik]

On 15.09.2014 19:27, Matthias Gatto wrote:

Add a value in the enum virQEMUCapsFlags for the qemu capability.
Set it with virQEMUCapsSet if the binary suport bps_max and they friends.

Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
  src/qemu/qemu_capabilities.c | 6 ++
  src/qemu/qemu_capabilities.h | 1 +
  2 files changed, 7 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 360cc67..a97ca03 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -265,6 +265,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
numa,
memory-backend-file,
usb-audio,
+  drive-iotune-max,
  );


@@ -1063,6 +1064,8 @@ virQEMUCapsComputeCmdFlags(const char *help,
  virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_COPY_ON_READ);
  if (strstr(help, bps=))
  virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE);
+if (strstr(help, bps_max=))
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX);
  }
  if ((p = strstr(help, -vga))  !strstr(help, -std-vga)) {
  const char *nl = strstr(p, \n);
@@ -3141,6 +3144,9 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
  if (qemuCaps-version = 1006000)
  virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);

+if (qemuCaps-version = 1007000)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX);
+


What's wrong with just the above chunk? I mean, version based checks are 
not good. I'd just drop this one.



  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 2911759..394a836 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -213,6 +213,7 @@ typedef enum {
  QEMU_CAPS_NUMA   = 171, /* newer -numa handling with disjoint 
cpu ranges */
  QEMU_CAPS_OBJECT_MEMORY_FILE = 172, /* -object memory-backend-file */
  QEMU_CAPS_OBJECT_USB_AUDIO   = 173, /* usb-audio device support */
+QEMU_CAPS_DRIVE_IOTUNE_MAX   = 174, /* -drive bps_max= and friends */

  QEMU_CAPS_LAST,   /* this must always be the last item */
  } virQEMUCapsFlags;



Michal

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

Re: [libvirt] [PATCH v2 4/9] qemu: Add bps_max and friends qemu driver

[Michal Privoznik]

On 15.09.2014 19:27, Matthias Gatto wrote:

Add support for bps_max and friends in the driver part.
In the part checking if a qemu is running, check if the running binary support 
bps_max,
if not print an error message, if yes add it to info variable

Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
---
  src/qemu/qemu_driver.c | 158 +++--
  1 file changed, 152 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2c3f179..7d024b3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -104,6 +104,7 @@ VIR_LOG_INIT(qemu.qemu_driver);
  #define QEMU_NB_MEM_PARAM  3

  #define QEMU_NB_BLOCK_IO_TUNE_PARAM  6
+#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX  13


Why? I think we can just proceed with current _PARAM changed so it 
reflects the number of arguments that can be returned.


Aha! going through the later patches I think I understand why you want 
this - in case qemu supports QEMU_CAPS_DRIVE_IOTUNE_MAX then we should 
report 13, if it doesn't then 6. Well, the code doesn't say that [2].




  #define QEMU_NB_NUMA_PARAM 2

@@ -15818,8 +15819,12 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
  int idx = -1;
  bool set_bytes = false;
  bool set_iops = false;
+bool set_bytes_max = false;
+bool set_iops_max = false;
+bool set_size_iops = false;
  virQEMUDriverConfigPtr cfg = NULL;
  virCapsPtr caps = NULL;
+info.suport_max_options = true;


This is useless ... [1]



  virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -15836,6 +15841,20 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
 VIR_TYPED_PARAM_ULLONG,
 VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
 VIR_TYPED_PARAM_ULLONG,
+   VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
+   VIR_TYPED_PARAM_ULLONG,
+   VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
+   VIR_TYPED_PARAM_ULLONG,
+   VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
+   VIR_TYPED_PARAM_ULLONG,
+   VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
+   VIR_TYPED_PARAM_ULLONG,
+   VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
+   VIR_TYPED_PARAM_ULLONG,
+   VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
+   VIR_TYPED_PARAM_ULLONG,
+   VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
+   VIR_TYPED_PARAM_ULLONG,
 NULL)  0)
  return -1;

@@ -15902,6 +15921,34 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
   VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) {
  info.write_iops_sec = param-value.ul;
  set_iops = true;
+} else if (STREQ(param-field,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX)) {
+info.total_bytes_sec_max = param-value.ul;
+set_bytes_max = true;
+} else if (STREQ(param-field,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX)) {
+info.read_bytes_sec_max = param-value.ul;
+set_bytes_max = true;
+} else if (STREQ(param-field,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX)) {
+info.write_bytes_sec_max = param-value.ul;
+set_bytes_max = true;
+} else if (STREQ(param-field,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX)) {
+info.total_iops_sec_max = param-value.ul;
+set_iops_max = true;
+} else if (STREQ(param-field,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX)) {
+info.read_iops_sec_max = param-value.ul;
+set_iops_max = true;
+} else if (STREQ(param-field,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX)) {
+info.write_iops_sec_max = param-value.ul;
+set_iops_max = true;
+} else if (STREQ(param-field,
+ VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC)) {
+info.size_iops_sec = param-value.ul;
+set_size_iops = true;
  }
  }

@@ -15919,11 +15966,33 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
  goto endjob;
  }

+if ((info.total_bytes_sec_max  info.read_bytes_sec_max) ||
+(info.total_bytes_sec_max  info.write_bytes_sec_max)) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+   _(total and read/write of bytes_sec_max cannot be set at 
the same time));
+goto endjob;
+}
+
+if ((info.total_iops_sec_max  info.read_iops_sec_max) ||
+

Re: [libvirt] [PATCH v2 0/5] Add rawio for hostdev

[Ján Tomko]

On 09/19/2014 12:17 PM, John Ferlan wrote:
 v1 is here:
 http://www.redhat.com/archives/libvir-list/2014-September/msg00502.html
 
 Changes since v1 (from code review):
 
  * Insert patch 1 to insert a missing goto if rawio is set, but the
build doesn't suport CAP_SYS_RAWIO.
  * Insert patch 2 to use the TristateBool for virDomainDiskDef and
the associated changes where it's used in code
  * Patch 3, 4, 5: Use TristateBool for hostdev rawio and the virYesNo
in the rng
 
 John Ferlan (5):
   qemu: Add missing goto on rawio
   domain_conf: Change virDomainDiskDef 'rawio' to use virTristateBool

   hostdev: Add rawio attribute to _virDomainHostdevSubsysSCSI

I think the above commit should be squashed with the domain_conf patch below.

   qemu: Process the hostdev rawio setting

And this one moved to the end, since it depends on it.

   domain_conf: Process the rawio for a hostdev device
 
  docs/formatdomain.html.in  | 12 +--
  docs/schemas/domaincommon.rng  | 12 +--
  src/conf/domain_conf.c | 40 
 +++---
  src/conf/domain_conf.h |  4 +--
  src/qemu/qemu_domain.c | 23 +++--
  src/qemu/qemu_domain.h |  4 +++
  src/qemu/qemu_driver.c |  1 +
  src/qemu/qemu_process.c| 23 -
  .../qemuxml2argv-hostdev-scsi-rawio.xml| 35 +++
  tests/qemuxml2xmltest.c|  1 +
  10 files changed, 132 insertions(+), 23 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml
 

ACK series

Jan



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

Re: [libvirt] [PATCH v2 4/5] qemu: Process the hostdev rawio setting

[Ján Tomko]

On 09/19/2014 12:17 PM, John Ferlan wrote:
 Mimic the Disk processing for 'rawio', but for a scsi_host hostdev
 lun device.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/qemu/qemu_domain.c  | 18 ++
  src/qemu/qemu_domain.h  |  4 
  src/qemu/qemu_driver.c  |  1 +
  src/qemu/qemu_process.c | 19 +++
  4 files changed, 42 insertions(+)
 

 @@ -4376,6 +4378,23 @@ int qemuProcessStart(virConnectPtr conn,
  goto cleanup;
  }
  
 +/* If rawio not already set, check hostdevs as well */
 +if (!rawio_set) {
 +for (i = 0; i  vm-def-nhostdevs; i++) {
 +virDomainHostdevSubsysSCSIPtr scsisrc =
 +vm-def-hostdevs[i]-source.subsys.u.scsi;
 +if (scsisrc-rawio == VIR_TRISTATE_BOOL_YES) {
 +#ifdef CAP_SYS_RAWIO
 +virCommandAllowCap(cmd, CAP_SYS_RAWIO);

You can break; here.

 +#else
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(Raw I/O is not supported on this 
 platform));
 +goto cleanup;
 +#endif
 +}
 +}
 +}
 +
  virCommandSetPreExecHook(cmd, qemuProcessHook, hookData);
  virCommandSetMaxProcesses(cmd, cfg-maxProcesses);
  virCommandSetMaxFiles(cmd, cfg-maxFiles);
 

Jan



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

Re: [libvirt] [PATCH v2 0/5] Add rawio for hostdev

[John Ferlan]

On 09/19/2014 07:38 AM, Ján Tomko wrote:
 On 09/19/2014 12:17 PM, John Ferlan wrote:
 v1 is here:
 http://www.redhat.com/archives/libvir-list/2014-September/msg00502.html

 Changes since v1 (from code review):

  * Insert patch 1 to insert a missing goto if rawio is set, but the
build doesn't suport CAP_SYS_RAWIO.
  * Insert patch 2 to use the TristateBool for virDomainDiskDef and
the associated changes where it's used in code
  * Patch 3, 4, 5: Use TristateBool for hostdev rawio and the virYesNo
in the rng

 John Ferlan (5):
   qemu: Add missing goto on rawio
   domain_conf: Change virDomainDiskDef 'rawio' to use virTristateBool
 
   hostdev: Add rawio attribute to _virDomainHostdevSubsysSCSI
 
 I think the above commit should be squashed with the domain_conf patch below.
 
   qemu: Process the hostdev rawio setting
 
 And this one moved to the end, since it depends on it.
 
   domain_conf: Process the rawio for a hostdev device

  docs/formatdomain.html.in  | 12 +--
  docs/schemas/domaincommon.rng  | 12 +--
  src/conf/domain_conf.c | 40 
 +++---
  src/conf/domain_conf.h |  4 +--
  src/qemu/qemu_domain.c | 23 +++--
  src/qemu/qemu_domain.h |  4 +++
  src/qemu/qemu_driver.c |  1 +
  src/qemu/qemu_process.c| 23 -
  .../qemuxml2argv-hostdev-scsi-rawio.xml| 35 +++
  tests/qemuxml2xmltest.c|  1 +
  10 files changed, 132 insertions(+), 23 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml

 
 ACK series
 
 Jan
 


I merged 35 and left 4 as the last one modifying it to add the break.
I went back and forth on which way to handle it...

Pushed - thanks for the review!

John

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

Re: [libvirt] [PATCH V3 2/3] qemu: Introduce vgamem attribute for video model

[Michal Privoznik]

On 14.08.2014 14:43, Wang Rui wrote:

From: Zeng Junliang zengjunli...@huawei.com

QEMU suopports to specifie the size of the framebuffer portion of
the ram region for vga, vmvga and qxl through the qemu command
line parameter vgamem_mb.
This patch introduces vgamem attribute for video model, makes the
vgamem_mb value configured in libvirt xml. Also, add test cases
and descriptions for it.

Libvirt xml configuration sample(based on VGA):
video
   model type='vga' vgamem='16384' heads='1'/
/video

The resulting qemu command line change is the addition of:
-vga std -global VGA.vgamem_mb=16
or
-device VGA,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2

If 'vgamem' is not configured by user, a default value will be assigned
by libvirt. Assume that user wants to start a VM without 'vgamem'
configuration in xml, if QEMU lacks the ability of vgamem, libvirt will
only report a warning rather than make starting failed even though libvirt
has assigned a default value to vgamem. It could avoid a regression.

It's less confusing to introduce the new vgamem attribute.
Prior to the change:
model   libvirt-attribute(xml)  qemu-attribute

qxl vramvram_size
qxl nonevgamem_mb
vga vramQEMU has no attribute named vram*
vga nonevgamem_mb
vmvga   vramQEMU has no attribute named vram*
vmvga   nonevgamem_mb

After the change:
model   libvirt attribute(xml)  QEMU attribute

qxl vramvram_size
qxl vgamem  vgamem_mb
vga vramQEMU has no attribute named vram*
vga vgamem  vgamem_mb
vmvga   vramQEMU has no attribute named vram*
vmvga   vgamem  vgamem_mb

Signed-off-by: Zeng Junliang zengjunli...@huawei.com
Signed-off-by: Wang Rui moon.wang...@huawei.com
---
  docs/formatdomain.html.in  |  35 ---
  docs/schemas/domaincommon.rng  |   5 +
  src/conf/domain_conf.c |  52 +-
  src/conf/domain_conf.h |   2 +
  src/libvirt_private.syms   |   1 +
  src/qemu/qemu_capabilities.c   |  15 +++
  src/qemu/qemu_capabilities.h   |   3 +
  src/qemu/qemu_command.c| 105 -
  tests/qemucapabilitiesdata/caps_1.2.2-1.caps   |   3 +
  tests/qemucapabilitiesdata/caps_1.3.1-1.caps   |   3 +
  tests/qemucapabilitiesdata/caps_1.4.2-1.caps   |   3 +
  tests/qemucapabilitiesdata/caps_1.5.3-1.caps   |   3 +
  tests/qemucapabilitiesdata/caps_1.6.0-1.caps   |   3 +
  tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |   3 +
  tests/qemuhelptest.c   |  10 +-
  .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml |   2 +-
  ...emuxml2argv-graphics-spice-agent-file-xfer.args |   5 +-
  ...qemuxml2argv-graphics-spice-agent-file-xfer.xml |   4 +-
  .../qemuxml2argv-graphics-spice-compression.args   |   4 +-
  .../qemuxml2argv-graphics-spice-compression.xml|   4 +-
  .../qemuxml2argv-graphics-spice-listen-network.xml |   4 +-
  .../qemuxml2argv-graphics-spice-qxl-vga.args   |   4 +-
  .../qemuxml2argv-graphics-spice-qxl-vga.xml|   4 +-
  .../qemuxml2argv-graphics-spice-sasl.args  |   3 +-
  .../qemuxml2argv-graphics-spice-sasl.xml   |   2 +-
  .../qemuxml2argv-graphics-spice-timeout.xml|   2 +-
  .../qemuxml2argv-graphics-spice.args   |   5 +-
  .../qemuxml2argv-graphics-spice.xml|   4 +-
  .../qemuxml2argv-graphics-vnc-std-vga.args |   4 +
  .../qemuxml2argv-graphics-vnc-std-vga.xml  |  36 +++
  .../qemuxml2argv-graphics-vnc-vmware-svga.args |   4 +
  .../qemuxml2argv-graphics-vnc-vmware-svga.xml  |  36 +++
  .../qemuxml2argv-net-bandwidth.xml |   2 +-
  .../qemuxml2argv-pcihole64-q35.args|   3 +-
  .../qemuxml2argv-pcihole64-q35.xml |   2 +-
  tests/qemuxml2argvdata/qemuxml2argv-q35.args   |   3 +-
  tests/qemuxml2argvdata/qemuxml2argv-q35.xml|   2 +-
  .../qemuxml2argv-serial-spiceport.args |   4 +-
  .../qemuxml2argv-serial-spiceport.xml  |   2 +-
  .../qemuxml2argv-video-device-pciaddr-default.args |   9 +-
  .../qemuxml2argv-video-device-pciaddr-default.xml  |   6 +-
  tests/qemuxml2argvtest.c   |  22 -
  .../qemuxml2xmlout-graphics-spice-timeout.xml  |   2 +-
  tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml|   2 +-
  tests/qemuxml2xmltest.c|   2 +
  45 files changed, 357 insertions(+), 77 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-std-vga.args
  create mode 

Re: [libvirt] [PATCH V3 3/3] qemu: Add secondary-vga support

[Michal Privoznik]

On 14.08.2014 14:43, Wang Rui wrote:

From: Zeng Junliang zengjunli...@huawei.com

Secondary-vga is supported by QEMU in currently master. Add it supported
in libvirt as qemu commandline show: '-device secondary-vga'. And it can
be used as secondary display device, like qxl. Also, add test cases and
descriptions for it.

Libvirt xml configuration sample:
video
   model type='secondary' vgamem='16384' heads='1'/
/video

The resulting qemu command line change is the addition of:
-device secondary-vga,id=video0,vgamem_mb=16,bus=pci.0,addr=0x5

Signed-off-by: Zeng Junliang zengjunli...@huawei.com
Signed-off-by: Wang Rui moon.wang...@huawei.com
---
  docs/formatdomain.html.in  | 13 +++---
  docs/schemas/domaincommon.rng  |  1 +
  src/conf/domain_conf.c |  9 ++--
  src/conf/domain_conf.h |  1 +
  src/qemu/qemu_capabilities.c   |  2 +
  src/qemu/qemu_capabilities.h   |  1 +
  src/qemu/qemu_command.c| 54 +++---
  .../qemuxml2argv-graphics-vnc-secondary-vga.args   |  7 +++
  .../qemuxml2argv-graphics-vnc-secondary-vga.xml| 39 
  tests/qemuxml2argvtest.c   |  4 ++
  tests/qemuxml2xmltest.c|  1 +
  11 files changed, 106 insertions(+), 26 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.args
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-secondary-vga.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index a0d15c4..b1d8ad4 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4440,9 +4440,9 @@ qemu-kvm -net nic,model=? /dev/null
  device in domain xml is the primary one, but the optional attribute
  codeprimary/code (span class=sincesince 1.0.2/span) with
  value 'yes' can be used to mark the primary in cases of multiple video
-device. The non-primary must be type of qxl. The optional attribute
-coderam/code (span class=sincesince 1.0.2/span) is allowed
-for qxl type only and specifies the size of the primary bar,
+device. The non-primary must be type of qxl or secondary. The
+optional attribute coderam/code (span class=sincesince 
1.0.2/span)
+is allowed for qxl type only and specifies the size of the primary 
bar,
  while codevram/code specifies the secondary bar size.
  If ram, vram or vgamem are not supplied a default value is used.
/dd
@@ -4451,8 +4451,9 @@ qemu-kvm -net nic,model=? /dev/null
dd
  The codemodel/code element has a mandatory codetype/code
  attribute which takes the value vga, cirrus, vmvga, xen,
-vbox, or qxl (span class=sincesince 0.8.6/span)
-depending on the hypervisor features available.
+vbox, qxl (span class=sincesince 0.8.6/span) or
+secondary (span class=sincesince 1.2.8/span) depending
+on the hypervisor features available.
  p
  codevram/code attribute specifies the amount of video memory
  in kibibytes (blocks of 1024 bytes). For type of kvm, it is only
@@ -4462,7 +4463,7 @@ qemu-kvm -net nic,model=? /dev/null
  codevgamem/code attribute span class=sincesince 1.2.8,
  QEMU and KVM only/span specifies the size of the framebuffer
  portion of the ram region. And it is only valid for type of
-vga, vmvga and qxl.
+vga, vmvga, qxl and secondary.
  /p
  p
  codeheads/code attribute specifies the number of screen.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b2cc218..db8dbde 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2768,6 +2768,7 @@
  valuevmvga/value
  valuexen/value
  valuevbox/value
+valuesecondary/value
/choice
  /attribute
  group
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7097570..c7b9a5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -490,7 +490,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
vmvga,
xen,
vbox,
-  qxl)
+  qxl,
+  secondary)

  VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST,
mouse,
@@ -9646,7 +9647,8 @@ virDomainVideoDefaultVgamem(int type)
  case VIR_DOMAIN_VIDEO_TYPE_VGA:
  case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
  case VIR_DOMAIN_VIDEO_TYPE_QXL:
-/* QEMU use 16M as default value for vga/vmvga/qxl device*/
+case VIR_DOMAIN_VIDEO_TYPE_SECONDARY:
+/* QEMU use 16M as default value for vga/vmvga/qxl/secondary device */
  return 16 * 1024;

  default:
@@ -9781,7 +9783,8 @@ 

Re: [libvirt] [PATCH V3 1/3] qemu: Hide vram attribute for some useless cases.

[Michal Privoznik]

On 14.08.2014 14:43, Wang Rui wrote:

From: Zeng Junliang zengjunli...@huawei.com

The vram attribute is never used for VGA, CIRRUS and VMVGA as
QEMU has no vram attribute for these models. Hide it for qemu
in qemuDomainDevicePostParse function. And update the
corresponding test cases and descriptions.

Signed-off-by: Zeng Junliang zengjunli...@huawei.com
Signed-off-by: Wang Rui moon.wang...@huawei.com
---
  docs/formatdomain.html.in  | 24 +++---
  src/qemu/qemu_command.c|  3 ++-
  src/qemu/qemu_domain.c | 12 +++
  ...qemuhotplug-console-compat-2+console-virtio.xml |  2 +-
  .../qemuxml2argv-console-compat-2.xml  |  2 +-
  .../qemuxml2argv-controller-order.xml  |  2 +-
  .../qemuxml2argv-graphics-listen-network.xml   |  2 +-
  .../qemuxml2argv-graphics-listen-network2.xml  |  2 +-
  .../qemuxml2argv-graphics-sdl-fullscreen.xml   |  2 +-
  .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml |  2 +-
  .../qemuxml2argv-graphics-spice-agentmouse.xml |  2 +-
  .../qemuxml2argv-graphics-spice-timeout.xml|  2 +-
  .../qemuxml2argv-graphics-vnc-policy.xml   |  2 +-
  .../qemuxml2argv-graphics-vnc-sasl.xml |  2 +-
  .../qemuxml2argv-graphics-vnc-socket.xml   |  2 +-
  .../qemuxml2argv-graphics-vnc-tls.xml  |  2 +-
  .../qemuxml2argv-graphics-vnc-websocket.xml|  2 +-
  .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml |  2 +-
  .../qemuxml2argv-net-bandwidth.xml |  2 +-
  .../qemuxml2argv-pci-autoadd-addr.xml  |  2 +-
  .../qemuxml2argv-pci-autoadd-idx.xml   |  2 +-
  tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml |  2 +-
  .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
  .../qemuxml2xmlout-graphics-listen-network2.xml|  2 +-
  .../qemuxml2xmlout-graphics-spice-timeout.xml  |  2 +-
  .../qemuxml2xmlout-pci-autoadd-addr.xml|  2 +-
  .../qemuxml2xmlout-pci-autoadd-idx.xml |  2 +-
  27 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index bd99ae0..3012e3c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4419,7 +4419,7 @@ qemu-kvm -net nic,model=? /dev/null
...
lt;devicesgt;
  lt;videogt;
-  lt;model type='vga' vram='8192' heads='1'gt;
+  lt;model type='vga' heads='1'gt;
  lt;acceleration accel3d='yes' accel2d='yes'/gt;
lt;/modelgt;
  lt;/videogt;
@@ -4434,17 +4434,16 @@ qemu-kvm -net nic,model=? /dev/null
  is set but there is a codegraphics/code in domain xml, then 
libvirt
  will add a default codevideo/code according to the guest type.
  For a guest of type kvm, the default codevideo/code for it is:
-codetype/code with value cirrus, codevram/code with value
-9216, and codeheads/code with value 1. By default, the first
-video device in domain xml is the primary one, but the optional
-attribute codeprimary/code (span class=sincesince 1.0.2/span)
-with value 'yes' can be used to mark the primary in cases of multiple
-video device. The non-primary must be type of qxl. The optional
-attribute coderam/code (span class=sincesince
-1.0.2/span) is allowed for qxl type only and specifies
-the size of the primary bar, while codevram/code specifies the
-secondary bar size.  If ram or vram are not supplied a default
-value is used.
+codetype/code with value cirrus, and codeheads/code with
+value 1. By default, the first video device in domain xml is the
+primary one, but the optional attribute codeprimary/code
+(span class=sincesince 1.0.2/span) with value 'yes' can be
+used to mark the primary in cases of multiple video device.
+The non-primary must be type of qxl. The optional attribute
+coderam/code (span class=sincesince 1.0.2/span) is allowed
+for qxl type only and specifies the size of the primary bar,
+while codevram/code specifies the secondary bar size.
+If ram or vram are not supplied a default value is used.
/dd



NACK to these two chunks! The fact that qemu doesn't support setting 
vram (or maybe it does meanwhile, but we're missing implementation) 
doesn't mean it should be removed from the XML - moreover, some other 
drivers support this setting (e.g. vmx and virtualbox). We must keep it 
there otherwise vmx users won't like us anymore.




dtcodemodel/code/dt
@@ -4456,6 +4455,7 @@ qemu-kvm -net nic,model=? /dev/null
  You can also provide the amount of video memory in kibibytes
  (blocks of 1024 bytes) using
  codevram/code and the number of screen with codeheads/code.
+For type of kvm codevram/code attribute is only valid for qxl.


True. This is a good note 

Re: [libvirt] [PATCH v1 04/10] locking: Add virLockSeclabelProtocol

[Ján Tomko]

On 09/10/2014 03:26 PM, Michal Privoznik wrote:
 So far no ConnectOpen() is introduced as it's not needed for such
 simple use case like this. It's crucial to separate this from
 virLockSpace program that already exists. Not only it requires
 virDomainObjPtr for its ConnectOpen() (subsequently all security
 drivers would need rework as they use virDomainDefPtr), but from
 nature of things it doesn't belong there either. virLockSpace handles
 disk locking, not labeling and it's not clean to pollute its
 namespace anyway.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  .gitignore   |  2 ++
  src/Makefile.am  | 27 ++
  src/lock_seclabel_protocol-structs   | 21 ++
  src/locking/lock_seclabel_protocol.x | 53 
 
  4 files changed, 98 insertions(+), 5 deletions(-)
  create mode 100644 src/lock_seclabel_protocol-structs
  create mode 100644 src/locking/lock_seclabel_protocol.x
 

 @@ -387,7 +397,8 @@ EXTRA_DIST +=  $(REMOTE_DRIVER_PROTOCOL) \
  # The alternation of the following regexps matches both cases.
  r1 = /\* \d+ \*/
  r2 = /\* [[:xdigit:]]+ \S+:\d+ \*/
 -struct_prefix = (remote_|qemu_|lxc_|keepalive|vir(Net|LockSpace|LXCMonitor))
 +struct_prefix1 = (remote_|qemu_|lxc_|keepalive)
 +struct_prefix2 = vir(Net|LockSpace|LockSeclabel|LXCMonitor)
  
  # Depending on configure options, libtool creates one or both of
  # remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o.  We want
 @@ -406,7 +417,8 @@ PDWTAGS = \
 else  \
   $(PERL) -0777 -n\
   -e 'foreach my $$p (split m!\n*(?:$(r1)|$(r2))\n!) {'   \
 - -e '  if ($$p =~ /^(struct|enum) $(struct_prefix)/ ||'  \
 + -e '  if ($$p =~ /^(struct|enum) $(struct_prefix1)/ ||' \
 + -e '  $$P =~ /^(struct|enum) $(struct_prefix2)/ ||' \

This p should be lowercase.

Jan



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

Re: [libvirt] [PATCH] libxl: Implement basic video device selection

[Stefan Bader]

On 19.09.2014 05:01, Jim Fehlig wrote:
 Stefan Bader wrote:
 Re-pushing this as the old thread got rather stale.
 
 Thanks.
 
  Some of the
 VFB setup went in a bug fix. Not sure I missed a detail in rebasing
 bug the keyboard setting may be the only thing missing...
   
 
 Yes, agreed.
 
 -Stefan

 [v2: Check return code of VIR_STRDUP and fix indentation]
 [v3: Split out VRAM fixup and return error for unsupported video type]
 [v4: Re-arrange code and move VFB setup into libxlMakeVfbList]
 [v5: Rebased against head which already had some VFB setup code]

 From b3ff8f4c658d29f15e673af88b9ae2fdfa3c1317 Mon Sep 17 00:00:00 2001
 From: Stefan Bader stefan.ba...@canonical.com
 Date: Thu, 27 Mar 2014 16:01:18 +0100
 Subject: [PATCH] libxl: Implement basic video device selection

 This started as an investigation into an issue where libvirt (using the
 libxl driver) and the Xen host, like an old couple, could not agree on
 who is responsible for selecting the VNC port to use.

 Things usually (and a bit surprisingly) did work because, just like that
 old couple, they had the same idea on what to do by default. However it
 was possible that this ended up in a big argument.

 The problem is that display information exists in two different places:
 in the vfbs list and in the build info. And for launching the device model,
 only the latter is used. But that never gets initialized from libvirt. So
 Xen allows the device model to select a default port while libvirt thinks
 it has told Xen that this is done by libvirt (though the vfbs config).

 While fixing that, I made a stab at actually evaluating the configuration
 of the video device. So that it is now possible to at least decide between
 a Cirrus or standard VGA emulation and to modify the VRAM within certain
 limits using libvirt.

 Signed-off-by: Stefan Bader stefan.ba...@canonical.com
 ---
  src/libxl/libxl_conf.c | 50 
 ++
  1 file changed, 50 insertions(+)
   
 
 This patch suffers the same issues as the last version.  And when
 commenting on that version, I promised to work on a followup to address
 my concerns
 
 https://www.redhat.com/archives/libvir-list/2014-July/msg00931.html

Oh my, I have to admit I completely forgot about that.

 
 Your repost poked me into reworking my first attempt, the result of
 which is below.  I should probably look at a sensible split-up of these
 patches that would be easier to review, but in the meantime comments on
 my followup would be appreciated.
 
 With both patches, my tests are passing and my concerns are subdued :-).

There seem to be some parts of code suggesting support for anything beside Xen
(assumed to be alias to VGA) and Cirrus. As much as I know Xen handles only the
two (not qxl or anything else).

I believe the xen specific qemu variant was the only one called qemu-dm (and the
upstream variant always being qemu-system-i386). But checking the help message
probably is safer and not called that often so I should not worry about
performance).

I tried the variant you proposed and it looks to be ok. Still have to carry a
piece which is not going upstream if I wan't to paper over the virt-manager
issue. But at least now the failure is clearly showing what is wrong. So I am
happy enough. :)

-Stefan
 
 Regards,
 Jim
  
 




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

Re: [libvirt] Adding multiple graphics devices to a vm

[bancfc]

Thanks that was it. Apparently Debian includes the package 
xserver-xorg-video-qxl but doesn't have it setup/installed out of the 
box. After I did this, I was able to get high resolutions with SPICE.


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

[libvirt] Plan for next release

[Daniel Veillard]

 If we want to push 1.2.9 around Oct 1st, I suggest to be prepared
to enter freeze on the 25th, so there is a small week to push patches
if you want features in the freeze,

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [PATCH] libxl: Drop driver lock in libxlDomainDefineXML

[Jim Fehlig]

Michal Privoznik wrote:
 On 18.09.2014 23:17, Jim Fehlig wrote:
 There is no need to acquire the driver-wide lock in
 libxlDomainDefineXML.  When switching to jobs in the libxl
 driver, most driver-wide locks were removed.  The locking here
 was preserved since I mistakenly thought virDomainObjListAdd
 needed protection.  This is not the case, so remove the
 unnecessary locking.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
   src/libxl/libxl_driver.c | 13 +++--
   1 file changed, 3 insertions(+), 10 deletions(-)
[...]

 ACK

Thanks; pushed.

Regards,
Jim

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

Re: [libvirt] [PATCH v1 01/10] locking: Allow seclabel remembering

[Daniel P. Berrange]

On Wed, Sep 10, 2014 at 03:26:07PM +0200, Michal Privoznik wrote:
 To keep original seclabel for files libvirt is touching we need a
 single point where the original seclabels can be stored. Instead
 of inventing a new one we can misuse virtlockd which already has
 nearly all the infrastructure we need. As nice feature, it keeps
 its internal state between virtlockd restarts. Again, it's
 something we are going to need, as we don't want to lose the
 original labels on the lock daemon restart.
 
 In this commit two functions are introduced:
 
 virLockManagerRememberSeclabel that takes three arguments:
 
 path, model and seclabel
 
 where @path is unique identifier for the file we are about to
 label, @model and @seclabel then represents original seclabel.
 
 virLockManagerRecallSeclabel then takes:
 
 path, model, *seclabel
 
 and returns number of references held on @path. If the return
 value is zero, *seclabel contains the original label stored by
 first call of RememberSeclabel(). If a positive value is
 returned, other domains are still using the @path and the
 original label shall not be restored.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/libvirt_private.syms   |  2 ++
  src/locking/lock_driver.h  | 41 +
  src/locking/lock_manager.c | 26 ++
  src/locking/lock_manager.h |  9 +
  4 files changed, 78 insertions(+)

 diff --git a/src/locking/lock_manager.h b/src/locking/lock_manager.h
 index 4189759..3fd5b9a 100644
 --- a/src/locking/lock_manager.h
 +++ b/src/locking/lock_manager.h
 @@ -67,4 +67,13 @@ int virLockManagerInquire(virLockManagerPtr manager,
  
  int virLockManagerFree(virLockManagerPtr manager);
  
 +int virLockManagerRememberSeclabel(virLockManagerPtr lock,
 +   const char *path,
 +   const char *model,
 +   const char *seclabel);
 +int virLockManagerRecallSeclabel(virLockManagerPtr lock,
 + const char *path,
 + const char *model,
 + char **seclabel);

Can add ATTRIBUTE_NONNULL for all of the args in these methods.


ACK if that's changed.

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 v1 02/10] locking: Implement seclabel stubs for NOP

[Daniel P. Berrange]

On Wed, Sep 10, 2014 at 03:26:08PM +0200, Michal Privoznik wrote:
 The NOP drivers needs implementation, although the function
 bodies are empty. This is because the virLockManager APIs check
 for driver implementation and returns an error if there's none.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/locking/lock_driver_nop.c | 22 ++
  1 file changed, 22 insertions(+)

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 v1 01/10] locking: Allow seclabel remembering

[Daniel P. Berrange]

On Wed, Sep 10, 2014 at 03:26:07PM +0200, Michal Privoznik wrote:
 To keep original seclabel for files libvirt is touching we need a
 single point where the original seclabels can be stored. Instead
 of inventing a new one we can misuse virtlockd which already has
 nearly all the infrastructure we need. As nice feature, it keeps
 its internal state between virtlockd restarts. Again, it's
 something we are going to need, as we don't want to lose the
 original labels on the lock daemon restart.
 
 In this commit two functions are introduced:
 
 virLockManagerRememberSeclabel that takes three arguments:
 
 path, model and seclabel
 
 where @path is unique identifier for the file we are about to
 label, @model and @seclabel then represents original seclabel.
 
 virLockManagerRecallSeclabel then takes:
 
 path, model, *seclabel
 
 and returns number of references held on @path. If the return
 value is zero, *seclabel contains the original label stored by
 first call of RememberSeclabel(). If a positive value is
 returned, other domains are still using the @path and the
 original label shall not be restored.

 +int virLockManagerRememberSeclabel(virLockManagerPtr lock,
 +   const char *path,
 +   const char *model,
 +   const char *seclabel)
 +{
 +VIR_DEBUG(lock=%p path=%s model=%s seclabel=%s,
 +  lock, path, model, seclabel);
 +
 +CHECK_MANAGER(drvRemember, -1);
 +
 +return lock-driver-drvRemember(lock, path, model, seclabel);
 +}
 +
 +int virLockManagerRecallSeclabel(virLockManagerPtr lock,
 + const char *path,
 + const char *model,
 + char **seclabel)
 +{
 +VIR_DEBUG(lock=%p path=%s model=%s seclabel=%p,
 +  lock, path, model, seclabel);
 +
 +CHECK_MANAGER(drvRecall, -1);

I thin kwe should do

  *seclabel = NULL;

to protect against drivers forgetting todo it

 +
 +return lock-driver-drvRecall(lock, path, model, seclabel);
 +}

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 v1 03/10] domain_lock: Introduce seclabel APIs

[Daniel P. Berrange]

On Wed, Sep 10, 2014 at 03:26:09PM +0200, Michal Privoznik wrote:
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/libvirt_private.syms|  2 ++
  src/locking/domain_lock.c   | 65 
 +
  src/locking/domain_lock.h   | 10 +++
  src/locking/lock_driver.h   |  2 ++
  src/locking/lock_driver_lockd.c |  4 +++
  5 files changed, 83 insertions(+)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index cdc476a..db65aa5 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -874,6 +874,8 @@ virDomainLockProcessInquire;
  virDomainLockProcessPause;
  virDomainLockProcessResume;
  virDomainLockProcessStart;
 +virDomainLockRecallSeclabel;
 +virDomainLockRememberSeclabel;
  
  
  # locking/lock_manager.h
 diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c
 index d7b681e..7de56b3 100644
 --- a/src/locking/domain_lock.c
 +++ b/src/locking/domain_lock.c
 @@ -164,6 +164,30 @@ static virLockManagerPtr 
 virDomainLockManagerNew(virLockManagerPluginPtr plugin,
  }
  
  
 +static virLockManagerPtr
 +virDomainLockManagerSeclabelNew(virLockManagerPluginPtr plugin)
 +{
 +virLockManagerPtr lock;
 +virLockManagerParam params[] = {
 +/* nada */
 +};

Thinking ahead to the time when we have to persist the lock information
to disk, shared between multiple virtlockds, we might want to take some
parameters here. Specifically a hostname and/or host uuid ?  Though I
guess we could possibly wait until that time - depends on impact on the
RPC protocol in later patches, so will comment there.

 +VIR_DEBUG(plugin=%p, plugin);
 +
 +if (!(lock = virLockManagerNew(virLockManagerPluginGetDriver(plugin),
 +   VIR_LOCK_MANAGER_OBJECT_TYPE_SECLABEL,
 +   ARRAY_CARDINALITY(params),
 +   params,
 +   0)))
 +goto error;
 +
 +return lock;
 +
 + error:
 +virLockManagerFree(lock);
 +return NULL;
 +}

Tentative 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 v1 04/10] locking: Add virLockSeclabelProtocol

[Daniel P. Berrange]

On Wed, Sep 10, 2014 at 03:26:10PM +0200, Michal Privoznik wrote:
 So far no ConnectOpen() is introduced as it's not needed for such
 simple use case like this. It's crucial to separate this from
 virLockSpace program that already exists. Not only it requires
 virDomainObjPtr for its ConnectOpen() (subsequently all security
 drivers would need rework as they use virDomainDefPtr), but from
 nature of things it doesn't belong there either. virLockSpace handles
 disk locking, not labeling and it's not clean to pollute its
 namespace anyway.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  .gitignore   |  2 ++
  src/Makefile.am  | 27 ++
  src/lock_seclabel_protocol-structs   | 21 ++
  src/locking/lock_seclabel_protocol.x | 53 
 
  4 files changed, 98 insertions(+), 5 deletions(-)
  create mode 100644 src/lock_seclabel_protocol-structs
  create mode 100644 src/locking/lock_seclabel_protocol.x



 diff --git a/src/lock_seclabel_protocol-structs 
 b/src/lock_seclabel_protocol-structs
 new file mode 100644
 index 000..46f1eae
 --- /dev/null
 +++ b/src/lock_seclabel_protocol-structs
 @@ -0,0 +1,21 @@
 +/* -*- c -*- */
 +struct virLockSeclabelProtocolRememberSeclabelArgs {
 +virLockSeclabelProtocolNonNullString path;
 +virLockSeclabelProtocolNonNullString model;
 +virLockSeclabelProtocolNonNullString seclabel;
 +};
 +struct virLockSeclabelProtocolRememberSeclabelRet {
 +intret;

What are the values of the 'ret' variable. Generally the
RPC methods deal with error status at the protocol header
level ?

 +};
 +struct virLockSeclabelProtocolRecallSeclabelArgs {
 +virLockSeclabelProtocolNonNullString path;
 +virLockSeclabelProtocolNonNullString model;
 +};
 +struct virLockSeclabelProtocolRecallSeclabelRet {
 +virLockSeclabelProtocolNonNullString seclabel;
 +intret;
 +};
 +enum virLockSeclabelProtocolProcedure {
 +VIR_LOCK_SECLABEL_PROTOCOL_PROC_REMEMBER_SECLABEL = 1,
 +VIR_LOCK_SECLABEL_PROTOCOL_PROC_RECALL_SECLABEL = 2,
 +};

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 v1 06/10] lock_daemon: Implement server dispatch

[Daniel P. Berrange]

On Wed, Sep 10, 2014 at 03:26:12PM +0200, Michal Privoznik wrote:
 And register the program, finally. Although, only stub
 implementation of the Remember() and Recall() functions
 is added. The real implementation will follow.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/locking/lock_daemon.c  | 37 ++
  src/locking/lock_daemon.h  |  8 
  src/locking/lock_daemon_dispatch.c | 77 
 ++
  src/locking/lock_daemon_dispatch.h |  3 ++
  4 files changed, 125 insertions(+)

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 v1 05/10] driver_lockd: Implement seclabel APIs

[Daniel P. Berrange]

On Wed, Sep 10, 2014 at 03:26:11PM +0200, Michal Privoznik wrote:
 This is the client side, so there's nothing more we need to do
 than call the RPC. Although, there's one interesting change: new
 virLockManagerLockSeclabelConnect() had to be implemented since the
 VIR_LOCK_SPACE_PROTOCOL_PROGRAM doesn't have any ConnectOpen()
 procedure.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/locking/lock_driver_lockd.c | 114 
 ++--
  1 file changed, 109 insertions(+), 5 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 v1 07/10] lock_daemon: Implement seclabel APIs

[Daniel P. Berrange]

On Wed, Sep 10, 2014 at 03:26:13PM +0200, Michal Privoznik wrote:
 The most interesting part where all the hard work takes place.
 
 For storing triplets of strings a two dimensional table would be
 the most intuitive approach. Or, if the triplet is rearranged
 into pair of one string and a pair, a hash table can be used. But
 hey, we already have those! Excellent. In other words,
 
 path, model, seclabel
 
 is turned into:
 
 path, model, seclabel
 
 The @path is then the key that the hash table is filled with. The
 inner pair is turned into linked list (hash table in a hash table
 would be awful really). Moreover, we need to store yet another
 item: reference counter.

I wonder, since 'model' is mandatory and (path, model) should
be unique, could we not avoid the linked list complexity by
using a string of 'model:path' as the hash key, then we only
have a single hash item too and O(1) access to values ?


  int
 -virLockDaemonRememberSeclabel(virLockDaemonPtr lockd ATTRIBUTE_UNUSED,
 -  const char *path ATTRIBUTE_UNUSED,
 -  const char *model ATTRIBUTE_UNUSED,
 -  const char *seclabel ATTRIBUTE_UNUSED)
 +virLockDaemonRememberSeclabel(virLockDaemonPtr lockd,
 +  const char *path,
 +  const char *model,
 +  const char *seclabel)
  {
 -/* Implement me */
 -return -1;
 +int ret = -1;
 +virSeclabelListPtr list = NULL;
 +virSeclabelListPtr tmp = NULL;
 +bool list_created = false;
 +
 +virMutexLock(lockd-lock);
 +
 +if (!(list = virHashLookup(lockd-seclabels, path))) {
 +if (VIR_ALLOC(list)  0)
 +goto cleanup;
 +
 +if (virHashAddEntry(lockd-seclabels, path, list)  0)
 +goto cleanup;
 +
 +list_created = true;
 +}
 +
 +tmp = list;
 +if (!(tmp = virSeclabelListFind(list, model))) {
 +/* seclabel for @model doesn't exit yet */
 +if (!(tmp = virSeclabelListAdd(list, model, seclabel)))
 +goto cleanup;
 +}
 +
 +/* Update refcount */
 +ret = ++tmp-item-refcount;
 +
 + cleanup:
 +virMutexUnlock(lockd-lock);
 +if (ret  0  list_created) {
 +virHashRemoveEntry(lockd-seclabels, path);
 +virSeclabelListFree(list);
 +}
 +return ret;
  }

What concerns me a little is that we don't have any association between
the label, use count and the running VMs that triggered the need for the
relabelling.

Lets say that a virtualization host with VMs running crashes. The libvirtd
and virtlocked on that host will loose all state. That's fine currently
since you're not sharing the data across hosts. We're going to need to
share the data though, so when we do that sharing, I think we're going
to need to remember information about the VMs + hosts associated with
the label, so that when a host crashes, we can have a chance of purging
the now stale information.

Now, as discussed on IRC it isn't critical to support multi-host mode
right away, but this does impact on the RPC protocol, since the messages
we are passing do not include any information about the VM or host.

So I'm thinking that this series should record the VM name + uuid and
the host name + uuid against the seclabels and plumb that through the
RPC protocol at least.

If we did that now, I think we might not even need to add new RPC
protocol for lockd, and instead just re-use the existing protocol
messages and at least some of the client/dispatcher code too.
It would be more like a case of just setting up a new lockspace
for recording info about labels, instead of a completely new set
of infrastructure side-by-side.

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

[libvirt] [PATCH 1/1] lxc: allow fallback to no apparmor.

[Serge Hallyn]

The security_driver line in /etc/libvirt/qemu.conf is best-effort - if
selinux is not available on the host, then 'none' will be used.

The security_driver line in /etc/libvirt/lxc.conf doesn't behave the
same way - if apparmor is specified but policies are not available
on the host, then container creation fails.

This patch always tries to fall back to 'none' if the requested
driver is not available.  A better patch would allow an option list
like qemu.conf allows, but this patch doesn't do that.

Signed-off-by: Serge Hallyn serge.hal...@ubuntu.com
---
 src/lxc/lxc_driver.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c3cd62c..233e558 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1541,6 +1541,11 @@ lxcSecurityInit(virLXCDriverConfigPtr cfg)
   
cfg-securityDefaultConfined,
   
cfg-securityRequireConfined);
 if (!mgr)
+mgr = virSecurityManagerNew(NULL, LXC_DRIVER_NAME, false,
+  
cfg-securityDefaultConfined,
+  
cfg-securityRequireConfined);
+
+if (!mgr)
 goto error;
 
 return mgr;
-- 
2.1.0

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

Re: [libvirt] [PATCHv2] Renamed internal __mon_yday into mon_yday to avoid conflicts

[Martin Kletzander]

On Fri, Sep 19, 2014 at 08:49:55AM +0200, Cédric Bosdonnat wrote:

libc has another constant with the same name, which leads to
redefinition error when building against static libvirt
---
src/util/virtime.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)



ACK,

Martin


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

[libvirt] [PATCH 2/5] Xen: Defer setting default vram value to Xen drivers

[Jim Fehlig]

Allow the Xen drivers to determine default vram values.  Sane
default vaules depend on the device model being used, so the
drivers are in the best position to determine the defaults.

For the legacy xen driver, it is best to maintain the existing
logic for setting default vram values to ensure there are no
regressions.  The libxl driver currently does not support
configuring a video device.  Support will be added in a
subsequent patch, where the benefit of this change will be
reaped.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/conf/domain_conf.c |  4 
 src/xen/xen_driver.c   | 19 +++
 2 files changed, 23 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bb4a4cb..1de2650 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9696,6 +9696,10 @@ int
 virDomainVideoDefaultRAM(const virDomainDef *def,
  int type)
 {
+/* Defer setting default vram to the Xen drivers */
+if (def-virtType == VIR_DOMAIN_VIRT_XEN)
+return 0;
+
 switch (type) {
 /* Weird, QEMU defaults to 9 MB ??! */
 case VIR_DOMAIN_VIDEO_TYPE_VGA:
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 11ae8f9..7438ebb 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -353,6 +353,25 @@ xenDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 return -1;
 }
 
+if (dev-type == VIR_DOMAIN_DEVICE_VIDEO  dev-data.video-vram == 0) {
+switch (dev-data.video-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
+case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
+dev-data.video-vram = 9 * 1024;
+break;
+
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+/* Original Xen PVFB hardcoded to 4 MB */
+dev-data.video-vram = 4 * 1024;
+break;
+
+case VIR_DOMAIN_VIDEO_TYPE_QXL:
+/* Use 64M as the minimal video video memory for qxl device */
+return 64 * 1024;
+}
+}
+
 return 0;
 }
 
-- 
1.8.4.5

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

[libvirt] [PATCH 1/5] libxl: Copy user-specified keymap to libxl build info struct

[Jim Fehlig]

Commit 4dfc34c3 missed copying the user-specified keymap to
libxl_domain_build_info struct when creating a VFB device.

Signed-off-by: Stefan Bader stefan.ba...@canonical.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_conf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index acba69c..0a781f9 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1200,6 +1200,9 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports,
 if (VIR_STRDUP(b_info-u.hvm.sdl.xauthority, vfb.sdl.xauthority)  
0)
 goto error;
 }
+
+if (VIR_STRDUP(b_info-u.hvm.keymap, vfb.keymap)  0)
+goto error;
 }
 
 return 0;
-- 
1.8.4.5

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

[libvirt] [PATCH 4/5] libxl: Implement basic video device selection

[Jim Fehlig]

From: Stefan Bader stefan.ba...@canonical.com

This started as an investigation into an issue where libvirt (using the
libxl driver) and the Xen host, like an old couple, could not agree on
who is responsible for selecting the VNC port to use.

Things usually (and a bit surprisingly) did work because, just like that
old couple, they had the same idea on what to do by default. However it
was possible that this ended up in a big argument.

The problem is that display information exists in two different places:
in the vfbs list and in the build info. And for launching the device model,
only the latter is used. But that never gets initialized from libvirt. So
Xen allows the device model to select a default port while libvirt thinks
it has told Xen that this is done by libvirt (though the vfbs config).

While fixing that, I made a stab at actually evaluating the configuration
of the video device. So that it is now possible to at least decide between
a Cirrus or standard VGA emulation and to modify the VRAM within certain
limits using libvirt.

Signed-off-by: Stefan Bader stefan.ba...@canonical.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_conf.c   | 74 
 src/libxl/libxl_domain.c | 22 ++
 2 files changed, 96 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index ff3f6b5..09211f8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1433,6 +1433,72 @@ libxlMakePCIList(virDomainDefPtr def, 
libxl_domain_config *d_config)
 return -1;
 }
 
+static int
+libxlMakeVideo(virDomainDefPtr def, libxl_domain_config *d_config)
+
+{
+libxl_domain_build_info *b_info = d_config-b_info;
+int dm_type = libxlDomainGetEmulatorType(def);
+
+if (d_config-c_info.type != LIBXL_DOMAIN_TYPE_HVM)
+return 0;
+
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ */
+if (def-nvideos) {
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+if (def-videos[0]-vram  16 * 1024) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(videoram must be at least 16MB for 
VGA));
+return -1;
+}
+} else {
+if (def-videos[0]-vram  8 * 1024) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(videoram must be at least 8MB for VGA));
+return -1;
+}
+}
+break;
+
+case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
+if (def-videos[0]-vram  8 * 1024) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(videoram must be at least 8MB for 
CIRRUS));
+return -1;
+}
+} else {
+if (def-videos[0]-vram  4 * 1024) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(videoram must be at least 4MB for 
CIRRUS));
+return -1;
+}
+}
+break;
+
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(video type %s is not supported by libxl),
+   virDomainVideoTypeToString(def-videos[0]-type));
+return -1;
+}
+/* vram validated for each video type, now set it */
+b_info-video_memkb = def-videos[0]-vram;
+} else {
+libxl_defbool_set(b_info-u.hvm.nographic, 1);
+}
+
+return 0;
+}
+
 int
 libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info)
 {
@@ -1523,6 +1589,14 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
 if (libxlMakePCIList(def, d_config)  0)
 return -1;
 
+/*
+ * Now that any potential VFBs are defined, update the build info with
+ * the data of the primary display. Some day libxl might implicitely do
+ * so but as it does not right now, better be explicit.
+ */
+if (libxlMakeVideo(def, d_config)  0)
+return -1;
+
 d_config-on_reboot = libxlActionFromVirLifecycle(def-onReboot);
 d_config-on_poweroff = libxlActionFromVirLifecycle(def-onPoweroff);
 d_config-on_crash = libxlActionFromVirLifecycleCrash(def-onCrash);
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 557fc20..f2cd07b 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -510,6 +510,28 @@ 

[libvirt] [PATCH 0/5] libxl: user-specified domain config improvements

[Jim Fehlig]

This is essentially a V2 of Stefan's patch to add support
for user-specified video device in the libxl driver.

https://www.redhat.com/archives/libvir-list/2014-September/msg01157.html

It goes a bit further and adds support for user-specfied emulator,
which was trivial to add after introducing libxlDomainGetEmulatorType
in patch 3.  To ease review, the series has been split up as follows:

- Missed keymap config from commit 4dfc34c3 split out to patch 1
- Patch 2 added to defer setting default vram value to the Xen drivers
- Patch 3 added to detect old qemu-dm vs qemu with xen support
- Patch 4 is a slightly reworked version of Stefan's patch
- Patch 5 added to support user-specified emulator

Jim Fehlig (4):
  libxl: Copy user-specified keymap to libxl build info struct
  Xen: Defer setting default vram value to Xen drivers
  libxl: Add function to determine device model type
  libxl: Support user-specified emulator

Stefan Bader (1):
  libxl: Implement basic video device selection

 src/conf/domain_conf.c   |   4 ++
 src/libxl/libxl_conf.c   | 124 +++
 src/libxl/libxl_conf.h   |   3 ++
 src/libxl/libxl_domain.c |  22 +
 src/xen/xen_driver.c |  19 
 5 files changed, 172 insertions(+)

-- 
1.8.4.5

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

[libvirt] [PATCH 3/5] libxl: Add function to determine device model type

[Jim Fehlig]

This patch introduces a function to detect whether the specified
emulator is QEMU_XEN or QEMU_XEN_TRADITIONAL.  Detection is based on the
string Options specific to the Xen version: in '$qemu -help' output.
AFAIK, the only qemu containing that string in help output is the
old Xen fork (aka qemu-dm).

Note:
QEMU_XEN means a qemu that contains support for Xen.

QEMU_XEN_TRADITIONAL means Xen's old forked qemu 0.10.2

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_conf.c | 32 
 src/libxl/libxl_conf.h |  3 +++
 2 files changed, 35 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 0a781f9..ff3f6b5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -40,6 +40,7 @@
 #include viralloc.h
 #include viruuid.h
 #include capabilities.h
+#include vircommand.h
 #include libxl_domain.h
 #include libxl_conf.h
 #include libxl_utils.h
@@ -796,6 +797,37 @@ libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
 }
 
 
+#define LIBXL_QEMU_DM_STR  Options specific to the Xen version:
+
+int
+libxlDomainGetEmulatorType(const virDomainDef *def)
+{
+int ret = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+virCommandPtr cmd = NULL;
+char *output = NULL;
+
+if (STREQ(def-os.type, hvm)) {
+if (def-emulator) {
+cmd = virCommandNew(def-emulator);
+
+virCommandAddArgList(cmd, -help, NULL);
+virCommandSetOutputBuffer(cmd, output);
+
+if (virCommandRun(cmd, NULL)  0)
+goto cleanup;
+
+if (strstr(output, LIBXL_QEMU_DM_STR))
+ret = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+}
+}
+
+ cleanup:
+VIR_FREE(output);
+virCommandFree(cmd);
+return ret;
+}
+
+
 int
 libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
 {
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index da66b4e..25f77ea 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -163,6 +163,9 @@ virCapsPtr
 libxlMakeCapabilities(libxl_ctx *ctx);
 
 int
+libxlDomainGetEmulatorType(const virDomainDef *def);
+
+int
 libxlMakeDisk(virDomainDiskDefPtr l_dev, libxl_device_disk *x_dev);
 int
 libxlMakeNic(virDomainDefPtr def,
-- 
1.8.4.5

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

[libvirt] [PATCH 5/5] libxl: Support user-specified emulator

[Jim Fehlig]

With the introduction of the libxlDomainGetEmulatorType function,
it is trivial to support a user-specfied emulator in the libxl
driver.  This patch is based loosely on David Scott's old patch
to do the same

https://www.redhat.com/archives/libvir-list/2013-April/msg02119.html

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
 src/libxl/libxl_conf.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 09211f8..882dcff 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -705,6 +705,21 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
 if (VIR_STRDUP(b_info-u.hvm.boot, bootorder)  0)
 goto error;
 
+if (def-emulator) {
+if (!virFileExists(def-emulator)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(emulator '%s' not found),
+   def-emulator);
+goto error;
+}
+
+VIR_FREE(b_info-device_model);
+if (VIR_STRDUP(b_info-device_model, def-emulator)  0)
+goto error;
+
+b_info-device_model_version = libxlDomainGetEmulatorType(def);
+}
+
 if (def-nserials) {
 if (def-nserials  1) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-- 
1.8.4.5

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

Re: [libvirt] [PATCH] libvirt-guests: wait for ntp service

[Jim Fehlig]

Michal Privoznik wrote:
 On 08.09.2014 18:30, Jim Fehlig wrote:
 If an NTP server is configured on the host, it is possible for
 libvirt-guests to start before the NTP service, in which case
 guest clocks won't be synchronized to the host clock.

 Add ntp-wait.service to After in libvirt-guests systemd service
 file, ensuring NTP has synchronized the host clock before starting
 any guests.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
   tools/libvirt-guests.service.in | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/tools/libvirt-guests.service.in
 b/tools/libvirt-guests.service.in
 index d8d7adf..226b3bd 100644
 --- a/tools/libvirt-guests.service.in
 +++ b/tools/libvirt-guests.service.in
 @@ -1,6 +1,6 @@
   [Unit]
   Description=Suspend Active Libvirt Guests
 -After=network.target libvirtd.service
 +After=network.target libvirtd.service ntp-wait.service
   Documentation=man:libvirtd(8)
   Documentation=http://libvirt.org



 Well, guest can have their own ntp-client (and in most cases they do,
 right?).

I think most do, but know of at least two users who want to use kvmclock
with no ntp in the guests :).

 I mean, since guests can be paused, saved  restored back, their time
 is often off. So the best is to have an ntp-client running inside the
 guest.

Yep.  I mentioned this, but seems they don't use save, restore, migrate,
et. al., since it wasn't a concern.  But I'm fine handling this
downstream.  Thanks!

Regards,
Jim

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

[libvirt] How do I modify an existing network filter?

[duyah...@kingsoft.com]

Hi All,
We have a requirement to allow more than one IP connect to external network 
through one mac address. We suppose we need to modify the network filter but we 
can't find the python API or anything else to change the existing network 
filter. Does anyone know what I should do?

We tried another way to add a parameter to 'filterref' in the instance xml file 
like below blue part and it worked. But we have to restart the instance to make 
this configufation effective. Can I make the xml change effective without 
restart the instance? Any help will be appreciated.
interface type='bridge'

  mac address='fa:16:3e:2f:00:09'/

  source bridge='br_vm'/

  model type='virtio'/

  filterref filter='nova-instance-instance-0018-fa163e2f0009'

parameter name='DHCPSERVER' value='50.1.1.1'/

parameter name='IP' value='50.1.1.223'/

parameter name='IP' value='50.1.1.222'/

  /filterref

  address type='pci' domain='0x' bus='0x00' slot='0x03' 
function='0x0'/

/interface



duyah...@kingsoft.com

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

Re: [libvirt] [PATCH] libvirt-guests: wait for ntp service

[Martin Kletzander]

On Fri, Sep 19, 2014 at 02:37:12PM -0600, Jim Fehlig wrote:

Michal Privoznik wrote:

On 08.09.2014 18:30, Jim Fehlig wrote:

If an NTP server is configured on the host, it is possible for
libvirt-guests to start before the NTP service, in which case
guest clocks won't be synchronized to the host clock.

Add ntp-wait.service to After in libvirt-guests systemd service
file, ensuring NTP has synchronized the host clock before starting
any guests.

Signed-off-by: Jim Fehlig jfeh...@suse.com
---
  tools/libvirt-guests.service.in | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libvirt-guests.service.in
b/tools/libvirt-guests.service.in
index d8d7adf..226b3bd 100644
--- a/tools/libvirt-guests.service.in
+++ b/tools/libvirt-guests.service.in
@@ -1,6 +1,6 @@
  [Unit]
  Description=Suspend Active Libvirt Guests
-After=network.target libvirtd.service
+After=network.target libvirtd.service ntp-wait.service
  Documentation=man:libvirtd(8)
  Documentation=http://libvirt.org




Well, guest can have their own ntp-client (and in most cases they do,
right?).


I think most do, but know of at least two users who want to use kvmclock
with no ntp in the guests :).



I'm sure there are way more users without ntp clients inside their
guests.  I'm just wondering what's the difference when libvirt-guests
starts before or after ntp has synchronized their clocks.  Is it that
they have the time reset to a little bit inaccurate time?  Or are they
off way too much?


I mean, since guests can be paused, saved  restored back, their time
is often off. So the best is to have an ntp-client running inside the
guest.




Yes, but if it's way off, ntp will refuse to update the time; that's
why we are resetting the time, isn't it?


Yep.  I mentioned this, but seems they don't use save, restore, migrate,
et. al., since it wasn't a concern.  But I'm fine handling this
downstream.  Thanks!



Well, if they use libvirt-guests, they use at least save/restore :)

Unfortunately I'm not very familiar with systemd files, but my guess
is that After=ntp-wait.service means it should be started after the
time is synchronized if and only if the ntp-wait.service unit is
enabled, otherwise it doesn't require it.  My point is that if this
doesn't enable ntp synchronization for users that don't want it, then
we should probably push his upstream.  There is slight added benefit
and no drawbacks.

Martin


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

Re: [libvirt] [PATCH] libvirt-guests: wait for ntp service

[Jim Fehlig]

Martin Kletzander wrote:
 On Fri, Sep 19, 2014 at 02:37:12PM -0600, Jim Fehlig wrote:
 Michal Privoznik wrote:
 On 08.09.2014 18:30, Jim Fehlig wrote:
 If an NTP server is configured on the host, it is possible for
 libvirt-guests to start before the NTP service, in which case
 guest clocks won't be synchronized to the host clock.

 Add ntp-wait.service to After in libvirt-guests systemd service
 file, ensuring NTP has synchronized the host clock before starting
 any guests.

 Signed-off-by: Jim Fehlig jfeh...@suse.com
 ---
   tools/libvirt-guests.service.in | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/tools/libvirt-guests.service.in
 b/tools/libvirt-guests.service.in
 index d8d7adf..226b3bd 100644
 --- a/tools/libvirt-guests.service.in
 +++ b/tools/libvirt-guests.service.in
 @@ -1,6 +1,6 @@
   [Unit]
   Description=Suspend Active Libvirt Guests
 -After=network.target libvirtd.service
 +After=network.target libvirtd.service ntp-wait.service
   Documentation=man:libvirtd(8)
   Documentation=http://libvirt.org



 Well, guest can have their own ntp-client (and in most cases they do,
 right?).

 I think most do, but know of at least two users who want to use kvmclock
 with no ntp in the guests :).


 I'm sure there are way more users without ntp clients inside their
 guests.  I'm just wondering what's the difference when libvirt-guests
 starts before or after ntp has synchronized their clocks.  Is it that
 they have the time reset to a little bit inaccurate time?  Or are they
 off way too much?

They are off by the ntp adjustment.  As I understand it, the guests
start and read the host clock, which is later adjusted by ntp.


 I mean, since guests can be paused, saved  restored back, their time
 is often off. So the best is to have an ntp-client running inside the
 guest.


 Yes, but if it's way off, ntp will refuse to update the time; that's
 why we are resetting the time, isn't it?

 Yep.  I mentioned this, but seems they don't use save, restore, migrate,
 et. al., since it wasn't a concern.  But I'm fine handling this
 downstream.  Thanks!


 Well, if they use libvirt-guests, they use at least save/restore :)

They have ON_SHUTDOWN=shutdown.


 Unfortunately I'm not very familiar with systemd files, but my guess
 is that After=ntp-wait.service means it should be started after the
 time is synchronized if and only if the ntp-wait.service unit is
 enabled, otherwise it doesn't require it.

Yes, this is my understanding too.

Regards,
Jim

   My point is that if this
 doesn't enable ntp synchronization for users that don't want it, then
 we should probably push his upstream.  There is slight added benefit
 and no drawbacks.

 Martin

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