Re: [libvirt] [[PATCH v5] autocreate tap device for VIR_DOMAIN_NET_TYPE_ETHERNET] autocreate tap device for VIR_DOMAIN_NET_TYPE_ETHERNET

2015-06-18 Thread Vasiliy Tolstov
2015-06-18 14:50 GMT+03:00 Vasiliy Tolstov v.tols...@selfip.ru:
 If a user specify ehernet device create it via libvirt and run
 script if it provided. After this commit user does not need to
 run external script to create tap device or add root to qemu
 process.

 Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru
 ---
  src/qemu/qemu_command.c | 139 
 ++--
  src/qemu/qemu_hotplug.c |  13 ++---
  src/qemu/qemu_process.c |   6 +++
  3 files changed, 98 insertions(+), 60 deletions(-)


This new version applies to master and have fix for setting mac
address for tap device after creation.

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

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


[libvirt] [PATCH 2/2] qemu: Fix double space in error message in qemuDomainGetVcpusFlags

2015-06-18 Thread Peter Krempa
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3a70b18..43228fd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5506,7 +5506,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int 
flags)
 if (!virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_INVALID_ARG, %s,
_(vCPU count provided by the guest agent can only 
be 
-  requested for live domains));
+ requested for live domains));
 goto cleanup;
 }

-- 
2.4.1

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


Re: [libvirt] [PATCH v2] qemu: implement address for isa-serial

2015-06-18 Thread John Ferlan


On 06/16/2015 09:07 AM, James Cowgill wrote:
 I needed to specify the iobase address for certain exotic mips configurations.
 
 Signed-off-by: James Cowgill james...@cowgill.org.uk
 ---
  src/qemu/qemu_command.c| 12 ++--
  .../qemuxml2argv-serial-dev-chardev-iobase.args|  7 +
  .../qemuxml2argv-serial-dev-chardev-iobase.xml | 36 
 ++
  tests/qemuxml2argvtest.c   |  2 ++
  4 files changed, 55 insertions(+), 2 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev-iobase.args
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev-iobase.xml
 
 v2:
  - add the serial-dev-chardev-iobase testcase
 

Thanks for the test..

ACK and pushed

John

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 3886b4f..f63d2a1 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -2693,6 +2693,10 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
info-addr.ccw.cssid,
info-addr.ccw.ssid,
info-addr.ccw.devno);
 +} else if (info-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) {
 +virBufferAsprintf(buf, ,iobase=0x%x,irq=0x%x,
 +  info-addr.isa.iobase,
 +  info-addr.isa.irq);
  }
  
  ret = 0;
 @@ -10982,11 +10986,15 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
  break;
  
  case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
 -if (serial-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
 +if (serial-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE 
 +serial-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 -   _(no addresses are supported for 
 isa-serial));
 +   _(isa-serial requires address of isa type));
  goto error;
  }
 +
 +if (qemuBuildDeviceAddressStr(cmd, def, serial-info, 
 qemuCaps)  0)
 +goto error;
  break;
  
  case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI:
 diff --git 
 a/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev-iobase.args 
 b/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev-iobase.args
 new file mode 100644
 index 000..9d5de02
 --- /dev/null
 +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev-iobase.args
 @@ -0,0 +1,7 @@
 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test 
 QEMU_AUDIO_DRV=none \
 +/usr/bin/qemu -S -M \
 +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,\
 +id=charmonitor,path=/tmp/test-monitor,server,nowait -mon 
 chardev=charmonitor,\
 +id=monitor,mode=readline -no-acpi -boot c -usb -hda /dev/HostVG/QEMUGuest1 
 -chardev \
 +tty,id=charserial0,path=/dev/ttyS2 -device isa-serial,chardev=charserial0,\
 +id=serial0,iobase=0x3f8,irq=0x4 -device 
 virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
 diff --git 
 a/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev-iobase.xml 
 b/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev-iobase.xml
 new file mode 100644
 index 000..eb84574
 --- /dev/null
 +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev-iobase.xml
 @@ -0,0 +1,36 @@
 +domain type='qemu'
 +  nameQEMUGuest1/name
 +  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
 +  memory unit='KiB'219100/memory
 +  currentMemory unit='KiB'219100/currentMemory
 +  vcpu placement='static'1/vcpu
 +  os
 +type arch='i686' machine='pc'hvm/type
 +boot dev='hd'/
 +  /os
 +  clock offset='utc'/
 +  on_poweroffdestroy/on_poweroff
 +  on_rebootrestart/on_reboot
 +  on_crashdestroy/on_crash
 +  devices
 +emulator/usr/bin/qemu/emulator
 +disk type='block' device='disk'
 +  source dev='/dev/HostVG/QEMUGuest1'/
 +  target dev='hda' bus='ide'/
 +  address type='drive' controller='0' bus='0' target='0' unit='0'/
 +/disk
 +controller type='usb' index='0'/
 +controller type='ide' index='0'/
 +serial type='dev'
 +  source path='/dev/ttyS2'/
 +  target port='0'/
 +  address type='isa' iobase='0x3f8' irq='0x4'/
 +/serial
 +console type='dev'
 +  source path='/dev/ttyS2'/
 +  target port='0'/
 +  address type='isa' iobase='0x3f8' irq='0x4'/
 +/console
 +memballoon model='virtio'/
 +  /devices
 +/domain
 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
 index a90f9a6..56b18eb 100644
 --- a/tests/qemuxml2argvtest.c
 +++ b/tests/qemuxml2argvtest.c
 @@ -1045,6 +1045,8 @@ mymain(void)
  QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
  DO_TEST(serial-dev-chardev,
  QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
 +DO_TEST(serial-dev-chardev-iobase,
 +QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, 

Re: [libvirt] [PATCH 2/4] parallels: Fix false error messages in libvirt log

2015-06-18 Thread Dmitry Guryanov

On 06/17/2015 03:35 PM, Mikhail Feoktistov wrote:

There was many errors in libvirt.log caused by prlsdkDelNet function because
job variable was always initialized as PRL_INVALID_HANDLE
In this patch job variable gets return value of PrlSrv_DeleteVirtualNetwork 
function()


Pushed, thanks!


---
  src/vz/vz_sdk.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index c36dad6..98f7a57 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2900,7 +2900,7 @@ static void prlsdkDelNet(vzConnPtr privconn, 
virDomainNetDefPtr net)
  pret = PrlVirtNet_SetNetworkId(vnet, net-data.network.name);
  prlsdkCheckRetGoto(pret, cleanup);
  
-PrlSrv_DeleteVirtualNetwork(privconn-server, vnet, 0);

+job = PrlSrv_DeleteVirtualNetwork(privconn-server, vnet, 0);
  if (PRL_FAILED(pret = waitJob(job)))
  goto cleanup;
  



--
Dmitry Guryanov

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


[libvirt] [PATCH 3/4] virNetSocket: Fix @watch corner case

2015-06-18 Thread Michal Privoznik
Although highly unlikely, nobody says that virEventAddHandle()
can't return 0 as a handle to socket callback. It can't happen
with our default implementation since all watches will have value
1 or greater, but users can register their own callback functions
(which can re-use unused watch IDs for instance). If this is the
case, weird things may happen.

Also, there's a little bug I'm fixing too, upon
virNetSocketRemoveIOCallback(), the variable holding callback ID
was not reset. Therefore calling AddIOCallback() once again would
fail. Not that we are doing it right now, but we might.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/rpc/virnetsocket.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 81903e7..cef3f36 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -245,6 +245,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr 
localAddr,
 sock-fd = fd;
 sock-errfd = errfd;
 sock-pid = pid;
+sock-watch = -1;
 
 /* Disable nagle for TCP sockets */
 if (sock-localAddr.data.sa.sa_family == AF_INET ||
@@ -1153,7 +1154,7 @@ void virNetSocketDispose(void *obj)
 PROBE(RPC_SOCKET_DISPOSE,
   sock=%p, sock);
 
-if (sock-watch  0) {
+if (sock-watch = 0) {
 virEventRemoveHandle(sock-watch);
 sock-watch = -1;
 }
@@ -1941,7 +1942,7 @@ int virNetSocketAddIOCallback(virNetSocketPtr sock,
 
 virObjectRef(sock);
 virObjectLock(sock);
-if (sock-watch  0) {
+if (sock-watch = 0) {
 VIR_DEBUG(Watch already registered on socket %p, sock);
 goto cleanup;
 }
@@ -1971,7 +1972,7 @@ void virNetSocketUpdateIOCallback(virNetSocketPtr sock,
   int events)
 {
 virObjectLock(sock);
-if (sock-watch = 0) {
+if (sock-watch  0) {
 VIR_DEBUG(Watch not registered on socket %p, sock);
 virObjectUnlock(sock);
 return;
@@ -1986,7 +1987,7 @@ void virNetSocketRemoveIOCallback(virNetSocketPtr sock)
 {
 virObjectLock(sock);
 
-if (sock-watch = 0) {
+if (sock-watch  0) {
 VIR_DEBUG(Watch not registered on socket %p, sock);
 virObjectUnlock(sock);
 return;
@@ -1994,6 +1995,7 @@ void virNetSocketRemoveIOCallback(virNetSocketPtr sock)
 
 virEventRemoveHandle(sock-watch);
 /* Don't unref @sock, it's done via evenloop callback. */
+sock-watch = -1;
 
 virObjectUnlock(sock);
 }
-- 
2.3.6

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


[libvirt] [PATCH 0/2] Two trivial fixes

2015-06-18 Thread Peter Krempa
Both are now pushed.

Peter Krempa (2):
  qemu: Jump to correct label in qemuDomainPinIOThread
  qemu: Fix double space in error message in qemuDomainGetVcpusFlags

 src/qemu/qemu_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.4.1

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


Re: [libvirt] [PATCH 0/2] Yet again a admin API fixup series.

2015-06-18 Thread Peter Krempa
On Thu, Jun 18, 2015 at 11:53:55 +0200, Michal Privoznik wrote:
 On 18.06.2015 11:44, Peter Krempa wrote:
  Fluffy bunny ears of shame will be administered to Martin.
  
  Peter Krempa (2):
rpc: Actually increase reference count on @srv in
  virNetDaemonAddServer
daemon: Add the admin service to the admin server only if it was
  allocated
  
   daemon/libvirtd.c  | 6 +++---
   src/rpc/virnetdaemon.c | 2 +-
   2 files changed, 4 insertions(+), 4 deletions(-)
  
 
 ACK series. Funny, I was just about to send the 1/2 patch too.

Pushed; Thanks.

Peter


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

Re: [libvirt] [PATCH 1/4] parallels: Fix initialization of buflen variable in each loop iteration

2015-06-18 Thread Dmitry Guryanov

On 06/17/2015 03:35 PM, Mikhail Feoktistov wrote:

We need to initialize buflen every time when we get network adapter's
friendly name because we call PrlVmDev_GetFriendlyName in a loop

Acked and pushed, thanks!


---
  src/vz/vz_sdk.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f9cde44..c36dad6 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -3150,6 +3150,7 @@ prlsdkGetDiskIndex(PRL_HANDLE sdkdom, virDomainDiskDefPtr 
disk)
  pret = PrlVmCfg_GetHardDisk(sdkdom, i, hdd);
  prlsdkCheckRetGoto(pret, cleanup);
  
+buflen = 0;

  pret = PrlVmDev_GetFriendlyName(hdd, 0, buflen);
  prlsdkCheckRetGoto(pret, cleanup);
  



--
Dmitry Guryanov

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


Re: [libvirt] [PATCHv1.5 1/2] cpu:x86: fix specified features will disappear after migrate/restore

2015-06-18 Thread Jiri Denemark
On Wed, Jun 17, 2015 at 22:22:44 +0800, Luyao Huang wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1207095
 
 We allow set the feature with the host-passthrough cpu,
 but won't save them in the migration xml, the features we
 specified will disappear after migrate/restore. This is because
 we skip the virCPUDefUpdateFeature if cpu mode is host-passthrough.
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  v1.5: just update the description.
 
  src/cpu/cpu_x86.c | 9 -
  1 file changed, 4 insertions(+), 5 deletions(-)
 
 diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
 index 2a14705..26601b6 100644
 --- a/src/cpu/cpu_x86.c
 +++ b/src/cpu/cpu_x86.c
 @@ -2087,8 +2087,7 @@ x86UpdateCustom(virCPUDefPtr guest,
  
  static int
  x86UpdateHostModel(virCPUDefPtr guest,
 -   const virCPUDef *host,
 -   bool passthrough)
 +   const virCPUDef *host)
  {
  virCPUDefPtr oldguest = NULL;
  const struct x86_map *map;
 @@ -2124,7 +2123,7 @@ x86UpdateHostModel(virCPUDefPtr guest,
  }
  }
  }
 -for (i = 0; !passthrough  i  oldguest-nfeatures; i++) {
 +for (i = 0; i  oldguest-nfeatures; i++) {
  if (virCPUDefUpdateFeature(guest,
 oldguest-features[i].name,
 oldguest-features[i].policy)  0)

While this looks correct, save/restore (and likely migration too) with
-cpu host has even more bugs (e.g., -cpu host turns into -cpu host,+...
after save/restore) and I'd like to fix all that at once.

Jirka

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


Re: [libvirt] [PATCHv3] qemu: add a check for slot and base when build dimm address

2015-06-18 Thread John Ferlan


On 06/15/2015 08:33 AM, Luyao Huang wrote:
 When hot-plug a memory device, we don't check if there
 is a memory device have the same address with the memory device
 we want hot-pluged. Qemu forbid use/hot-plug 2 memory device
 with same slot or the same base(qemu side this elemnt named addr).
 
 Introduce a address check when build memory device qemu command line.
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---

NOTE: Used the following for commit message:

qemu: Add a check for slot and base dimm address conflicts

When hotplugging a memory device, there wasn't a check to determine
if there is a conflict with the address space being used by the to
be added memory device and any existing device which is disallowed by qemu.

This patch adds a check to ensure the new device address doesn't
conflict with any existing device.


  v3:
   rename qemuBuildMemoryDeviceAddr to qemuCheckMemoryDimmConflict and
   remove the refactor.
 
  src/qemu/qemu_command.c | 37 +
  1 file changed, 37 insertions(+)
 

ACK

Adjusted patch as shown below and pushed.

John

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 0a6d92f..d3f0a23 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -4952,6 +4952,40 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr 
 mem,
  }
  
  
 +static bool
 +qemuCheckMemoryDimmConflict(virDomainDefPtr def,
 +virDomainMemoryDefPtr mem)
 +{
 +size_t i;
 +
 +for (i = 0; i  def-nmems; i++) {
 + virDomainMemoryDefPtr tmp = def-mems[i];
 +
 + if (tmp == mem ||
 + tmp-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM)
 + continue;
 +
 + if (mem-info.addr.dimm.slot == tmp-info.addr.dimm.slot) {
 + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +_(memory device slot '%u' is already being
 +   used by another memory device),

Placed space at end of first line rather than at the beginning of the
appended string (eg ...being  and used by...)

 +mem-info.addr.dimm.slot);
 + return true;
 + }
 +
 + if (mem-info.addr.dimm.base != 0  tmp-info.addr.dimm.base != 0 
 
 + mem-info.addr.dimm.base == tmp-info.addr.dimm.base) {

Used:
 if (mem-info.addr.dimm.base != 0 
 mem-info.addr.dimm.base == tmp-info.addr.dimm.base) {


 + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +_(memory device base '0x%llx' is already being
 +   used by another memory device),

Similar space adjustment here

 +mem-info.addr.dimm.base);
 + return true;
 + }
 +}
 +
 +return false;
 +}
 +
  char *
  qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
   virDomainDefPtr def,
 @@ -4993,6 +5027,9 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
mem-targetNode, mem-info.alias, mem-info.alias);
  
  if (mem-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
 +if (qemuCheckMemoryDimmConflict(def, mem))
 +return NULL;
 +
  virBufferAsprintf(buf, ,slot=%d, mem-info.addr.dimm.slot);
  virBufferAsprintf(buf, ,addr=%llu, mem-info.addr.dimm.base);
  }
 

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


Re: [libvirt] [PATCH v4 6/9] util: virTypedParams{Filter, GetAllStrings}

2015-06-18 Thread Jiri Denemark
On Tue, Jun 16, 2015 at 17:44:11 +0200, Michal Privoznik wrote:
 On 16.06.2015 00:42, Pavel Boldin wrote:
  Add multikey API:
  
   * virTypedParamsFilter that filters all the parameters with specified name.
   * virTypedParamsGetAllStrings that returns a list with all the values for
 specified name and string type.
  
  Signed-off-by: Pavel Boldin pbol...@mirantis.com
  Signed-off-by: Michal Privoznik mpriv...@redhat.com
  ---
   include/libvirt/libvirt-host.h |   5 ++
   src/libvirt_public.syms|   5 ++
   src/util/virtypedparam.c   | 102 
  +
   src/util/virtypedparam.h   |   9 
   tests/Makefile.am  |   2 +-
   tests/virtypedparamtest.c  | 100 
  
   6 files changed, 222 insertions(+), 1 deletion(-)
  
  diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h
  index 070550b..8222cfb 100644
  --- a/include/libvirt/libvirt-host.h
  +++ b/include/libvirt/libvirt-host.h
  @@ -284,6 +284,11 @@ virTypedParamsGetString (virTypedParameterPtr params,
const char *name,
const char **value);
   int
  +virTypedParamsGetAllStrings(virTypedParameterPtr params,

Hmm, I apologize for not noticing it in my earlier comment, but since
the corresponding API for adding multiple strings is called
virTypedParamsAddStringList, we should call this
virTypedParamsGetStringList.

  + int nparams,
  + const char *name,
  + const char ***values);

Wrong indentation.

  +int
   virTypedParamsAddInt(virTypedParameterPtr *params,
int *nparams,
int *maxparams,
  diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
  index 716dd2f..0a1feea 100644
  --- a/src/libvirt_public.syms
  +++ b/src/libvirt_public.syms
  @@ -715,4 +715,9 @@ LIBVIRT_1.2.16 {
   virDomainSetUserPassword;
   } LIBVIRT_1.2.15;
   
  +LIBVIRT_1.3.0 {
  +global:
  +virTypedParamsGetAllStrings;
  +} LIBVIRT_1.2.16;
  +
 
 I don't think this symbol needs to be exported.

Yeah, we have no API which would return multiple values for a single
parameter so there's no reason to export this now. We can export it
later when introducing such API.

Jirka

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


Re: [libvirt] [Qemu-block] [Qemu-devel] [PATCH v7 00/11] Support streaming to an intermediate layer

2015-06-18 Thread Eric Blake
[adding libvirt, to make sure I capture a design idea]

On 06/18/2015 06:36 AM, Eric Blake wrote:
 On 06/18/2015 06:07 AM, Alberto Garcia wrote:
 On Thu 18 Jun 2015 01:47:20 PM CEST, Kevin Wolf wrote:

 I believe our conclusion from an earlier version of the series was
 that we need QAPI introspection so that libvirt can detect the
 presence of the feature.
 
 Detecting the presence of a feature allows libvirt the luxury of giving
 its own error message, rather than relying on the qemu message. But
 that's not to say libvirt HAS to use its own error message, and
 therefore being unable to detect the feature may not be the end of the
 world.
 

 That said, I would prefer a way to detect the feature that does not
 involve testing commands for their error codes, but is there any? What
 does libvirt generally do in order to detect new features that don't
 depend on API changes?
 
 But libvirt has not yet set up node name management (I'm about to revive
 Jeff's patch for auto-node-naming simultaneously with a libvirt patch
 series that proves that it helps libvirt), and libvirt will need a new
 API to allow users a way to request streams to an intermediate image.
 So anything libvirt does to interact with the new stream-to-intermediate
 will have to be new code, and I can worry about whether the qemu error
 message is good enough, or whether I have to contrive some probing test
 to see if it even works; but my initial thought is that merely probing
 to see if auto-node-naming is in place is a good approximation filter
 (if libvirt isn't managing its own node names, then the only way to use
 stream-to-intermediate is via a node name automatically supplied by
 qemu, especially nice if both features land in 2.4).

Actually, in thinking more about it, libvirt won't need a new API; the
existing virDomainBlockPull() and virDomainBlockRebase() are sufficient,
if I allow libvirt to treat vda[1] as a destination (which is the
first backing image of disk vda; pretty much similar to how qemu is
adding node names rather than device names as a way to make the existing
block-stream now stream to intermediate).  And that is consistent with
the way we have been retrofitting other existing libvirt API to refer to
specific backing images.  On libvirt's front, I may want to add a new
flag (where the flag must be present to make it clear that
stream-to-intermediate is desired, so that upper level applications can
use the absence of the libvirt flag as their feature probe), but that
has no bearing on what qemu has to do to turn on the feature.

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



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

Re: [libvirt] [PATCH 2/2] qemu: fix a document issue as we support host-passthrough with features

2015-06-18 Thread Jiri Denemark
On Wed, Jun 17, 2015 at 22:22:45 +0800, Luyao Huang wrote:
 From the documentation :With this mode, the CPU visible to
 the guest should be exactly the same as the host CPU even in
 the aspects that libvirt does not understand.
 
 So this place document need fix.
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  src/qemu/qemu_process.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
 index 64ee049..3cd0ff4 100644
 --- a/src/qemu/qemu_process.c
 +++ b/src/qemu/qemu_process.c
 @@ -4024,8 +4024,10 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
  bool ret = false;
  size_t i;
  
 -/* no features are passed to QEMU with -cpu host
 - * so it makes no sense to verify them */
 +/* Although we allow set features with host-passthrough
 + * cpu mode, we allow user require/disable the feature
 + * that libvirt does not understand, so it makes no sense
 + * to verify them */
  if (def-cpu  def-cpu-mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
  return true;
  

NACK. We certainly don't want features to be just passed through without
any validation if used with host-passthrough. We rather need to fix the
code check the features used in a guest cpu element are all known to
libvirt.

Jirka

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


[libvirt] [PATCH 1/2] qemu: Jump to correct label in qemuDomainPinIOThread

2015-06-18 Thread Peter Krempa
If virDomainObjGetDefs used in qemuDomainPinIOThread would fail the code
would jump to the 'cleanup' label after acquiring the job, thus the VM
would be locked forever.

Introduced in commit cac6d639.
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index eb07b7a..3a70b18 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5790,7 +5790,7 @@ qemuDomainPinIOThread(virDomainPtr dom,
 goto cleanup;

 if (virDomainObjGetDefs(vm, flags, def, persistentDef)  0)
-goto cleanup;
+goto endjob;

 if (!(pcpumap = virBitmapNewData(cpumap, maplen)))
 goto endjob;
-- 
2.4.1

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


[libvirt] [[PATCH v5] autocreate tap device for VIR_DOMAIN_NET_TYPE_ETHERNET] autocreate tap device for VIR_DOMAIN_NET_TYPE_ETHERNET

2015-06-18 Thread Vasiliy Tolstov
If a user specify ehernet device create it via libvirt and run
script if it provided. After this commit user does not need to
run external script to create tap device or add root to qemu
process.

Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru
---
 src/qemu/qemu_command.c | 139 ++--
 src/qemu/qemu_hotplug.c |  13 ++---
 src/qemu/qemu_process.c |   6 +++
 3 files changed, 98 insertions(+), 60 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3886b4f..f9008e4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -332,10 +332,39 @@ static int 
qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg,
 return *tapfd  0 ? -1 : 0;
 }
 
+/**
+ * qemuExecuteEthernetScript:
+ * @ifname: the interface name
+ * @script: the script name
+ *
+ * This function executes script for new tap device created by libvirt.
+ * Returns 0 in case of success or -1 on failure
+ */
+static int
+qemuExecuteEthernetScript(const char *ifname, const char *script)
+{
+virCommandPtr cmd;
+int ret;
+
+cmd = virCommandNew(script);
+virCommandAddArgFormat(cmd, %s, ifname);
+virCommandClearCaps(cmd);
+#ifdef CAP_NET_ADMIN
+virCommandAllowCap(cmd, CAP_NET_ADMIN);
+#endif
+virCommandAddEnvPassCommon(cmd);
+
+ret = virCommandRun(cmd, NULL);
+
+virCommandFree(cmd);
+return ret;
+}
+
 /* qemuNetworkIfaceConnect - *only* called if actualType is
- * VIR_DOMAIN_NET_TYPE_NETWORK or VIR_DOMAIN_NET_TYPE_BRIDGE (i.e. if
- * the connection is made with a tap device connecting to a bridge
- * device)
+ * VIR_DOMAIN_NET_TYPE_NETWORK, VIR_DOMAIN_NET_TYPE_BRIDGE
+ * VIR_DOMAIN_NET_TYPE_ETHERNET (i.e. if the connection is
+ * made with a tap device connecting to a bridge device or
+ * use plain tap device)
  */
 int
 qemuNetworkIfaceConnect(virDomainDefPtr def,
@@ -351,6 +380,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
 bool template_ifname = false;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 const char *tunpath = /dev/net/tun;
+virMacAddr tapmac;
 
 if (net-backend.tap) {
 tunpath = net-backend.tap;
@@ -361,11 +391,6 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
 }
 }
 
-if (!(brname = virDomainNetGetActualBridgeName(net))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Missing bridge name));
-goto cleanup;
-}
-
 if (!net-ifname ||
 STRPREFIX(net-ifname, VIR_NET_GENERATED_PREFIX) ||
 strchr(net-ifname, '%')) {
@@ -381,40 +406,62 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
 tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
 }
 
-if (cfg-privileged) {
-if (virNetDevTapCreateInBridgePort(brname, net-ifname, net-mac,
-   def-uuid, tunpath, tapfd, 
*tapfdSize,
-   
virDomainNetGetActualVirtPortProfile(net),
-   virDomainNetGetActualVlan(net),
-   tap_create_flags)  0) {
+if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_ETHERNET) {
+if (virNetDevTapCreate(net-ifname, tunpath, tapfd, *tapfdSize,
+   tap_create_flags)  0) {
 virDomainAuditNetDevice(def, net, tunpath, false);
 goto cleanup;
 }
-if (virDomainNetGetActualBridgeMACTableManager(net)
-== VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LIBVIRT) {
-/* libvirt is managing the FDB of the bridge this device
- * is attaching to, so we need to turn off learning and
- * unicast_flood on the device to prevent the kernel from
- * adding any FDB entries for it. We will add add an fdb
- * entry ourselves (during qemuInterfaceStartDevices(),
- * using the MAC address from the interface config.
- */
-if (virNetDevBridgePortSetLearning(brname, net-ifname, false)  0)
-goto cleanup;
-if (virNetDevBridgePortSetUnicastFlood(brname, net-ifname, false) 
 0)
+virMacAddrSet(tapmac, net-mac);
+
+if (virNetDevSetMAC(net-ifname, tapmac)  0)
+goto cleanup;
+
+if (net-script) {
+if (qemuExecuteEthernetScript(net-ifname, net-script)  0)
 goto cleanup;
 }
 } else {
-if (qemuCreateInBridgePortWithHelper(cfg, brname,
- net-ifname,
- tapfd, tap_create_flags)  0) {
-virDomainAuditNetDevice(def, net, tunpath, false);
+if (!(brname = virDomainNetGetActualBridgeName(net))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s, _(Missing bridge 
name));
 goto cleanup;
 }
-/* qemuCreateInBridgePortWithHelper can only create a single FD */
-if (*tapfdSize  1) {
-  

Re: [libvirt] [PATCH v4 0/9] Selective block device migration implementation

2015-06-18 Thread Jiri Denemark
On Wed, Jun 17, 2015 at 17:31:03 +0200, Kashyap Chamarthy wrote:
 On Tue, Jun 16, 2015 at 01:42:02AM +0300, Pavel Boldin wrote:
...
 libvirtd debug log[1] from source (destination log is empty)):
 
 [. . .]
 2015-06-17 15:13:53.317+: 781: debug : 
 virDomainMigratePerform3Params:5202 : dom=0x7f2118f13c40, (VM: name=cvm1, 
 uuid=ab4c412b-6fdc-4fc4-b78c-f1d49db10d4e), 
 dconnuri=qemu+tcp://root@devstack3/system, params=0x7f2118f12a90, nparams=1, 
 cookiein=(nil), cookieinlen=0, cookieout=0x7f2106f38ba8, 
 cookieoutlen=0x7f2106f38ba4, flags=3
 2015-06-17 15:13:53.317+: 781: debug : 
 virDomainMigratePerform3Params:5203 : params[migrate_disks]=(string)vdb
 2015-06-17 15:13:53.317+: 781: debug : qemuMigrationPerform:5238 : 
 driver=0x7f20f416b840, conn=0x7f20dc005c30, vm=0x7f20f41e9640, xmlin=null, 
 dconnuri=qemu+tcp://root@devstack3/system, uri=null, graphicsuri=null, 
 listenAddress=null, nmigrate_disks=1, migrate_disks=0x7f2118f13930, 
 cookiein=null, cookieinlen=0, cookieout=0x7f2106f38ba8, 
 cookieoutlen=0x7f2106f38ba4, flags=3, dname=null, resource=0, v3proto=1
 2015-06-17 15:13:53.317+: 781: debug : qemuDomainObjBeginJobInternal:1397 
 : Starting async job: none (async=migration out vm=0x7f20f41e9640 name=cvm1)
 2015-06-17 15:13:53.317+: 781: debug : qemuDomainObjBeginJobInternal:1414 
 : Waiting for async job (vm=0x7f20f41e9640 name=cvm1)
 2015-06-17 15:13:53.821+: 782: debug : virThreadJobSet:96 : Thread 782 
 (virNetServerHandleJob) is now running job remoteDispatchDomainGetJobInfo
 2015-06-17 15:13:53.821+: 782: debug : virDomainGetJobInfo:8808 : 
 dom=0x7f20dc008c30, (VM: name=cvm1, 
 uuid=ab4c412b-6fdc-4fc4-b78c-f1d49db10d4e), info=0x7f2106737b50
 2015-06-17 15:13:53.821+: 782: debug : virThreadJobClear:121 : Thread 782 
 (virNetServerHandleJob) finished job remoteDispatchDomainGetJobInfo with ret=0
 2015-06-17 15:13:54.325+: 780: debug : virThreadJobSet:96 : Thread 780 
 (virNetServerHandleJob) is now running job remoteDispatchDomainGetJobInfo
 2015-06-17 15:13:54.325+: 780: debug : virDomainGetJobInfo:8808 : 
 dom=0x7f20dc008c30, (VM: name=cvm1, 
 uuid=ab4c412b-6fdc-4fc4-b78c-f1d49db10d4e), info=0x7f2107739b50
 2015-06-17 15:13:54.325+: 780: debug : virThreadJobClear:121 : Thread 780 
 (virNetServerHandleJob) finished job remoteDispatchDomainGetJobInfo with ret=0
 [. . .]
 remoteDispatchDomainMigratePerform3Params, 784 
 remoteDispatchDomainMigratePerform3Params) for (520s, 520s)
 2015-06-17 15:14:23.320+: 781: error : qemuDomainObjBeginJobInternal:1492 
 : Timed out during operation: cannot acquire state change lock (held by 
 remoteDispatchDomainMigratePerform3Params)
 2015-06-17 15:14:23.320+: 781: debug : virThreadJobClear:121 : Thread 781 
 (virNetServerHandleJob) finished job 
 remoteDispatchDomainMigratePerform3Params with ret=-1
 2015-06-17 15:14:23.320+: 783: debug : virThreadJobSet:96 : Thread 783 
 (virNetServerHandleJob) is now running job remoteDispatchConnectClose
 2015-06-17 15:14:23.320+: 783: debug : virThreadJobClear:121 : Thread 783 
 (virNetServerHandleJob) finished job remoteDispatchConnectClose with ret=0
 
 
 How can I mitigate this? (I realize this is not due to these patches,
 proably something with my test environment.)
 
 Since this is non-shared storage migration, I tried to supply
 '--copy-storage-inc' to no avail (same error as above).
 
 Probably I should test by building local RPMs.
 
 [1] 
 https://kashyapc.fedorapeople.org/virt/temp/libvirtd-log-selective-blockdev-failed.log

Could you upload a complete log somewhere? It seems a previously started
migration is waiting for a response from QEMU. Or alternatively, it
failed to release the jobs. I'd like to see the logs from the previous
migration attempt.

Jirka

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


[libvirt] [PATCH 1/4] daemonSetupNetworking: Don't leak services

2015-06-18 Thread Michal Privoznik
When setting up the daemon networking, new services are created. These
services then have sockets to listen on. Once created, the service
objects are added to corresponding server object. However, during that
process, server increases reference counter of the service object. So,
at the end of the function, we should decrease it again. This way the
service objects will have only 1 reference, but that's okay since
servers are the only objects having a reference.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 daemon/libvirtd.c | 47 ---
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 286512a..20e0b2f 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -455,33 +455,34 @@ daemonSetupNetworking(virNetServerPtr srv,
 int unix_sock_ro_mask = 0;
 int unix_sock_rw_mask = 0;
 int unix_sock_adm_mask = 0;
+int ret = -1;
 
 unsigned int cur_fd = STDERR_FILENO + 1;
 unsigned int nfds = virGetListenFDs();
 
 if (config-unix_sock_group) {
 if (virGetGroupID(config-unix_sock_group, unix_sock_gid)  0)
-return -1;
+return ret;
 }
 
 if (nfds  (sock_path_ro ? 2 : 1)) {
 VIR_ERROR(_(Too many (%u) FDs passed from caller), nfds);
-return -1;
+return ret;
 }
 
 if (virStrToLong_i(config-unix_sock_ro_perms, NULL, 8, 
unix_sock_ro_mask) != 0) {
 VIR_ERROR(_(Failed to parse mode '%s'), config-unix_sock_ro_perms);
-goto error;
+goto cleanup;
 }
 
 if (virStrToLong_i(config-unix_sock_admin_perms, NULL, 8, 
unix_sock_adm_mask) != 0) {
 VIR_ERROR(_(Failed to parse mode '%s'), 
config-unix_sock_admin_perms);
-goto error;
+goto cleanup;
 }
 
 if (virStrToLong_i(config-unix_sock_rw_perms, NULL, 8, 
unix_sock_rw_mask) != 0) {
 VIR_ERROR(_(Failed to parse mode '%s'), config-unix_sock_rw_perms);
-goto error;
+goto cleanup;
 }
 
 if (!(svc = virNetServerServiceNewFDOrUNIX(sock_path,
@@ -495,7 +496,7 @@ daemonSetupNetworking(virNetServerPtr srv,
config-max_queued_clients,
config-max_client_requests,
nfds, cur_fd)))
-goto error;
+goto cleanup;
 if (sock_path_ro) {
 if (!(svcRO = virNetServerServiceNewFDOrUNIX(sock_path_ro,
  unix_sock_ro_mask,
@@ -508,18 +509,18 @@ daemonSetupNetworking(virNetServerPtr srv,
  
config-max_queued_clients,
  
config-max_client_requests,
  nfds, cur_fd)))
-goto error;
+goto cleanup;
 }
 
 if (virNetServerAddService(srv, svc,
config-mdns_adv  !ipsock ?
_libvirt._tcp :
NULL)  0)
-goto error;
+goto cleanup;
 
 if (svcRO 
 virNetServerAddService(srv, svcRO, NULL)  0)
-goto error;
+goto cleanup;
 
 if (sock_path_adm) {
 VIR_DEBUG(Registering unix socket %s, sock_path_adm);
@@ -533,10 +534,10 @@ daemonSetupNetworking(virNetServerPtr srv,
   true,
   
config-admin_max_queued_clients,
   
config-admin_max_client_requests)))
-goto error;
+goto cleanup;
 
 if (virNetServerAddService(srvAdm, svcAdm, NULL)  0)
-goto error;
+goto cleanup;
 }
 
 if (ipsock) {
@@ -553,11 +554,11 @@ daemonSetupNetworking(virNetServerPtr srv,
  false,
  
config-max_queued_clients,
  
config-max_client_requests)))
-goto error;
+goto cleanup;
 
 if (virNetServerAddService(srv, svcTCP,
config-mdns_adv ? _libvirt._tcp : 
NULL)  0)
-goto error;
+goto cleanup;
 }
 
 #if WITH_GNUTLS
@@ -574,14 +575,14 @@ daemonSetupNetworking(virNetServerPtr srv,
(const char 
*const*)config-tls_allowed_dn_list,

config-tls_no_sanity_certificate ? false : true,

config-tls_no_verify_certificate ? false : true)))
-goto error;
+goto cleanup;
 } else {
 if (!(ctxt = virNetTLSContextNewServerPath(NULL,
  

[libvirt] [PATCH 4/4] virNetServerServiceClose: Don't leak sockets

2015-06-18 Thread Michal Privoznik
Well, if a server is being destructed, all underlying services and
their sockets should disappear with it. But due to bug in our
implementation this is not the case. Yes, we are closing the sockets,
but that's not enough. We must also:

1) Unregister them from the event loop
2) Unref the service for each socket

The last step is needed, because each socket callback holds a
reference to the service object. Since in the first step we are
unregistering the callbacks, they no longer need the reference.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/rpc/virnetserverservice.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index 4df26cb..3b35fc0 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -520,6 +520,9 @@ void virNetServerServiceClose(virNetServerServicePtr svc)
 if (!svc)
 return;
 
-for (i = 0; i  svc-nsocks; i++)
+for (i = 0; i  svc-nsocks; i++) {
+virNetSocketRemoveIOCallback(svc-socks[i]);
 virNetSocketClose(svc-socks[i]);
+virObjectUnref(svc);
+}
 }
-- 
2.3.6

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


[libvirt] [PATCH 0/4] A cople of memleak fixes

2015-06-18 Thread Michal Privoznik
One thing that I still haven't figured out is, when the daemon is waiting in
the even loop, in poll() and SIGINT comes, it jumps out and cleanup phase
begins. However, the event loop may still be the one and only holding a
reference to some objects, so we kind of want it to at least remove stale
handlers. Well, they will be deleted after the cleanup work in the daemon is
done. Yeah, messy and probably not worth breaking up the code into smaller
pieces just to cleanup deleted handles.

Michal Privoznik (4):
  daemonSetupNetworking: Don't leak services
  virNetSocketRemoveIOCallback: Be explicit about unref
  virNetSocket: Fix @watch corner case
  virNetServerServiceClose: Don't leak sockets

 daemon/libvirtd.c | 47 ++-
 src/rpc/virnetserverservice.c |  5 -
 src/rpc/virnetsocket.c| 11 ++
 3 files changed, 35 insertions(+), 28 deletions(-)

-- 
2.3.6

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


[libvirt] [PATCH 2/4] virNetSocketRemoveIOCallback: Be explicit about unref

2015-06-18 Thread Michal Privoznik
When going through the code I've notice that
virNetSocketAddIOCallback() increases the reference counter of
@socket. However, its counter part RemoveIOCallback does not. It took
me a while to realize this disproportion. The AddIOCallback registers
our own callback which eventually calls the desired callback and then
unref the @sock. Yeah, a bit complicated but it works. So, lets note
this hard learned fact in a comment in RemoveIOCallback().

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/rpc/virnetsocket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 2497f67..81903e7 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1993,6 +1993,7 @@ void virNetSocketRemoveIOCallback(virNetSocketPtr sock)
 }
 
 virEventRemoveHandle(sock-watch);
+/* Don't unref @sock, it's done via evenloop callback. */
 
 virObjectUnlock(sock);
 }
-- 
2.3.6

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


Re: [libvirt] [RFC] qemu: Redesigning guest CPU configuration

2015-06-18 Thread Daniel P. Berrange
On Wed, Jun 17, 2015 at 05:37:42PM +0200, Jiri Denemark wrote:
 Hi all (and sorry for the long email),
 
 The current way QEMU driver handles guest CPU configuration is not
 ideal. We detect host CPU capabilities only by querying the CPU and we
 don't check with QEMU what features it supports. We don't check QEMU's
 definitions of CPU models, which may be different from libvirt's
 definitions. All this results in several issues:
 
 - guest CPU may change druing migration, save/restore
 - libvirt may ask for a CPU which QEMU cannot provide; the guest will
   see a slightly different CPU but libvirt client won't know about it
 - libvirt may come up with a CPU that doesn't make sense and which won't
   work for a guest (the guest may even crash)
 
 Although usually everything just works, it is very fragile.

A third issue is that if there is no cpu in the guest config, we
just delegate CPU choice to QEMU and then ignore any CPU checks when
migrating. If libvirt owns the full CPU config, we'd probably want
to also decide the default ourselves, so that we will always be able
todo migrate CPU checks.

 Since we want to fix all these issues, we need to:
 - guarantee stable guest ABI (a single domain XML should always results
   in the same guest ABI). Once a domain is started, its CPU definition
   should never change (unless someone changes the XML, of course,
   similar to, e.g. PCI addresses). However, there are a few exceptions:
 - host-passthrough CPU mode will always result in -cpu host
 - host-model CPU mode should recompute the CPU model on every start,
   but the CPU must not change during migration
 - always make sure QEMU provides the CPU we asked for. Starting a domain
   should fail in case QEMU cannot provide exactly the CPU we asked for.
 - provide usable host-model mode and custom mode with minimum match. We
   need to generate CPU configurations that actually work, i.e., we need
   to ask QEMU what CPU it can provide on current host rather than
   requesting a bunch of features on top of a CPU model which does not
   always match the host CPU.
 
 QEMU already provides or will soon provide everything we need to meet
 these requirements:
 - we can cover every configurable part of a CPU in our cpu_map.xml and
   instead of asking QEMU for a specific CPU model we can use -cpu
   custom with a fully specified CPU
 - we can use the additional data about CPU models to choose the right
   one for a host CPU
 - when starting a domain we can check whether QEMU filtered out any of
   the features we asked for and refuse to start the domain
 - we can ask QEMU what would -cpu host look like and use that for
   host-model and minimum match CPUs (it won't work for TCG mode, though,
   but we can keep using the current CPUID detection code for TCG)

In TCG mode of course, 'host-model' and 'host-passthrough' are
effectively identical, and don't actually need the host to support
all the featues, since TCG is fully emulated. Which means that you
can migrated TCG guests to anyhost with any model :-) I wonder if
we are probably accidentally restricting that today, becuase we
assume KVM needs host support.

 Once we start maintaining CPU models with all the details, we will
 likely meet the same issues QEMU folks meet, i.e., we will need to fix
 bugs in existing CPU models. And it's not just about adding removing CPU
 features but also fixing other parameters, such as wrong level, etc.
 It's clear every change will require a new CPU model to be defined. But
 I think we should do it in a way that applications or users should not
 need (if they don't want to) to care about it. I'm thinking about doing
 something similar to machine types. Each CPU model could be defined in
 several versions and a CPU specs without a version would be an alias to
 the latest version.

Agreed, I think that versioning CPU models, independantly of machine
types makes sense. It is probably a little more complex - in most cases
we'd increase the version, but in some cases I think we'd end up wanting
to define new named models. For example, with the recent TSX scenario we
had, using versions would not have been appropriate, because Intel in
fact ship 2 variants of the silicon. So even with with versioning, we
would still have wanted to introduce the noTSX variants of the models.

 The problem is, we need to maintain backward compatibility and we should
 avoid breaking existing domains (shouldn't we?) which just work even
 though their guest CPUs do not exactly match the domain XML definitions.

Yep breaking existing domains isn't too pleasant!

 So either we need to define all existing CPU models in all their
 variants used for various machine types and have a mapping between
 (model without a version, machine type) to a specific version of the
 model (which may be quite hard) or we need to be able to distinguish
 between an existing domain and a new domain with no CPU model version.
 While host-model and host-passthrough CPU modes 

Re: [libvirt] libvirt proxy

2015-06-18 Thread Daniel P. Berrange
On Thu, Jun 18, 2015 at 01:20:59PM +0300, Vasiliy Tolstov wrote:
 2015-06-18 12:01 GMT+03:00 Daniel P. Berrange berra...@redhat.com:
  FWIW, we really consider the RPC protocol to be a private impl detail
  and strongly discourage people from trying to re-implement it
  themselves.
 
  Can you explain a bit more about what you're trying to achieve by
  creating an RPC proxy
 
 
 I have DO like cloud hosting and want to provide libvirt compat
 endpoint to create/start/stop servers.

I really discourage anyone from trying to fit the libvirt API over a
cloud system.

In general the libvirt API is designed to be fairly host-centric. We did
once have a libvirt driver that targetted a cloud system, but we deleted
it because the libvirt API was a bad fit for case where the concept of
individual hosts is hidden from the user.

I'd really recommend that any cloud hosting exposes one or more of the
common cloud APIs, in particular EC2 compatible API, and possibly an
OpenStack compatible API.

There are also some projects that attempt to provide a cloud API
abstraction layer, in the same way that libvirt provides a host
API abstraction layer. eg libcloud, or the (now defunct IIRC)
deltacloud.

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 v1] update sheepdog client] update sheepdog client path

2015-06-18 Thread Vasiliy Tolstov
Nnever sheepdog versions have dog client binary
while old have collie. Check them both.

Signed-off-by: Vasiliy Tolstov v.tols...@selfip.ru
---
 configure.ac   | 10 +-
 src/storage/storage_backend_sheepdog.c | 12 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index 408ee11..93f9e38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1959,14 +1959,14 @@ AC_SUBST([LIBRBD_LIBS])
 
 if test $with_storage_sheepdog = yes ||
test $with_storage_sheepdog = check; then
-  AC_PATH_PROG([COLLIE], [collie], [], [$PATH:/sbin:/usr/sbin])
+  AC_PATH_PROGS([SHEEPDOGCLI], [collie dog], [], [$PATH:/sbin:/usr/sbin])
 
   if test $with_storage_sheepdog = yes; then
-if test -z $COLLIE; then
-  AC_MSG_ERROR([We need collie for Sheepdog storage driver])
+if test -z $SHEEPDOGCLI; then
+  AC_MSG_ERROR([We need sheepdog client for Sheepdog storage driver])
 fi
   else
-if test -z $COLLIE; then
+if test -z $SHEEPDOGCLI; then
   with_storage_sheepdog=no
 fi
 
@@ -1978,7 +1978,7 @@ if test $with_storage_sheepdog = yes ||
   if test $with_storage_sheepdog = yes; then
 AC_DEFINE_UNQUOTED([WITH_STORAGE_SHEEPDOG], 1,
   [whether Sheepdog backend for storage driver is enabled])
-AC_DEFINE_UNQUOTED([COLLIE],[$COLLIE],[Location of collie program])
+AC_DEFINE_UNQUOTED([SHEEPDOGCLI],[$SHEEPDOGCLI],[Location of sheepdog 
client program])
   fi
 fi
 AM_CONDITIONAL([WITH_STORAGE_SHEEPDOG],
diff --git a/src/storage/storage_backend_sheepdog.c 
b/src/storage/storage_backend_sheepdog.c
index af15c3b..69ba7836 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -150,7 +150,7 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 char **cells = NULL;
 size_t i;
 
-virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, list, -r, 
NULL);
+virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, vdi, list, -r, 
NULL);
 virStorageBackendSheepdogAddHostArg(cmd, pool);
 virCommandSetOutputBuffer(cmd, output);
 if (virCommandRun(cmd, NULL)  0)
@@ -195,7 +195,7 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 char *output = NULL;
 virCommandPtr cmd;
 
-cmd = virCommandNewArgList(COLLIE, node, info, -r, NULL);
+cmd = virCommandNewArgList(SHEEPDOGCLI, node, info, -r, NULL);
 virStorageBackendSheepdogAddHostArg(cmd, pool);
 virCommandSetOutputBuffer(cmd, output);
 if (virCommandRun(cmd, NULL)  0)
@@ -221,7 +221,7 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 virCheckFlags(0, -1);
 
-virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, delete, 
vol-name, NULL);
+virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, vdi, delete, 
vol-name, NULL);
 virStorageBackendSheepdogAddHostArg(cmd, pool);
 int ret = virCommandRun(cmd, NULL);
 
@@ -273,7 +273,7 @@ virStorageBackendSheepdogBuildVol(virConnectPtr conn,
 goto cleanup;
 }
 
-cmd = virCommandNewArgList(COLLIE, vdi, create, vol-name, NULL);
+cmd = virCommandNewArgList(SHEEPDOGCLI, vdi, create, vol-name, NULL);
 virCommandAddArgFormat(cmd, %llu, vol-target.capacity);
 virStorageBackendSheepdogAddHostArg(cmd, pool);
 if (virCommandRun(cmd, NULL)  0)
@@ -358,7 +358,7 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 int ret;
 char *output = NULL;
 
-virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, list, vol-name, 
-r, NULL);
+virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, vdi, list, 
vol-name, -r, NULL);
 virStorageBackendSheepdogAddHostArg(cmd, pool);
 virCommandSetOutputBuffer(cmd, output);
 ret = virCommandRun(cmd, NULL);
@@ -394,7 +394,7 @@ virStorageBackendSheepdogResizeVol(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 virCheckFlags(0, -1);
 
-virCommandPtr cmd = virCommandNewArgList(COLLIE, vdi, resize, 
vol-name, NULL);
+virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, vdi, resize, 
vol-name, NULL);
 virCommandAddArgFormat(cmd, %llu, capacity);
 virStorageBackendSheepdogAddHostArg(cmd, pool);
 int ret = virCommandRun(cmd, NULL);
-- 
2.3.3

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


Re: [libvirt] [PATCH 02/13] conf: Introduce helper to help getting correct def for getter functions

2015-06-18 Thread Peter Krempa
On Wed, Jun 17, 2015 at 16:53:14 -0400, John Ferlan wrote:
 
 
 On 06/15/2015 03:47 PM, Peter Krempa wrote:
  virDomainObjGetOneDef will help to retrieve the correct definition
  pointer from @vm in cases where VIR_DOMAIN_AFFECT_LIVE and
  VIR_DOMAIN_AFFECT_CONFIG are mutually exclusive. The function simply
  returns the correct pointer. This similarly to virDomainObjGetDefs will
  greatly simplify the code.
  ---
   src/conf/domain_conf.c   | 36 
   src/conf/domain_conf.h   |  1 +
   src/libvirt_private.syms |  1 +
   3 files changed, 38 insertions(+)
  
 
 I dunno - I just have this little voice asking is there some corner case
 from the prior virDomainLiveConfigHelperMethod thruough possibly
 creating 'newDef' in virDomainObjSetDefTransient that this new code will
 miss...  IOW - is there a condition where CONFIG is wanted, domain is
 running, but newDef is NULL.

Well, up until now I've inspected the qemu driver and the test driver
and both set the transient def upon start of a VM, so we should be fine
using this in those.

Honestly I think that all drivers should be fixed and
virDomainLiveConfigHelperMethod should be killed with fire forever.

 
 OTOH - one wonders if the previous code went through the patch into
 virDomainObjSetDefTransient
 
 All the callers check for NULL, so that's not an issue - just
 considering some strange corner case for a transient domain.
 
  diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
  index 3cc182b..35e1cb4 100644
  --- a/src/conf/domain_conf.c
  +++ b/src/conf/domain_conf.c
  @@ -2948,6 +2948,42 @@ virDomainObjGetDefs(virDomainObjPtr vm,
   }
  
  
  +/**
  + * virDomainObjGetOneDef:
  + *
  + * @vm: Domain object
  + * @flags: for virDomainModificationImpact
  + *
  + * Helper function to resolve @flags and return the correct domain pointer
  + * object. This function returns one of @vm-def or @vm-persistentDef
  + * according to @flags. This helper should be used only in APIs that 
  guarantee
  + * that @flags contains exactly one of VIR_DOMAIN_AFFECT_LIVE,
 
 s/,/ or/
 
  + * VIR_DOMAIN_AFFECT_CONFIG.
 
 and to be really redundant ;-) - couldn't resist.
 
 s/./ and not both./
 
 
 ACK - although GetOneDef isn't my favorite, I don't have a better
 suggestion either.

Me neither but that can be changed in the future.

 
 John

I've pushed this series with the suggested changes and I'll follow up
with fixes to the endjob label and the double space.

Thanks.

Peter


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

Re: [libvirt] [PATCH v4 0/9] Selective block device migration implementation

2015-06-18 Thread Kashyap Chamarthy
On Thu, Jun 18, 2015 at 02:25:08PM +0200, Jiri Denemark wrote:
 On Wed, Jun 17, 2015 at 17:31:03 +0200, Kashyap Chamarthy wrote:
  On Tue, Jun 16, 2015 at 01:42:02AM +0300, Pavel Boldin wrote:
 ...
  libvirtd debug log[1] from source (destination log is empty)):
  
  [. . .]
  2015-06-17 15:13:53.317+: 781: debug : 
  virDomainMigratePerform3Params:5202 : dom=0x7f2118f13c40, (VM: name=cvm1, 
  uuid=ab4c412b-6fdc-4fc4-b78c-f1d49db10d4e), 
  dconnuri=qemu+tcp://root@devstack3/system, params=0x7f2118f12a90, 
  nparams=1, cookiein=(nil), cookieinlen=0, cookieout=0x7f2106f38ba8, 
  cookieoutlen=0x7f2106f38ba4, flags=3
  2015-06-17 15:13:53.317+: 781: debug : 
  virDomainMigratePerform3Params:5203 : params[migrate_disks]=(string)vdb
  2015-06-17 15:13:53.317+: 781: debug : qemuMigrationPerform:5238 : 
  driver=0x7f20f416b840, conn=0x7f20dc005c30, vm=0x7f20f41e9640, 
  xmlin=null, dconnuri=qemu+tcp://root@devstack3/system, uri=null, 
  graphicsuri=null, listenAddress=null, nmigrate_disks=1, 
  migrate_disks=0x7f2118f13930, cookiein=null, cookieinlen=0, 
  cookieout=0x7f2106f38ba8, cookieoutlen=0x7f2106f38ba4, flags=3, 
  dname=null, resource=0, v3proto=1
  2015-06-17 15:13:53.317+: 781: debug : 
  qemuDomainObjBeginJobInternal:1397 : Starting async job: none 
  (async=migration out vm=0x7f20f41e9640 name=cvm1)
  2015-06-17 15:13:53.317+: 781: debug : 
  qemuDomainObjBeginJobInternal:1414 : Waiting for async job 
  (vm=0x7f20f41e9640 name=cvm1)
  2015-06-17 15:13:53.821+: 782: debug : virThreadJobSet:96 : Thread 782 
  (virNetServerHandleJob) is now running job remoteDispatchDomainGetJobInfo
  2015-06-17 15:13:53.821+: 782: debug : virDomainGetJobInfo:8808 : 
  dom=0x7f20dc008c30, (VM: name=cvm1, 
  uuid=ab4c412b-6fdc-4fc4-b78c-f1d49db10d4e), info=0x7f2106737b50
  2015-06-17 15:13:53.821+: 782: debug : virThreadJobClear:121 : Thread 
  782 (virNetServerHandleJob) finished job remoteDispatchDomainGetJobInfo 
  with ret=0
  2015-06-17 15:13:54.325+: 780: debug : virThreadJobSet:96 : Thread 780 
  (virNetServerHandleJob) is now running job remoteDispatchDomainGetJobInfo
  2015-06-17 15:13:54.325+: 780: debug : virDomainGetJobInfo:8808 : 
  dom=0x7f20dc008c30, (VM: name=cvm1, 
  uuid=ab4c412b-6fdc-4fc4-b78c-f1d49db10d4e), info=0x7f2107739b50
  2015-06-17 15:13:54.325+: 780: debug : virThreadJobClear:121 : Thread 
  780 (virNetServerHandleJob) finished job remoteDispatchDomainGetJobInfo 
  with ret=0
  [. . .]
  remoteDispatchDomainMigratePerform3Params, 784 
  remoteDispatchDomainMigratePerform3Params) for (520s, 520s)
  2015-06-17 15:14:23.320+: 781: error : 
  qemuDomainObjBeginJobInternal:1492 : Timed out during operation: cannot 
  acquire state change lock (held by 
  remoteDispatchDomainMigratePerform3Params)
  2015-06-17 15:14:23.320+: 781: debug : virThreadJobClear:121 : Thread 
  781 (virNetServerHandleJob) finished job 
  remoteDispatchDomainMigratePerform3Params with ret=-1
  2015-06-17 15:14:23.320+: 783: debug : virThreadJobSet:96 : Thread 783 
  (virNetServerHandleJob) is now running job remoteDispatchConnectClose
  2015-06-17 15:14:23.320+: 783: debug : virThreadJobClear:121 : Thread 
  783 (virNetServerHandleJob) finished job remoteDispatchConnectClose with 
  ret=0
  
  
  How can I mitigate this? (I realize this is not due to these patches,
  proably something with my test environment.)
  
  Since this is non-shared storage migration, I tried to supply
  '--copy-storage-inc' to no avail (same error as above).
  
  Probably I should test by building local RPMs.
  
  [1] 
  https://kashyapc.fedorapeople.org/virt/temp/libvirtd-log-selective-blockdev-failed.log
 
 Could you upload a complete log somewhere? It seems a previously started
 migration is waiting for a response from QEMU. Or alternatively, it
 failed to release the jobs. I'd like to see the logs from the previous
 migration attempt.

I'm afraid, too late -- I blew that environment away and re-created
libvirt RPMs. This time, with Michal's branch from here, which also has
the additional diff he posted in his review:

https://github.com/zippy2/libvirt/tree/storage_migration2

I did a preliminary test and it seems to have worked:

On source:

$ virsh domblklist cvm1 Target Source

vda/var/lib/libvirt/images/cirros-0.3.3-x86_64-disk.img
vdb/export/disk2.img

$ virsh migrate --verbose --p2p --copy-storage-inc \
--migratedisks vda  --live cvm1 qemu+tcp://root@devstack3/system
Migration: [100 %]


On Dest:
---

Where vdb was already present.

$ virsh list
 IdName   State

 2 cvm1   running


$ virsh domblklist cvm1
Target Source

vda/var/lib/libvirt/images/cirros-0.3.3-x86_64-disk.img
vdb

Re: [libvirt] [PATCH v4 0/9] Selective block device migration implementation

2015-06-18 Thread Michal Privoznik
On 16.06.2015 17:47, Michal Privoznik wrote:
 On 16.06.2015 00:42, Pavel Boldin wrote:
 Behold of the fourth re-roll of the selective block device migration patch.
 In this patch we don't only fix the issue with NBD migration format auto-
 detection but also introduce the patches that do enhance the NBD migration
 triggered by `drive-mirror` QEMU command with ability for the user to select
 what disks are to be migrated based on the target name.

 First two patches fix some nitpicks, third one fixes the issue with NBD 
 format
 auto-detection.

 Middle ones introduce a necessary API to keep a list of block devices to
 migrate in the virTypedParameter array and to work with this list.

 Of the two last patches first introduces the `migrate_disks' qemuMigration*
 parameter and pushes it down the call stack making the code to consult it 
 when
 there is a decision to be made whether the block device is to be migrated to
 the new host. When there is no `migrate_disks' parameter given then the old
 scheme is used: only non-shared non-readonly disks with a source are 
 migrated.

 The last patch promotes this ability up to the virsh utility and documents
 it as appropriate.

 Michal Privoznik (3):
   virDomainDiskGetSource: Mark passed disk as 'const'
   qemuMigrationBeginPhase: Fix function header indentation
   qemuMigrationDriveMirror: Force raw format for NBD

 Pavel Boldin (6):
   util: multi-value virTypedParameter
   util: multi-value parameters in virTypedParamsAdd*
   util: virTypedParams{Filter,GetAllStrings}
   util: add virTypedParamsAddStringList
   qemu: migration: selective block device migration
   virsh: selective block device migration

  include/libvirt/libvirt-domain.h |   9 ++
  include/libvirt/libvirt-host.h   |  11 ++
  src/conf/domain_conf.c   |   2 +-
  src/conf/domain_conf.h   |   2 +-
  src/libvirt_public.syms  |   6 +
  src/qemu/qemu_driver.c   |  78 ---
  src/qemu/qemu_migration.c| 264 +--
  src/qemu/qemu_migration.h|  24 ++--
  src/util/virtypedparam.c | 259 +++---
  src/util/virtypedparam.h |  19 +++
  tests/Makefile.am|   6 +
  tests/virtypedparamtest.c| 295 
 +++
  tools/virsh-domain.c |  23 +++
  tools/virsh.pod  |  21 +--
  14 files changed, 854 insertions(+), 165 deletions(-)
  create mode 100644 tests/virtypedparamtest.c

 


 I have it squashed into the corresponding commits. So with this - you
 have my ACK, although it feels a bit weird to ACK my own patches.
 Therefore, I'm giving others some time before merging this to express
 their feelings.
 

This is now pushed. Let me congratulate Pavel on his first libvirt
contribution. And what a contribution it was!

Michal


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


Re: [libvirt] Socket files in virt-aa-helper

2015-06-18 Thread Michał Dubiel
Thanks Jamie for the explanation.

Regards,
Michal

On 16 June 2015 at 17:15, Jamie Strandboge ja...@canonical.com wrote:

 On 06/16/2015 08:40 AM, Michał Dubiel wrote:
  Hi all,
 
  May I kindly ask someone for some advice on this topic?
 
  Regards,
  Michal
 
  On 21 May 2015 at 20:23, Michał Dubiel m...@semihalf.com
  mailto:m...@semihalf.com wrote:
 
  Hi guys,
 
  I have got a question. I need to add apparmor support for vhost-user
 socket
  files used to communicate with the vhost-user server app. Those ones
 defined
  with something like:
  interface type='vhostuser'
mac address='02:ed:f3:5d:de:f3'/
source type='unix'
 path='/var/run/vrouter/uvh_vif_tapa8396c51-2a'
  mode='client'/
model type='virtio'/
address type='pci' domain='0x' bus='0x00' slot='0x03'
  function='0x0'/
  /interface
 
  I added something like this into get_files() function in
 virt-aa-helper.c:
  for (i = 0; i  ctl-def-nnets; i++) {
  if (ctl-def-nets[i] 
  ctl-def-nets[i]-type ==
 VIR_DOMAIN_NET_TYPE_VHOSTUSER 
  ctl-def-nets[i]-data.vhostuser) {
  virDomainChrSourceDefPtr vhu =
 ctl-def-nets[i]-data.vhostuser;
 
  if (vah_add_file_chardev(buf, vhu-data.nix.path, rw,
 vhu-type) != 0)
  goto cleanup;
  }
  }
 
  However, there is a restriction for the socket file types in
 valid_path()
  function:
  switch (sb.st_mode  S_IFMT) {
  case S_IFSOCK:
  return 1;
  break;
  default:
  break;
  }
  That prevents this from working.
 
  May I ask why the socket file types are restricted? Vhost-user uses
 sockets
  so if I want to use apparmor virt-aa-helper has to be able to add
 the line
  for the socket file into /etc/apparmor.d/libvirt/libvirt-UUID.files.
 

 They are restricted only because at the time virt-aa-helper.c was written
 there
 wasn't a valid use for them. There were more checks in this part of the
 code but
 over the years as more valid types were added to libvirt, they've been
 removed
 and now we are left with just this one. Since there is now a valid usecase
 for
 S_IFSOCK, it seems this can simply be removed.

 --
 Jamie Strandboge http://www.ubuntu.com/


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

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

Re: [libvirt] Socket files in virt-aa-helper

2015-06-18 Thread Serge Hallyn
Quoting Jamie Strandboge (ja...@canonical.com):
 On 06/16/2015 08:40 AM, Michał Dubiel wrote:
  Hi all,
  
  May I kindly ask someone for some advice on this topic?
  
  Regards,
  Michal
  
  On 21 May 2015 at 20:23, Michał Dubiel m...@semihalf.com
  mailto:m...@semihalf.com wrote:
  
  Hi guys,
  
  I have got a question. I need to add apparmor support for vhost-user 
  socket
  files used to communicate with the vhost-user server app. Those ones 
  defined
  with something like:
  interface type='vhostuser'
mac address='02:ed:f3:5d:de:f3'/
source type='unix' path='/var/run/vrouter/uvh_vif_tapa8396c51-2a'
  mode='client'/
model type='virtio'/
address type='pci' domain='0x' bus='0x00' slot='0x03'
  function='0x0'/
  /interface
  
  I added something like this into get_files() function in 
  virt-aa-helper.c:
  for (i = 0; i  ctl-def-nnets; i++) {
  if (ctl-def-nets[i] 
  ctl-def-nets[i]-type == 
  VIR_DOMAIN_NET_TYPE_VHOSTUSER 
  ctl-def-nets[i]-data.vhostuser) {
  virDomainChrSourceDefPtr vhu = 
  ctl-def-nets[i]-data.vhostuser;
  
  if (vah_add_file_chardev(buf, vhu-data.nix.path, rw,
 vhu-type) != 0)
  goto cleanup;
  }
  }
  
  However, there is a restriction for the socket file types in 
  valid_path()
  function:
  switch (sb.st_mode  S_IFMT) {
  case S_IFSOCK:
  return 1;
  break;
  default:
  break;
  }
  That prevents this from working.
  
  May I ask why the socket file types are restricted? Vhost-user uses 
  sockets
  so if I want to use apparmor virt-aa-helper has to be able to add the 
  line
  for the socket file into /etc/apparmor.d/libvirt/libvirt-UUID.files.
  
 
 They are restricted only because at the time virt-aa-helper.c was written 
 there
 wasn't a valid use for them. There were more checks in this part of the code 
 but
 over the years as more valid types were added to libvirt, they've been removed
 and now we are left with just this one. Since there is now a valid usecase for
 S_IFSOCK, it seems this can simply be removed.

So something like

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 35423b5..bb5e449 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -592,18 +592,8 @@ valid_path(const char *path, const bool readonly)
 
 if (!virFileExists(path)) {
 vah_warning(_(path does not exist, skipping file type checks));
-} else {
-if (stat(path, sb) == -1)
-return -1;
-
-switch (sb.st_mode  S_IFMT) {
-case S_IFSOCK:
-return 1;
-break;
-default:
-break;
-}
-}
+} else if (stat(path, sb) == -1)
+return -1;
 
 opaths = sizeof(override)/sizeof(*(override));
 
should be ok?

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

Re: [libvirt] [PATCH 2/5] Print SCSI logical unit as unsigned integer

2015-06-18 Thread John Ferlan


On 06/16/2015 11:29 PM, Eric Farman wrote:
 The logical unit field is an unsigned integer, we should
 use the appropriate substitution when printing it.
 
 Signed-off-by: Eric Farman far...@linux.vnet.ibm.com
 ---
  src/conf/domain_audit.c | 2 +-
  src/conf/domain_conf.c  | 2 +-
  src/qemu/qemu_hotplug.c | 4 ++--
  src/util/virhostdev.c   | 6 +++---
  src/util/virscsi.c  | 6 +++---
  tools/virsh-domain.c| 2 +-
  6 files changed, 11 insertions(+), 11 deletions(-)
 

Similar to 1/5 - why only adjust unit, adjust bus  target too since
they're incorrect.  I will adjust when I push (and change commit message
to reflect that).

John
 diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
 index 1900039..c94cae8 100644
 --- a/src/conf/domain_audit.c
 +++ b/src/conf/domain_audit.c
 @@ -427,7 +427,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
 virDomainHostdevDefPtr hostdev,
  } else {
  virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
  scsisrc-u.host;
 -if (virAsprintfQuiet(address, %s:%d:%d:%d,
 +if (virAsprintfQuiet(address, %s:%d:%d:%u,
   scsihostsrc-adapter, scsihostsrc-bus,
   scsihostsrc-target,
   scsihostsrc-unit)  0) {
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 9e77b87..7e3ca36 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -18940,7 +18940,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
  virBufferAsprintf(buf, adapter name='%s'/\n,
scsihostsrc-adapter);
  virBufferAsprintf(buf,
 -  address %sbus='%d' target='%d' 
 unit='%d'/\n,
 +  address %sbus='%d' target='%d' 
 unit='%u'/\n,
includeTypeInAddr ? type='scsi'  : ,
scsihostsrc-bus, scsihostsrc-target,
scsihostsrc-unit);
 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
 index cc86a3b..1d538a0 100644
 --- a/src/qemu/qemu_hotplug.c
 +++ b/src/qemu/qemu_hotplug.c
 @@ -1938,7 +1938,7 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
  } else {
  virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host;
  virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(Unable to prepare scsi hostdev: %s:%d:%d:%d),
 +   _(Unable to prepare scsi hostdev: %s:%d:%d:%u),
 scsihostsrc-adapter, scsihostsrc-bus,
 scsihostsrc-target, scsihostsrc-unit);
  }
 @@ -3873,7 +3873,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
   virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
   scsisrc-u.host;
   virReportError(VIR_ERR_OPERATION_FAILED,
 -_(host scsi device %s:%d:%d.%d not found),
 +_(host scsi device %s:%d:%d.%u not found),
  scsihostsrc-adapter, scsihostsrc-bus,
  scsihostsrc-target, scsihostsrc-unit);
  }
 diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
 index 1c8f31e..ea0076c 100644
 --- a/src/util/virhostdev.c
 +++ b/src/util/virhostdev.c
 @@ -1482,7 +1482,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr 
 hostdev_mgr,
scsihostsrc-adapter, scsihostsrc-bus,
scsihostsrc-target, scsihostsrc-unit,
hostdev-readonly, hostdev-shareable))) {
 -VIR_WARN(Unable to reattach SCSI device %s:%d:%d:%d on domain %s,
 +VIR_WARN(Unable to reattach SCSI device %s:%d:%d:%u on domain %s,
   scsihostsrc-adapter, scsihostsrc-bus, scsihostsrc-target,
   scsihostsrc-unit, dom_name);
  return;
 @@ -1492,7 +1492,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr 
 hostdev_mgr,
   * because qemuProcessStart could fail half way through. */
  
  if (!(tmp = virSCSIDeviceListFind(hostdev_mgr-activeSCSIHostdevs, 
 scsi))) {
 -VIR_WARN(Unable to find device %s:%d:%d:%d 
 +VIR_WARN(Unable to find device %s:%d:%d:%u 
   in list of active SCSI devices,
   scsihostsrc-adapter, scsihostsrc-bus,
   scsihostsrc-target, scsihostsrc-unit);
 @@ -1500,7 +1500,7 @@ virHostdevReAttachSCSIHostDevices(virHostdevManagerPtr 
 hostdev_mgr,
  return;
  }
  
 -VIR_DEBUG(Removing %s:%d:%d:%d dom=%s from activeSCSIHostdevs,
 +VIR_DEBUG(Removing %s:%d:%d:%u dom=%s from activeSCSIHostdevs,
 scsihostsrc-adapter, scsihostsrc-bus, scsihostsrc-target,
 scsihostsrc-unit, dom_name);
  
 diff --git 

Re: [libvirt] [PATCH v4 0/9] Selective block device migration implementation

2015-06-18 Thread Kashyap Chamarthy
On Wed, Jun 17, 2015 at 05:31:03PM +0200, Kashyap Chamarthy wrote:
 On Tue, Jun 16, 2015 at 01:42:02AM +0300, Pavel Boldin wrote:
 
 [. . .]
 
  Michal Privoznik (3):
virDomainDiskGetSource: Mark passed disk as 'const'
qemuMigrationBeginPhase: Fix function header indentation
qemuMigrationDriveMirror: Force raw format for NBD
  
  Pavel Boldin (6):
util: multi-value virTypedParameter
util: multi-value parameters in virTypedParamsAdd*
util: virTypedParams{Filter,GetAllStrings}
util: add virTypedParamsAddStringList
qemu: migration: selective block device migration
virsh: selective block device migration
  
   include/libvirt/libvirt-domain.h |   9 ++
   include/libvirt/libvirt-host.h   |  11 ++
   src/conf/domain_conf.c   |   2 +-
   src/conf/domain_conf.h   |   2 +-
   src/libvirt_public.syms  |   6 +
   src/qemu/qemu_driver.c   |  78 ---
   src/qemu/qemu_migration.c| 264 +--
   src/qemu/qemu_migration.h|  24 ++--
   src/util/virtypedparam.c | 259 +++---
   src/util/virtypedparam.h |  19 +++
   tests/Makefile.am|   6 +
   tests/virtypedparamtest.c| 295 
  +++
   tools/virsh-domain.c |  23 +++
   tools/virsh.pod  |  21 +--
   14 files changed, 854 insertions(+), 165 deletions(-)
   create mode 100644 tests/virtypedparamtest.c
  
 
 New test with this revision of patches applied.
 
 
[. . .]

 Probably I should test by building local RPMs.
 

I missed to apply the diff from Michal earlier. So, I tested again from
git, now that Michal pushed them already :-). With both
--copy-storage-all and --copy-storage-inc.

Simple test seems to work:

  - On source:
  
  $ virsh domblklist cvm1
  Target Source
  
  vda/var/lib/libvirt/images/cirros-0.3.3-x86_64-disk.img
  vdb/export/disk2.img
  
  - On destination, /export/disk2.img (vdb) already exists.
  
  - On source, just migrate the 'vda' device:
  
  $ virsh migrate --verbose --p2p --copy-storage-all \
  --migrate-disks vda \
  --live cvm1 qemu+tcp://root@devstack3/system
  Migration: [100 %]
  
  - On source, cvm1 is down (as expected).
  
  - On destination, cvm1 is running, and both the block devices (vda,
vdb) are enumerated.


As a test 2, I tried the opposite:

- Ensured the vda
  (/var/lib/libvirt/images/cirros-0.3.3-x86_64-disk.img) is in place
  on destination.

- Did a `virsh pool-refresh` on both existing pools on destination,
  for good measure.
 
- This time, tried to migrate only 'vdb', and the result is the
  migrate command just hung on source:

$ virsh migrate --verbose --p2p --copy-storage-all \
--migrate-disks vdb \
--live cvm1 qemu+tcp://root@devstack3/system
# Here the command is just hung

- On destination, the guest is in paused state:
   
$ virsh list
 IdName   State

 15cvm1   paused

Here's the libvirt debug log from *destination*, if anyone is
interested:


https://kashyapc.fedorapeople.org/virt/temp/libvirtd-log-dst-hung-blockdev-mig.log

-- 
/kashyap

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


Re: [libvirt] [PATCH 1/5] Print SCSI logical unit as a positive integer

2015-06-18 Thread John Ferlan

On 06/16/2015 11:29 PM, Eric Farman wrote:
 A logical unit address is a positive integer, so it would be wise
 to reject any negative inputs.
 
 Signed-off-by: Eric Farman far...@linux.vnet.ibm.com
 ---
  src/conf/domain_conf.c | 2 +-
  tools/virsh-domain.c   | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
 

While we're at it - why not bus and target as well?  That's doable in
one patch...  I'll adjust this in order

Looking back through history it seems commit id '5c811dce' added this
functionality to domain.conf.c and commit id 'e962a579' added address
parsing for the attach-disk command.

I also peeked at the review cycle of the domain_conf.c change and found
my name, but at the time I didn't notice that we were expecting a non
negative value storing as unsigned, but printing as signed... In my
defense I was newer to the group though ;-)

So, I'll add bus/target as _uip and then adjust the title/commit message
appropriately when I push the changes.

John
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index ca55981..9e77b87 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -4954,7 +4954,7 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr 
 sourcenode,
  goto cleanup;
  }
  
 -if (virStrToLong_ui(unit, NULL, 0, scsihostsrc-unit)  0) {
 +if (virStrToLong_uip(unit, NULL, 0, scsihostsrc-unit)  0) 
 {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(cannot parse unit '%s'), unit);
  goto cleanup;
 diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
 index 4c47473..0bea462 100644
 --- a/tools/virsh-domain.c
 +++ b/tools/virsh-domain.c
 @@ -488,7 +488,7 @@ static int str2SCSIAddress(const char *str, struct 
 SCSIAddress *scsiAddr)
  return -1;
  
  unit++;
 -if (virStrToLong_ui(unit, NULL, 0, scsiAddr-unit) != 0)
 +if (virStrToLong_uip(unit, NULL, 0, scsiAddr-unit) != 0)
  return -1;
  
  return 0;
 

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


Re: [libvirt] [PATCH 0/5 v2] Corrections to SCSI logical unit handling

2015-06-18 Thread John Ferlan


On 06/16/2015 11:29 PM, Eric Farman wrote:
 While working with the hostdev tag and SCSI LUNs, a problem was
 discovered with the XML schema (see commit message in patch 4).
 This spawned some further corrections to the handling of the
 logical unit field throughout libvirt.
 
 This series was split from a single patch, from this feedback:
 http://www.redhat.com/archives/libvir-list/2015-June/msg00489.html
 
 Eric Farman (5):
   Print SCSI logical unit as a positive integer
   Print SCSI logical unit as unsigned integer
   Convert SCSI logical unit from int to long long
   docs: Fix XML schema handling of LUN address in hostdev tag
   docs: Correct typos in scsi hostdev and address elements
 
  docs/formatdomain.html.in | 10 +++---
  docs/schemas/domaincommon.rng | 14 --
  src/conf/domain_audit.c   |  2 +-
  src/conf/domain_conf.c|  4 ++--
  src/conf/domain_conf.h|  2 +-
  src/qemu/qemu_command.h   |  2 +-
  src/qemu/qemu_hotplug.c   |  4 ++--
  src/util/virhostdev.c |  6 +++---
  src/util/virscsi.c| 16 
  src/util/virscsi.h|  8 
  tests/testutilsqemu.c |  2 +-
  tools/virsh-domain.c  |  6 +++---
  12 files changed, 45 insertions(+), 31 deletions(-)
 


While I understand the comments regarding the step by step process -
consider bisection when/if something went wrong with these patches in
the future...  Was the issue because we parsed a positive integer?  Was
the issue because we printed an unsigned value?  Was the issue because
we printed a long long unsigned? Easier to revert something smaller and
rework, than have to go through everything.

I've made adjustments in my local branch to add the unsigned for bus and
target, as well as a couple of other adjustments I've noted. I'll give
it a day or so to percolate before pushing though just in case I've
missed something...

Tks -

John

John

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


Re: [libvirt] [PATCH 4/5] docs: Fix XML schema handling of LUN address in hostdev tag

2015-06-18 Thread John Ferlan


On 06/16/2015 11:29 PM, Eric Farman wrote:
 Defining a domain with a SCSI disk attached via a hostdev
 tag and a source address unit value longer than two digits
 causes an error when editing the domain with virsh edit,
 even if no changes are made to the domain definition.
 The error suggests invalid XML, somewhere:
 
   # virsh edit lmb_guest
   error: XML document failed to validate against schema:
   Unable to validate doc against /usr/local/share/libvirt/schemas/domain.rng
   Extra element devices in interleave
   Element domain failed to validate content
 
 The virt-xml-validate tool fails with a similar error:
 
   # virt-xml-validate lmb_guest.xml
   Relax-NG validity error : Extra element devices in interleave
   lmb_guest.xml:17: element devices: Relax-NG validity error :
   Element domain failed to validate content
   lmb_guest.xml fails to validate
 
 The hostdev tag requires a source address to be specified,
 which includes bus, target, and unit address attributes.
 According to the SCSI Architecture Model spec (section
 4.9 of SAM-2), a LUN address is 64 bits and thus could be
 up to 20 decimal digits long.  Unfortunately, the XML
 schema limits this string to just two digits.  Similarly,
 the target field can be up to 32 bits in length, which
 would be 10 decimal digits.
 
   # lsscsi -xx
   [0:0:19:0x40224011]  diskIBM  2107900  3.44 /dev/sda
   # lsscsi
   [0:0:19:1074872354]diskIBM  2107900  3.44  /dev/sda
   # cat lmb_guest.xml
   domain type='kvm'
 namelmb_guest/name
 memory unit='MiB'1024/memory
   ...trimmed...
 devices
   controller type='scsi' model='virtio-scsi' index='0'/
   hostdev mode='subsystem' type='scsi'
 source
   adapter name='scsi_host0'/
   address bus='0' target='19' unit='1074872354'/
 /source
   /hostdev
   ...trimmed...
 
 Since the reference unit and target fields are used in
 several places in the XML schema, create a separate one
 specific for SCSI Logical Units that will permit the
 greater length.  This permits both the validation utility
 and the virsh edit command to succeed when a hostdev
 tag is included.
 
 Signed-off-by: Eric Farman far...@linux.vnet.ibm.com
 Reviewed-by: Matthew Rosato mjros...@linux.vnet.ibm.com
 Reviewed-by: Stefan Zimmermann s...@linux.vnet.ibm.com
 Reviewed-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com
 ---
  docs/formatdomain.html.in |  6 +-
  docs/schemas/domaincommon.rng | 14 --
  2 files changed, 17 insertions(+), 3 deletions(-)
 


 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 4e85b51..c88c4a6 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -3256,7 +3256,11 @@
/dd
dtscsi/dt
ddSCSI devices are described by both the codeadapter/code
 -and codeaddress/code elements.
 +and codeaddress/code elements. The codeaddress/code
 +element includes a codebus/code attribute (a 2-digit bus
 +number), a codetarget/code attribute (a 10-digit target
 +number), and a codeunit/code attribute (a 20-digit unit
 +number on the bus).

Since we know from the v1 comments that qemu has a max of 16384 units, add:

Not all hypervisors support larger codeunit/code values. It is up to
each hypervisor to determine the maximum value supported for the adapter.

I could add codetarget/code and  if you think that would be better..


  p
  span class=sinceSince 1.2.8/span, the codesource/code
  element of a SCSI device may contain the codeprotocol/code
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index f0f7400..b3c5cb8 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -3833,10 +3833,10 @@
ref name=driveBus/
  /attribute
  attribute name=target
 -  ref name=driveTarget/
 +  ref name=driveSCSITarget/
  /attribute
  attribute name=unit
 -  ref name=driveUnit/
 +  ref name=driveSCSIUnit/
  /attribute
/define
define name=usbportaddress
 @@ -5129,11 +5129,21 @@
param name=pattern[0-9]{1,2}/param
  /data
/define
 +  define name=driveSCSITarget
 +data type=string
 +  param name=pattern[0-9]{1,10}/param
 +/data
 +  /define
define name=driveUnit
  data type=string
param name=pattern[0-9]{1,2}/param
  /data
/define
 +  define name=driveSCSIUnit
 +data type=string
 +  param name=pattern[0-9]{1,20}/param
 +/data
 +  /define
define name=featureName
  data type=string
param name='pattern'[a-zA-Z0-9\-_\.]+/param
 

I'm going to add a test as well. It's essentially a copy of
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-scsi.xml
adjusted to have a larger unit value (it'll be attached)

From 09518db2ddf099d6c2862899b1a71a6e58d4314d Mon Sep 17 00:00:00 2001
From: 

Re: [libvirt] [PATCH 5/5] docs: Correct typos in scsi hostdev and address elements

2015-06-18 Thread John Ferlan


On 06/16/2015 11:29 PM, Eric Farman wrote:
 The type='scsi' parameter of an address element is ignored
 if placed within a hostdev section, and rejected by the XML
 schema used by virt-xml-validate. Remove it from the doc,
 and correct a typo in the remaining address arguments.
 
 Signed-off-by: Eric Farman far...@linux.vnet.ibm.com
 Reviewed-by: Matthew Rosato mjros...@linux.vnet.ibm.com
 Reviewed-by: Stefan Zimmermann s...@linux.vnet.ibm.com
 Reviewed-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com
 ---
  docs/formatdomain.html.in | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 

ACK

John

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


Re: [libvirt] [PATCH 1/5] Print SCSI logical unit as a positive integer

2015-06-18 Thread Eric Farman



On 06/18/2015 12:47 PM, John Ferlan wrote:

On 06/16/2015 11:29 PM, Eric Farman wrote:

A logical unit address is a positive integer, so it would be wise
to reject any negative inputs.

Signed-off-by: Eric Farman far...@linux.vnet.ibm.com
---
  src/conf/domain_conf.c | 2 +-
  tools/virsh-domain.c   | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)


While we're at it - why not bus and target as well?  That's doable in
one patch...  I'll adjust this in order


The linux kernel/qemu code have some more confusion in how the bus and 
target are printed, but that's certainly not the responsibility of this 
patch series.  The LUN is always printed as %llu in the other layers of 
the stack, so I just wanted to get them in sync.


So yes, I'm fine with cleaning up bus and target in both these patches.

Thanks!

- Eric



Looking back through history it seems commit id '5c811dce' added this
functionality to domain.conf.c and commit id 'e962a579' added address
parsing for the attach-disk command.

I also peeked at the review cycle of the domain_conf.c change and found
my name, but at the time I didn't notice that we were expecting a non
negative value storing as unsigned, but printing as signed... In my
defense I was newer to the group though ;-)

So, I'll add bus/target as _uip and then adjust the title/commit message
appropriately when I push the changes.

John

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca55981..9e77b87 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4954,7 +4954,7 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr 
sourcenode,
  goto cleanup;
  }
  
-if (virStrToLong_ui(unit, NULL, 0, scsihostsrc-unit)  0) {

+if (virStrToLong_uip(unit, NULL, 0, scsihostsrc-unit)  0) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(cannot parse unit '%s'), unit);
  goto cleanup;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 4c47473..0bea462 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -488,7 +488,7 @@ static int str2SCSIAddress(const char *str, struct 
SCSIAddress *scsiAddr)
  return -1;
  
  unit++;

-if (virStrToLong_ui(unit, NULL, 0, scsiAddr-unit) != 0)
+if (virStrToLong_uip(unit, NULL, 0, scsiAddr-unit) != 0)
  return -1;
  
  return 0;




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


Re: [libvirt] [Qemu-devel] incorrect memory size inside vm

2015-06-18 Thread Piotr Rybicki

W dniu 2015-06-18 o 00:57, Vasiliy Tolstov pisze:

2015-06-18 1:52 GMT+03:00 Andrey Korolyov and...@xdel.ru:

Whoosh... technically it is possible but it would be an incompatible
fork for the upstreams for both SeaBIOS and Qemu, because the generic
way of plugging DIMMs in is available down to at least generic 2.6.32.
Except may be Centos where broken kabi would bring great consequences,
it may be better to just provide a backport repository with newer
kernels, but it doesn`t sound very optimistic. For the history
records, the initial hotplug support proposal provided by Vasilis
Liaskovitis a couple of years ago worked in an exact way you are
suggesting to, but its resurrection would mean emulator and rom code
alteration, as I said above.



Ok, i'm try to build latest libvirt and check all oses for memory
hotplug support =).



Hi guys.

I'm actually investigating mem waste issue at my lab. I'm using libvirt 
+ qemu on gentoo.


# libvirtd -v
2015-06-18 14:50:56.619+: 11720: info : libvirt version: 1.2.16

# qemu-x86_64 -version
qemu-x86_64 version 2.3.0, Copyright (c) 2003-2008 Fabrice Bellard

# uname -a
Linux vms06 3.18.14-gentoo #1 SMP Wed Jun 17 14:55:27 CEST 2015 x86_64 
AMD Opteron(tm) Processor 6380 AuthenticAMD GNU/Linux



When not using dimm (static or hotplugged) - only 'main' memory, waste 
is huge, especially if one define big limit.


my test with only 'plain' memory:
define domain.xml with different mem setting, and see mem sizes (virsh 
domstats DOMAIN --balloon from host and 'free' command from guest)


libvirt max: 2GB, curr: 2GB
system total: 2001
balloon.current=2097152
balloon.maximum=2097152


libvirt max: 4GB, curr: 4GB
system total: 3953
balloon.current=4194304
balloon.maximum=4194304

libvirt max: 4GB, curr: 2GB
system total: 1905
balloon.current=2097152
balloon.maximum=4194304


libvirt max: 8GB, curr: 8GB
system total: 7985
balloon.current=8388608
balloon.maximum=8388608

libvirt max: 8GB, curr: 4GB
system total: 3889
balloon.current=4194304
balloon.maximum=8388608

libvirt max: 8GB, curr: 2GB
system total: 1841
balloon.current=2097152
balloon.maximum=8388608


libvirt max: 16GB, curr: 16GB
system total: 16049
balloon.current=16777216
balloon.maximum=16777216

libvirt max: 16GB, curr: 8GB
system total: 7857
balloon.current=8388608
balloon.maximum=16777216

libvirt max: 16GB, curr: 4GB
system total: 3761
balloon.current=4194304
balloon.maximum=16777216

libvirt max: 16GB, curr: 2GB
system total: 1713
balloon.current=2097152
balloon.maximum=16777216

As You can see, when one set 16GB max mem and define current mem to 2GB, 
guest only see 1713 MB. When You set 2GB max mem and leave 2GB mem, 
guest see 2001. So there are 288 MB wasted.


Later, i tried to define 15 static 1GB dimms and 1GB 'main' memory, and 
later on decreased guest memory via balloon to 1GB, and there was no 
waste in guest.


But, when i checked RES size of qemu process on host, It was about 
5,5GB! In contrast, when using only 'main' memory, results were as 
expected (res mem for qemu was amount ram for guest + qemu itself).


Do You see similar results at Your side?

Best regards
Piotr Rybicki

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


Re: [libvirt] [PATCH 3/5] Convert SCSI logical unit from int to long long

2015-06-18 Thread John Ferlan


On 06/16/2015 11:29 PM, Eric Farman wrote:
 The SCSI Architecture Model defines a logical unit address
 as 64-bits in length, so change the field accordingly so
 that the entire value could be stored.
 
 Signed-off-by: Eric Farman far...@linux.vnet.ibm.com
 ---
  src/conf/domain_audit.c |  2 +-
  src/conf/domain_conf.c  |  4 ++--
  src/conf/domain_conf.h  |  2 +-
  src/qemu/qemu_command.h |  2 +-
  src/qemu/qemu_hotplug.c |  4 ++--
  src/util/virhostdev.c   |  6 +++---
  src/util/virscsi.c  | 16 
  src/util/virscsi.h  |  8 
  tests/testutilsqemu.c   |  2 +-
  tools/virsh-domain.c|  6 +++---
  10 files changed, 26 insertions(+), 26 deletions(-)
 

ACK -

John

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


Re: [libvirt] [PATCH 4/5] docs: Fix XML schema handling of LUN address in hostdev tag

2015-06-18 Thread Eric Farman



On 06/18/2015 02:38 PM, John Ferlan wrote:


On 06/16/2015 11:29 PM, Eric Farman wrote:

Defining a domain with a SCSI disk attached via a hostdev
tag and a source address unit value longer than two digits
causes an error when editing the domain with virsh edit,
even if no changes are made to the domain definition.
The error suggests invalid XML, somewhere:

   # virsh edit lmb_guest
   error: XML document failed to validate against schema:
   Unable to validate doc against /usr/local/share/libvirt/schemas/domain.rng
   Extra element devices in interleave
   Element domain failed to validate content

The virt-xml-validate tool fails with a similar error:

   # virt-xml-validate lmb_guest.xml
   Relax-NG validity error : Extra element devices in interleave
   lmb_guest.xml:17: element devices: Relax-NG validity error :
   Element domain failed to validate content
   lmb_guest.xml fails to validate

The hostdev tag requires a source address to be specified,
which includes bus, target, and unit address attributes.
According to the SCSI Architecture Model spec (section
4.9 of SAM-2), a LUN address is 64 bits and thus could be
up to 20 decimal digits long.  Unfortunately, the XML
schema limits this string to just two digits.  Similarly,
the target field can be up to 32 bits in length, which
would be 10 decimal digits.

   # lsscsi -xx
   [0:0:19:0x40224011]  diskIBM  2107900  3.44 /dev/sda
   # lsscsi
   [0:0:19:1074872354]diskIBM  2107900  3.44  /dev/sda
   # cat lmb_guest.xml
   domain type='kvm'
 namelmb_guest/name
 memory unit='MiB'1024/memory
   ...trimmed...
 devices
   controller type='scsi' model='virtio-scsi' index='0'/
   hostdev mode='subsystem' type='scsi'
 source
   adapter name='scsi_host0'/
   address bus='0' target='19' unit='1074872354'/
 /source
   /hostdev
   ...trimmed...

Since the reference unit and target fields are used in
several places in the XML schema, create a separate one
specific for SCSI Logical Units that will permit the
greater length.  This permits both the validation utility
and the virsh edit command to succeed when a hostdev
tag is included.

Signed-off-by: Eric Farman far...@linux.vnet.ibm.com
Reviewed-by: Matthew Rosato mjros...@linux.vnet.ibm.com
Reviewed-by: Stefan Zimmermann s...@linux.vnet.ibm.com
Reviewed-by: Boris Fiuczynski fiu...@linux.vnet.ibm.com
---
  docs/formatdomain.html.in |  6 +-
  docs/schemas/domaincommon.rng | 14 --
  2 files changed, 17 insertions(+), 3 deletions(-)




diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4e85b51..c88c4a6 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3256,7 +3256,11 @@
/dd
dtscsi/dt
ddSCSI devices are described by both the codeadapter/code
-and codeaddress/code elements.
+and codeaddress/code elements. The codeaddress/code
+element includes a codebus/code attribute (a 2-digit bus
+number), a codetarget/code attribute (a 10-digit target
+number), and a codeunit/code attribute (a 20-digit unit
+number on the bus).

Since we know from the v1 comments that qemu has a max of 16384 units, add:

Not all hypervisors support larger codeunit/code values. It is up to
each hypervisor to determine the maximum value supported for the adapter.

I could add codetarget/code and  if you think that would be better..


The text with both target and unit sounds good to me.





  p
  span class=sinceSince 1.2.8/span, the codesource/code
  element of a SCSI device may contain the codeprotocol/code
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f0f7400..b3c5cb8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3833,10 +3833,10 @@
ref name=driveBus/
  /attribute
  attribute name=target
-  ref name=driveTarget/
+  ref name=driveSCSITarget/
  /attribute
  attribute name=unit
-  ref name=driveUnit/
+  ref name=driveSCSIUnit/
  /attribute
/define
define name=usbportaddress
@@ -5129,11 +5129,21 @@
param name=pattern[0-9]{1,2}/param
  /data
/define
+  define name=driveSCSITarget
+data type=string
+  param name=pattern[0-9]{1,10}/param
+/data
+  /define
define name=driveUnit
  data type=string
param name=pattern[0-9]{1,2}/param
  /data
/define
+  define name=driveSCSIUnit
+data type=string
+  param name=pattern[0-9]{1,20}/param
+/data
+  /define
define name=featureName
  data type=string
param name='pattern'[a-zA-Z0-9\-_\.]+/param


I'm going to add a test as well. It's essentially a copy of
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-scsi.xml
adjusted to have a larger unit value (it'll be attached)


Ok.  If you push it as a separate 

Re: [libvirt] [Qemu-devel] incorrect memory size inside vm

2015-06-18 Thread Andrey Korolyov
 Do You see similar results at Your side?

 Best regards


Would you mind to share you argument set to an emulator? As far as I
understood you are using plain ballooning with most results from above
for which those numbers are expected. The case with 5+gig memory
consumption for deflated 1G guest looks like a bug with mixed
dimm/balloon configuration if you are tried against latest qemu, so
please describe a setup a bit more verbosely too.

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


[libvirt] [PATCH] conf: Don't allow duplicated targets regardless of bus

2015-06-18 Thread John Ferlan
https://bugzilla.redhat.com/show_bug.cgi?id=1142631

Commit id 'e0e290552' added a check to determine if the same bus
had the same target value.  It seems that's not quite good enough
as the check should check the target name value regardless of bus type.

Also added a DO_TEST_DIFFERENT to exhibit the issue

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_conf.c |  3 +-
 .../qemuxml2argv-disk-same-targets.xml | 35 ++
 tests/qemuxml2argvtest.c   |  3 ++
 3 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-same-targets.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b10f6cd..7855bcb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12590,8 +12590,7 @@ virDomainDiskDefDstDuplicates(virDomainDefPtr def)
 
 for (i = 1; i  def-ndisks; i++) {
 for (j = 0; j  i; j++) {
-if (def-disks[i]-bus == def-disks[j]-bus 
-STREQ(def-disks[i]-dst, def-disks[j]-dst)) {
+if (STREQ(def-disks[i]-dst, def-disks[j]-dst)) {
 virReportError(VIR_ERR_XML_ERROR,
_(target '%s' duplicated for disk sources 
  '%s' and '%s'),
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-same-targets.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-disk-same-targets.xml
new file mode 100644
index 000..3276ce5
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-same-targets.xml
@@ -0,0 +1,35 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219136/memory
+  currentMemory unit='KiB'219136/currentMemory
+  vcpu placement='static'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+/disk
+disk type='file' device='disk'
+  source file='/tmp/usbdisk.img'/
+  target dev='sda' bus='usb'/
+/disk
+disk type='file' device='disk'
+  source file='/tmp/idedisk.img'/
+  target dev='sda' bus='ide'/
+/disk
+disk type='file' device='disk'
+  source file='/tmp/scsidisk.img'/
+  target dev='sda' bus='scsi'/
+/disk
+memballoon model='virtio'/
+  /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index be82dd2..b066681 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -882,6 +882,9 @@ mymain(void)
 QEMU_CAPS_DEVICE);
 DO_TEST(disk-snapshot,
 QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_CACHE_V2, QEMU_CAPS_DRIVE_FORMAT);
+DO_TEST_FAILURE(disk-same-targets,
+QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_SCSI_LSI,
+QEMU_CAPS_DEVICE_USB_STORAGE, QEMU_CAPS_NODEFCONFIG);
 DO_TEST(event_idx,
 QEMU_CAPS_DRIVE,
 QEMU_CAPS_VIRTIO_BLK_EVENT_IDX,
-- 
2.1.0

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


Re: [libvirt] [PATCH 0/5 v2] Corrections to SCSI logical unit handling

2015-06-18 Thread Stefan Zimmermann
On Wed, 2015-06-17 at 10:29 -0400, Matthew Rosato wrote:
 On 06/16/2015 11:29 PM, Eric Farman wrote:
  While working with the hostdev tag and SCSI LUNs, a problem was
  discovered with the XML schema (see commit message in patch 4).
  This spawned some further corrections to the handling of the
  logical unit field throughout libvirt.
  
  This series was split from a single patch, from this feedback:
  http://www.redhat.com/archives/libvir-list/2015-June/msg00489.html
  
  Eric Farman (5):
Print SCSI logical unit as a positive integer
Print SCSI logical unit as unsigned integer
Convert SCSI logical unit from int to long long
docs: Fix XML schema handling of LUN address in hostdev tag
docs: Correct typos in scsi hostdev and address elements
 
 I get the value of small patches  agree with the way patches 4 and 5
 are split out, but patch 1 and 2 are completely replaced by patch 3 with
 a different type.  These are pretty straightforward changes, so I'd
 suggest squashing patches 1-3 as a single patch that just goes from
 signed int -- unsigned long long and has a commit that explains why we
 needed to change both size and sign...  I see now that it was a v1
 comment from John, so I'll leave it to his judgment.
 
 Regardless, code still looks good to me so you can feel free to keep my
 reviewed-by tag for the set, whether patches 1-3 are squashed or not.
 
 Reviewed-by: Matthew Rosato mjros...@linux.vnet.ibm.com
I'm with you.
Reviewed-by: Stefan Zimmermann s...@linux.vnet.ibm.com

   docs/formatdomain.html.in | 10 +++---
   docs/schemas/domaincommon.rng | 14 --
   src/conf/domain_audit.c   |  2 +-
   src/conf/domain_conf.c|  4 ++--
   src/conf/domain_conf.h|  2 +-
   src/qemu/qemu_command.h   |  2 +-
   src/qemu/qemu_hotplug.c   |  4 ++--
   src/util/virhostdev.c |  6 +++---
   src/util/virscsi.c| 16 
   src/util/virscsi.h|  8 
   tests/testutilsqemu.c |  2 +-
   tools/virsh-domain.c  |  6 +++---
   12 files changed, 45 insertions(+), 31 deletions(-)
  
 


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


Re: [libvirt] [PATCH 0/2] Yet again a admin API fixup series.

2015-06-18 Thread Michal Privoznik
On 18.06.2015 11:44, Peter Krempa wrote:
 Fluffy bunny ears of shame will be administered to Martin.
 
 Peter Krempa (2):
   rpc: Actually increase reference count on @srv in
 virNetDaemonAddServer
   daemon: Add the admin service to the admin server only if it was
 allocated
 
  daemon/libvirtd.c  | 6 +++---
  src/rpc/virnetdaemon.c | 2 +-
  2 files changed, 4 insertions(+), 4 deletions(-)
 

ACK series. Funny, I was just about to send the 1/2 patch too.

Michal

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


Re: [libvirt] libvirt proxy

2015-06-18 Thread Vasiliy Tolstov
2015-06-18 12:01 GMT+03:00 Daniel P. Berrange berra...@redhat.com:
 FWIW, we really consider the RPC protocol to be a private impl detail
 and strongly discourage people from trying to re-implement it
 themselves.

 Can you explain a bit more about what you're trying to achieve by
 creating an RPC proxy


I have DO like cloud hosting and want to provide libvirt compat
endpoint to create/start/stop servers.

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

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


Re: [libvirt] [PATCH] remove redundant condition

2015-06-18 Thread Peter Krempa
On Wed, Jun 17, 2015 at 18:01:38 +0200, Ján Tomko wrote:
 If the address type is SPAPRVIO, it will match the != NONE condition.
 ---
  src/qemu/qemu_command.c | 2 --
  1 file changed, 2 deletions(-)


ACK,

Peter


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

Re: [libvirt] [PATCH] daemon: only unref virNetServers once

2015-06-18 Thread Peter Krempa
On Wed, Jun 17, 2015 at 18:29:16 +0200, Ján Tomko wrote:
 After we add srv and srvAdm to the virNetDaemon,
 it's its responsibility to unref them in virNetDaemonDispose.
 
 Added by commits fa14207 and d5f4241.
 ---
  daemon/libvirtd.c | 2 --
  1 file changed, 2 deletions(-)
 
 diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
 index 368e10c..9e6c489 100644
 --- a/daemon/libvirtd.c
 +++ b/daemon/libvirtd.c
 @@ -1589,8 +1589,6 @@ int main(int argc, char **argv) {
  virObjectUnref(adminProgram);
  virNetDaemonClose(dmn);
  virObjectUnref(dmn);
 -virObjectUnref(srv);
 -virObjectUnref(srvAdm);

virNetDaemonAddServer that adds them to the virNetDaemon increases the
reference count, thus the main function should retain it's reference.

I think these should remain and the bug needs to be fixed elsewhere.
I'm giving it a brief glance before I'll go complain to Martin for
breaking it.

Peter


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

Re: [libvirt] [PATCH] qemu: Report all supported machine types in capabilities

2015-06-18 Thread Jiri Denemark
On Wed, Jun 17, 2015 at 14:18:23 -0400, John Ferlan wrote:
 
 
 On 06/12/2015 08:53 AM, Jiri Denemark wrote:
  Some machine types are only reported as canonical names for other
  machine types, which make it a bit harder to find what machine types are
  supported by a specific QEMU binary. Ideally, one would just use
  /capabilities/guest/arch[@name='...']/machine/text() XPath to get a list
  of all supported machine types, but it doesn't work right now.
  
  For example, we report
  
  machine canonical='pc-i440fx-2.3' maxCpus='255'pc/machine
  
  in guest capabilities, but the corresponding
  
  machine maxCpus='255'pc-i440fx-2.3/machine
  
  is missing.
  
  This is a result of QMP probing. With -machine ? parsing QEMU sends
  us two lines:
  
  pc   Standard PC (i440FX + PIIX, 1996) (alias of 
  pc-i440fx-2.3)
  pc-i440fx-2.3Standard PC (i440FX + PIIX, 1996) (default)
  
  while query-machines QMP command reports both in the same entry:
  
  {name: pc-i440fx-2.3, is-default: true, cpu-max: 255, alias: pc}
  
  Let's make sure we always report separate machine/ for both the
  canonical name and its alias and using the canonical name as the default
  machine type (i.e., inserting it before its alias) in case is-default is
  true.
  
  https://bugzilla.redhat.com/show_bug.cgi?id=1229666
  
  Signed-off-by: Jiri Denemark jdene...@redhat.com
  ---
   src/qemu/qemu_capabilities.c | 38 ++
   1 file changed, 38 insertions(+)
  
 
 There's no existing test for this? I guess I would have expected to see
 differences in output and thus test adjustments as well.
 
 The code seems to be doing what is stated though... and the
 tests/capabilityschemadata/caps-qemu-kvm.xml seems to show the output as
 desired.
 
 ACK -

Pushed, thanks.

Jirka

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


Re: [libvirt] [PATCH v2] virSysinfo: Introduce SMBIOS type 2 support

2015-06-18 Thread Michal Privoznik
On 17.06.2015 15:44, John Ferlan wrote:
 
 
 On 06/12/2015 06:02 AM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1220527

 This type of information defines attributes of a system
 baseboard.  With one caveat: in qemu they call it family, while
 in the specification it's referred to as type. I'm sticking with
 the latter.
 
 Perhaps should update this since 'family' and 'type' aren't processed by
 libvirt.
 

 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---

 diff to v1:
 -I've dropped the 'family' attribute. It's not implemented in qemu yet. We 
 can
 add it later.

  docs/formatdomain.html.in  |  37 -
  docs/schemas/domaincommon.rng  |  23 +++
  src/conf/domain_conf.c |  63 
  src/libvirt_private.syms   |   1 +
  src/qemu/qemu_command.c|  54 +++
  src/util/virsysinfo.c  | 170 
 -
  src/util/virsysinfo.h  |  16 ++
  .../qemuxml2argv-smbios-multiple-type2.xml |  58 +++
  tests/qemuxml2argvdata/qemuxml2argv-smbios.args|   2 +
  tests/qemuxml2argvdata/qemuxml2argv-smbios.xml |   8 +
  tests/qemuxml2xmltest.c|   1 +
  11 files changed, 427 insertions(+), 6 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-smbios-multiple-type2.xml

 
 There's a couple issues pointed out below, fixable without need for a v3
 I believe...
 
 ACK with the adjustments
 
 John
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 0478cb2..977660e 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -375,6 +375,12 @@
lt;entry name='product'gt;Virt-Managerlt;/entrygt;
lt;entry name='version'gt;0.9.4lt;/entrygt;
  lt;/systemgt;
 +lt;baseBoardgt;
 +  lt;entry name='manufacturer'gt;LENOVOlt;/entrygt;
 +  lt;entry name='product'gt;20BE0061MClt;/entrygt;
 +  lt;entry name='version'gt;0B98401 Prolt;/entrygt;
 +  lt;entry name='serial'gt;W1KS427111Elt;/entrygt;
 +lt;/baseBoardgt;
lt;/sysinfogt;
.../pre
  
 @@ -435,11 +441,32 @@
  dtcodefamily/code/dt
  ddIdentify the family a particular computer belongs 
 to./dd
  /dl
 -NB: Incorrectly supplied entries in either the codebios/code
 -or codesystem/code blocks will be ignored without error.
 -Other than codeuuid/code validation and codedate/code
 -format checking, all values are passed as strings to the
 -hypervisor driver.
 +  /dd
 +  dtcodebaseBoard/code/dt
 +  dd
 +This is block 2 of SMBIOS. This element can be repeated multiple
 +times, to describe all the base boards. However, not all
 
 s/times,/times
 s/boards. However,/boards; however,
 +hypervisors support the repetition necessarily. The element can
 
 s/support the repetition necessarily/necessarily support the repitition
 
 or
 
 s/support the repetition necessarily./support multiple base boards./
 
 +have the following children:
 +dl
 +dtcodemanufacturer/code/dt
 +ddManufacturer of BIOS/dd
 +dtcodeproduct/code/dt
 +ddProduct Name/dd
 +dtcodeversion/code/dt
 +ddVersion of the product/dd
 +dtcodeserial/code/dt
 +ddSerial number/dd
 +dtcodeasset/code/dt
 +ddAsset tag/dd
 +dtcodelocation/code/dt
 +ddLocation in chassis/dd
 +/dl
 +NB: Incorrectly supplied entries for the
 +codebios/code, codesystem/code or codebaseBoard/code
 +blocks will be ignored without error.  Other than 
 codeuuid/code
 +validation and codedate/code format checking, all values are
 +passed as strings to the hypervisor driver.
/dd
  /dl
/dd
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index e38b927..32d28cd 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -4248,6 +4248,18 @@
  /oneOrMore
/element
  /optional
 +zeroOrMore
 +  element name=baseBoard
 +oneOrMore
 +  element name=entry
 +attribute name=name
 +  ref name=sysinfo-baseBoard-name/
 +/attribute
 +ref name=sysinfo-value/
 +  /element
 +/oneOrMore
 +  /element
 +/zeroOrMore
/interleave
  /element
/define
 @@ -4273,6 +4285,17 @@
  /choice
/define
  
 +  define name=sysinfo-baseBoard-name
 +choice
 +  valuemanufacturer/value
 +  valueproduct/value
 +  

Re: [libvirt] libvirt proxy

2015-06-18 Thread Daniel P. Berrange
On Thu, Jun 18, 2015 at 01:55:47AM +0300, Vasiliy Tolstov wrote:
 Hello. I need libvirt proxy, but not simple proxy connection, but
 something like dedicated libvirt server, that can modify requests or
 doing something based on incoming request.
 I'm use go for this, i'm already parse xdr data, but i'm stuck at
 payload. Where i can find payloads examples or how can i determine
 needed payload for specific procedure ? (now i'm use wireshark with
 libvirt dissector).
 I don't need to provide all libvirt functions, now i need very basic
 sets like open/close, list, create , destroy domains..

FWIW, we really consider the RPC protocol to be a private impl detail
and strongly discourage people from trying to re-implement it
themselves.

Can you explain a bit more about what you're trying to achieve by
creating an RPC proxy

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] [RFC v2][remote]

2015-06-18 Thread Daniel P. Berrange
On Thu, Jun 18, 2015 at 11:05:13AM +0300, Nikolay Shirokovskiy wrote:
 Hello.
 
 I'll reword the previous letter.
   
   In libvirt we have connection close callback for drivers that have 
 persistent
 internal connection to notify client that this connection is closed. Vz driver
 has internal persistent connection so I wanted to add support of this callback
 to vz driver. Now clients with connections urls like 'vz:///system' are
 notified of connection close event. But if connection url is like
 'vz+transport://system we have a remote driver between client and vz driver
 and client is not notified as remote driver doesn't handle this event.
 
   The problem is that remote driver can't just relay this event as domain one
 as there is no means to do it in driver interface. The quick fix is to close
 the connection between daemon and remote driver from daemon side is case of
 close event. This will trigger connection close event in remote driver and
 client finally will be notified. I doubt this is a good approach as it looks
 like we forced to take specific action of closing connection to daemon only 
 due
 to lack of appropriate driver interface.
 
   So the proposition is to move connection close event
 registration/deregistration/notifying to driver level so remote driver could
 relay them just like say domain events.

Ok, I think I understand the problem you're getting at. The parallels
driver is stateless, so it usually executes in the context of the app
linking to libvirt.so As such you would normally just emit the connection
close event directly in your driver code.

Even though it is stateless, you have the option of connecting via
libvirtd, which allows for remote access. Normally the remote_driver.c
client deals with emitting the connection close event when the connection
to libvirtd goes down, but you want to be able to emit events from
the parallels driver, even when libvirtd conncetion is still operational.

This is indeed a new scenario we've not had to deal with before, but
it sounds entirely reasonable to want todo this.

If we moved the registration/deregistration to the driver level, then
we could also add RPC calls for reg/dereg, that could be forwarded
onto the remotely running driver. We'd also need an RPC event added
to do the notification. The remote driver client would receive the
events and emit them.

I think this is all doable, so if you want to propose patches, go
ahead.

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] Overhead for a default cpu cg placement scheme

2015-06-18 Thread Andrey Korolyov
On Thu, Jun 18, 2015 at 12:09 PM, Daniel P. Berrange
berra...@redhat.com wrote:
 On Wed, Jun 17, 2015 at 10:55:35PM +0300, Andrey Korolyov wrote:

 Sorry for a delay, the 'perf numa numa-mem -p 8 -t 2 -P 384 -C 0 -M 0
 -s 200 -zZq --thp 1 --no-data_rand_walk' exposes a difference of value
 0.96 by 1. The trick I did (and successfully forget) before is in
 setting the value of the cfs_quota in a machine wide group, up one
 level from individual vcpus.

 Right now, libvirt sets values from
 cputune
 period10/period
 quota20/quota
 /cputune
 for each vCPU thread cgroup, which is a bit wrong by my understanding , like
 /cgroup/cpu/machine/vmxx/vcpu0: period=10, quota=200
 /cgroup/cpu/machine/vmxx/vcpu1: period=10, quota=200
 /cgroup/cpu/machine/vmxx/vcpu2: period=10, quota=200
 /cgroup/cpu/machine/vmxx/vcpu3: period=10, quota=200


 In other words, the user (me) assumed that he limited total
 consumption of the VM by two cores total, though all every thread can
 consume up to a single CPU, resulting in a four-core consumption
 instead. With different cpu count/quota/host cpu count ratios there
 would be different practical limitations with same period to quota
 ratio, where a single total quota will result in much more predictable
 top consumption. I had put the same quota to period ratio in a
 VM-level directory to meet the expectancies from a config setting and
 there one can observe a mentioned performance drop.

 With default placement there is no difference in a performance
 numbers, but the behavior of the libvirt itself is kinda controversial
 there. The documentation says that this is a right behavior as well,
 but I think that the limiting the vcpu group with total quota is far
 more flexible than per-vcpu limitations which can negatively impact
 single-threaded processes in the guest, plus the overall consumption
 should be recalculated every time when host core count or guest core
 count changes. Sorry for not mentioning the custom scheme before, if
 mine assumption about execution flexibility is plainly wrong, I`ll
 withdraw my concerns from above. I am using the 'mine' scheme for a
 couple of years in production and it is proved (for me) to be a far
 less complex for a workload balancing for a cpu-congested hypervisor
 than a generic one.

 As you say there are two possible directions libvirt was able to take
 when implementing the schedular tunables. Either apply them to the
 VM as a whole, or apply them to the individual vCPUS. We debated this
 a fair bit, but in the end we took the per-VCPU approach. There were
 two real compelling reasons. First, if users have 2 guests with
 identical configurations, but give one of the guests 2 vCPUs and the
 other guest 4 vCPUs, the general expectation is that the one with
 4 vCPUS will have twice the performance. If we apply the CFS tuning
 at the VM level, then as you added vCPUs you'd get no increase in
 performance.  The second reason was that people wanted to be able to
 control performance of the emulator threads, separately from the
 vCPU threads. Now we also have dedicated I/O threads that can have
 different tuning set. This would be impossible if we were always
 setting stuff at the VM level.

 It would in theory be possible for us to add a further tunable to the
 VM config which allowed VM level tuning.  eg we could define something
 like

  vmtune
period10/period
quota20/quota
  /vmtune

 Semantically, if vmtune was set, we would then forbid use of the
 cputune and emulatortune configurations, as they'd be mutually
 exclusive. In such a case we'd avoid creating the sub-cgroups for
 vCPUs and emulator threads, etc.

 The question is whether the benefit would outweigh the extra code
 complexity to deal with this. I appreciate you would desire this
 kind of setup, but I think we'd probably need more than one person
 requesting use of this kind of setup in order to justify the work
 involved.


Thanks for a quite awesome explanation! I see, the thing that is
obvious for Xen-era hosting (more vCPUs means more power) is not an
obvious thing for myself. I agree with the fact that less count of
more powerful cores is always preferable over a large set of 'weak on
average' cores with the approach I proposed. The thing that is still
confusing is that the one should mind *three* exact things while
setting a limit in a current scheme - real or HT core count, the VM`
core count and the quota to period ratio itself to determine an upper
cap for a designated VM` consumption, and it would be even more
confusing when we will talk for a share ratios - for me, it is
completely unclear how two VMs with 2:1 share ratio for both vCPUs and
emulator would behave, will the emulator thread starve first on a CPU
congestion or vice-versa, will the many vCPU processes with equal
share to an emulator make enough influence inside a capped node to
displace the actual available bandwidths from 2:1, will the guest
emulator 

[libvirt] [PATCH 0/2] Yet again a admin API fixup series.

2015-06-18 Thread Peter Krempa
Fluffy bunny ears of shame will be administered to Martin.

Peter Krempa (2):
  rpc: Actually increase reference count on @srv in
virNetDaemonAddServer
  daemon: Add the admin service to the admin server only if it was
allocated

 daemon/libvirtd.c  | 6 +++---
 src/rpc/virnetdaemon.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.4.1

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


[libvirt] [PATCH 1/2] rpc: Actually increase reference count on @srv in virNetDaemonAddServer

2015-06-18 Thread Peter Krempa
VIR_APPEND_ELEMENT would clear @srv to NULL after it successfully
inserted it thus the reference count could not be increased afterwards.

Switch to VIR_APPEND_ELEMENT_COPY. This fixes crash after terminating
the daemon.
---
 src/rpc/virnetdaemon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index a6d6a4b..67dff14 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -162,7 +162,7 @@ virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr 
srv)

 virObjectLock(dmn);

-if (VIR_APPEND_ELEMENT(dmn-servers, dmn-nservers, srv)  0)
+if (VIR_APPEND_ELEMENT_COPY(dmn-servers, dmn-nservers, srv)  0)
 goto cleanup;

 virObjectRef(srv);
-- 
2.4.1

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


[libvirt] [PATCH] nodedev: add RDMA and tx-udp_tnl-segmentation NIC capabilities

2015-06-18 Thread Moshe Levi
Adding functionality to libvirt that will allow
it query the interface for the availability of RDMA and
tx-udp_tnl-segmentation Offloading NIC capabilities

Here is an example of the feature XML definition:

device
namenet_eth4_90_e2_ba_5e_a5_45/name
  path/sys/devices/pci:00/:00:03.0/:08:00.1/net/eth4/path
  parentpci__08_00_1/parent
  capability type='net'
interfaceeth4/interface
address90:e2:ba:5e:a5:45/address
link speed='1' state='up'/
feature name='rx'/
feature name='tx'/
feature name='sg'/
feature name='tso'/
feature name='gso'/
feature name='gro'/
feature name='rxvlan'/
feature name='txvlan'/
feature name='rxhash'/
feature name='rdma'/
feature name='tx-udp_tnl-segmentation'/
capability type='80203'/
  /capability
/device
---
 docs/formatnode.html.in |2 +
 src/conf/device_conf.c  |4 +-
 src/conf/device_conf.h  |2 +
 src/util/virnetdev.c|   97 ++
 src/util/virnetdev.h|1 +
 5 files changed, 88 insertions(+), 18 deletions(-)

diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 3ff1bef..9b32dd1 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -199,6 +199,8 @@
 dtcodetxvlan/code/dtddtx-vlan-offload/dd
 dtcodentuple/code/dtddntuple-filters/dd
 dtcoderxhash/code/dtddreceive-hashing/dd
+
dtcoderdma/code/dtddremote-direct-memory-access/dd
+
dtcodetx-udp_tnl-segmentation/code/dtddtx-udp-tunnel-segmentation/dd
 /dl
   /dd
   dtcodecapability/code/dt
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 98808e2..8e8d557 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -51,7 +51,9 @@ VIR_ENUM_IMPL(virNetDevFeature,
   rxvlan,
   txvlan,
   ntuple,
-  rxhash)
+  rxhash,
+  rdma,
+  tx-udp_tnl-segmentation)
 
 int virDevicePCIAddressIsValid(virDevicePCIAddressPtr addr)
 {
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 7ea90f6..07298c9 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -74,6 +74,8 @@ typedef enum {
 VIR_NET_DEV_FEAT_TXVLAN,
 VIR_NET_DEV_FEAT_NTUPLE,
 VIR_NET_DEV_FEAT_RXHASH,
+VIR_NET_DEV_FEAT_RDMA,
+VIR_NET_DEV_FEAT_TX_UDP_TNL_SEGMENTATION,
 VIR_NET_DEV_FEAT_LAST
 } virNetDevFeature;
 
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index e4fcd81..3086616 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -87,6 +87,14 @@ VIR_LOG_INIT(util.netdev);
 # define VIR_IFF_ALLMULTI 0
 #endif
 
+#define RESOURCE_FILE_LEN 4096
+#define TX_UDP_TNL 25
+#define GFEATURES_SIZE 2
+#define FEATURE_WORD(blocks, index, field)  ((blocks)[(index) / 32U].field)
+#define FEATURE_FIELD_FLAG(index)  (1U  (index) % 32U)
+#define FEATURE_BIT_IS_SET(blocks, index, field)\
+(FEATURE_WORD(blocks, index, field)  FEATURE_FIELD_FLAG(index))
+
 typedef enum {
 VIR_MCAST_TYPE_INDEX_TOKEN,
 VIR_MCAST_TYPE_NAME_TOKEN,
@@ -2943,6 +2951,58 @@ int virNetDevGetRxFilter(const char *ifname,
 return ret;
 }
 
+
+/**
+ * virNetDevRDMAFeature
+ * This function checks for the availability of RDMA feature
+ * and add it to bitmap
+ *
+ * @ifname: name of the interface
+ * @out: add RDMA feature if exist to bitmap
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int
+virNetDevRDMAFeature(const char *ifname,
+ virBitmapPtr *out)
+{
+char *eth_devpath = NULL;
+char *ib_devpath = NULL;
+char *eth_res_buf = NULL;
+char *ib_res_buf = NULL;
+struct dirent *dp;
+
+DIR *dirp = opendir(SYSFS_INFINIBAND_DIR);
+if (dirp == NULL) {
+virReportSystemError(errno,
+ _(Failed to opendir path '%s'),
+ SYSFS_INFINIBAND_DIR);
+return -1;
+}
+
+if (virAsprintf(eth_devpath, SYSFS_NET_DIR %s/device/resource, ifname) 
 0)
+return -1;
+if (!virFileExists(eth_devpath))
+return 0;
+if (virFileReadAll(eth_devpath, RESOURCE_FILE_LEN, eth_res_buf)  0)
+return -1;
+while (virDirRead(dirp, dp, SYSFS_INFINIBAND_DIR)  0) {
+if (STREQ(dp-d_name, .) ||
+STREQ(dp-d_name, ..))
+continue;
+
+if (virAsprintf(ib_devpath, SYSFS_INFINIBAND_DIR 
%s/device/resource, dp-d_name)  0)
+continue;
+if (virFileReadAll(ib_devpath, RESOURCE_FILE_LEN, ib_res_buf)  0)
+continue;
+if  (STREQ(eth_res_buf, ib_res_buf)) {
+ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_RDMA));
+break;
+}
+}
+return 0;
+}
+
 #if defined(SIOCETHTOOL)  defined(HAVE_STRUCT_IFREQ)
 
 /**
@@ -2952,12 +3012,10 @@ int virNetDevGetRxFilter(const char *ifname,
  * @ifname: name of the interface
 

Re: [libvirt] Overhead for a default cpu cg placement scheme

2015-06-18 Thread Daniel P. Berrange
On Wed, Jun 17, 2015 at 10:55:35PM +0300, Andrey Korolyov wrote:
 
 Sorry for a delay, the 'perf numa numa-mem -p 8 -t 2 -P 384 -C 0 -M 0
 -s 200 -zZq --thp 1 --no-data_rand_walk' exposes a difference of value
 0.96 by 1. The trick I did (and successfully forget) before is in
 setting the value of the cfs_quota in a machine wide group, up one
 level from individual vcpus.
 
 Right now, libvirt sets values from
 cputune
 period10/period
 quota20/quota
 /cputune
 for each vCPU thread cgroup, which is a bit wrong by my understanding , like
 /cgroup/cpu/machine/vmxx/vcpu0: period=10, quota=200
 /cgroup/cpu/machine/vmxx/vcpu1: period=10, quota=200
 /cgroup/cpu/machine/vmxx/vcpu2: period=10, quota=200
 /cgroup/cpu/machine/vmxx/vcpu3: period=10, quota=200
 
 
 In other words, the user (me) assumed that he limited total
 consumption of the VM by two cores total, though all every thread can
 consume up to a single CPU, resulting in a four-core consumption
 instead. With different cpu count/quota/host cpu count ratios there
 would be different practical limitations with same period to quota
 ratio, where a single total quota will result in much more predictable
 top consumption. I had put the same quota to period ratio in a
 VM-level directory to meet the expectancies from a config setting and
 there one can observe a mentioned performance drop.
 
 With default placement there is no difference in a performance
 numbers, but the behavior of the libvirt itself is kinda controversial
 there. The documentation says that this is a right behavior as well,
 but I think that the limiting the vcpu group with total quota is far
 more flexible than per-vcpu limitations which can negatively impact
 single-threaded processes in the guest, plus the overall consumption
 should be recalculated every time when host core count or guest core
 count changes. Sorry for not mentioning the custom scheme before, if
 mine assumption about execution flexibility is plainly wrong, I`ll
 withdraw my concerns from above. I am using the 'mine' scheme for a
 couple of years in production and it is proved (for me) to be a far
 less complex for a workload balancing for a cpu-congested hypervisor
 than a generic one.

As you say there are two possible directions libvirt was able to take
when implementing the schedular tunables. Either apply them to the
VM as a whole, or apply them to the individual vCPUS. We debated this
a fair bit, but in the end we took the per-VCPU approach. There were
two real compelling reasons. First, if users have 2 guests with
identical configurations, but give one of the guests 2 vCPUs and the
other guest 4 vCPUs, the general expectation is that the one with
4 vCPUS will have twice the performance. If we apply the CFS tuning
at the VM level, then as you added vCPUs you'd get no increase in
performance.  The second reason was that people wanted to be able to
control performance of the emulator threads, separately from the
vCPU threads. Now we also have dedicated I/O threads that can have
different tuning set. This would be impossible if we were always
setting stuff at the VM level.

It would in theory be possible for us to add a further tunable to the
VM config which allowed VM level tuning.  eg we could define something
like

 vmtune
   period10/period
   quota20/quota
 /vmtune

Semantically, if vmtune was set, we would then forbid use of the
cputune and emulatortune configurations, as they'd be mutually
exclusive. In such a case we'd avoid creating the sub-cgroups for
vCPUs and emulator threads, etc.

The question is whether the benefit would outweigh the extra code
complexity to deal with this. I appreciate you would desire this
kind of setup, but I think we'd probably need more than one person
requesting use of this kind of setup in order to justify the work
involved.

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] [RFC v2][remote]

2015-06-18 Thread Nikolay Shirokovskiy
Hello.

I'll reword the previous letter.
  
  In libvirt we have connection close callback for drivers that have persistent
internal connection to notify client that this connection is closed. Vz driver
has internal persistent connection so I wanted to add support of this callback
to vz driver. Now clients with connections urls like 'vz:///system' are
notified of connection close event. But if connection url is like
'vz+transport://system we have a remote driver between client and vz driver
and client is not notified as remote driver doesn't handle this event.

  The problem is that remote driver can't just relay this event as domain one
as there is no means to do it in driver interface. The quick fix is to close
the connection between daemon and remote driver from daemon side is case of
close event. This will trigger connection close event in remote driver and
client finally will be notified. I doubt this is a good approach as it looks
like we forced to take specific action of closing connection to daemon only due
to lack of appropriate driver interface.

  So the proposition is to move connection close event
registration/deregistration/notifying to driver level so remote driver could
relay them just like say domain events.

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


Re: [libvirt] [RFC v2][remote]

2015-06-18 Thread Nikolay Shirokovskiy


On 18.06.2015 12:00, Daniel P. Berrange wrote:
 On Thu, Jun 18, 2015 at 11:05:13AM +0300, Nikolay Shirokovskiy wrote:
 Hello.

 I'll reword the previous letter.
   
   In libvirt we have connection close callback for drivers that have 
 persistent
 internal connection to notify client that this connection is closed. Vz 
 driver
 has internal persistent connection so I wanted to add support of this 
 callback
 to vz driver. Now clients with connections urls like 'vz:///system' are
 notified of connection close event. But if connection url is like
 'vz+transport://system we have a remote driver between client and vz driver
 and client is not notified as remote driver doesn't handle this event.

   The problem is that remote driver can't just relay this event as domain one
 as there is no means to do it in driver interface. The quick fix is to close
 the connection between daemon and remote driver from daemon side is case of
 close event. This will trigger connection close event in remote driver and
 client finally will be notified. I doubt this is a good approach as it looks
 like we forced to take specific action of closing connection to daemon only 
 due
 to lack of appropriate driver interface.

   So the proposition is to move connection close event
 registration/deregistration/notifying to driver level so remote driver could
 relay them just like say domain events.
 
 Ok, I think I understand the problem you're getting at. The parallels
 driver is stateless, so it usually executes in the context of the app
 linking to libvirt.so As such you would normally just emit the connection
 close event directly in your driver code.
 
 Even though it is stateless, you have the option of connecting via
 libvirtd, which allows for remote access. Normally the remote_driver.c
 client deals with emitting the connection close event when the connection
 to libvirtd goes down, but you want to be able to emit events from
 the parallels driver, even when libvirtd conncetion is still operational.
 
 This is indeed a new scenario we've not had to deal with before, but
 it sounds entirely reasonable to want todo this.
 
 If we moved the registration/deregistration to the driver level, then
 we could also add RPC calls for reg/dereg, that could be forwarded
 onto the remotely running driver. We'd also need an RPC event added
 to do the notification. The remote driver client would receive the
 events and emit them.
 
 I think this is all doable, so if you want to propose patches, go
 ahead.
Ok, I'll take it.
 
 Regards,
 Daniel
 

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


[libvirt] [PATCH v2 2/3] vz: add vcpu statistics

2015-06-18 Thread Nikolay Shirokovskiy
From: Nikolay Shirokovskiy nshirokovs...@parallels.com

Comments.

Replace vzDomObjFromDomain/virObjectUnlock pair
to vzDomObjFromDomainRef/virDomainObjEndAPI as we
use prlsdkGetStatsParam. See previous statistics
comments.

Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
---
 src/vz/vz_driver.c |   16 ++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index deac572..4197569 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -825,8 +825,9 @@ vzDomainGetVcpus(virDomainPtr domain,
 size_t i;
 int v, maxcpu, hostcpus;
 int ret = -1;
+char *name = NULL;
 
-if (!(privdom = vzDomObjFromDomain(domain)))
+if (!(privdom = vzDomObjFromDomainRef(domain)))
 goto cleanup;
 
 if (!virDomainObjIsActive(privdom)) {
@@ -847,8 +848,18 @@ vzDomainGetVcpus(virDomainPtr domain,
 if (info != NULL) {
 memset(info, 0, sizeof(*info) * maxinfo);
 for (i = 0; i  maxinfo; i++) {
+long long vcpuTime = 0;
+
 info[i].number = i;
 info[i].state = VIR_VCPU_RUNNING;
+
+if (virAsprintf(name, guest.vcpu%u.time, (unsigned int)i)  
0)
+goto cleanup;
+if (prlsdkGetStatsParam(privdom, name, vcpuTime)  0)
+goto cleanup;
+if (vcpuTime != -1)
+info[i].cpuTime = vcpuTime;
+VIR_FREE(name);
 }
 }
 if (cpumaps != NULL) {
@@ -871,7 +882,8 @@ vzDomainGetVcpus(virDomainPtr domain,
 
  cleanup:
 if (privdom)
-virObjectUnlock(privdom);
+virDomainObjEndAPI(privdom);
+VIR_FREE(name);
 return ret;
 }
 
-- 
1.7.1

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


[libvirt] [PATCH v2 3/3] vz: add memory statistics

2015-06-18 Thread Nikolay Shirokovskiy
From: Nikolay Shirokovskiy nshirokovs...@parallels.com

Implemented counters:
 VIR_DOMAIN_MEMORY_STAT_SWAP_IN
 VIR_DOMAIN_MEMORY_STAT_SWAP_OUT
 VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT
 VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT
 VIR_DOMAIN_MEMORY_STAT_AVAILABLE
 VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON
 VIR_DOMAIN_MEMORY_STAT_UNUSED

Comments.

1. Use vzDomObjFromDomainRef/virDomainObjEndAPI pair to get domain
object as we use prlsdkGetStatsParam. See previous statistics
comments.

2. Balloon statistics is not applicable to containers. Fault
statistics for containers not provided in PCS6 yet.

Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
---
 src/vz/vz_driver.c |   73 
 1 files changed, 73 insertions(+), 0 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 4197569..8dae7c4 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1377,6 +1377,78 @@ vzDomainInterfaceStats(virDomainPtr domain,
 return ret;
 }
 
+static int
+vzDomainMemoryStats(virDomainPtr domain,
+  virDomainMemoryStatPtr stats,
+  unsigned int nr_stats,
+  unsigned int flags)
+{
+virDomainObjPtr dom = NULL;
+int ret = -1;
+long long v = 0, t = 0, u = 0;
+size_t i = 0;
+
+virCheckFlags(0, -1);
+if (!(dom = vzDomObjFromDomainRef(domain)))
+return -1;
+
+#define PARALLELS_GET_COUNTER(NAME, VALUE)  \
+if (prlsdkGetStatsParam(dom, NAME, VALUE)  0) \
+goto cleanup;   \
+
+#define PARALLELS_MEMORY_STAT_SET(TAG, VALUE)   \
+if (i  nr_stats) { \
+stats[i].tag = (TAG);   \
+stats[i].val = (VALUE); \
+i++;\
+}
+
+#define PARALLELS_COUNTER_PROTECT(VALUE) VALUE == -1 : ?
+
+i = 0;
+
+// count to kb
+PARALLELS_GET_COUNTER(guest.ram.swap_in, v)
+if (v != -1)
+PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_SWAP_IN, v  12)
+
+PARALLELS_GET_COUNTER(guest.ram.swap_out, v)
+if (v != -1)
+PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_SWAP_OUT, v  12)
+
+PARALLELS_GET_COUNTER(guest.ram.minor_fault, v)
+if (v != -1)
+PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_MINOR_FAULT, v)
+
+PARALLELS_GET_COUNTER(guest.ram.major_fault, v)
+if (v != -1)
+PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_MAJOR_FAULT, v)
+
+PARALLELS_GET_COUNTER(guest.ram.total, v)
+if (v != -1)
+PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_AVAILABLE, v  10)
+
+PARALLELS_GET_COUNTER(guest.ram.balloon_actual, v)
+if (v != -1)
+PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_ACTUAL_BALLOON, v  
10)
+
+PARALLELS_GET_COUNTER(guest.ram.usage, u)
+PARALLELS_GET_COUNTER(guest.ram.total, t)
+if (u != -1  t != -1)
+PARALLELS_MEMORY_STAT_SET(VIR_DOMAIN_MEMORY_STAT_UNUSED, (t - u)  10)
+
+
+#undef PARALLELS_GET_COUNTER
+#undef PARALLELS_MEMORY_STAT_SET
+
+ret = i;
+ cleanup:
+if (dom)
+virDomainObjEndAPI(dom);
+
+return ret;
+}
+
 static virHypervisorDriver vzDriver = {
 .name = vz,
 .connectOpen = vzConnectOpen,/* 0.10.0 */
@@ -1429,6 +1501,7 @@ static virHypervisorDriver vzDriver = {
 .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */
 .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */
 .domainInterfaceStats = vzDomainInterfaceStats, /* 1.3.0 */
+.domainMemoryStats = vzDomainMemoryStats, /* 1.3.0 */
 };
 
 static virConnectDriver vzConnectDriver = {
-- 
1.7.1

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


[libvirt] [PATCH v2 0/3] vz: add statistics

2015-06-18 Thread Nikolay Shirokovskiy
Add vz statistics for network, cpu and memory.

 src/vz/vz_driver.c |  133 +++-
 src/vz/vz_sdk.c|   69 ++-
 src/vz/vz_sdk.h|4 ++

Changes from v1.
subject prefix changed from 'parallels' to 'vz'

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


[libvirt] [PATCH v2 1/3] vz: add net dev statistiscs

2015-06-18 Thread Nikolay Shirokovskiy
From: Nikolay Shirokovskiy nshirokovs...@parallels.com

Populate counters SDK currenly supports:
 rx_bytes
 rx_packets
 tx_bytes
 tx_packets

Comments.

1. Use vzDomObjFromDomainRef/virDomainObjEndAPI pair to get domain
object as we use prlsdkGetStatsParam that can release domain
object lock and thus we need a reference in case domain
is deleated meanwhile.

2. Introduce prlsdkGetAdapterIndex to convert from
device name to SDK device index. We need this index
as it is used in statistics data from SDK identify
network device. Unfortunately we need to enumerate
network devices to discover this mapping.

Signed-off-by: Nikolay Shirokovskiy nshirokovs...@virtuozzo.com
---
 src/vz/vz_driver.c |   44 +
 src/vz/vz_sdk.c|   69 +++-
 src/vz/vz_sdk.h|4 +++
 3 files changed, 116 insertions(+), 1 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index cef3c77..deac572 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1321,6 +1321,49 @@ vzDomainBlockStatsFlags(virDomainPtr domain,
 return ret;
 }
 
+static int
+vzDomainInterfaceStats(virDomainPtr domain,
+ const char *path,
+ virDomainInterfaceStatsPtr stats)
+{
+virDomainObjPtr dom = NULL;
+int ret = -1;
+int net_index = -1;
+char *name = NULL;
+
+if (!(dom = vzDomObjFromDomainRef(domain)))
+return -1;
+
+net_index = prlsdkGetAdapterIndex(dom, path);
+if (net_index  0)
+return -1;
+
+#define PARALLELS_GET_NET_COUNTER(VAL, NAME)\
+if (virAsprintf(name, net.nic%d.%s, net_index, NAME)  0)\
+goto cleanup;   \
+if (prlsdkGetStatsParam(dom, name, stats-VAL)  0)\
+goto cleanup;   \
+VIR_FREE(name);
+
+PARALLELS_GET_NET_COUNTER(rx_bytes, bytes_in)
+PARALLELS_GET_NET_COUNTER(rx_packets, pkts_in)
+PARALLELS_GET_NET_COUNTER(tx_bytes, bytes_out)
+PARALLELS_GET_NET_COUNTER(tx_packets, pkts_out)
+stats-rx_errs = -1;
+stats-rx_drop = -1;
+stats-tx_errs = -1;
+stats-tx_drop = -1;
+
+#undef PARALLELS_GET_NET_COUNTER
+ret = 0;
+
+ cleanup:
+VIR_FREE(name);
+if (dom)
+virDomainObjEndAPI(dom);
+
+return ret;
+}
 
 static virHypervisorDriver vzDriver = {
 .name = vz,
@@ -1373,6 +1416,7 @@ static virHypervisorDriver vzDriver = {
 .domainGetMaxMemory = vzDomainGetMaxMemory, /* 1.2.15 */
 .domainBlockStats = vzDomainBlockStats, /* 1.3.0 */
 .domainBlockStatsFlags = vzDomainBlockStatsFlags, /* 1.3.0 */
+.domainInterfaceStats = vzDomainInterfaceStats, /* 1.3.0 */
 };
 
 static virConnectDriver vzConnectDriver = {
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f9cde44..0956b58 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -3562,7 +3562,7 @@ prlsdkExtractStatsParam(PRL_HANDLE sdkstats, const char 
*name, long long *val)
 
 #define PARALLELS_STATISTICS_TIMEOUT (60 * 1000)
 
-static int
+int
 prlsdkGetStatsParam(virDomainObjPtr dom, const char *name, long long *val)
 {
 vzDomObjPtr privdom = dom-privateData;
@@ -3658,3 +3658,70 @@ prlsdkGetBlockStats(virDomainObjPtr dom, 
virDomainDiskDefPtr disk, virDomainBloc
 VIR_FREE(name);
 return ret;
 }
+
+static PRL_HANDLE
+prlsdkFindNetAdapter(virDomainObjPtr dom, const char *path)
+{
+PRL_UINT32 count = 0;
+vzDomObjPtr privdom = dom-privateData;
+PRL_UINT32 buflen = 0;
+PRL_RESULT pret;
+size_t i;
+char *name = NULL;
+PRL_HANDLE net = PRL_INVALID_HANDLE;
+
+pret = PrlVmCfg_GetNetAdaptersCount(privdom-sdkdom, count);
+prlsdkCheckRetGoto(pret, error);
+
+for (i = 0; i  count; ++i) {
+pret = PrlVmCfg_GetNetAdapter(privdom-sdkdom, i, net);
+prlsdkCheckRetGoto(pret, error);
+
+pret = PrlVmDevNet_GetHostInterfaceName(net, NULL, buflen);
+prlsdkCheckRetGoto(pret, error);
+
+if (VIR_ALLOC_N(name, buflen)  0)
+goto error;
+
+pret = PrlVmDevNet_GetHostInterfaceName(net, name, buflen);
+prlsdkCheckRetGoto(pret, error);
+
+if (STREQ(name, path))
+break;
+
+VIR_FREE(name);
+PrlHandle_Free(net);
+net = PRL_INVALID_HANDLE;
+}
+
+if (net == PRL_INVALID_HANDLE)
+virReportError(VIR_ERR_INVALID_ARG,
+   _(invalid path, '%s' is not a known interface), path);
+return net;
+
+ error:
+VIR_FREE(name);
+PrlHandle_Free(net);
+return PRL_INVALID_HANDLE;
+}
+
+int
+prlsdkGetAdapterIndex(virDomainObjPtr dom, const char *path)
+{
+PRL_HANDLE net = PRL_INVALID_HANDLE;
+PRL_UINT32 net_index = -1;
+PRL_RESULT pret;
+int ret = -1;
+
+net = prlsdkFindNetAdapter(dom, path);
+if (net == PRL_INVALID_HANDLE)
+return -1;
+
+pret = PrlVmDev_GetIndex(net, net_index);
+

[libvirt] [PATCH 2/2] daemon: Add the admin service to the admin server only if it was allocated

2015-06-18 Thread Peter Krempa
If the admin service is disabled it would not be allocated, but the NULL
pointer still would be added to the admin server. Since
virNetServerAddService would dereference it, the daemon would crash.

Move the service registration into the block that allocates it.
---
 daemon/libvirtd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 368e10c..286512a 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -534,10 +534,10 @@ daemonSetupNetworking(virNetServerPtr srv,
   
config-admin_max_queued_clients,
   
config-admin_max_client_requests)))
 goto error;
-}

-if (virNetServerAddService(srvAdm, svcAdm, NULL)  0)
-goto error;
+if (virNetServerAddService(srvAdm, svcAdm, NULL)  0)
+goto error;
+}

 if (ipsock) {
 if (config-listen_tcp) {
-- 
2.4.1

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


Re: [libvirt] libvirt proxy

2015-06-18 Thread Stefan Hajnoczi
On Thu, Jun 18, 2015 at 11:20 AM, Vasiliy Tolstov v.tols...@selfip.ru wrote:
 2015-06-18 12:01 GMT+03:00 Daniel P. Berrange berra...@redhat.com:
 FWIW, we really consider the RPC protocol to be a private impl detail
 and strongly discourage people from trying to re-implement it
 themselves.

 Can you explain a bit more about what you're trying to achieve by
 creating an RPC proxy


 I have DO like cloud hosting and want to provide libvirt compat
 endpoint to create/start/stop servers.

Would a hypervisor driver like the QEMU or Xen drivers in libvirt.git
work for you?

Instead of writing a libvirt proxy, add a driver for your cloud to libvirt.

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