Re: [libvirt] [PATCH 0/3] vz: fixes after commits refactoring common snapshot code

[Maxim Nestratov]

08-Apr-19 11:42, Nikolay Shirokovskiy пишет:

> Nikolay Shirokovskiy (3):
>vz: fix for tracking current snapshot
>vz: fixes: snapshot: Switch type of virDomainSnapshotObj.def
>vz: fixes: snapshot: Factor out virDomainMomentDef class
>
>   src/vz/vz_driver.c | 36 +++-
>   src/vz/vz_sdk.c| 24 +++-
>   2 files changed, 18 insertions(+), 42 deletions(-)


ACK. Thanks.

Maxim Nestratov


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

Re: [libvirt] [PATCH 0/2] vz: build fixes

[Maxim Nestratov]

17-Oct-17 20:40, Jiri Denemark пишет:


On Tue, Oct 17, 2017 at 16:55:13 +0300, Nikolay Shirokovskiy wrote:

Nikolay Shirokovskiy (2):
   vz: missing pieces for fd885a06 for vz driver
   vz: fix typo for 0d3d020b

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

ACK series

Jirka

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


Thank you. Pushed both.

Maxim

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

Re: [libvirt] [PATCH v2 0/2] vz: fix vz driver compilation

[Maxim Nestratov]

29-Jun-17 13:17, Mikhail Feoktistov пишет:


Mikhail Feoktistov (2):
   vz: add argument xmlopt for virDomainDefCheckABIStability call
   vz: nseclabels member is moved to virDomainChrSourceDef struct

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



ACK. Pushed, thanks.

Maxim

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

Re: [libvirt] [PATCH] docs: install html fonts and related

[Maxim Nestratov]

11-May-17 12:15, Daniel P. Berrange пишет:


On Thu, May 11, 2017 at 12:01:27PM +0300, Nikolay Shirokovskiy wrote:

---
  docs/Makefile.am | 22 --
  1 file changed, 20 insertions(+), 2 deletions(-)

ACK


Regards,
Daniel


Pushed.

Thanks,
Maxim

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

Re: [libvirt] [PATCH 1/2] vz: support virDomainSetVcpus

[Maxim Nestratov]

14-Apr-17 17:53, Konstantin Neumoin пишет:


Acked-by: Nikolay Shirokovskiy 
Signed-off-by: Konstantin Neumoin 
---
  src/vz/vz_driver.c | 43 +++
  src/vz/vz_sdk.c| 23 +++
  src/vz/vz_sdk.h|  1 +
  3 files changed, 67 insertions(+)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index da83a8f..ed7132f 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3905,6 +3905,47 @@ vzDomainReset(virDomainPtr domain, unsigned int flags)
  return ret;
  }

+static int vzDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus,
+ unsigned int flags)
+{
+virDomainObjPtr dom = NULL;
+int ret = -1;
+bool job = false;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(dom = vzDomObjFromDomainRef(domain)))
+goto cleanup;
+
+if (vzCheckConfigUpdateFlags(dom, ) < 0)
+goto cleanup;
+
+if (virDomainSetVcpusFlagsEnsureACL(domain->conn, dom->def, flags) < 0)
+goto cleanup;
+
+if (vzDomainObjBeginJob(dom) < 0)
+goto cleanup;
+job = true;
+
+if (vzEnsureDomainExists(dom) < 0)
+goto cleanup;
+
+ret = prlsdkSetCpuCount(dom, nvcpus);
+
+ cleanup:
+if (job)
+vzDomainObjEndJob(dom);
+virDomainObjEndAPI();
+return ret;
+}
+
+static int vzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
+{
+return vzDomainSetVcpusFlags(dom, nvcpus,
+ VIR_DOMAIN_AFFECT_LIVE | 
VIR_DOMAIN_AFFECT_CONFIG);
+}
+
  static virHypervisorDriver vzHypervisorDriver = {
  .name = "vz",
  .connectOpen = vzConnectOpen,/* 0.10.0 */
@@ -3954,6 +3995,8 @@ static virHypervisorDriver vzHypervisorDriver = {
  .domainDetachDeviceFlags = vzDomainDetachDeviceFlags, /* 1.2.15 */
  .domainIsActive = vzDomainIsActive, /* 1.2.10 */
  .domainIsUpdated = vzDomainIsUpdated, /* 1.2.21 */
+.domainSetVcpus = vzDomainSetVcpus, /* 3.3.0 */
+.domainSetVcpusFlags = vzDomainSetVcpusFlags, /* 3.3.0 */
  .domainGetVcpusFlags = vzDomainGetVcpusFlags, /* 1.2.21 */
  .domainGetMaxVcpus = vzDomainGetMaxVcpus, /* 1.2.21 */
  .domainSetUserPassword = vzDomainSetUserPassword, /* 2.0.0 */
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index c1a50fd..2daa44a 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4902,3 +4902,26 @@ int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,
   cleanup:
  return ret;
  }
+
+int prlsdkSetCpuCount(virDomainObjPtr dom, unsigned int count)
+{
+vzDomObjPtr privdom = dom->privateData;
+PRL_HANDLE job;
+PRL_RESULT pret;
+
+job = PrlVm_BeginEdit(privdom->sdkdom);
+if (PRL_FAILED(waitDomainJob(job, dom)))
+goto error;
+
+pret = PrlVmCfg_SetCpuCount(privdom->sdkdom, count);
+prlsdkCheckRetGoto(pret, error);
+
+job = PrlVm_CommitEx(privdom->sdkdom, 0);
+if (PRL_FAILED(waitDomainJob(job, dom)))
+goto error;
+
+return 0;
+
+ error:
+return -1;
+}
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index f8da2ad..100a5e3 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -71,6 +71,7 @@ int
  prlsdkGetMemoryStats(PRL_HANDLE sdkstas, virDomainMemoryStatPtr stats, 
unsigned int nr_stats);
  /* memsize is in MiB */
  int prlsdkSetMemsize(virDomainObjPtr dom, unsigned int memsize);
+int prlsdkSetCpuCount(virDomainObjPtr dom, unsigned int count);
  int
  prlsdkDomainSetUserPassword(virDomainObjPtr dom,
  const char *user,
--
2.7.4

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


ACK to both and pushed.

Thanks,
Maxim

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

Re: [libvirt] [RFC v2 0/2] vireventpoll implimentation using epoll

[Maxim Nestratov]

22-Feb-17 17:35, Dmitry Derbyshev пишет:


Ping.
Also, noticed that I renamed patch series, old name was "Substitute poll by epoll in 
virEventPollRunOnce". My bad.



A few issues required to be fixed to make 'make syntax-check' pass. Otherwise 
looks good to me.

Maxim


/14/2017 9:04 PM, Derbyshev Dmitriy пишет:

From: Derbyshev Dmitry 

Provides about 20% boost on local machine with 35 vms.
virEventPollDispatchHandles can also be split to pass cb via epoll
data field.

Should start sending as PATCH instead?

Changes since v1:
  * ifdef supstituded by 2 .c files with vireventpollinternal.h
implementations
  * PROBE purged

Derbyshev Dmitry (2):
   vireventpoll: isolate common code
   vireventpoll implimentation using epoll

  configure.ac  |  28 +
  src/Makefile.am   |  12 +-
  src/util/vireventepoll.c  | 201 +++
  src/util/vireventpoll.c   | 700 ++
  src/util/{vireventpoll.c => vireventpollcommon.c} | 231 ++-
  src/util/vireventpollinternal.h   |  91 +++
  tests/commanddata/{test14.log => test3epoll.log}  |   2 +
  tests/commandtest.c   |   4 +
  8 files changed, 451 insertions(+), 818 deletions(-)
  create mode 100644 src/util/vireventepoll.c
  copy src/util/{vireventpoll.c => vireventpollcommon.c} (78%)
  create mode 100644 src/util/vireventpollinternal.h
  copy tests/commanddata/{test14.log => test3epoll.log} (94%)



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


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

Re: [libvirt] [PATCH v2] cpu: fix typo: rename __kvm_hv_spinlock to __kvm_hv_spinlocks

[Maxim Nestratov]

09-Feb-17 15:57, Jiri Denemark пишет:


On Tue, Jan 31, 2017 at 17:12:13 +0300, Maxim Nestratov wrote:

Strings associated with virDomainHyperv values in domain_conf.c are used to
construct HyperV CPU features names to be compared with names defined in
cpu_x86_data.h and the names for HyperV "spinlocks" feature don't match.
This leads to a misleading warning:
"host doesn't support hyperv 'spinlocks' feature" even when it's supported.
Let's fix it and rename along with it VIR_CPU_x86_KVM_HV_SPINLOCK to
VIR_CPU_x86_KVM_HV_SPINLOCKS.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/cpu/cpu_x86.c  | 4 ++--
  src/cpu/cpu_x86_data.h | 8 +++-
  2 files changed, 9 insertions(+), 3 deletions(-)

ACK and pushed. I just updated the comment to explicitly mention
virDomainHyperv.

Jirka


Thanks!

Maxim

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

Re: [libvirt] [PATCH] vz: cleanup: remove unused constant

[Maxim Nestratov]

08-Feb-17 21:19, Andrea Bolognani пишет:


On Wed, 2017-02-08 at 18:26 +0300, Maxim Nestratov wrote:

PARALLELS_STATISTICS_DROP_COUNT isn't used anymore

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>

ACK


Pushed.

Thanks,
Maxim

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

Re: [libvirt] [PATCH 2/2] vz: fix event handle leak in prlsdkHandlePerfEvent

[Maxim Nestratov]

08-Feb-17 19:07, Daniel P. Berrange пишет:


On Wed, Feb 08, 2017 at 06:28:34PM +0300, Maxim Nestratov wrote:

When we happen to lose a domain but still get a performance event
for it, we should also free the event handle.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/vz/vz_sdk.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index ec954dd..614dca1 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2197,8 +2197,10 @@ prlsdkHandlePerfEvent(vzDriverPtr driver,
  virDomainObjPtr dom = NULL;
  vzDomObjPtr privdom = NULL;
  
-if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid)))

+if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid))) {
+PrlHandle_Free(event);
  return;
+}
  
  privdom = dom->privateData;

  PrlHandle_Free(privdom->stats);

ACK

Regards,
Daniel


Pushed the series.

Thanks,
Maxim

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

[libvirt] [PATCH 2/2] vz: fix event handle leak in prlsdkHandlePerfEvent

[Maxim Nestratov]

When we happen to lose a domain but still get a performance event
for it, we should also free the event handle.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index ec954dd..614dca1 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2197,8 +2197,10 @@ prlsdkHandlePerfEvent(vzDriverPtr driver,
 virDomainObjPtr dom = NULL;
 vzDomObjPtr privdom = NULL;
 
-if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid)))
+if (!(dom = virDomainObjListFindByUUID(driver->domains, uuid))) {
+PrlHandle_Free(event);
 return;
+}
 
 privdom = dom->privateData;
 PrlHandle_Free(privdom->stats);
-- 
2.4.11

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

[libvirt] [PATCH] vz: cleanup: remove unused constant

[Maxim Nestratov]

PARALLELS_STATISTICS_DROP_COUNT isn't used anymore

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 614dca1..30eb743 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2187,8 +2187,6 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
 return;
 }
 
-#define PARALLELS_STATISTICS_DROP_COUNT 3
-
 static void
 prlsdkHandlePerfEvent(vzDriverPtr driver,
   PRL_HANDLE event,
-- 
2.4.11

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

[libvirt] [PATCH 1/2] vz: fix handle leak in prlsdkHandleVmStateEvent

[Maxim Nestratov]

Every successful call of PrlEvent_GetParamByName allocates a handle,
which has to be freed.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 727b572..ec954dd 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2114,6 +2114,7 @@ prlsdkHandleVmStateEvent(vzDriverPtr driver,
 prlsdkSendEvent(driver, dom, lvEventType, lvEventTypeDetails);
 
  cleanup:
+PrlHandle_Free(eventParam);
 virObjectUnlock(dom);
 return;
 }
-- 
2.4.11

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

[libvirt] [PATCH 0/2] vz: fix two handle leaks

[Maxim Nestratov]

Maxim Nestratov (2):
  vz: fix handle leak in prlsdkHandleVmStateEvent
  vz: fix event handle leak in prlsdkHandlePerfEvent

 src/vz/vz_sdk.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.4.11

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

Re: [libvirt] [RFC PATCH 00/10] introduce push backups

[Maxim Nestratov]

08-Feb-17 17:52, Martin Kletzander пишет:


On Wed, Jan 18, 2017 at 01:21:48PM +0300, Nikolay Shirokovskiy wrote:

By the way we came to agreement to create distinct API to support backup 
operations in [1]
and there is a discussion of pull backup series in [2]. The latter is blocked
as some necessary operations are still experimental in qemu.

[1] https://www.redhat.com/archives/libvir-list/2016-March/msg00937.html
[2] https://www.redhat.com/archives/libvir-list/2016-September/msg00192.html



I went through the conversations and I feel quite better about
understanding the code, would you mind sending a new version (no matter
whether it's an RFC or not) to the list so it's applicable (and
hopefully will draw more attention)?



Just to have more context. The continuation of the thread [1] went to
https://www.redhat.com/archives/libvir-list/2016-April/msg00401.html

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

Re: [libvirt] [PATCH] vz: change printing format specifier for network statistics

[Maxim Nestratov]

31-Jan-17 15:24, Nikolay Shirokovskiy пишет:



On 31.01.2017 14:01, Maxim Nestratov wrote:

This is necessary to be able to get statistics for venet0 or
"host-routed" adapter, which has -1 index and thus, its statistics
is shown as "net.nic4294967295".

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/vz/vz_sdk.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f63b9a3..d5688e1 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4388,7 +4388,7 @@ prlsdkGetNetStats(PRL_HANDLE sdkstats, PRL_HANDLE sdkdom, 
const char *path,
  prlsdkCheckRetGoto(pret, cleanup);
  
  #define PRLSDK_GET_NET_COUNTER(VAL, NAME)   \

-if (virAsprintf(, "net.nic%d.%s", net_index, NAME) < 0)\
+if (virAsprintf(, "net.nic%u.%s", net_index, NAME) < 0)\
  goto cleanup;   \
  if (prlsdkExtractStatsParam(sdkstats, name, >VAL) < 0)   \
  goto cleanup;   \


ACK


Pushed.

Thanks,
Maxim

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

[libvirt] [PATCH v2] cpu: fix typo: rename __kvm_hv_spinlock to __kvm_hv_spinlocks

[Maxim Nestratov]

Strings associated with virDomainHyperv values in domain_conf.c are used to
construct HyperV CPU features names to be compared with names defined in
cpu_x86_data.h and the names for HyperV "spinlocks" feature don't match.
This leads to a misleading warning:
"host doesn't support hyperv 'spinlocks' feature" even when it's supported.
Let's fix it and rename along with it VIR_CPU_x86_KVM_HV_SPINLOCK to
VIR_CPU_x86_KVM_HV_SPINLOCKS.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/cpu/cpu_x86.c  | 4 ++--
 src/cpu/cpu_x86_data.h | 8 +++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 23a519e..40d98ee 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -100,7 +100,7 @@ KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_STIMER,
 0x4003, 0x0008);
 KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_RELAXED,
 0x4003, 0x0020);
-KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_SPINLOCK,
+KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_SPINLOCKS,
 0x4003, 0x0022);
 KVM_FEATURE_DEF(VIR_CPU_x86_KVM_HV_VAPIC,
 0x4003, 0x0030);
@@ -124,7 +124,7 @@ static virCPUx86Feature x86_kvm_features[] =
 KVM_FEATURE(VIR_CPU_x86_KVM_HV_SYNIC),
 KVM_FEATURE(VIR_CPU_x86_KVM_HV_STIMER),
 KVM_FEATURE(VIR_CPU_x86_KVM_HV_RELAXED),
-KVM_FEATURE(VIR_CPU_x86_KVM_HV_SPINLOCK),
+KVM_FEATURE(VIR_CPU_x86_KVM_HV_SPINLOCKS),
 KVM_FEATURE(VIR_CPU_x86_KVM_HV_VAPIC),
 KVM_FEATURE(VIR_CPU_x86_KVM_HV_VPINDEX),
 KVM_FEATURE(VIR_CPU_x86_KVM_HV_RESET),
diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index 4660ab6..4c51e43 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -49,11 +49,17 @@ struct _virCPUx86CPUID {
 # define VIR_CPU_x86_KVM_PV_EOI   "__kvm_pv_eoi"
 # define VIR_CPU_x86_KVM_PV_UNHALT"__kvm_pv_unhalt"
 # define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable"
+
+/*
+ * The following HyperV feature names suffixes must exactly match corresponding
+ * ones defined in domain_conf.c. E.g "__kvm_runtime" -> "runtime",
+ * "__kvm_hv_spinlocks" -> "spinlocks" etc.
+*/
 # define VIR_CPU_x86_KVM_HV_RUNTIME   "__kvm_hv_runtime"
 # define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic"
 # define VIR_CPU_x86_KVM_HV_STIMER"__kvm_hv_stimer"
 # define VIR_CPU_x86_KVM_HV_RELAXED   "__kvm_hv_relaxed"
-# define VIR_CPU_x86_KVM_HV_SPINLOCK  "__kvm_hv_spinlock"
+# define VIR_CPU_x86_KVM_HV_SPINLOCKS  "__kvm_hv_spinlocks"
 # define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic"
 # define VIR_CPU_x86_KVM_HV_VPINDEX   "__kvm_hv_vpindex"
 # define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset"
-- 
2.4.11

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

Re: [libvirt] [PATCH 0/2] vz: support more API calls

[Maxim Nestratov]

31-Jan-17 10:44, Nikolay Shirokovskiy пишет:


Nikolay Shirokovskiy (2):
   vz: support virDomainAbortJob
   vz: support virDomainReset

  src/vz/vz_driver.c | 53 +
  src/vz/vz_sdk.c| 48 
  src/vz/vz_sdk.h|  2 ++
  src/vz/vz_utils.c  |  1 +
  src/vz/vz_utils.h  |  2 ++
  5 files changed, 106 insertions(+)



ACK the series and pushed.

Thanks,
Maxim

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

Re: [libvirt] [PATCH] cpu: fix typo: rename __kvm_hv_spinlock to __kvm_hv_spinlocks

[Maxim Nestratov]

31-Jan-17 14:54, Jiri Denemark пишет:


On Tue, Jan 31, 2017 at 13:59:37 +0300, Maxim Nestratov wrote:

Corresponding hv cpu feature name is 'spinlocks' thus, we should have similar
name with __kvm_hv_ prefix. Otherwise we have misleading warning:
"host doesn't support hyperv 'spinlocks' feature" even when it's supported.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/cpu/cpu_x86_data.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index 4660ab6..4f3940b 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -53,7 +53,7 @@ struct _virCPUx86CPUID {
  # define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic"
  # define VIR_CPU_x86_KVM_HV_STIMER"__kvm_hv_stimer"
  # define VIR_CPU_x86_KVM_HV_RELAXED   "__kvm_hv_relaxed"
-# define VIR_CPU_x86_KVM_HV_SPINLOCK  "__kvm_hv_spinlock"
+# define VIR_CPU_x86_KVM_HV_SPINLOCK  "__kvm_hv_spinlocks"
  # define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic"
  # define VIR_CPU_x86_KVM_HV_VPINDEX   "__kvm_hv_vpindex"
  # define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset"

It took me a while to realized how this change can fix the issue. It's
because the strings associated with virDomainHyperv values are used to
construct these CPU features names and the names for this particular
feature didn't match. To avoid possible confusion in the future I'd
suggest adding a comment in cpu_x86_data.h saying the feature names must
match the strings for virDomainHyperv in domain_conf.c.

And while at it, I think you can
s/VIR_CPU_x86_KVM_HV_SPINLOCK/VIR_CPU_x86_KVM_HV_SPINLOCKS/ too.


Ok. Actually I had one version but somehow decided to minimize the difference.


Jirka


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

[libvirt] [PATCH] vz: change printing format specifier for network statistics

[Maxim Nestratov]

This is necessary to be able to get statistics for venet0 or
"host-routed" adapter, which has -1 index and thus, its statistics
is shown as "net.nic4294967295".

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f63b9a3..d5688e1 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4388,7 +4388,7 @@ prlsdkGetNetStats(PRL_HANDLE sdkstats, PRL_HANDLE sdkdom, 
const char *path,
 prlsdkCheckRetGoto(pret, cleanup);
 
 #define PRLSDK_GET_NET_COUNTER(VAL, NAME)   \
-if (virAsprintf(, "net.nic%d.%s", net_index, NAME) < 0)\
+if (virAsprintf(, "net.nic%u.%s", net_index, NAME) < 0)\
 goto cleanup;   \
 if (prlsdkExtractStatsParam(sdkstats, name, >VAL) < 0)   \
 goto cleanup;   \
-- 
2.4.11

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

[libvirt] [PATCH] cpu: fix typo: rename __kvm_hv_spinlock to __kvm_hv_spinlocks

[Maxim Nestratov]

Corresponding hv cpu feature name is 'spinlocks' thus, we should have similar
name with __kvm_hv_ prefix. Otherwise we have misleading warning:
"host doesn't support hyperv 'spinlocks' feature" even when it's supported.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/cpu/cpu_x86_data.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index 4660ab6..4f3940b 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -53,7 +53,7 @@ struct _virCPUx86CPUID {
 # define VIR_CPU_x86_KVM_HV_SYNIC "__kvm_hv_synic"
 # define VIR_CPU_x86_KVM_HV_STIMER"__kvm_hv_stimer"
 # define VIR_CPU_x86_KVM_HV_RELAXED   "__kvm_hv_relaxed"
-# define VIR_CPU_x86_KVM_HV_SPINLOCK  "__kvm_hv_spinlock"
+# define VIR_CPU_x86_KVM_HV_SPINLOCK  "__kvm_hv_spinlocks"
 # define VIR_CPU_x86_KVM_HV_VAPIC "__kvm_hv_vapic"
 # define VIR_CPU_x86_KVM_HV_VPINDEX   "__kvm_hv_vpindex"
 # define VIR_CPU_x86_KVM_HV_RESET "__kvm_hv_reset"
-- 
2.4.11

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

Re: [libvirt] [PATCH] vz: don't show bootorder for containers

[Maxim Nestratov]

30-Jan-17 18:35, Maxim Nestratov пишет:


29-Dec-16 12:58, Nikolay Shirokovskiy пишет:


Because this is invalid xml for containers. This patch almost
reverts 7eda8369, but still skips converting vz sdk bootorder
for containers to libvirt bootorder because we use boot order
in containers for quite different purpurse.

---

I know I reviewed this code just recently. It is just got out
of my sight.

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

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index ced58e5..43a6340 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1695,21 +1695,6 @@ prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE 
sdkType, int sdkIndex,
  return ret;
  }
  -static void
-prlsdkConvertBootOrderCt(virDomainDefPtr def)
-{
-size_t i;
-for (i = 0; i < def->nfss; i++) {
-
-if (STREQ(def->fss[i]->dst, "/")) {
-def->os.nBootDevs = 0;
-return;
-}
-}
-def->os.nBootDevs = 1;
-def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
-}
-
  static int
  prlsdkConvertBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def)
  {
@@ -1870,12 +1855,8 @@ prlsdkLoadDomain(vzDriverPtr driver,
  goto error;
/* depends on prlsdkAddDomainHardware */
-if (IS_CT(def)) {
-prlsdkConvertBootOrderCt(def);
-} else {
-if (prlsdkConvertBootOrderVm(sdkdom, def) < 0)
-goto error;
-}
+if (!IS_CT(def) && prlsdkConvertBootOrderVm(sdkdom, def) < 0)
+goto error;
pret = PrlVmCfg_GetEnvId(sdkdom, );
  prlsdkCheckRetGoto(pret, error);


Trying to make things look better I didn't take into account the fact that it will be impossible to use such xmls to 
define CTs correctly thus, ACK.


Maxim



Pushed now.

Maxim

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

Re: [libvirt] [PATCH 0/5] implement .connectGetAllDomainStats

[Maxim Nestratov]

12-Dec-16 10:56, Nikolay Shirokovskiy пишет:


Nikolay Shirokovskiy (5):
   vz: provide block stats for all domain stats
   vz: add net group to all domain stats
   vz: add vcpu group to all domain stats
   vz: add balloon group to all domain stats
   vz: add state group to all domain stats

  src/vz/vz_driver.c | 366 +
  src/vz/vz_sdk.c|   8 ++
  2 files changed, 374 insertions(+)



Pushed now.

Thanks,
Maxim

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

Re: [libvirt] [PATCH 5/5] vz: add state group to all domain stats

[Maxim Nestratov]

12-Dec-16 10:56, Nikolay Shirokovskiy пишет:


---
  src/vz/vz_driver.c | 25 +
  1 file changed, 25 insertions(+)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 01c1a96..d9bd2cd 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3729,6 +3729,28 @@ vzDomainGetBalloonStats(virDomainObjPtr dom,
  return 0;
  }
  
+static int

+vzDomainGetStateStats(virDomainObjPtr dom,
+  virDomainStatsRecordPtr record,
+  int *maxparams)
+{
+if (virTypedParamsAddInt(>params,
+ >nparams,
+ maxparams,
+ "state.state",
+ dom->state.state) < 0)
+return -1;
+
+if (virTypedParamsAddInt(>params,
+ >nparams,
+ maxparams,
+ "state.reason",
+ dom->state.reason) < 0)
+return -1;
+
+return 0;
+}
+
  static virDomainStatsRecordPtr
  vzDomainGetAllStats(virConnectPtr conn,
  virDomainObjPtr dom)
@@ -3739,6 +3761,9 @@ vzDomainGetAllStats(virConnectPtr conn,
  if (VIR_ALLOC(stat) < 0)
  return NULL;
  
+if (vzDomainGetStateStats(dom, stat, ) < 0)

+goto error;
+
  if (vzDomainGetBlockStats(dom, stat, ) < 0)
  goto error;
  


ACK  for patches 2-5 also.

Maxim

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

Re: [libvirt] [PATCH 1/5] vz: provide block stats for all domain stats

[Maxim Nestratov]

12-Dec-16 10:56, Nikolay Shirokovskiy пишет:


---
  src/vz/vz_driver.c | 189 +
  src/vz/vz_sdk.c|   8 +++
  2 files changed, 197 insertions(+)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 08f7961..6d173ef 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3508,6 +3508,194 @@ vzDomainGetJobStats(virDomainPtr domain,
  return ret;
  }
  
+#define VZ_ADD_STAT_PARAM_UUL(group, field, counter)\

+do {\
+if (stat.field != -1) { \
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,  \
+ group ".%zu." counter, i); \
+if (virTypedParamsAddULLong(>params,\
+>nparams,   \
+maxparams,  \
+param_name, \
+stat.field) < 0)\
+return -1;  \
+}   \
+} while (0)
+
+static int
+vzDomainGetBlockStats(virDomainObjPtr dom,
+  virDomainStatsRecordPtr record,
+  int *maxparams)
+{
+vzDomObjPtr privdom = dom->privateData;
+size_t i;
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+
+if (virTypedParamsAddUInt(>params,
+  >nparams,
+  maxparams,
+  "block.count",
+  dom->def->ndisks) < 0)
+return -1;
+
+for (i = 0; i < dom->def->ndisks; i++) {
+virDomainBlockStatsStruct stat;
+virDomainDiskDefPtr disk = dom->def->disks[i];
+
+if (prlsdkGetBlockStats(privdom->stats, disk, ) < 0)
+return -1;


This needs to be adapted to the latest HEAD as this function got additional 
parameter. I can do this while pushing.


+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "block.%zu.name", i);
+if (virTypedParamsAddString(>params,
+>nparams,
+maxparams,
+param_name,
+disk->dst) < 0)
+return -1;
+
+if (virStorageSourceIsLocalStorage(disk->src) && disk->src->path) {
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "block.%zu.path", i);
+if (virTypedParamsAddString(>params,
+>nparams,
+maxparams,
+param_name,
+disk->src->path) < 0)
+return -1;
+}
+
+VZ_ADD_STAT_PARAM_UUL("block", rd_req, "rd.reqs");
+VZ_ADD_STAT_PARAM_UUL("block", rd_bytes, "rd.bytes");
+VZ_ADD_STAT_PARAM_UUL("block", wr_req, "wr.reqs");
+VZ_ADD_STAT_PARAM_UUL("block", wr_bytes, "wr.bytes");
+
+if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "block.%zu.capacity", i);
+if (virTypedParamsAddULLong(>params,
+>nparams,
+maxparams,
+param_name,
+disk->src->capacity) < 0)
+return -1;
+}
+
+}
+
+return 0;
+}
+
+static virDomainStatsRecordPtr
+vzDomainGetAllStats(virConnectPtr conn,
+virDomainObjPtr dom)
+{
+virDomainStatsRecordPtr stat;
+int maxparams = 0;
+
+if (VIR_ALLOC(stat) < 0)
+return NULL;
+
+if (vzDomainGetBlockStats(dom, stat, ) < 0)
+goto error;
+
+if (!(stat->dom = virGetDomain(conn, dom->def->name, dom->def->uuid)))
+goto error;
+
+return stat;
+
+ error:
+virTypedParamsFree(stat->params, stat->nparams);
+virObjectUnref(stat->dom);
+VIR_FREE(stat);
+return NULL;
+}
+
+static int
+vzConnectGetAllDomainStats(virConnectPtr conn,
+   virDomainPtr *domains,
+   unsigned int ndomains,
+   unsigned int stats,
+   virDomainStatsRecordPtr **retStats,
+   unsigned int flags)
+{
+vzConnPtr privconn = conn->privateData;
+vzDriverPtr driver = privconn->driver;
+unsigned int lflags = flags & (VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE |
+   VIR_CONNECT_LIST_DOMAINS_FILTERS_PERSISTENT 
|
+   VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE);
+unsigned int supported = VIR_DOMAIN_STATS_STATE |
+  

Re: [libvirt] [PATCH] vz: don't show bootorder for containers

[Maxim Nestratov]

29-Dec-16 12:58, Nikolay Shirokovskiy пишет:


Because this is invalid xml for containers. This patch almost
reverts 7eda8369, but still skips converting vz sdk bootorder
for containers to libvirt bootorder because we use boot order
in containers for quite different purpurse.

---

I know I reviewed this code just recently. It is just got out
of my sight.

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

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index ced58e5..43a6340 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1695,21 +1695,6 @@ prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE 
sdkType, int sdkIndex,
  return ret;
  }
  
-static void

-prlsdkConvertBootOrderCt(virDomainDefPtr def)
-{
-size_t i;
-for (i = 0; i < def->nfss; i++) {
-
-if (STREQ(def->fss[i]->dst, "/")) {
-def->os.nBootDevs = 0;
-return;
-}
-}
-def->os.nBootDevs = 1;
-def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
-}
-
  static int
  prlsdkConvertBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def)
  {
@@ -1870,12 +1855,8 @@ prlsdkLoadDomain(vzDriverPtr driver,
  goto error;
  
  /* depends on prlsdkAddDomainHardware */

-if (IS_CT(def)) {
-prlsdkConvertBootOrderCt(def);
-} else {
-if (prlsdkConvertBootOrderVm(sdkdom, def) < 0)
-goto error;
-}
+if (!IS_CT(def) && prlsdkConvertBootOrderVm(sdkdom, def) < 0)
+goto error;
  
  pret = PrlVmCfg_GetEnvId(sdkdom, );

  prlsdkCheckRetGoto(pret, error);


Trying to make things look better I didn't take into account the fact that it will be impossible to use such xmls to 
define CTs correctly thus, ACK.


Maxim

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

[libvirt] [PATCH] docs: Add pit timer policy change description to news.xml

[Maxim Nestratov]

Add bug fix description of appying correct pit timer policy.
---
 docs/news.xml | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index e341ba2..d616ebd 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -184,6 +184,17 @@
   libvirt can recognize.
 
   
+  
+
+  qemu: Fix pit timer tick policy
+
+
+  By a mistake, when policy=delay pit timer tick policy
+  was specified, policy=discard was used instead. Now it
+  is possible to use both discard and delay
+  policies correctly.
+
+  
 
   
   
-- 
2.4.11

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

Re: [libvirt] [PATCH 3/3] qemu: tests: add "no-kvm-pit-device" testcase

[Maxim Nestratov]

05-Jan-17 19:53, John Ferlan пишет:



On 12/09/2016 09:28 AM, Maxim Nestratov wrote:

As specifying both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY
capabilities has no practical sense in tests, and we already have tests
for QEMU_CAPS_KVM_PIT_TICK_POLICY, it's better to add a separate one with
QEMU_CAPS_NO_KVM_PIT set.

My suggested alteration:

tests: Add "no-kvm-pit-device" testcase

Add a test case for when the QEMU_CAPS_NO_KVM_PIT capability is set.
This capability is mutually exclusive to QEMU_CAPS_KVM_PIT_TICK_POLICY
and results in the same output regardless of whether "discard" or
"delay" was specified in the guest XML for 'tickpolicy'.

ACK -

John


Your wording is better. Thanks for the review.

Maxim

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

Re: [libvirt] [PATCH 2/3] qemu: allow to specify pit timer tick policy=discard

[Maxim Nestratov]

05-Jan-17 19:52, John Ferlan пишет:



On 12/09/2016 09:28 AM, Maxim Nestratov wrote:

Reuse "kvm-pit-device" test case for testing it.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/qemu/qemu_command.c| 8 +++-
  tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml | 2 +-
  tests/qemuxml2argvtest.c   | 1 +
  3 files changed, 9 insertions(+), 2 deletions(-)


My suggestion here - let's rename *pit-device.xml to *pit-discard.xml
and do the same for the .args file.  That'll make it clearer. Of course
that means modifying argvtest.c as well.


I thought about it too but decided to leave it as is to reduce changes, thus I 
don't mind.


This would alter the commit message to:

 qemu: Allow to specify pit timer tick policy=discard

 Separate out the "policy=discard" into it's own specific
 qemu command line.

 We'll rename "kvm-pit-device" test case to be "kvm-pit-discard"
 since it has the syntax we'd be using.



Agree.




diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cd243e4..7f10d75 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6158,8 +6158,14 @@ qemuBuildClockCommandLine(virCommandPtr cmd,
  return -1;
  }
  break;
-case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
  case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD:
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY))
+virCommandAddArgList(cmd, "-global",
+ "kvm-pit.lost_tick_policy=discard", 
NULL);
+else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
+virCommandAddArg(cmd, "-no-kvm-pit-reinjection");
+break;
+case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
  /* no way to support these modes for pit in qemu */

s/these modes/this mode/


ACK w/ these adjustments (I can do this as well if you want)

John


This would be very kind of you as I'll able to fix it on monday only as we have 
holidays currently.

Maxim


  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 _("unsupported pit tickpolicy '%s'"),
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml
index 7835a1b..d8ddcba 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml
@@ -9,7 +9,7 @@
  


-
+

destroy
restart
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b8619dd..713a8fe 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2060,6 +2060,7 @@ mymain(void)
  qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
  
  DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY);

+DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY);
  
  DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC,

  QEMU_CAPS_NODEFCONFIG);



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

Re: [libvirt] [PATCH 1/3] qemu: fix pit timer tick policy=delay

[Maxim Nestratov]

05-Jan-17 19:52, John Ferlan пишет:


Since this has been sitting unreviewed for a while...


On 12/09/2016 09:28 AM, Maxim Nestratov wrote:

By a mistake, 'delay' libvirt xml parameter was converted to 'discard' QEMU
command line string one.

Test "kvm-pit-delay" is fixed accordinly, so that redundant test cases removed
as there is no need to specify both QEMU_CAPS_NO_KVM_PIT and
QEMU_CAPS_KVM_PIT_TICK_POLICY simultaneusly in tests as they are mutually
exclusive and "kvm-pit-device" becomes just the same as "kvm-pit-delay".

I'd like to alter the commit to be:

qemu: Fix pit timer tick policy=delay

By a mistake, for the VIR_DOMAIN_TIMER_TICKPOLICY_DELAY qemu
command line creation, 'discard' was used instead of 'delay'
in commit id '1569fa14'.

Test "kvm-pit-delay" is fixed accordingly to show the correct
option being generated.

Remove the (now) redundant kvm-pit-device tests. As it turns
out there is no need to specify both QEMU_CAPS_NO_KVM_PIT and
QEMU_CAPS_KVM_PIT_TICK_POLICY since they are mutually exclusive
and "kvm-pit-device" becomes just the same as "kvm-pit-delay".

...

ACK - (let me know if you agree with the adjust commit - I can push if
you would like, although I know you have that access.)

John


Agree with the your changes and don't mind if you push it.

Maxim

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

Re: [libvirt] [PATCH v2 5/5] vz: get disks statistics for CTs

[Maxim Nestratov]

22-Dec-16 12:13, Daniel P. Berrange пишет:


On Wed, Dec 21, 2016 at 10:38:52PM +0300, Maxim Nestratov wrote:

A CT disk statistics is reported with prefix "hdd" and we should use
it to extract data.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/vz/vz_driver.c | 10 --
  src/vz/vz_sdk.c|  5 +++--
  src/vz/vz_sdk.h|  2 +-
  3 files changed, 12 insertions(+), 5 deletions(-)

ACK


Regards,
Daniel
Though you ACKed this version I changed it a bit addressing Nickolay's comments on this patch and pushed modified 
version. The new chunk is as follows:


 address = >info.addr.drive;
-switch (disk->bus) {
-case VIR_DOMAIN_DISK_BUS_IDE:
-prefix = "ide";
-idx = address->bus * 2 + address->unit;
-break;
-case VIR_DOMAIN_DISK_BUS_SATA:
-prefix = "sata";
-idx = address->unit;
-break;
-case VIR_DOMAIN_DISK_BUS_SCSI:
-prefix = "scsi";
+
+if (isCt) {
+prefix = "hdd";
 idx = address->unit;
-break;
-default:
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unknown disk bus: %X"), disk->bus);
-goto cleanup;
+} else {
+switch (disk->bus) {
+case VIR_DOMAIN_DISK_BUS_IDE:
+prefix = "ide";
+idx = address->bus * 2 + address->unit;
+break;
+case VIR_DOMAIN_DISK_BUS_SATA:
+prefix = "sata";
+idx = address->unit;
+break;
+case VIR_DOMAIN_DISK_BUS_SCSI:
+prefix = "scsi";
+idx = address->unit;
+break;
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unknown disk bus: %X"), disk->bus);
+goto cleanup;
+}
 }

Thus, although we report CT's disk bus as scsi in most cases, we don't really care what exact bus is and explicitly set 
prefix to "hdd" to extract statistics.


With this change I pushed the series.

Maxim

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

[libvirt] [PATCH v2 1/5] vz: report "scsi" bus for disks when nothing was set explixitly

[Maxim Nestratov]

This is necessary to show CTs created out of libvirt correctly.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 64748fa..2c8ce03 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -563,6 +563,7 @@ prlsdkGetDiskId(PRL_HANDLE disk, int *bus, char **dst)
 *dst = virIndexToDiskName(pos, "hd");
 break;
 case PMS_SCSI_DEVICE:
+case PMS_UNKNOWN_DEVICE:
 *bus = VIR_DOMAIN_DISK_BUS_SCSI;
 *dst = virIndexToDiskName(pos, "sd");
 break;
-- 
2.4.11

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

[libvirt] [PATCH v2 4/5] vz: set boot from disk for CT only when there is no root filesystem

[Maxim Nestratov]

Before, boot devices information for CTs was always empty and we
didn't indicate that containers can boot from disk.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 54a21a3..cc077f5 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1704,8 +1704,23 @@ prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE 
sdkType, int sdkIndex,
 return ret;
 }
 
+static void
+prlsdkConvertBootOrderCt(virDomainDefPtr def)
+{
+size_t i;
+for (i = 0; i < def->nfss; i++) {
+
+if (STREQ(def->fss[i]->dst, "/")) {
+def->os.nBootDevs = 0;
+return;
+}
+}
+def->os.nBootDevs = 1;
+def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
+}
+
 static int
-prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def)
+prlsdkConvertBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def)
 {
 int ret = -1;
 PRL_RESULT pret;
@@ -1864,8 +1879,12 @@ prlsdkLoadDomain(vzDriverPtr driver,
 goto error;
 
 /* depends on prlsdkAddDomainHardware */
-if (prlsdkConvertBootOrder(sdkdom, def) < 0)
-goto error;
+if (IS_CT(def)) {
+prlsdkConvertBootOrderCt(def);
+} else {
+if (prlsdkConvertBootOrderVm(sdkdom, def) < 0)
+goto error;
+}
 
 pret = PrlVmCfg_GetEnvId(sdkdom, );
 prlsdkCheckRetGoto(pret, error);
-- 
2.4.11

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

[libvirt] [PATCH v2 3/5] vz: report disks either as disks or filesystems depending on original xml

[Maxim Nestratov]

Virtuozzo SDK interface doesn't differ filesystems from disks and sees them as 
disks.
Before, we always mistakenly presented disks based on files as filesystems, 
which is
not completely correct. Now we are going to show either disks or filesystems 
depending
on a hint, which uses boot device section of VZ config. Though this information
doesn't change booting order of a CT, it is used by vz libvirt interface as a 
hint
for libvirt representation of disks. Since now, if we have filesystems in input 
xml,
then we add them to VZ booting devices list and rely on this information to show
corresponding libvirt xml.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 96 ++---
 1 file changed, 85 insertions(+), 11 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 8d75399..54a21a3 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -50,6 +50,9 @@ static PRL_HANDLE
 prlsdkFindNetByMAC(PRL_HANDLE sdkdom, virMacAddrPtr mac);
 static PRL_HANDLE
 prlsdkGetDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk);
+static bool
+prlsdkInBootList(PRL_HANDLE sdkdom,
+ PRL_HANDLE sdktargetdev);
 
 /*
  * Log error description
@@ -758,7 +761,8 @@ prlsdkAddDomainHardDisksInfo(vzDriverPtr driver, PRL_HANDLE 
sdkdom, virDomainDef
 pret = PrlVmDev_GetEmulatedType(hdd, );
 prlsdkCheckRetGoto(pret, error);
 
-if (PDT_USE_REAL_DEVICE != emulatedType && IS_CT(def)) {
+if (IS_CT(def) &&
+prlsdkInBootList(sdkdom, hdd)) {
 
 if (!(fs = virDomainFSDefNew()))
 goto error;
@@ -1555,6 +1559,69 @@ virFindDiskBootIndex(virDomainDefPtr def, 
virDomainDiskDevice type, int index)
 return NULL;
 }
 
+static bool
+prlsdkInBootList(PRL_HANDLE sdkdom,
+ PRL_HANDLE sdktargetdev)
+{
+bool ret = false;
+PRL_RESULT pret;
+PRL_UINT32 bootNum;
+PRL_HANDLE bootDev = PRL_INVALID_HANDLE;
+PRL_BOOL inUse;
+PRL_DEVICE_TYPE sdkType, targetType;
+PRL_UINT32 sdkIndex, targetIndex;
+PRL_HANDLE dev = PRL_INVALID_HANDLE;
+size_t i;
+
+pret = PrlVmDev_GetType(sdktargetdev, );
+prlsdkCheckRetExit(pret, -1);
+
+pret = PrlVmDev_GetIndex(sdktargetdev, );
+prlsdkCheckRetExit(pret, -1);
+
+pret = PrlVmCfg_GetBootDevCount(sdkdom, );
+prlsdkCheckRetExit(pret, -1);
+
+for (i = 0; i < bootNum; ++i) {
+pret = PrlVmCfg_GetBootDev(sdkdom, i, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlBootDev_IsInUse(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+if (!inUse) {
+PrlHandle_Free(bootDev);
+bootDev = PRL_INVALID_HANDLE;
+continue;
+}
+
+pret = PrlBootDev_GetType(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlBootDev_GetIndex(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+dev = prlsdkGetDevByDevIndex(sdkdom, sdkType, sdkIndex);
+if (dev == PRL_INVALID_HANDLE)
+goto cleanup;
+
+PrlHandle_Free(dev);
+dev = PRL_INVALID_HANDLE;
+
+PrlHandle_Free(bootDev);
+bootDev = PRL_INVALID_HANDLE;
+
+if (sdkIndex == targetIndex && sdkType == targetType) {
+ret = true;
+break;
+}
+}
+
+ cleanup:
+PrlHandle_Free(dev);
+PrlHandle_Free(bootDev);
+return ret;
+}
 static int
 prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE sdkType, int sdkIndex,
  virDomainDefPtr def, int bootIndex)
@@ -3740,23 +3807,26 @@ prlsdkSetBootOrderCt(PRL_HANDLE sdkdom, virDomainDefPtr 
def)
 size_t i;
 PRL_HANDLE hdd = PRL_INVALID_HANDLE;
 PRL_RESULT pret;
+bool rootfs = false;
 int ret = -1;
 
-/* if we have root mounted we don't need to explicitly set boot order */
 for (i = 0; i < def->nfss; i++) {
+
+pret = prlsdkAddDeviceToBootList(sdkdom, i, PDE_HARD_DISK, i + 1);
+prlsdkCheckRetExit(pret, -1);
+
 if (STREQ(def->fss[i]->dst, "/"))
-return 0;
+rootfs = true;
 }
 
-/* else set first hard disk as boot device */
-pret = prlsdkAddDeviceToBootList(sdkdom, 0, PDE_HARD_DISK, 0);
-prlsdkCheckRetExit(pret, -1);
-
-pret = PrlVmCfg_GetHardDisk(sdkdom, 0, );
-prlsdkCheckRetExit(pret, -1);
+if (!rootfs) {
+/* if we have root mounted we don't need to explicitly set boot order 
*/
+pret = PrlVmCfg_GetHardDisk(sdkdom, def->nfss, );
+prlsdkCheckRetExit(pret, -1);
 
-PrlVmDevHd_SetMountPoint(hdd, "/");
-prlsdkCheckRetGoto(pret, cleanup);
+PrlVmDevHd_SetMountPoint(hdd, "/");
+prlsdkCheckRetGoto(pret, cleanup);
+}
 
 ret = 0;
 
@@ -3926,11 +3996,15 @@ prlsdkDoApplyConfig(vzDriverPtr driver,
 goto error;
 }
 
+/* It is important that 

[libvirt] [PATCH v2 0/5] vz: fix some CT disk representation cases and its statistics

[Maxim Nestratov]

v1-v2 changes - comments addressed

Maxim Nestratov (5):
  vz: report "scsi" bus for disks when nothing was set explixitly
  vz: don't add implicit devices for CTs
  vz: report disks either as disks or filesystems depending on original
xml
  vz: set boot from disk for CT only when there is no root filesystem
  vz: get disks statistics for CTs

 src/vz/vz_driver.c |  10 -
 src/vz/vz_sdk.c| 129 ++---
 src/vz/vz_sdk.h|   2 +-
 3 files changed, 121 insertions(+), 20 deletions(-)

-- 
2.4.11

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

[libvirt] [PATCH v2 5/5] vz: get disks statistics for CTs

[Maxim Nestratov]

A CT disk statistics is reported with prefix "hdd" and we should use
it to extract data.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_driver.c | 10 --
 src/vz/vz_sdk.c|  5 +++--
 src/vz/vz_sdk.h|  2 +-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index af9ef7f..93d8552 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1743,7 +1743,10 @@ vzDomainBlockStatsImpl(virDomainObjPtr dom,
 virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path);
 return -1;
 }
-if (prlsdkGetBlockStats(privdom->stats, dom->def->disks[idx], stats) < 
0)
+if (prlsdkGetBlockStats(privdom->stats,
+dom->def->disks[idx],
+stats,
+IS_CT(dom->def)) < 0)
 return -1;
 } else {
 virDomainBlockStatsStruct s;
@@ -1756,7 +1759,10 @@ vzDomainBlockStatsImpl(virDomainObjPtr dom,
 #undef PARALLELS_ZERO_STATS
 
 for (i = 0; i < dom->def->ndisks; i++) {
-if (prlsdkGetBlockStats(privdom->stats, dom->def->disks[i], ) < 
0)
+if (prlsdkGetBlockStats(privdom->stats,
+dom->def->disks[i],
+,
+IS_CT(dom->def)) < 0)
 return -1;
 
 #define PARALLELS_SUM_STATS(VAR, TYPE, NAME)\
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index cc077f5..630a18f 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4376,7 +4376,8 @@ prlsdkExtractStatsParam(PRL_HANDLE sdkstats, const char 
*name, long long *val)
 int
 prlsdkGetBlockStats(PRL_HANDLE sdkstats,
 virDomainDiskDefPtr disk,
-virDomainBlockStatsPtr stats)
+virDomainBlockStatsPtr stats,
+bool isCt)
 {
 virDomainDeviceDriveAddressPtr address;
 int idx;
@@ -4395,7 +4396,7 @@ prlsdkGetBlockStats(PRL_HANDLE sdkstats,
 idx = address->unit;
 break;
 case VIR_DOMAIN_DISK_BUS_SCSI:
-prefix = "scsi";
+prefix = isCt ? "hdd" : "scsi";
 idx = address->unit;
 break;
 default:
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index ef789ab..e4e46dc 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -63,7 +63,7 @@ prlsdkDetachDevice(vzDriverPtr driver, virDomainObjPtr dom, 
virDomainDeviceDefPt
 int
 prlsdkUpdateDevice(vzDriverPtr driver, virDomainObjPtr dom, 
virDomainDeviceDefPtr dev);
 int
-prlsdkGetBlockStats(PRL_HANDLE sdkstats, virDomainDiskDefPtr disk, 
virDomainBlockStatsPtr stats);
+prlsdkGetBlockStats(PRL_HANDLE sdkstats, virDomainDiskDefPtr disk, 
virDomainBlockStatsPtr stats, bool isCt);
 int
 prlsdkGetNetStats(PRL_HANDLE sdkstas, PRL_HANDLE sdkdom, const char *path, 
virDomainInterfaceStatsPtr stats);
 int
-- 
2.4.11

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

[libvirt] [PATCH v2 2/5] vz: don't add implicit devices for CTs

[Maxim Nestratov]

Implicit devices like controllers are confusing for CTs and
function virDomainDefAddImplicitDevices never intended to be called
for CTs.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 2c8ce03..8d75399 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1815,7 +1815,7 @@ prlsdkLoadDomain(vzDriverPtr driver,
 if (prlsdkGetDomainState(dom, sdkdom, ) < 0)
 goto error;
 
-if (virDomainDefAddImplicitDevices(def) < 0)
+if (!IS_CT(def) && virDomainDefAddImplicitDevices(def) < 0)
 goto error;
 
 if (def->ngraphics > 0) {
-- 
2.4.11

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

Re: [libvirt] [PATCH 4/5] vz: report disks either as disks or filesystems depending on original xml

[Maxim Nestratov]

21-Dec-16 10:25, Nikolay Shirokovskiy пишет:



On 09.12.2016 17:36, Maxim Nestratov wrote:

Virtuozzo SDK interface doesn't differ filesystems from disks and sees them as 
disks.
Before, we always mistakenly presented disks based on files as filesystems, 
which is
not completely correct. Now we are going to show either disks or filesystems 
depending
on a hint, which uses boot device section of VZ config. Though this information
doesn't change booting order of a CT, it is used by vz libvirt interface as a 
hint
for libvirt representation of disks. Since now, if we have filesystems in input 
xml,
then we add them to VZ booting devices list and rely on this information to show
corresponding libvirt xml.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/vz/vz_sdk.c | 96 +++--
  1 file changed, 86 insertions(+), 10 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 1030a1b..9b8b48e 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -50,6 +50,9 @@ static PRL_HANDLE
  prlsdkFindNetByMAC(PRL_HANDLE sdkdom, virMacAddrPtr mac);
  static PRL_HANDLE
  prlsdkGetDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk);
+static bool
+prlsdkInBootList(PRL_HANDLE sdkdom,
+ PRL_HANDLE sdktargetdev);
  
  /*

   * Log error description
@@ -754,7 +757,8 @@ prlsdkAddDomainHardDisksInfo(vzDriverPtr driver, PRL_HANDLE 
sdkdom, virDomainDef
  pret = PrlVmDev_GetEmulatedType(hdd, );
  prlsdkCheckRetGoto(pret, error);
  
-if (PDT_USE_REAL_DEVICE != emulatedType && IS_CT(def)) {

+if (IS_CT(def) &&
+prlsdkInBootList(sdkdom, hdd)) {
  
  if (!(fs = virDomainFSDefNew()))

  goto error;
@@ -1551,6 +1555,75 @@ virFindDiskBootIndex(virDomainDefPtr def, 
virDomainDiskDevice type, int index)
  return NULL;
  }
  
+static bool

+prlsdkInBootList(PRL_HANDLE sdkdom,
+ PRL_HANDLE sdktargetdev)
+{
+bool ret = false;
+PRL_RESULT pret;
+PRL_UINT32 bootNum;
+PRL_HANDLE bootDev = PRL_INVALID_HANDLE;
+PRL_BOOL inUse;
+PRL_DEVICE_TYPE sdkType;
+PRL_UINT32 sdkIndex, bootIndex;
+PRL_UINT32 pos, targetpos;
+PRL_HANDLE dev = PRL_INVALID_HANDLE;
+size_t i;
+
+pret = PrlVmDev_GetStackIndex(sdktargetdev, );
+prlsdkCheckRetExit(pret, -1);
+
+pret = PrlVmCfg_GetBootDevCount(sdkdom, );
+prlsdkCheckRetExit(pret, -1);
+
+if (bootNum > VIR_DOMAIN_MAX_BOOT_DEVS) {
+bootNum = VIR_DOMAIN_MAX_BOOT_DEVS;
+VIR_WARN("Too many boot devices");
+}

This is not nesessary. We have VIR_DOMAIN_MAX_BOOT_DEVS limit
only if we want to present boot order in domain definition.


Ok. Seems to be true.


+
+for (i = 0; i < bootNum; ++i) {
+pret = PrlVmCfg_GetBootDev(sdkdom, i, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlBootDev_IsInUse(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+if (!inUse) {
+PrlHandle_Free(bootDev);
+bootDev = PRL_INVALID_HANDLE;
+continue;
+}
+
+pret = PrlBootDev_GetSequenceIndex(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);

bootIndex is not used


Yeah, my first intention was to return it, then I reconsidered it but left the 
variable.




+
+pret = PrlBootDev_GetType(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlBootDev_GetIndex(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+dev = prlsdkGetDevByDevIndex(sdkdom, sdkType, sdkIndex);
+if (dev == PRL_INVALID_HANDLE)
+goto cleanup;
+
+pret = PrlVmDev_GetStackIndex(dev, );
+prlsdkCheckRetExit(pret, -1);

I would use PrlVmDev_GetType and PrlVmDev_GetIndex for given device
and compare obtained values against sdkType and sdkIndex. It is more straight 
forward.


Ok. I'll try to do it this way.




+
+PrlHandle_Free(bootDev);
+bootDev = PRL_INVALID_HANDLE;
+
+if (pos == targetpos) {
+ret = true;
+break;
+}
+}
+
+ cleanup:
+PrlHandle_Free(dev);
+PrlHandle_Free(bootDev);
+return ret;
+}
  static int
  prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE sdkType, int sdkIndex,
   virDomainDefPtr def, int bootIndex)
@@ -3741,23 +3814,26 @@ prlsdkSetBootOrderCt(PRL_HANDLE sdkdom, virDomainDefPtr 
def)
  size_t i;
  PRL_HANDLE hdd = PRL_INVALID_HANDLE;
  PRL_RESULT pret;
+bool rootfs = false;
  int ret = -1;
  
  /* if we have root mounted we don't need to explicitly set boot order */

let's move this comment to 'if (!rootfs) {'


Ok


  for (i = 0; i < def->nfss; i++) {
+
+pret = prlsdkAddDeviceToBootList(sdkdom, i, PDE_HARD_DISK, i + 1);
+prlsdkCheckRetExit(pret, -1);

As this relies on the fact that we add disks for filesystems first and
then disks for di

Re: [libvirt] [PATCH 3/5] vz: don't add implicit devices for CTs

[Maxim Nestratov]

20-Dec-16 17:44, Nikolay Shirokovskiy пишет:



On 09.12.2016 17:36, Maxim Nestratov wrote:

Implicit devices like controllers are confusing for CTs as they are faked.
Other devices like video we require to be specified explicitly.

I would drop last sentence. prlsdkApplyVideoParams shows that
we ignore video params for containers.

Let's reword commit message as we discussed offline - 
virDomainDefAddImplicitDevices
is just never intended for containers.


Ok

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

Re: [libvirt] [PATCH 2/5] vz: don't query boot devices information for VZ, set boot from disk always

[Maxim Nestratov]

20-Dec-16 13:50, Nikolay Shirokovskiy пишет:



On 09.12.2016 17:36, Maxim Nestratov wrote:

Before, boot devices information for CTs was always empty and we
didn't indicate that containers always boot from disk.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/vz/vz_sdk.c | 13 +
  1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 9976e4c..a6236d0 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1634,7 +1634,7 @@ prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE 
sdkType, int sdkIndex,
  }
  
  static int

-prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def)
+prlsdkConvertBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def)
  {
  int ret = -1;
  PRL_RESULT pret;
@@ -1792,9 +1792,14 @@ prlsdkLoadDomain(vzDriverPtr driver,
  if (prlsdkAddDomainHardware(driver, sdkdom, def) < 0)
  goto error;
  
-/* depends on prlsdkAddDomainHardware */

-if (prlsdkConvertBootOrder(sdkdom, def) < 0)
-goto error;
+if (IS_CT(def)) {
+def->os.nBootDevs = 1;
+def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
+} else {
+/* depends on prlsdkAddDomainHardware */
+if (prlsdkConvertBootOrderVm(sdkdom, def) < 0)
+goto error;
+}
  
  pret = PrlVmCfg_GetEnvId(sdkdom, );

  prlsdkCheckRetGoto(pret, error);


I think we'd better make this code symmetrical to prlsdkSetBootOrderCt,
that is report boot order only if there is no filesystem mounted to root.
Also we need to fix prlsdkCheckUnsupportedParams to allow boot order
if there is no root filesystems but still forbid boot from network etc.

Nikolay


Ok. Makes sense to move this change after #4 and rely on def->fss previously 
constructed

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

Re: [libvirt] [PATCH 1/5] vz: report "scsi" bus for disks when nothing was set explixitly

[Maxim Nestratov]

20-Dec-16 13:20, Nikolay Shirokovskiy пишет:



On 09.12.2016 17:36, Maxim Nestratov wrote:

This is necessary for to show CTs created out of libvirt correctly.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/vz/vz_sdk.c | 11 ---
  1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index d5688e1..9976e4c 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -562,18 +562,15 @@ prlsdkGetDiskId(PRL_HANDLE disk, int *bus, char **dst)
  *bus = VIR_DOMAIN_DISK_BUS_IDE;
  *dst = virIndexToDiskName(pos, "hd");
  break;
-case PMS_SCSI_DEVICE:
-*bus = VIR_DOMAIN_DISK_BUS_SCSI;
-*dst = virIndexToDiskName(pos, "sd");
-break;
  case PMS_SATA_DEVICE:
  *bus = VIR_DOMAIN_DISK_BUS_SATA;
  *dst = virIndexToDiskName(pos, "sd");
  break;
+case PMS_SCSI_DEVICE:
  default:
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unknown disk bus: %X"), ifType);
-return -1;
+*bus = VIR_DOMAIN_DISK_BUS_SCSI;
+*dst = virIndexToDiskName(pos, "sd");
+break;
  }
  
  if (NULL == *dst)



So this is special case only for containers and only for special
'undefined' value of bus type (we don't set/report bus type if create
containers with help of virtuozzo tools). I would code the condition exactly as
it is. This patch can turn into scsi unexpected cases.

Nikolay


Actually I agree. Will fix in next version

Maxim

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

[libvirt] [PATCH 1/3] qemu: fix pit timer tick policy=delay

[Maxim Nestratov]

By a mistake, 'delay' libvirt xml parameter was converted to 'discard' QEMU
command line string one.

Test "kvm-pit-delay" is fixed accordinly, so that redundant test cases removed
as there is no need to specify both QEMU_CAPS_NO_KVM_PIT and
QEMU_CAPS_KVM_PIT_TICK_POLICY simultaneusly in tests as they are mutually
exclusive and "kvm-pit-device" becomes just the same as "kvm-pit-delay".

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/qemu/qemu_command.c| 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args | 2 +-
 tests/qemuxml2argvtest.c   | 5 +
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 815abff..cd243e4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6139,7 +6139,7 @@ qemuBuildClockCommandLine(virCommandPtr cmd,
(-no-kvm-pit), otherwise, the default is catchup. */
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY))
 virCommandAddArgList(cmd, "-global",
- "kvm-pit.lost_tick_policy=discard", 
NULL);
+ "kvm-pit.lost_tick_policy=delay", 
NULL);
 else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
 virCommandAddArg(cmd, "-no-kvm-pit-reinjection");
 break;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args 
b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args
index 1d69797..7a02d36 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args
@@ -14,7 +14,7 @@ QEMU_AUDIO_DRV=none \
 -nographic \
 -nodefaults \
 -monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
--no-kvm-pit-reinjection \
+-global kvm-pit.lost_tick_policy=delay \
 -no-acpi \
 -boot c \
 -usb \
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 90d6aaf..b8619dd 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2059,10 +2059,7 @@ mymain(void)
 QEMU_CAPS_KVM);
 qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
 
-DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY);
-DO_TEST("kvm-pit-delay", QEMU_CAPS_NO_KVM_PIT);
-DO_TEST("kvm-pit-device", QEMU_CAPS_NO_KVM_PIT,
-QEMU_CAPS_KVM_PIT_TICK_POLICY);
+DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY);
 
 DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC,
 QEMU_CAPS_NODEFCONFIG);
-- 
2.4.11

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

[libvirt] [PATCH 3/3] qemu: tests: add "no-kvm-pit-device" testcase

[Maxim Nestratov]

As specifying both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY
capabilities has no practical sense in tests, and we already have tests
for QEMU_CAPS_KVM_PIT_TICK_POLICY, it's better to add a separate one with
QEMU_CAPS_NO_KVM_PIT set.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 .../qemuxml2argv-no-kvm-pit-device.args| 23 +
 .../qemuxml2argv-no-kvm-pit-device.xml | 29 ++
 tests/qemuxml2argvtest.c   |  1 +
 3 files changed, 53 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args 
b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args
new file mode 100644
index 000..1d69797
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args
@@ -0,0 +1,23 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 2,sockets=2,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-kvm-pit-reinjection \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml
new file mode 100644
index 000..d8ddcba
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml
@@ -0,0 +1,29 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  2
+  
+hvm
+
+  
+  
+
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+
+
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 713a8fe..77af591 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2061,6 +2061,7 @@ mymain(void)
 
 DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY);
 DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY);
+DO_TEST("no-kvm-pit-device", QEMU_CAPS_NO_KVM_PIT);
 
 DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC,
 QEMU_CAPS_NODEFCONFIG);
-- 
2.4.11

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

[libvirt] [PATCH 2/3] qemu: allow to specify pit timer tick policy=discard

[Maxim Nestratov]

Reuse "kvm-pit-device" test case for testing it.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/qemu/qemu_command.c| 8 +++-
 tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml | 2 +-
 tests/qemuxml2argvtest.c   | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cd243e4..7f10d75 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6158,8 +6158,14 @@ qemuBuildClockCommandLine(virCommandPtr cmd,
 return -1;
 }
 break;
-case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
 case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD:
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY))
+virCommandAddArgList(cmd, "-global",
+ "kvm-pit.lost_tick_policy=discard", 
NULL);
+else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT))
+virCommandAddArg(cmd, "-no-kvm-pit-reinjection");
+break;
+case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE:
 /* no way to support these modes for pit in qemu */
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported pit tickpolicy '%s'"),
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml
index 7835a1b..d8ddcba 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml
@@ -9,7 +9,7 @@
 
   
   
-
+
   
   destroy
   restart
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b8619dd..713a8fe 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2060,6 +2060,7 @@ mymain(void)
 qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
 
 DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY);
+DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY);
 
 DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC,
 QEMU_CAPS_NODEFCONFIG);
-- 
2.4.11

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

[libvirt] [PATCH 0/3] qemu: fix pit timer tick policy

[Maxim Nestratov]

Maxim Nestratov (3):
  qemu: fix pit timer tick policy=delay
  qemu: allow to specify pit timer tick policy=discard
  qemu: tests: add "no-kvm-pit-device" testcase

 src/qemu/qemu_command.c| 10 ++--
 .../qemuxml2argv-kvm-pit-delay.args|  2 +-
 .../qemuxml2argv-kvm-pit-device.xml|  2 +-
 .../qemuxml2argv-no-kvm-pit-device.args| 23 +
 .../qemuxml2argv-no-kvm-pit-device.xml | 29 ++
 tests/qemuxml2argvtest.c   |  5 ++--
 6 files changed, 64 insertions(+), 7 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml

-- 
2.4.11

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

[libvirt] [PATCH 3/5] vz: don't add implicit devices for CTs

[Maxim Nestratov]

Implicit devices like controllers are confusing for CTs as they are faked.
Other devices like video we require to be specified explicitly.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index a6236d0..1030a1b 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1816,7 +1816,7 @@ prlsdkLoadDomain(vzDriverPtr driver,
 if (prlsdkGetDomainState(dom, sdkdom, ) < 0)
 goto error;
 
-if (virDomainDefAddImplicitDevices(def) < 0)
+if (!IS_CT(def) && virDomainDefAddImplicitDevices(def) < 0)
 goto error;
 
 if (def->ngraphics > 0) {
-- 
2.4.11

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

[libvirt] [PATCH 5/5] vz: get disks statistics for CTs

[Maxim Nestratov]

A CT disk statistics is reported with prefix "hdd" and we should use
it to extract data.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_driver.c | 10 --
 src/vz/vz_sdk.c|  5 +++--
 src/vz/vz_sdk.h|  2 +-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index a634ed2..3a22b07 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1743,7 +1743,10 @@ vzDomainBlockStatsImpl(virDomainObjPtr dom,
 virReportError(VIR_ERR_INVALID_ARG, _("invalid path: %s"), path);
 return -1;
 }
-if (prlsdkGetBlockStats(privdom->stats, dom->def->disks[idx], stats) < 
0)
+if (prlsdkGetBlockStats(privdom->stats,
+dom->def->disks[idx],
+stats,
+IS_CT(dom->def)) < 0)
 return -1;
 } else {
 virDomainBlockStatsStruct s;
@@ -1756,7 +1759,10 @@ vzDomainBlockStatsImpl(virDomainObjPtr dom,
 #undef PARALLELS_ZERO_STATS
 
 for (i = 0; i < dom->def->ndisks; i++) {
-if (prlsdkGetBlockStats(privdom->stats, dom->def->disks[i], ) < 
0)
+if (prlsdkGetBlockStats(privdom->stats,
+dom->def->disks[i],
+,
+IS_CT(dom->def)) < 0)
 return -1;
 
 #define PARALLELS_SUM_STATS(VAR, TYPE, NAME)\
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 9b8b48e..dff531f 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4360,7 +4360,8 @@ prlsdkExtractStatsParam(PRL_HANDLE sdkstats, const char 
*name, long long *val)
 int
 prlsdkGetBlockStats(PRL_HANDLE sdkstats,
 virDomainDiskDefPtr disk,
-virDomainBlockStatsPtr stats)
+virDomainBlockStatsPtr stats,
+bool isCt)
 {
 virDomainDeviceDriveAddressPtr address;
 int idx;
@@ -4379,7 +4380,7 @@ prlsdkGetBlockStats(PRL_HANDLE sdkstats,
 idx = address->unit;
 break;
 case VIR_DOMAIN_DISK_BUS_SCSI:
-prefix = "scsi";
+prefix = isCt ? "hdd" : "scsi";
 idx = address->unit;
 break;
 default:
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index ef789ab..e4e46dc 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -63,7 +63,7 @@ prlsdkDetachDevice(vzDriverPtr driver, virDomainObjPtr dom, 
virDomainDeviceDefPt
 int
 prlsdkUpdateDevice(vzDriverPtr driver, virDomainObjPtr dom, 
virDomainDeviceDefPtr dev);
 int
-prlsdkGetBlockStats(PRL_HANDLE sdkstats, virDomainDiskDefPtr disk, 
virDomainBlockStatsPtr stats);
+prlsdkGetBlockStats(PRL_HANDLE sdkstats, virDomainDiskDefPtr disk, 
virDomainBlockStatsPtr stats, bool isCt);
 int
 prlsdkGetNetStats(PRL_HANDLE sdkstas, PRL_HANDLE sdkdom, const char *path, 
virDomainInterfaceStatsPtr stats);
 int
-- 
2.4.11

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

[libvirt] [PATCH 1/5] vz: report "scsi" bus for disks when nothing was set explixitly

[Maxim Nestratov]

This is necessary for to show CTs created out of libvirt correctly.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index d5688e1..9976e4c 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -562,18 +562,15 @@ prlsdkGetDiskId(PRL_HANDLE disk, int *bus, char **dst)
 *bus = VIR_DOMAIN_DISK_BUS_IDE;
 *dst = virIndexToDiskName(pos, "hd");
 break;
-case PMS_SCSI_DEVICE:
-*bus = VIR_DOMAIN_DISK_BUS_SCSI;
-*dst = virIndexToDiskName(pos, "sd");
-break;
 case PMS_SATA_DEVICE:
 *bus = VIR_DOMAIN_DISK_BUS_SATA;
 *dst = virIndexToDiskName(pos, "sd");
 break;
+case PMS_SCSI_DEVICE:
 default:
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unknown disk bus: %X"), ifType);
-return -1;
+*bus = VIR_DOMAIN_DISK_BUS_SCSI;
+*dst = virIndexToDiskName(pos, "sd");
+break;
 }
 
 if (NULL == *dst)
-- 
2.4.11

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

[libvirt] [PATCH 4/5] vz: report disks either as disks or filesystems depending on original xml

[Maxim Nestratov]

Virtuozzo SDK interface doesn't differ filesystems from disks and sees them as 
disks.
Before, we always mistakenly presented disks based on files as filesystems, 
which is
not completely correct. Now we are going to show either disks or filesystems 
depending
on a hint, which uses boot device section of VZ config. Though this information
doesn't change booting order of a CT, it is used by vz libvirt interface as a 
hint
for libvirt representation of disks. Since now, if we have filesystems in input 
xml,
then we add them to VZ booting devices list and rely on this information to show
corresponding libvirt xml.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 96 +++--
 1 file changed, 86 insertions(+), 10 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 1030a1b..9b8b48e 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -50,6 +50,9 @@ static PRL_HANDLE
 prlsdkFindNetByMAC(PRL_HANDLE sdkdom, virMacAddrPtr mac);
 static PRL_HANDLE
 prlsdkGetDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk);
+static bool
+prlsdkInBootList(PRL_HANDLE sdkdom,
+ PRL_HANDLE sdktargetdev);
 
 /*
  * Log error description
@@ -754,7 +757,8 @@ prlsdkAddDomainHardDisksInfo(vzDriverPtr driver, PRL_HANDLE 
sdkdom, virDomainDef
 pret = PrlVmDev_GetEmulatedType(hdd, );
 prlsdkCheckRetGoto(pret, error);
 
-if (PDT_USE_REAL_DEVICE != emulatedType && IS_CT(def)) {
+if (IS_CT(def) &&
+prlsdkInBootList(sdkdom, hdd)) {
 
 if (!(fs = virDomainFSDefNew()))
 goto error;
@@ -1551,6 +1555,75 @@ virFindDiskBootIndex(virDomainDefPtr def, 
virDomainDiskDevice type, int index)
 return NULL;
 }
 
+static bool
+prlsdkInBootList(PRL_HANDLE sdkdom,
+ PRL_HANDLE sdktargetdev)
+{
+bool ret = false;
+PRL_RESULT pret;
+PRL_UINT32 bootNum;
+PRL_HANDLE bootDev = PRL_INVALID_HANDLE;
+PRL_BOOL inUse;
+PRL_DEVICE_TYPE sdkType;
+PRL_UINT32 sdkIndex, bootIndex;
+PRL_UINT32 pos, targetpos;
+PRL_HANDLE dev = PRL_INVALID_HANDLE;
+size_t i;
+
+pret = PrlVmDev_GetStackIndex(sdktargetdev, );
+prlsdkCheckRetExit(pret, -1);
+
+pret = PrlVmCfg_GetBootDevCount(sdkdom, );
+prlsdkCheckRetExit(pret, -1);
+
+if (bootNum > VIR_DOMAIN_MAX_BOOT_DEVS) {
+bootNum = VIR_DOMAIN_MAX_BOOT_DEVS;
+VIR_WARN("Too many boot devices");
+}
+
+for (i = 0; i < bootNum; ++i) {
+pret = PrlVmCfg_GetBootDev(sdkdom, i, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlBootDev_IsInUse(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+if (!inUse) {
+PrlHandle_Free(bootDev);
+bootDev = PRL_INVALID_HANDLE;
+continue;
+}
+
+pret = PrlBootDev_GetSequenceIndex(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlBootDev_GetType(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+pret = PrlBootDev_GetIndex(bootDev, );
+prlsdkCheckRetGoto(pret, cleanup);
+
+dev = prlsdkGetDevByDevIndex(sdkdom, sdkType, sdkIndex);
+if (dev == PRL_INVALID_HANDLE)
+goto cleanup;
+
+pret = PrlVmDev_GetStackIndex(dev, );
+prlsdkCheckRetExit(pret, -1);
+
+PrlHandle_Free(bootDev);
+bootDev = PRL_INVALID_HANDLE;
+
+if (pos == targetpos) {
+ret = true;
+break;
+}
+}
+
+ cleanup:
+PrlHandle_Free(dev);
+PrlHandle_Free(bootDev);
+return ret;
+}
 static int
 prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE sdkType, int sdkIndex,
  virDomainDefPtr def, int bootIndex)
@@ -3741,23 +3814,26 @@ prlsdkSetBootOrderCt(PRL_HANDLE sdkdom, virDomainDefPtr 
def)
 size_t i;
 PRL_HANDLE hdd = PRL_INVALID_HANDLE;
 PRL_RESULT pret;
+bool rootfs = false;
 int ret = -1;
 
 /* if we have root mounted we don't need to explicitly set boot order */
 for (i = 0; i < def->nfss; i++) {
+
+pret = prlsdkAddDeviceToBootList(sdkdom, i, PDE_HARD_DISK, i + 1);
+prlsdkCheckRetExit(pret, -1);
+
 if (STREQ(def->fss[i]->dst, "/"))
-return 0;
+rootfs = true;
 }
 
-/* else set first hard disk as boot device */
-pret = prlsdkAddDeviceToBootList(sdkdom, 0, PDE_HARD_DISK, 0);
-prlsdkCheckRetExit(pret, -1);
-
-pret = PrlVmCfg_GetHardDisk(sdkdom, 0, );
-prlsdkCheckRetExit(pret, -1);
+if (!rootfs) {
+pret = PrlVmCfg_GetHardDisk(sdkdom, 0, );
+prlsdkCheckRetExit(pret, -1);
 
-PrlVmDevHd_SetMountPoint(hdd, "/");
-prlsdkCheckRetGoto(pret, cleanup);
+PrlVmDevHd_SetMountPoint(hdd, "/");
+prlsdkCheckRetGoto(pret, cleanup);
+}
 
 ret = 0;
 
-- 
2.4.11

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

[libvirt] [PATCH 0/5] vz: fix some CT disk representation cases and its statistics

[Maxim Nestratov]

Maxim Nestratov (5):
  vz: report "scsi" bus for disks when nothing was set explixitly
  vz: don't query boot devices information for VZ, set boot from disk
always
  vz: don't add implicit devices for CTs
  vz: report disks either as disks or filesystems depending on original
xml
  vz: get disks statistics for CTs

 src/vz/vz_driver.c |  10 -
 src/vz/vz_sdk.c| 127 +++--
 src/vz/vz_sdk.h|   2 +-
 3 files changed, 112 insertions(+), 27 deletions(-)

-- 
2.4.11

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

[libvirt] [PATCH 2/5] vz: don't query boot devices information for VZ, set boot from disk always

[Maxim Nestratov]

Before, boot devices information for CTs was always empty and we
didn't indicate that containers always boot from disk.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 9976e4c..a6236d0 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1634,7 +1634,7 @@ prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE 
sdkType, int sdkIndex,
 }
 
 static int
-prlsdkConvertBootOrder(PRL_HANDLE sdkdom, virDomainDefPtr def)
+prlsdkConvertBootOrderVm(PRL_HANDLE sdkdom, virDomainDefPtr def)
 {
 int ret = -1;
 PRL_RESULT pret;
@@ -1792,9 +1792,14 @@ prlsdkLoadDomain(vzDriverPtr driver,
 if (prlsdkAddDomainHardware(driver, sdkdom, def) < 0)
 goto error;
 
-/* depends on prlsdkAddDomainHardware */
-if (prlsdkConvertBootOrder(sdkdom, def) < 0)
-goto error;
+if (IS_CT(def)) {
+def->os.nBootDevs = 1;
+def->os.bootDevs[0] = VIR_DOMAIN_BOOT_DISK;
+} else {
+/* depends on prlsdkAddDomainHardware */
+if (prlsdkConvertBootOrderVm(sdkdom, def) < 0)
+goto error;
+}
 
 pret = PrlVmCfg_GetEnvId(sdkdom, );
 prlsdkCheckRetGoto(pret, error);
-- 
2.4.11

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

Re: [libvirt] [PATCH 1/2] vz: set PVMT_DONT_CREATE_DISK migration flag

[Maxim Nestratov]

08-Dec-16 11:49, Pavel Glushchak пишет:


This flag tells backend not to create instance
disks making behavior the same as in qemu driver.
Disk files have to be created beforehand on target
host manually or by upper management layer i.e.
OpenStack Nova.

Signed-off-by: Pavel Glushchak 
---
  src/vz/vz_sdk.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index d13c86a..4cd988a 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4740,7 +4740,7 @@ int prlsdkSwitchToSnapshot(virDomainObjPtr dom, const 
char *uuid, bool paused)
   * connection to dispatcher
   */
  
-#define PRLSDK_MIGRATION_FLAGS (PSL_HIGH_SECURITY)

+#define PRLSDK_MIGRATION_FLAGS (PSL_HIGH_SECURITY | PVMT_DONT_CREATE_DISK)
  
  int prlsdkMigrate(virDomainObjPtr dom, virURIPtr uri,

const unsigned char *session_uuid,


ACKed and pushed the series.

Thanks,
Maxim

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

Re: [libvirt] [PATCH v2] storage: vz storage pool support

[Maxim Nestratov]

08-Dec-16 15:17, John Ferlan пишет:



On 12/08/2016 04:19 AM, Maxim Nestratov wrote:

08-Dec-16 02:22, John Ferlan пишет:


[...]


I see what you mean; however, IMO vstorage should be separate. Maybe
there's another opinion out there, but since you're requiring
"something" else to be installed in order to get the WITH_VSTORAGE
to be
set to 1, then a separate file is in order.

Not sure they're comparable, but zfs has its own. Having separated
vstorage reduces the chance that some day some incompatible logic is
added/altered in the *fs.c (or vice versa).

Ok. I will try.


I think you should consider the *_fs.c code to be the "default" of
sorts. That is default file/dir structure with netfs added in. The
vstorage may just be some file system, but it's not something (yet) on
"every" distribution.

I did not understand actually, what you mean  "be the "default" of
sorts."
As I have understood - what I need to do is to create backend_vstorage.c
with all create/delete/* functionality.


Sorry - I was trying to think of a better way to explain... The 'fs' and
'nfs' pool are default of sorts because one can "ls" (on UNIX/Linux) or
"dir" (on Windows) and get a list of files.

"ls" and "dir" are inherent to the OS, while in this case vstorage
commands are installed separately.

Once you mounted your vstorage cluster to a local filesystem you can
also "ls" it. Thus, I can't see much difference from nfs here.


So if it's more like NFS, then how does one ensure that the local userid
X is the same as the remote userid X? NFS has a root-squashing concept
that results in numerous shall we say "interesting" issues.


Vstorage doesn't have users concept. Authentication is made by a password per 
node just once.
If authentication passes, a key is stored in 
/etc/vstorage/clusters/CLUSTER_NAME/auth_digest.key
Then, permissions are set to a mount point during mounting with -u USER -g 
GROUP -m  MODE options
provided to vstorage-mount command.


Check out the virFileOpen*, virDirCreate, and virFileRemove...

Also what about viFileIsShareFSType? And security_selinux.c code for
NFS? If you use cscope, just search on NFS.

In the virStorageBackendVzStart, I see:

VSTORAGE_MOUNT -c $pool.source.name $pool.target.path


This call certainly lacks user/group/mode parameters and should be fixed in the 
next series.



where VSTORAGE_MOUNT is a build (configure.ac) definition that is the
"Location or name of vstorage-mount program" which would only be set if
the proper package was installed.

In the virStorageBackendVzfindPoolSources, I see:

VSTORAGE discover

which I assume generates some list of remote "services" (for lack of a
better term) which can be used as/for pool.source.name in order to be
well mounted by the VSTORAGE_MOUNT program.

Compare that to NFS, which uses mount which is included in well every
distro I can think of. That's a big difference. Also let's face it, NFS
has been the essential de facto goto tool to access remote storage for a
long time. Personally, I'd rather see the NFS code split out of the
*_fs.c backend, but I don't have the desire/time to do it - so it stays
as is.


To sum this up, you still think that copy and paste isn't a problem here and 
will create more value than do any harm, right?

Maxim

[snip]

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

Re: [libvirt] [PATCH v2] storage: vz storage pool support

[Maxim Nestratov]

08-Dec-16 02:22, John Ferlan пишет:


[...]


I see what you mean; however, IMO vstorage should be separate. Maybe
there's another opinion out there, but since you're requiring
"something" else to be installed in order to get the WITH_VSTORAGE to be
set to 1, then a separate file is in order.

Not sure they're comparable, but zfs has its own. Having separated
vstorage reduces the chance that some day some incompatible logic is
added/altered in the *fs.c (or vice versa).

Ok. I will try.


I think you should consider the *_fs.c code to be the "default" of
sorts. That is default file/dir structure with netfs added in. The
vstorage may just be some file system, but it's not something (yet) on
"every" distribution.

I did not understand actually, what you mean  "be the "default" of sorts."
As I have understood - what I need to do is to create backend_vstorage.c
with all create/delete/* functionality.


Sorry - I was trying to think of a better way to explain... The 'fs' and
'nfs' pool are default of sorts because one can "ls" (on UNIX/Linux) or
"dir" (on Windows) and get a list of files.

"ls" and "dir" are inherent to the OS, while in this case vstorage
commands are installed separately.


Once you mounted your vstorage cluster to a local filesystem you can also "ls" it. Thus, I can't see much difference 
from nfs here.



When you create a vstorage "file" is that done via touch? or edit some
path or some other "common" OS command?  Or is there a vstorage command
that needs to be used.  If the former, then using the common
storage_backend API's should be possible.


vstorage is just another "remote filesystem" type of distributed software defined storage. In terms of starting to use 
it, it doesn't differ from nfs - you should mount it and then you can use any POSIX calls to control files and 
directories resided on it.


Maxim


John



Also I forgot to mention yesterday - you need to update the
docs/formatstorage.html.in at the very least and also the storage driver
page docs/storage.html.in.
In addition there are storagepool tests (xml2xml) that would need to be
updated to validate the new storage pool type. The tests would "show"
how the pool XML would appear and validate whatever parsing has been
done.

I know. Will fix.


[...]


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

Re: [libvirt] [PATCH] Revert "vz: fixed race in vzDomainAttach/DettachDevice"

[Maxim Nestratov]

22-Nov-16 15:05, Nikolay Shirokovskiy пишет:


ACK


Pushed now. Thanx.

Maxim

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

Re: [libvirt] [PATCH 2/2] qemu: agent: fix unsafe agent access

[Maxim Nestratov]

23-Nov-16 11:00, Nikolay Shirokovskiy пишет:



On 23.11.2016 10:50, Michal Privoznik wrote:

On 23.11.2016 08:08, Nikolay Shirokovskiy wrote:


On 22.11.2016 17:49, Michal Privoznik wrote:

On 14.11.2016 15:24, Nikolay Shirokovskiy wrote:

qemuDomainObjExitAgent is unsafe.

First it accesses domain object without domain lock.
Second it uses outdated logic that goes back to commit 79533da1 of
year 2009 when code was quite different. (unref function
instead of unreferencing only unlocked and disposed object
in case of last reference and leaved unlocking to the caller otherwise).
Nowadays this logic may lead to disposing locked object
i guess.

Well, I agree that the order of those two steps should be reversed, so
ACK to that.


Another problem is that the callers of qemuDomainObjEnterAgent
use domain object again (namely priv->agent) without domain lock.

But why is this a problem?

qemuProcessHandleAgentEOF for example can zero out priv->agent after
we call qemuDomainObjEnterAgent and before we call 
qemuAgentGetTime(priv->agent, seconds, nseconds).
Because we drop domain lock in qemuDomainObjEnterAgent and
qemuProcessHandleAgentEOF does not acquire job condition, only domain lock.
At the same time qemuAgentGetTime is not ready for NULL agent and will crash.
It could get even worse as priv->agent is not atomic and instead of
NULL we can get garbage there. Long story short we just should
not access domain state without lock. Thats why I change all the places
so we get copy of priv->agent before we drop domain lock.

Ah. Sounds reasonable. Mind adding it to the commit message?

Of course not.


ACK then.


Thanx! I have no push rights so can you push this and another agent series
you ACKed recently instead of me? (Sorry this will take changing commit
message too:).




I went ahead and pushed this and another series you mentioned.

Maxim


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

Re: [libvirt] [PATCH v2 0/3] qemu: agent: cleanup agent error flag correctly

[Maxim Nestratov]

22-Nov-16 18:17, Michal Privoznik пишет:


On 16.11.2016 14:43, Nikolay Shirokovskiy wrote:

Patches 1 and 2 are refactorings to help review patch 3.

diff from v1:
=
1. error flag cleanup logic clarified
2. patches that touch io loop are dropped

Nikolay Shirokovskiy (3):
   qemu: agent: handle agent connection errors in one place
   qemu: agent: remove redundant check
   qemu: agent: clenup agent error flag correctly

  src/qemu/qemu_driver.c| 12 +++---
  src/qemu/qemu_migration.c | 13 ++
  src/qemu/qemu_process.c   | 61 +--
  3 files changed, 22 insertions(+), 64 deletions(-)


ACK series.

Michal




Pushed the series.

Maxim

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

Re: [libvirt] [PATCH] libvirtd: systemd: add special target for system shutdown

[Maxim Nestratov]

21-Nov-16 15:22, Martin Kletzander пишет:


On Fri, Oct 14, 2016 at 10:13:48AM +0300, Nikolay Shirokovskiy wrote:

It is already discussed in "[RFC] daemon: remove hardcode dep on 
libvirt-guests" [1].

Mgmt can use means to save/restore domains on system shutdown/boot other then
libvirt-guests.service. Thus we need to specify appropriate ordering dependency 
between
libvirtd, domains and save/restore service. This patch takes approach suggested
in RFC and introduces a systemd target, so that ordering can be built next way:

libvirtd -> domain -> virt-guest-shutdown.target -> save-restore.service.

This way domains are decoupled from specific shutdown service via intermediate
target.



I already thought I replied to this.  I just now see I haven't.  I tried
everything that came to my mind and everything worked as expected.  So
ACK.  Sorry for waiting.


Pushed now.

Maxim

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

[libvirt] [PATCH] Revert "vz: fixed race in vzDomainAttach/DettachDevice"

[Maxim Nestratov]

This reverts commit 3a6cf6fc16.

Mistakenly this commit was pushed because I thought I missed the
corret one b880ff42ddb while in fact I didn't.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index d61bccf..f63b9a3 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -3552,12 +3552,6 @@ prlsdkAttachDevice(vzDriverPtr driver,
 return -1;
 }
 
-if (prlsdkUpdateDomain(driver, dom) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-_("Failed to save new config"));
-return -1;
-}
-
 job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE);
 if (PRL_FAILED(waitDomainJob(job, dom)))
 return -1;
@@ -3623,12 +3617,6 @@ prlsdkDetachDevice(vzDriverPtr driver ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 
-if (prlsdkUpdateDomain(driver, dom) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-_("Failed to save new config"));
-goto cleanup;
-}
-
 job = PrlVm_CommitEx(privdom->sdkdom, PVCF_DETACH_HDD_BUNDLE);
 if (PRL_FAILED(waitDomainJob(job, dom)))
 goto cleanup;
-- 
2.4.11

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

Re: [libvirt] [PATCH v3] qemu: fix libvirtd crash when querying halted cpus info

[Maxim Nestratov]

15-Nov-16 17:33, Peter Krempa пишет:


On Tue, Nov 15, 2016 at 17:09:33 +0300, Maxim Nestratov wrote:

It was introduced by commit 7a51d9ebb, which started to use
monitor commands without job acquiring, which is unsafe and leads
to simultaneous access to vm->mon structure by different threads.

Crash backtrace is the following (shortened):

Program received signal SIGSEGV, Segmentation fault.
qemuMonitorSend (mon=mon@entry=0x7f4ef4000d20, msg=msg@entry=0x7f4f18e78640) at 
qemu/qemu_monitor.c:1011
1011while (!mon->msg->finished) {

0  qemuMonitorSend () at qemu/qemu_monitor.c:1011
1  0x7f691abdc720 in qemuMonitorJSONCommandWithFd () at 
qemu/qemu_monitor_json.c:298
2  0x7f691abde64a in qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328
3  qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408
4  0x7f691abcaebd in qemuMonitorGetCPUInfo g@entry=false) at 
qemu/qemu_monitor.c:1931
5  0x7f691ab96863 in qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309
6  0x7f691ac0af99 in qemuDomainGetStatsVcpu at qemu/qemu_driver.c:18945
7  0x7f691abef921 in qemuDomainGetStats  at qemu/qemu_driver.c:19469
8  qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559
9  0x7f693382e806 in virConnectGetAllDomainStats at libvirt-domain.c:11546
10 0x7f6934470c40 in remoteDispatchConnectGetAllDomainStats at remote.c:6267

(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0

This change fixes it by calling qemuDomainRefreshVcpuHalted only when job is 
acquired.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
v1-v2: don't output halted cpu info if it wasn't rathered
v2-v3: syntax-check recommendation

  src/qemu/qemu_driver.c | 18 --
  1 file changed, 12 insertions(+), 6 deletions(-)

ACK


Thank you. Pushed now.

Maxim

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

Re: [libvirt] [PATCH v2] qemu: fix libvirtd crash when querying halted cpus info

[Maxim Nestratov]

15-Nov-16 16:44, Maxim Nestratov пишет:


It was introduced by commit 7a51d9ebb, which started to use
monitor commands without job acquiring, which is unsafe and leads
to simultaneous access to vm->mon structure by different threads.

Crash backtrace is the following (shortened):

Program received signal SIGSEGV, Segmentation fault.
qemuMonitorSend (mon=mon@entry=0x7f4ef4000d20, msg=msg@entry=0x7f4f18e78640) at 
qemu/qemu_monitor.c:1011
1011while (!mon->msg->finished) {

0  qemuMonitorSend () at qemu/qemu_monitor.c:1011
1  0x7f691abdc720 in qemuMonitorJSONCommandWithFd () at 
qemu/qemu_monitor_json.c:298
2  0x7f691abde64a in qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328
3  qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408
4  0x7f691abcaebd in qemuMonitorGetCPUInfo g@entry=false) at 
qemu/qemu_monitor.c:1931
5  0x7f691ab96863 in qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309
6  0x7f691ac0af99 in qemuDomainGetStatsVcpu at qemu/qemu_driver.c:18945
7  0x7f691abef921 in qemuDomainGetStats  at qemu/qemu_driver.c:19469
8  qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559
9  0x7f693382e806 in virConnectGetAllDomainStats at libvirt-domain.c:11546
10 0x7f6934470c40 in remoteDispatchConnectGetAllDomainStats at remote.c:6267

(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0

This change fixes it by calling qemuDomainRefreshVcpuHalted only when job is 
acquired.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/qemu/qemu_driver.c | 17 +++--
  1 file changed, 11 insertions(+), 6 deletions(-)


Ignore this version please. The next one is sent with "syntax-check".

Maxim

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

[libvirt] [PATCH v3] qemu: fix libvirtd crash when querying halted cpus info

[Maxim Nestratov]

It was introduced by commit 7a51d9ebb, which started to use
monitor commands without job acquiring, which is unsafe and leads
to simultaneous access to vm->mon structure by different threads.

Crash backtrace is the following (shortened):

Program received signal SIGSEGV, Segmentation fault.
qemuMonitorSend (mon=mon@entry=0x7f4ef4000d20, msg=msg@entry=0x7f4f18e78640) at 
qemu/qemu_monitor.c:1011
1011while (!mon->msg->finished) {

0  qemuMonitorSend () at qemu/qemu_monitor.c:1011
1  0x7f691abdc720 in qemuMonitorJSONCommandWithFd () at 
qemu/qemu_monitor_json.c:298
2  0x7f691abde64a in qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328
3  qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408
4  0x7f691abcaebd in qemuMonitorGetCPUInfo g@entry=false) at 
qemu/qemu_monitor.c:1931
5  0x7f691ab96863 in qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309
6  0x7f691ac0af99 in qemuDomainGetStatsVcpu at qemu/qemu_driver.c:18945
7  0x7f691abef921 in qemuDomainGetStats  at qemu/qemu_driver.c:19469
8  qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559
9  0x7f693382e806 in virConnectGetAllDomainStats at libvirt-domain.c:11546
10 0x7f6934470c40 in remoteDispatchConnectGetAllDomainStats at remote.c:6267

(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0

This change fixes it by calling qemuDomainRefreshVcpuHalted only when job is 
acquired.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
v1-v2: don't output halted cpu info if it wasn't rathered
v2-v3: syntax-check recommendation 

 src/qemu/qemu_driver.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a82e58b..05a88c2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18978,7 +18978,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
virDomainObjPtr dom,
virDomainStatsRecordPtr record,
int *maxparams,
-   unsigned int privflags ATTRIBUTE_UNUSED)
+   unsigned int privflags)
 {
 size_t i;
 int ret = -1;
@@ -19005,10 +19005,16 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
 VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
 goto cleanup;
 
-if (qemuDomainRefreshVcpuHalted(driver, dom,
-QEMU_ASYNC_JOB_NONE) == 0 &&
-VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0)
-goto cleanup;
+if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) {
+if (qemuDomainRefreshVcpuHalted(driver, dom,
+QEMU_ASYNC_JOB_NONE) < 0) {
+/* it's ok to be silent and go ahead, because halted vcpu info
+ * wasn't here from the beginning */
+virResetLastError();
+} else if (VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0) 
{
+goto cleanup;
+}
+}
 
 if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
  virDomainDefGetVcpus(dom->def),
@@ -19462,7 +19468,7 @@ static struct qemuDomainGetStatsWorker 
qemuDomainGetStatsWorkers[] = {
 { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false },
 { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false },
 { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true },
-{ qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false },
+{ qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, true },
 { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false },
 { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true },
 { qemuDomainGetStatsPerf, VIR_DOMAIN_STATS_PERF, false },
-- 
2.4.11

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

[libvirt] [PATCH v2] qemu: fix libvirtd crash when querying halted cpus info

[Maxim Nestratov]

It was introduced by commit 7a51d9ebb, which started to use
monitor commands without job acquiring, which is unsafe and leads
to simultaneous access to vm->mon structure by different threads.

Crash backtrace is the following (shortened):

Program received signal SIGSEGV, Segmentation fault.
qemuMonitorSend (mon=mon@entry=0x7f4ef4000d20, msg=msg@entry=0x7f4f18e78640) at 
qemu/qemu_monitor.c:1011
1011while (!mon->msg->finished) {

0  qemuMonitorSend () at qemu/qemu_monitor.c:1011
1  0x7f691abdc720 in qemuMonitorJSONCommandWithFd () at 
qemu/qemu_monitor_json.c:298
2  0x7f691abde64a in qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328
3  qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408
4  0x7f691abcaebd in qemuMonitorGetCPUInfo g@entry=false) at 
qemu/qemu_monitor.c:1931
5  0x7f691ab96863 in qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309
6  0x7f691ac0af99 in qemuDomainGetStatsVcpu at qemu/qemu_driver.c:18945
7  0x7f691abef921 in qemuDomainGetStats  at qemu/qemu_driver.c:19469
8  qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559
9  0x7f693382e806 in virConnectGetAllDomainStats at libvirt-domain.c:11546
10 0x7f6934470c40 in remoteDispatchConnectGetAllDomainStats at remote.c:6267

(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0

This change fixes it by calling qemuDomainRefreshVcpuHalted only when job is 
acquired.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/qemu/qemu_driver.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a82e58b..55b9566 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18978,7 +18978,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
virDomainObjPtr dom,
virDomainStatsRecordPtr record,
int *maxparams,
-   unsigned int privflags ATTRIBUTE_UNUSED)
+   unsigned int privflags)
 {
 size_t i;
 int ret = -1;
@@ -19005,10 +19005,15 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
 VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
 goto cleanup;
 
-if (qemuDomainRefreshVcpuHalted(driver, dom,
-QEMU_ASYNC_JOB_NONE) == 0 &&
-VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0)
-goto cleanup;
+if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) {
+if (qemuDomainRefreshVcpuHalted(driver, dom,
+QEMU_ASYNC_JOB_NONE) < 0) {
+/* it's ok to be silent and go ahead, because halted vcpu info
+ * wasn't here from the beginning */
+virResetLastError();
+} else if (VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0)
+goto cleanup;
+}
 
 if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
  virDomainDefGetVcpus(dom->def),
@@ -19462,7 +19467,7 @@ static struct qemuDomainGetStatsWorker 
qemuDomainGetStatsWorkers[] = {
 { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false },
 { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false },
 { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true },
-{ qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false },
+{ qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, true },
 { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false },
 { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true },
 { qemuDomainGetStatsPerf, VIR_DOMAIN_STATS_PERF, false },
-- 
2.4.11

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

Re: [libvirt] [PATCH] qemu: fix libvirtd crash when querying halted cpus info

[Maxim Nestratov]

15-Nov-16 15:38, Viktor Mihajlovski пишет:


On 15.11.2016 13:19, Peter Krempa wrote:

On Wed, Nov 02, 2016 at 18:56:49 +0300, Maxim Nestratov wrote:

It was introduced by commit 7a51d9ebb, which started to use
monitor commands without job acquiring, which is unsafe and
leads to simultaneous access to vm->mon structure by different
threads.

Crash backtrace is the following (shortened):

Program received signal SIGSEGV, Segmentation fault.
qemuMonitorSend (mon=mon@entry=0x7f4ef4000d20,
msg=msg@entry=0x7f4f18e78640) at qemu/qemu_monitor.c:1011 1011
while (!mon->msg->finished) {

0  qemuMonitorSend () at qemu/qemu_monitor.c:1011 1
0x7f691abdc720 in qemuMonitorJSONCommandWithFd () at
qemu/qemu_monitor_json.c:298 2  0x7f691abde64a in
qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328 3
qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408 4
0x7f691abcaebd in qemuMonitorGetCPUInfo g@entry=false) at
qemu/qemu_monitor.c:1931 5  0x7f691ab96863 in
qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309 6
0x7f691ac0af99 in qemuDomainGetStatsVcpu at
qemu/qemu_driver.c:18945 7  0x7f691abef921 in
qemuDomainGetStats  at qemu/qemu_driver.c:19469 8
qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559 9
0x7f693382e806 in virConnectGetAllDomainStats at
libvirt-domain.c:11546 10 0x7f6934470c40 in
remoteDispatchConnectGetAllDomainStats at remote.c:6267

(gdb) p mon->msg $1 = (qemuMonitorMessagePtr) 0x0

This change fixes it by calling qemuDomainRefreshVcpuHalted only
when job is acquired.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com> ---
src/qemu/qemu_driver.c | 20 +--- 1 file changed,
13 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dd1907b..43a546d 100644 --- a/src/qemu/qemu_driver.c +++
b/src/qemu/qemu_driver.c @@ -18915,7 +18915,7 @@
qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, virDomainObjPtr
dom, virDomainStatsRecordPtr record, int *maxparams, -
unsigned int privflags ATTRIBUTE_UNUSED) +
unsigned int privflags) { size_t i; int ret = -1; @@ -18939,14
+18939,20 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
return -1;

if (VIR_ALLOC_N(cpuinfo, virDomainDefGetVcpus(dom->def)) < 0 || -
VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0) -
goto cleanup; - -if (qemuDomainRefreshVcpuHalted(driver,
dom, -QEMU_ASYNC_JOB_NONE) ==
0 && +VIR_ALLOC_N(cpuwait,
virDomainDefGetVcpus(dom->def)) < 0 || VIR_ALLOC_N(cpuhalted,
virDomainDefGetVcpus(dom->def)) < 0) goto cleanup;

+if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) { +

Spurious empty line.


+if (qemuDomainRefreshVcpuHalted(driver, dom, +
QEMU_ASYNC_JOB_NONE) < 0) { +/* it's ok to be silent
and go ahead, because halted vcpu info + * wasn't
here from the beginning */ +virResetLastError(); +
} +} +

Not visible in this context: The output of the parameter needs to
be suppressed if the parameter can't be requested.

Peter


right, the VIR_ALLOC_N(cpuhalted...) should be conditional on the
success of qemuDomainRefreshVcpuHalted, as it was originally...



Agree. Sent a new version.

Maxim

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

Re: [libvirt] [PATCH] qemu: fix libvirtd crash when querying halted cpus info

[Maxim Nestratov]

15-Nov-16 14:26, Viktor Mihajlovski пишет:


On 02.11.2016 17:29, Viktor Mihajlovski wrote:

On 02.11.2016 16:56, Maxim Nestratov wrote:

It was introduced by commit 7a51d9ebb, which started to use
monitor commands without job acquiring, which is unsafe and leads
to simultaneous access to vm->mon structure by different threads.

Crash backtrace is the following (shortened):

Program received signal SIGSEGV, Segmentation fault.
qemuMonitorSend (mon=mon@entry=0x7f4ef4000d20, msg=msg@entry=0x7f4f18e78640) at 
qemu/qemu_monitor.c:1011
1011while (!mon->msg->finished) {

0  qemuMonitorSend () at qemu/qemu_monitor.c:1011
1  0x7f691abdc720 in qemuMonitorJSONCommandWithFd () at 
qemu/qemu_monitor_json.c:298
2  0x7f691abde64a in qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328
3  qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408
4  0x7f691abcaebd in qemuMonitorGetCPUInfo g@entry=false) at 
qemu/qemu_monitor.c:1931
5  0x7f691ab96863 in qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309
6  0x7f691ac0af99 in qemuDomainGetStatsVcpu at qemu/qemu_driver.c:18945
7  0x7f691abef921 in qemuDomainGetStats  at qemu/qemu_driver.c:19469
8  qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559
9  0x7f693382e806 in virConnectGetAllDomainStats at libvirt-domain.c:11546
10 0x7f6934470c40 in remoteDispatchConnectGetAllDomainStats at remote.c:6267

(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0

This change fixes it by calling qemuDomainRefreshVcpuHalted only when job is 
acquired.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/qemu/qemu_driver.c | 20 +---
  1 file changed, 13 insertions(+), 7 deletions(-)

[...]
Argh ... this fell through the cracks when I moved the code from
vcpuinfo to domstats. Thanks for catching and fixing that one.


It would be nice if the fix could be pushed before the next release
freeze. Thanks!


I regard your reply as ACK then.
Let's wait a bit to let people react to this and if there is no objection, I'll 
push shortly.

Maxim

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

Re: [libvirt] [PATCH] vz: fixed migration in p2p mode

[Maxim Nestratov]

14-Nov-16 18:20, Pavel Glushchak пишет:


dom xml generated on begin step should be passed
to perform step in VIR_MIGRATE_PARAM_DEST_XML parameter.
Otherwise 'XML error: failed to parse xml document' is
raised on destination host as dom xml is NULL.

Signed-off-by: Pavel Glushchak 
---
  src/vz/vz_driver.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index b7c37bb..b2c3e31 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3199,6 +3199,7 @@ vzDomainMigratePerformP2P(virDomainObjPtr dom,
  virConnectPtr dconn = NULL;
  virTypedParameterPtr params = NULL;
  int ret = -1;
+int maxparams = nparams;
  
  if (virTypedParamsCopy(, orig_params, nparams) < 0)

  return -1;
@@ -3210,6 +3211,10 @@ vzDomainMigratePerformP2P(virDomainObjPtr dom,
   , )))
  goto done;
  
+if (virTypedParamsAddString(, , ,

+VIR_MIGRATE_PARAM_DEST_XML, dom_xml) < 0)
+goto done;
+
  cookiein = cookieout;
  cookieinlen = cookieoutlen;
  cookieout = NULL;


ACKed and pushed.

Thanks,
Maxim

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

Re: [libvirt] [PATCH] qemu: fix one reboot scenario

[Maxim Nestratov]

07-Nov-16 20:15, Michal Privoznik пишет:


On 07.11.2016 17:19, Michal Privoznik wrote:

On 03.11.2016 19:11, Maxim Nestratov wrote:

Both qemuDomainReboot and qemuDomainShutdownFlags do the following if
they were called to reboot:
1. use agent and call qemuAgentShutdown
2. then if the above function doesn't succeed, try qemuMonitorSystemPowerdown

When the first step is called, it resets fakeReboot flag, while the second one,
opposite to that, sets it. Thus, in case we tried to use agent to reboot a
guest and failed for some reason, we end up with fakeReboot flag set. After 
that,
as qemuMonitorSystemPowerdown function was called, libvirt is notified with 
POWERDOWN.
The problem is that there is no callback routine set for it. The lack of 
monitor event
reaction leads to incorrect logic and guest doesn't restart or reboot correctly.
The patch simply sets domainPowerdown monitor callback to 
qemuProcessHandleShutdown as
powerdown event processing is actually equal to shutdown.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/qemu/qemu_process.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 04b25fe..0de9fa5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1630,6 +1630,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
  .diskSecretLookup = qemuProcessFindVolumeQcowPassphrase,
  .domainEvent = qemuProcessHandleEvent,
  .domainShutdown = qemuProcessHandleShutdown,
+.domainPowerdown = qemuProcessHandleShutdown,
  .domainStop = qemuProcessHandleStop,
  .domainResume = qemuProcessHandleResume,
  .domainReset = qemuProcessHandleReset,


Huh, this event was never ever handled. How did we even manage that?

Oh, now looking into the qemu code, maybe we don't want this to be
handled after all. POWERDOWN event is emitted on ACPI powerdown event,
which means that guest is still running and hence we don't want to
mangle our internal state (i.e. set it to SHUTDOWN).


ACK

I'm sorry, but I have to discard my own ACK. We need a different fix for
the scenario you're seeing.

Michal


Yeah, you are absolutely right. Please disregard the patch. I'll send another 
one.

Maxim

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

[libvirt] [PATCH] qemu: fix one reboot scenario

[Maxim Nestratov]

Both qemuDomainReboot and qemuDomainShutdownFlags do the following if
they were called to reboot:
1. use agent and call qemuAgentShutdown
2. then if the above function doesn't succeed, try qemuMonitorSystemPowerdown

When the first step is called, it resets fakeReboot flag, while the second one,
opposite to that, sets it. Thus, in case we tried to use agent to reboot a
guest and failed for some reason, we end up with fakeReboot flag set. After 
that,
as qemuMonitorSystemPowerdown function was called, libvirt is notified with 
POWERDOWN.
The problem is that there is no callback routine set for it. The lack of 
monitor event
reaction leads to incorrect logic and guest doesn't restart or reboot correctly.
The patch simply sets domainPowerdown monitor callback to 
qemuProcessHandleShutdown as
powerdown event processing is actually equal to shutdown.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/qemu/qemu_process.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 04b25fe..0de9fa5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1630,6 +1630,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
 .diskSecretLookup = qemuProcessFindVolumeQcowPassphrase,
 .domainEvent = qemuProcessHandleEvent,
 .domainShutdown = qemuProcessHandleShutdown,
+.domainPowerdown = qemuProcessHandleShutdown,
 .domainStop = qemuProcessHandleStop,
 .domainResume = qemuProcessHandleResume,
 .domainReset = qemuProcessHandleReset,
-- 
2.4.11

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

[libvirt] [PATCH] qemu: fix libvirtd crash when querying halted cpus info

[Maxim Nestratov]

It was introduced by commit 7a51d9ebb, which started to use
monitor commands without job acquiring, which is unsafe and leads
to simultaneous access to vm->mon structure by different threads.

Crash backtrace is the following (shortened):

Program received signal SIGSEGV, Segmentation fault.
qemuMonitorSend (mon=mon@entry=0x7f4ef4000d20, msg=msg@entry=0x7f4f18e78640) at 
qemu/qemu_monitor.c:1011
1011while (!mon->msg->finished) {

0  qemuMonitorSend () at qemu/qemu_monitor.c:1011
1  0x7f691abdc720 in qemuMonitorJSONCommandWithFd () at 
qemu/qemu_monitor_json.c:298
2  0x7f691abde64a in qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328
3  qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408
4  0x7f691abcaebd in qemuMonitorGetCPUInfo g@entry=false) at 
qemu/qemu_monitor.c:1931
5  0x7f691ab96863 in qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309
6  0x7f691ac0af99 in qemuDomainGetStatsVcpu at qemu/qemu_driver.c:18945
7  0x7f691abef921 in qemuDomainGetStats  at qemu/qemu_driver.c:19469
8  qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559
9  0x7f693382e806 in virConnectGetAllDomainStats at libvirt-domain.c:11546
10 0x7f6934470c40 in remoteDispatchConnectGetAllDomainStats at remote.c:6267

(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0

This change fixes it by calling qemuDomainRefreshVcpuHalted only when job is 
acquired.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/qemu/qemu_driver.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dd1907b..43a546d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18915,7 +18915,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
virDomainObjPtr dom,
virDomainStatsRecordPtr record,
int *maxparams,
-   unsigned int privflags ATTRIBUTE_UNUSED)
+   unsigned int privflags)
 {
 size_t i;
 int ret = -1;
@@ -18939,14 +18939,20 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
 return -1;
 
 if (VIR_ALLOC_N(cpuinfo, virDomainDefGetVcpus(dom->def)) < 0 ||
-VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
-goto cleanup;
-
-if (qemuDomainRefreshVcpuHalted(driver, dom,
-QEMU_ASYNC_JOB_NONE) == 0 &&
+VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0 ||
 VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0)
 goto cleanup;
 
+if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) {
+
+if (qemuDomainRefreshVcpuHalted(driver, dom,
+QEMU_ASYNC_JOB_NONE) < 0) {
+/* it's ok to be silent and go ahead, because halted vcpu info
+ * wasn't here from the beginning */
+virResetLastError();
+}
+}
+
 if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
  virDomainDefGetVcpus(dom->def),
  NULL, 0, cpuhalted) < 0) {
@@ -19399,7 +19405,7 @@ static struct qemuDomainGetStatsWorker 
qemuDomainGetStatsWorkers[] = {
 { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false },
 { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false },
 { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true },
-{ qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false },
+{ qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, true },
 { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false },
 { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true },
 { qemuDomainGetStatsPerf, VIR_DOMAIN_STATS_PERF, false },
-- 
2.4.11

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

Re: [libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Maxim Nestratov]

27-Oct-16 16:58, Maxim Nestratov пишет:


24-Oct-16 09:46, Nikolay Shirokovskiy пишет:



On 21.10.2016 18:20, Maxim Nestratov wrote:

07-Sep-16 14:00, Nikolay Shirokovskiy пишет:


On 05.09.2016 19:39, Maxim Nestratov wrote:

Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to  support it.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
   src/vz/vz_sdk.c | 100 

   1 file changed, 14 insertions(+), 86 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..933b222 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
virDomainNetDefPtr net, bool isCt)
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
   goto cleanup;
   } else {
-char *netid = NULL;
-
-if (!(netid =
+char *netid =
prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
-  netAdapter)))
-goto cleanup;
+  netAdapter);
   -/*
- * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters
- * except those whose Virtual Network Id differ from Parallels
- * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME
- * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME
- */
-if (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+if (emulatedType == PNA_BRIDGE) {
   net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-net->data.network.name = netid;
+if (netid)
+net->data.bridge.brname = netid;
   } else {
   net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-net->data.bridge.brname = netid;
+if (netid)
+net->data.network.name = netid;

not sure why check for not NULL here and above,

Assuming that I fix this and below, I have your ACK on the first two patches, 
right?

Maxim

Yes.

I just realized you removed NULL check on PrlVmDevNet_GetVirtualNetworkId. NULL 
is not a error case?

Nikolay


Yes, it is not, as it can be changed later and we don't want to prevent such 
domains to be listed.

Maxim



Pushed now.

Maxim

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

Re: [libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Maxim Nestratov]

24-Oct-16 09:46, Nikolay Shirokovskiy пишет:



On 21.10.2016 18:20, Maxim Nestratov wrote:

07-Sep-16 14:00, Nikolay Shirokovskiy пишет:


On 05.09.2016 19:39, Maxim Nestratov wrote:

Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to  support it.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
   src/vz/vz_sdk.c | 100 

   1 file changed, 14 insertions(+), 86 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..933b222 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
virDomainNetDefPtr net, bool isCt)
  PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
   goto cleanup;
   } else {
-char *netid = NULL;
-
-if (!(netid =
+char *netid =
 prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
-  netAdapter)))
-goto cleanup;
+  netAdapter);
   -/*
- * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters
- * except those whose Virtual Network Id differ from Parallels
- * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME
- * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME
- */
-if (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+if (emulatedType == PNA_BRIDGE) {
   net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-net->data.network.name = netid;
+if (netid)
+net->data.bridge.brname = netid;
   } else {
   net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-net->data.bridge.brname = netid;
+if (netid)
+net->data.network.name = netid;

not sure why check for not NULL here and above,

Assuming that I fix this and below, I have your ACK on the first two patches, 
right?

Maxim

Yes.

I just realized you removed NULL check on PrlVmDevNet_GetVirtualNetworkId. NULL 
is not a error case?

Nikolay


Yes, it is not, as it can be changed later and we don't want to prevent such 
domains to be listed.

Maxim

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

Re: [libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Maxim Nestratov]

07-Sep-16 14:00, Nikolay Shirokovskiy пишет:



On 05.09.2016 19:39, Maxim Nestratov wrote:

Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to  support it.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/vz/vz_sdk.c | 100 
  1 file changed, 14 insertions(+), 86 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..933b222 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
virDomainNetDefPtr net, bool isCt)
 PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
  goto cleanup;
  } else {
-char *netid = NULL;
-
-if (!(netid =
+char *netid =
prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
-  netAdapter)))
-goto cleanup;
+  netAdapter);
  
-/*

- * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters
- * except those whose Virtual Network Id differ from Parallels
- * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME
- * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME
- */
-if (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+if (emulatedType == PNA_BRIDGE) {
  net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-net->data.network.name = netid;
+if (netid)
+net->data.bridge.brname = netid;
  } else {
  net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-net->data.bridge.brname = netid;
+if (netid)
+net->data.network.name = netid;

not sure why check for not NULL here and above,


Assuming that I fix this and below, I have your ACK on the first two patches, 
right?

Maxim



1.) now i see my mistake introduced in 3a82c04c, setting net->data need to be 
swapped)


  }
-

here is my usual pedantic comment


  }
  
  if (!isCt) {

[snip]


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

Re: [libvirt] [PATCH 0/2] vz: add serial number to disk devices

[Maxim Nestratov]

22-Sep-16 17:55, Nikolay Shirokovskiy пишет:


Only the first patch is really on the subject. The second one is
bugfix that is included mainly because it touches the same place
(and nice to have to test first patch too...)

Nikolay Shirokovskiy (2):
   vz: add serial number to disk devices
   vz: set something in disk driver name

  src/vz/vz_sdk.c   | 16 
  src/vz/vz_utils.c |  6 +++---
  2 files changed, 19 insertions(+), 3 deletions(-)



After offline discussion we decided to stick to this patch series.
ACKed and pushed now.

Maxim

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

Re: [libvirt] [PATCH v2 0/3] qemu: util: vz: support setting disk serial number

[Maxim Nestratov]

21-Oct-16 12:50, Maxim Nestratov пишет:


Actually this is a combination of the two previous series
https://www.redhat.com/archives/libvir-list/2016-October/msg00890.html
and
https://www.redhat.com/archives/libvir-list/2016-September/msg01024.html

Maxim Nestratov (1):
   util: qemu: make qemuSafeSerialParamValue function usable by other
 drivers

Nikolay Shirokovskiy (2):
   vz: set something in disk driver name
   vz: add serial number to disk devices

  src/libvirt_private.syms |  1 +
  src/qemu/qemu_command.c  | 19 +--
  src/util/virutil.c   | 16 
  src/util/virutil.h   |  2 ++
  src/vz/vz_sdk.c  | 24 
  src/vz/vz_utils.c|  6 +++---
  6 files changed, 47 insertions(+), 21 deletions(-)



Disregard this please. I pushed the original series:
https://www.redhat.com/archives/libvir-list/2016-September/msg01024.html

Maxim


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

[libvirt] [PATCH v2 3/3] vz: add serial number to disk devices

[Maxim Nestratov]

From: Nikolay Shirokovskiy <nshirokovs...@virtuozzo.com>

vz sdk supports setting serial number only for disk devices.
Getting serial upon cdrom(for example) is error however
setting is just ignored. Let's check for disk device
explicitly for clarity in both cases.

Setting serial number for other devices is ignored
with an info note just as before.

We need usual conversion from "" to NULL in direction
vz sdk -> libvirt, because "" is not valid for libvirt
and "" means unspecifiend in vz sdk which is NULL for libvirt.

Signed-off-by: Nikolay Shirokovskiy <nshirokovs...@virtuozzo.com>
Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c   | 21 +
 src/vz/vz_utils.c |  6 +++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 8178d3b..e14caa5 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -590,6 +590,7 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
   bool isCt)
 {
 char *buf = NULL;
+char *serial = NULL;
 PRL_RESULT pret;
 PRL_UINT32 emulatedType;
 virDomainDeviceDriveAddressPtr address;
@@ -626,6 +627,20 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
 if (*buf != '\0' && virDomainDiskSetSource(disk, buf) < 0)
 goto cleanup;
 
+if (!isCdrom) {
+serial = prlsdkGetStringParamVar(PrlVmDevHd_GetSerialNumber, prldisk);
+if (serial) {
+if (virSafeSerialParamValue(serial) < 0)
+goto cleanup;
+
+if (*serial == '\0')
+VIR_FREE(serial);
+else
+disk->serial = serial;
+serial = NULL;
+}
+}
+
 if (prlsdkGetDiskId(prldisk, >bus, >dst) < 0)
 goto cleanup;
 
@@ -646,6 +661,7 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
 
  cleanup:
 VIR_FREE(buf);
+VIR_FREE(serial);
 return ret;
 }
 
@@ -3495,6 +3511,11 @@ static int prlsdkConfigureDisk(vzDriverPtr driver,
 pret = PrlVmDev_SetStackIndex(sdkdisk, idx);
 prlsdkCheckRetGoto(pret, cleanup);
 
+if (devType == PDE_HARD_DISK) {
+pret = PrlVmDevHd_SetSerialNumber(sdkdisk, disk->serial);
+prlsdkCheckRetGoto(pret, cleanup);
+}
+
 return 0;
  cleanup:
 PrlHandle_Free(sdkdisk);
diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
index eaf09f2..81429d2 100644
--- a/src/vz/vz_utils.c
+++ b/src/vz/vz_utils.c
@@ -323,9 +323,9 @@ vzCheckDiskUnsupportedParams(virDomainDiskDefPtr disk)
 return -1;
 }
 
-if (disk->serial) {
-VIR_INFO("%s", _("Setting disk serial number is not "
- "supported by vz driver."));
+if (disk->serial && disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
+VIR_INFO("%s", _("Setting disk serial number is "
+ "supported only for disk devices."));
 }
 
 if (disk->wwn) {
-- 
2.4.11

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

[libvirt] [PATCH v2 1/3] vz: set something in disk driver name

[Maxim Nestratov]

From: Nikolay Shirokovskiy 

Absent driver name attribute is invalid xml. Which in turn makes
unusable 'virsh edit' for example. The value does not make
much sense and ignored on input so nobody will hurt.
---
 src/vz/vz_sdk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f36064d..8178d3b 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -632,6 +632,9 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
 if (virDiskNameToBusDeviceIndex(disk, , ) < 0)
 goto cleanup;
 
+if (virDomainDiskSetDriver(disk, "vz") < 0)
+goto cleanup;
+
 address = >info.addr.drive;
 address->bus = busIdx;
 address->target = 0;
-- 
2.4.11

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

[libvirt] [PATCH v2 0/3] qemu: util: vz: support setting disk serial number

[Maxim Nestratov]

Actually this is a combination of the two previous series
https://www.redhat.com/archives/libvir-list/2016-October/msg00890.html
and
https://www.redhat.com/archives/libvir-list/2016-September/msg01024.html

Maxim Nestratov (1):
  util: qemu: make qemuSafeSerialParamValue function usable by other
drivers

Nikolay Shirokovskiy (2):
  vz: set something in disk driver name
  vz: add serial number to disk devices

 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  | 19 +--
 src/util/virutil.c   | 16 
 src/util/virutil.h   |  2 ++
 src/vz/vz_sdk.c  | 24 
 src/vz/vz_utils.c|  6 +++---
 6 files changed, 47 insertions(+), 21 deletions(-)

-- 
2.4.11

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

[libvirt] [PATCH v2 2/3] util: qemu: make qemuSafeSerialParamValue function usable by other drivers

[Maxim Nestratov]

Rename qemuSafeSerialParamValue to virSafeSerialParamValue and move it to utils

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  | 19 +--
 src/util/virutil.c   | 16 
 src/util/virutil.h   |  2 ++
 4 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bf503a5..170195b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2649,6 +2649,7 @@ virParseVersionString;
 virPipeReadUntilEOF;
 virReadFCHost;
 virReadSCSIUniqueId;
+virSafeSerialParamValue;
 virScaleInteger;
 virSetBlocking;
 virSetCloseExec;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8282162..0be2ffa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -427,23 +427,6 @@ qemuBuildIoEventFdStr(virBufferPtr buf,
 return 0;
 }
 
-#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
-  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ "
-
-static int
-qemuSafeSerialParamValue(const char *value)
-{
-if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen(value)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("driver serial '%s' contains unsafe characters"),
-   value);
-return -1;
-}
-
-return 0;
-}
-
-
 static int
 qemuNetworkDriveGetPort(int protocol,
 const char *port)
@@ -1600,7 +1583,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
 
 if (disk->serial &&
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_SERIAL)) {
-if (qemuSafeSerialParamValue(disk->serial) < 0)
+if (virSafeSerialParamValue(disk->serial) < 0)
 goto error;
 if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI &&
 disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 844c947..58ace3f 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2684,3 +2684,19 @@ virMemoryMaxValue(bool capped)
 else
 return LLONG_MAX;
 }
+
+#define VIR_SERIAL_PARAM_ACCEPTED_CHARS \
+  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ "
+
+int
+virSafeSerialParamValue(const char *value)
+{
+if (strspn(value, VIR_SERIAL_PARAM_ACCEPTED_CHARS) != strlen(value)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("driver serial '%s' contains unsafe characters"),
+   value);
+return -1;
+}
+
+return 0;
+}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 8c0d83c..f9b4831 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -252,6 +252,8 @@ unsigned long long virMemoryLimitTruncate(unsigned long 
long value);
 bool virMemoryLimitIsSet(unsigned long long value);
 unsigned long long virMemoryMaxValue(bool ulong);
 
+int virSafeSerialParamValue(const char *value);
+
 /**
  * VIR_ASSIGN_IS_OVERFLOW:
  * @rvalue: value that is checked (evaluated twice)
-- 
2.4.11

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

Re: [libvirt] [PATCH] vz: set localhost as vnc address

[Maxim Nestratov]

19-Oct-16 12:40, Nikolay Shirokovskiy пишет:



On 18.10.2016 19:19, Mikhail Feoktistov wrote:

We should set localhost as vnc address in case of empty string.
Because Virtuozzo sets 0.0.0.0 as default vnc address.
---
  src/vz/vz_sdk.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..7235172 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -2967,7 +2967,7 @@ static int prlsdkApplyGraphicsParams(PRL_HANDLE sdkdom,
  
  glisten = virDomainGraphicsGetListen(gr, 0);

  pret = PrlVmCfg_SetVNCHostName(sdkdom, glisten && glisten->address ?
-   glisten->address : "");
+   glisten->address : "127.0.0.1");
  prlsdkCheckRetGoto(pret, cleanup);
  
  ret = 0;



ACK


Pushed now. Thank you.

Maxim

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

Re: [libvirt] [PATCH 1/2] vz: add serial number to disk devices

[Maxim Nestratov]

22-Sep-16 17:55, Nikolay Shirokovskiy пишет:


vz sdk supports setting serial number only for disk devices.
Getting serial upon cdrom(for example) is error however
setting is just ignored. Let's check for disk device
explicitly for clarity in both cases.

Setting serial number for other devices is ignored
with an info note just as before.

We need usual conversion from "" to NULL in direction
vz sdk -> libvirt, because "" is not valid for libvirt
and "" means unspecifiend in vz sdk which is NULL for libvirt.
---
  src/vz/vz_sdk.c   | 13 +
  src/vz/vz_utils.c |  6 +++---
  2 files changed, 16 insertions(+), 3 deletions(-)


Hmm. I missed this series and sent mine on the matter yesterday. I see that 
your patch has some
useful changes regarding CDROM absent in my patch. So I'll merge these two and 
resend a
new version shortly.

Maxim


diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..a38f2af 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -639,6 +639,14 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
  
  disk->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
  
+if (!isCdrom) {

+if (!(disk->serial = 
prlsdkGetStringParamVar(PrlVmDevHd_GetSerialNumber, prldisk)))
+goto cleanup;
+
+if (*disk->serial == '\0')
+VIR_FREE(disk->serial);
+}
+
  ret = 0;
  
   cleanup:

@@ -3492,6 +3500,11 @@ static int prlsdkConfigureDisk(vzDriverPtr driver,
  pret = PrlVmDev_SetStackIndex(sdkdisk, idx);
  prlsdkCheckRetGoto(pret, cleanup);
  
+if (devType == PDE_HARD_DISK) {

+pret = PrlVmDevHd_SetSerialNumber(sdkdisk, disk->serial);
+prlsdkCheckRetGoto(pret, cleanup);
+}
+
  return 0;
   cleanup:
  PrlHandle_Free(sdkdisk);
diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
index eaf09f2..81429d2 100644
--- a/src/vz/vz_utils.c
+++ b/src/vz/vz_utils.c
@@ -323,9 +323,9 @@ vzCheckDiskUnsupportedParams(virDomainDiskDefPtr disk)
  return -1;
  }
  
-if (disk->serial) {

-VIR_INFO("%s", _("Setting disk serial number is not "
- "supported by vz driver."));
+if (disk->serial && disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) {
+VIR_INFO("%s", _("Setting disk serial number is "
+ "supported only for disk devices."));
  }
  
  if (disk->wwn) {


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

Re: [libvirt] [PATCH 2/2] vz: set something in disk driver name

[Maxim Nestratov]

22-Sep-16 17:55, Nikolay Shirokovskiy пишет:


Absent driver name attribute is invalid xml. Which in turn makes
unusable 'virsh edit' for example. The value does not make
much sense and ignored on input so nobody will hurt.
---
  src/vz/vz_sdk.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index a38f2af..0af450b 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -647,6 +647,9 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
  VIR_FREE(disk->serial);
  }
  
+if (virDomainDiskSetDriver(disk, "vz") < 0)

+goto cleanup;
+
  ret = 0;
  
   cleanup:


ACK

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

[libvirt] [PATCH 0/2] qemu: util: vz: support setting disk serial number

[Maxim Nestratov]

The first patch is a preparation moving qemuSafeSerialParamValue to util,
the second implements disk serial number setting in vz driver.

Maxim Nestratov (2):
  util: qemu: make qemuSafeSerialParamValue function usable by other
drivers
  vz: support setting disk serial number

 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  | 19 +--
 src/util/virutil.c   | 16 
 src/util/virutil.h   |  2 ++
 src/vz/vz_sdk.c  | 19 +++
 src/vz/vz_utils.c|  5 -
 6 files changed, 39 insertions(+), 23 deletions(-)

-- 
2.4.11

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

[libvirt] [PATCH 2/2] vz: support setting disk serial number

[Maxim Nestratov]

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c   | 19 +++
 src/vz/vz_utils.c |  5 -
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index b5b0197..f27441d 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -590,6 +590,7 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
   bool isCt)
 {
 char *buf = NULL;
+char *serial = NULL;
 PRL_RESULT pret;
 PRL_UINT32 emulatedType;
 virDomainDeviceDriveAddressPtr address;
@@ -626,6 +627,18 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
 if (*buf != '\0' && virDomainDiskSetSource(disk, buf) < 0)
 goto cleanup;
 
+serial = prlsdkGetStringParamVar(PrlVmDevHd_GetSerialNumber, prldisk);
+if (serial) {
+if (virSafeSerialParamValue(serial) < 0)
+goto cleanup;
+
+if (*serial == '\0')
+VIR_FREE(serial);
+else
+disk->serial = serial;
+serial = NULL;
+}
+
 if (prlsdkGetDiskId(prldisk, >bus, >dst) < 0)
 goto cleanup;
 
@@ -643,6 +656,7 @@ prlsdkGetDiskInfo(vzDriverPtr driver,
 
  cleanup:
 VIR_FREE(buf);
+VIR_FREE(serial);
 return ret;
 }
 
@@ -3489,6 +3503,11 @@ static int prlsdkConfigureDisk(vzDriverPtr driver,
 pret = PrlVmDev_SetIfaceType(sdkdisk, sdkbus);
 prlsdkCheckRetGoto(pret, cleanup);
 
+if (disk->serial) {
+pret = PrlVmDevHd_SetSerialNumber(sdkdisk, disk->serial);
+prlsdkCheckRetGoto(pret, cleanup);
+}
+
 pret = PrlVmDev_SetStackIndex(sdkdisk, idx);
 prlsdkCheckRetGoto(pret, cleanup);
 
diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c
index eaf09f2..f68b3a1 100644
--- a/src/vz/vz_utils.c
+++ b/src/vz/vz_utils.c
@@ -323,11 +323,6 @@ vzCheckDiskUnsupportedParams(virDomainDiskDefPtr disk)
 return -1;
 }
 
-if (disk->serial) {
-VIR_INFO("%s", _("Setting disk serial number is not "
- "supported by vz driver."));
-}
-
 if (disk->wwn) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Setting disk wwn id is not "
-- 
2.4.11

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

[libvirt] [PATCH 1/2] util: qemu: make qemuSafeSerialParamValue function usable by other drivers

[Maxim Nestratov]

Rename qemuSafeSerialParamValue to virSafeSerialParamValue and move it to utils

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  | 19 +--
 src/util/virutil.c   | 16 
 src/util/virutil.h   |  2 ++
 4 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 55b6a24..ec9fe1c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2649,6 +2649,7 @@ virParseVersionString;
 virPipeReadUntilEOF;
 virReadFCHost;
 virReadSCSIUniqueId;
+virSafeSerialParamValue;
 virScaleInteger;
 virSetBlocking;
 virSetCloseExec;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8282162..0be2ffa 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -427,23 +427,6 @@ qemuBuildIoEventFdStr(virBufferPtr buf,
 return 0;
 }
 
-#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
-  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ "
-
-static int
-qemuSafeSerialParamValue(const char *value)
-{
-if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen(value)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("driver serial '%s' contains unsafe characters"),
-   value);
-return -1;
-}
-
-return 0;
-}
-
-
 static int
 qemuNetworkDriveGetPort(int protocol,
 const char *port)
@@ -1600,7 +1583,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
 
 if (disk->serial &&
 virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_SERIAL)) {
-if (qemuSafeSerialParamValue(disk->serial) < 0)
+if (virSafeSerialParamValue(disk->serial) < 0)
 goto error;
 if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI &&
 disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) {
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 844c947..58ace3f 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2684,3 +2684,19 @@ virMemoryMaxValue(bool capped)
 else
 return LLONG_MAX;
 }
+
+#define VIR_SERIAL_PARAM_ACCEPTED_CHARS \
+  "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_ "
+
+int
+virSafeSerialParamValue(const char *value)
+{
+if (strspn(value, VIR_SERIAL_PARAM_ACCEPTED_CHARS) != strlen(value)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("driver serial '%s' contains unsafe characters"),
+   value);
+return -1;
+}
+
+return 0;
+}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 8c0d83c..f9b4831 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -252,6 +252,8 @@ unsigned long long virMemoryLimitTruncate(unsigned long 
long value);
 bool virMemoryLimitIsSet(unsigned long long value);
 unsigned long long virMemoryMaxValue(bool ulong);
 
+int virSafeSerialParamValue(const char *value);
+
 /**
  * VIR_ASSIGN_IS_OVERFLOW:
  * @rvalue: value that is checked (evaluated twice)
-- 
2.4.11

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

Re: [libvirt] [PATCH rfc v2 0/8] fspool: backend directory

[Maxim Nestratov]

07-Oct-16 20:09, Olga Krishtal пишет:


On 24/09/16 00:12, John Ferlan wrote:

On 09/23/2016 11:56 AM, Olga Krishtal wrote:

On 21/09/16 19:17, Maxim Nestratov wrote:

20 сент. 2016 г., в 23:52, John Ferlan<jfer...@redhat.com>  написал(а):




On 09/15/2016 03:32 AM, Olga Krishtal wrote:
Hi everyone, we would like to propose the first implementation of fspool
with directory backend.

Filesystem pools is a facility to manage filesystems resources similar
to how storage pools manages volume resources. Furthermore new API follows
storage API closely where it makes sense. Uploading/downloading operations
are not defined yet as it is not obvious how to make it properly. I guess
we can use some kind of tar to make a stream from a filesystem. Please share
you thoughts on this particular issue.

So how do you differentiate between with the existing 

Pool type=fs still provides volumes, i. e. block devices rather than 
filesystem, though this storage pool can mount file systems resided on a source 
block device.


http://libvirt.org/storage.html#StorageBackendFS

Sure the existing fs pool requires/uses a source block device as the
source path and this new variant doesn't require that source but seems
to use some item in order to dictate how to "define" the source on the
fly. Currently only a "DIR" is created - so how does that differ from a
"dir" pool.


Same here, storage "dir" provides files, which are in fact block devices for guests. 
While filesystem pool "dir" provides guests with file systems.



I think it'll be confusing to have and differentiate fspool and pool
commands.

Some time ago, we wrote the proposal description and asked for
everyone's advice and opinion.
The aim of fspool is to provide filesystems, not volumes. The simplest
type of fspool is directory pool and
it do has a lot in common with storage_backend_fs. However,  in the
proposal description we said that
the plan is to use other backends:  eg, storage volumes from storage
pool as the source of fs, zfs, etc.
  The final api for fspool will be significantly different, because of
the other backends needs.

Can you please try to create an extra line after the paragraph you're
responding to and the start of your paragraph and then one after.


Thanks for noticing. It looks better.


Anyway, as I pointed out - that description wasn't in my (short term)
memory. Keeping a trail of pointers to previous stuff helps those that
want to refresh their memory on the history.


I will hold this links through the next versions.
https://www.redhat.com/archives/libvir-list/2016-April/msg01941.html
https://www.redhat.com/archives/libvir-list/2016-May/msg00208.html



If you're going to "reuse" things, then using the 'src/util/*' is the
way to go rather than trying to drag in storage_{driver|backend*} APIs.
Crossing driver boundaries is something IIRC we try to avoid.


As I have written before at the moment we have only one backend for fspool -
directory. It is the simplest backend and only the starting point.
I think that it is too early to decide which parts should be moved to 
src/util/*.
Moreover,  as fspool items and storage pool volumes are pretty different, it
could be possible that they have very little in common. That said, I would leave
things as they are, but if you insist I can try.



If you meant the resulting code will have very little in common, then I would 
agree here.
More backends implementation will show us where common parts are
and we will have more basis for splitting out common parts.


I didn't dig through all the patches, but from the few I did look at it
seems as though all that's done is to rip out the guts of stuff not
desired from the storage pool driver and replace it with this new code
attributing all the work to the new author/copyright. IOW: Lots of
places where StoragePool appears to be exactly the same as the FSPool.


I have written this lines as a part of  GPLv2+ boilerplate:
https://www.redhat.com/archives/libvir-list/2016-August/msg01160.html, which I 
took from
other libvirt parts. And I guess it was naturally to change name and company, 
don't you?
And again, if you insist I can leave out the author/copyright as it wasn't the 
aim of this series.


Indeed, storage pool is very similar to FS pool but their items are not - 
volumes (block devices)
versus filesystems (directory trees). And intention here was to introduce a 
*new API*, which is
also very different from storage pool one, effectivly introducing a new driver. 
As driver
boundaries crossing isn't favored, the code was simply borrowed, following 
earlier practice used
by libvirt to get new drivers implemented.

John, keeping all said above in mind, do you think it's worth trying to reuse 
common code while
introducing a new API? It won't allow us to leave existing code untouched and 
it will increase the
series even more.





I think you need to find a different means to do what 

Re: [libvirt] [PATCH rfc v2 0/8] fspool: backend directory

[Maxim Nestratov]

23-Sep-16 23:51, John Ferlan пишет:

[snip]

I think rather than just copy what the storage pool does, I would think
the new driver could "build up" what it needs based on some consensus
based on what makes sense for the usage model.



Having a guest mount a host file system would seem to be possible
through other means. I also start wondering about security implications
for either side (haven't put too much thought into it). What can the
guest put "on" the host file system and vice versa where different
security policies may exist for allowing such placement.

Perhaps rather than a large dump of code the RFC should state the goal,
purpose, usage, etc. and see if that's what the community wants or is
willing to provide feedback on.

This was previously done in the mailing list many months ago now.


Well a pointer would have been nice... Obviously I didn't remember it!
There was an fspools v1 posted 8/19. I think there was an assumption
that list readers/reviewers would remember some original RFC. I didn't.
I've just been going through older patches that haven't had review and
this just came up as "next" (actually I had started thinking about the
v1 when v2 showed up).

John



Just a pointer to the previous disscussion:

https://www.redhat.com/archives/libvir-list/2016-April/msg01941.html
https://www.redhat.com/archives/libvir-list/2016-May/msg00208.html

Maxim

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

Re: [libvirt] [PATCH rfc v2 0/8] fspool: backend directory

[Maxim Nestratov]

> 20 сент. 2016 г., в 23:52, John Ferlan  написал(а):
> 
> 
> 
>> On 09/15/2016 03:32 AM, Olga Krishtal wrote:
>> Hi everyone, we would like to propose the first implementation of fspool
>> with directory backend.
>> 
>> Filesystem pools is a facility to manage filesystems resources similar
>> to how storage pools manages volume resources. Furthermore new API follows
>> storage API closely where it makes sense. Uploading/downloading operations
>> are not defined yet as it is not obvious how to make it properly. I guess
>> we can use some kind of tar to make a stream from a filesystem. Please share
>> you thoughts on this particular issue.
> 
> 
> So how do you differentiate between with the existing 

Pool type=fs still provides volumes, i. e. block devices rather than 
filesystem, though this storage pool can mount file systems resided on a source 
block device. 

> 
> http://libvirt.org/storage.html#StorageBackendFS
> 
> Sure the existing fs pool requires/uses a source block device as the
> source path and this new variant doesn't require that source but seems
> to use some item in order to dictate how to "define" the source on the
> fly. Currently only a "DIR" is created - so how does that differ from a
> "dir" pool.
> 

Same here, storage "dir" provides files, which are in fact block devices for 
guests. While filesystem pool "dir" provides guests with file systems. 


> I think it'll be confusing to have and differentiate fspool and pool
> commands.
> 
> I didn't dig through all the patches, but from the few I did look at it
> seems as though all that's done is to rip out the guts of stuff not
> desired from the storage pool driver and replace it with this new code
> attributing all the work to the new author/copyright. IOW: Lots of
> places where StoragePool appears to be exactly the same as the FSPool.
> 
> I think you need to find a different means to do what you want. It's not
> 100% what the end goal is.
> 
> I did download/git am the patches and scan a few patches...
>  * In patch 2 you've totally missed how to modify libvirt_public.syms.
>  * In patch 3, the build breaks in "conf/fs_conf" since the "if { if {}
> }" aren't done properly in virFSPoolDefFormatBuf.
>  * In patch 5 the remote_protocol_structs fails check/syntax-check... I
> stopped there in my build each patch test.
> 
> John
> 
>> 
>> The patchset provides 'dir' backend which simply expose directories in some
>> directory in host filesystem. The virsh commands are provided too. So it is
>> ready to play with, just replace 'pool' in xml descriptions and virsh 
>> commands
>> to 'fspool' and 'volume' to 'item'.
>> Examle and usage:
>> Define:
>> virsh -c qemu:///system fspool-define-as fs_pool_name dir --target 
>> /path/on/host
>> Build
>> virsh -c qemu:///system fspool-build fs_pool_name
>> Start
>> virsh -c qemu:///system fspool-start fs_pool_name
>> Look inside
>> virsh -c qemu:///system fspool-list (--all) fspool_name
>> 
>> Fspool called POOL, on the host fs uses /fs_driver to hold items.
>>  virsh -c qemu:///system fspool-dumpxml POOL
>>  
>>POOL
>>c57c9d7c-b1d5-4c45-ba9c-67f03d4da160
>>733722615808
>>1331486720
>>534810800128
>>
>>
>>
>>  /fs_driver
>>  
>>0755
>>0
>>0
>>  
>>
>>  
>> 
>> virsh -c qemu:///system fspool-info POOL
>> Name:   POOL
>> UUID:   c57c9d7c-b1d5-4c45-ba9c-67f03d4da160
>> State:  running
>> Persistent: yes
>> Autostart:  no autostart
>> Capacity:   683.33 GiB
>> Allocation: 1.24 GiB
>> Available:  498.08 GiB
>> 
>> virsh -c qemu+unix:///system item-list POOL
>>  Name Path
>> --
>> item1/fs_driver/item1
>> item10   /fs_driver/item10
>> item11   /fs_driver/item11
>> item12   /fs_driver/item12
>> item15   /fs_driver/item15
>> 
>> Fspool of directory type is some directory on host fs that holds items 
>> (subdirs).
>> Example of usage for items:
>> virsh -c vz+unix:///system item-create-as POOL item1 1g - create item
>> virsh -c qemu+unix:///system item-dumpxml item1 POOL
>> 
>>item1
>>/fs_driver/item1
>>
>>
>>0
>>0
>>
>>  
>>
>>  
>> 
>> virsh -c qemu+unix:///system item-info item1 POOL
>> Name:   item1
>> Type:   dir
>> Capacity:   683.33 GiB
>> Allocation: 634.87 MiB
>> Autostart:  no autostart
>> Capacity:   683.33 GiB
>> Allocation: 1.24 GiB
>> Available:  498.08 GiB
>> virsh -c qemu+unix:///system item-list POOL
>>  Name Path
>> --
>>  item1/fs_driver/item1
>>  item10   /fs_driver/item10
>>  item11   /fs_driver/item11
>>  item12   /fs_driver/item12
>>  item15   /fs_driver/item15
>> 
>> 

Re: [libvirt] [PATCH 0/3] add option to keep nvram file on undefine

[Maxim Nestratov]

27-May-16 11:05, Nikolay Shirokovskiy пишет:


   There is already a patch [1] on this topic with a different approach - keep
nvram file by default. There is also some discussion there. To sum up keeping
nvram on undefine could be useful in some usecases so there should be an option
to do it. On the other hand there is a danger of leaving domain assets after
its undefine and unsing them unintentionally on defining domain with the same
name.

   AFAIU keeping nvram by default was motivated by domain disks behaviour.
I think there is a difference as libvirt never create disks for domain as
opposed to nvram and managed save and without disks domain will not start so
user is quite aware of disks files. On the other hand one can start using nvram
file solely putting  in config and managed save is created on daemon
shutdown. So user is much less aware of nvram and managed save existence. Thus
one can easily mess up by unaware define $name/using/undefine/define $name again
usecase. Thus I vote for keeping said assets only if it is specified explicitly
so user knows what he is doing.

   Adding option to undefine is best solution I come up with. The other options
are add checks on define or start and both are impossible. Such a check should
be done without any extra flags for it to be useful but this way we break
existing users.

   As this a proof of concept this series does not add extra flag for managed 
save.

[1] https://www.redhat.com/archives/libvir-list/2015-February/msg00915.html


I'm OK with your approach. ACK from me.
Any other opinion?

Maxim

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

Re: [libvirt] [PATCH] util: fix crash in virClassIsDerivedFrom for CloseCallbacks objects

[Maxim Nestratov]

07-Sep-16 12:45, Michal Privoznik пишет:


On 07.09.2016 11:41, Maxim Nestratov wrote:

06-Sep-16 17:42, Michal Privoznik пишет:


On 05.09.2016 18:42, Maxim Nestratov wrote:

There is a possibility that qemu driver frees by unreferencing its
closeCallbacks pointer as it has the only reference to the object,
while in fact not all users of CloseCallbacks called thier
virCloseCallbacksUnset.

Backtrace is the following:
Thread #1:
0  in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
1  in virCondWait (c=, m=)
  at util/virthread.c:154
2  in virThreadPoolFree (pool=0x7f0810110b50)
  at util/virthreadpool.c:266
3  in qemuStateCleanup () at qemu/qemu_driver.c:1116
4  in virStateCleanup () at libvirt.c:808
5  in main (argc=, argv=)
  at libvirtd.c:1660

Thread #2:
0  in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7f0837c694d0)
at util/virobject.c:169
1  in virObjectIsClass (anyobj=anyobj@entry=0x7f08101d4760,
klass=) at util/virobject.c:365
2  in virObjectLock (anyobj=0x7f08101d4760) at util/virobject.c:317
3  in virCloseCallbacksUnset (closeCallbacks=0x7f08101d4760,
vm=vm@entry=0x7f08101d47b0, cb=cb@entry=0x7f081d078fc0
) at util/virclosecallbacks.c:163
4  in qemuProcessAutoDestroyRemove
(driver=driver@entry=0x7f081018be50, vm=vm@entry=0x7f08101d47b0) at
qemu/qemu_process.c:6368
5  in qemuProcessStop (driver=driver@entry=0x7f081018be50,
vm=vm@entry=0x7f08101d47b0,
reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN,
asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=flags@entry=0) at
qemu/qemu_process.c:5854
6  in processMonitorEOFEvent (vm=0x7f08101d47b0,
driver=0x7f081018be50) at qemu/qemu_driver.c:4585
7  qemuProcessEventHandler (data=,
opaque=0x7f081018be50) at qemu/qemu_driver.c:4629
8  in virThreadPoolWorker (opaque=opaque@entry=0x7f0837c4f820) at
util/virthreadpool.c:145
9  in virThreadHelper (data=) at util/virthread.c:206
10 in start_thread () from /lib64/libpthread.so.0

Let's reference CloseCallbacks object in virCloseCallbacksSet and
unreference in virCloseCallbacksUnset.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
   src/util/virclosecallbacks.c | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
index 82d4071..891a92b 100644
--- a/src/util/virclosecallbacks.c
+++ b/src/util/virclosecallbacks.c
@@ -141,6 +141,7 @@ virCloseCallbacksSet(virCloseCallbacksPtr
closeCallbacks,
   virObjectRef(vm);
   }
   +virObjectRef(closeCallbacks);
   ret = 0;
cleanup:
   virObjectUnlock(closeCallbacks);
@@ -180,6 +181,8 @@ virCloseCallbacksUnset(virCloseCallbacksPtr
closeCallbacks,
   ret = 0;
cleanup:
   virObjectUnlock(closeCallbacks);
+if (!ret)
+virObjectUnref(closeCallbacks);

Might as well put this a few lines above, just before 'ret = 0' line.

ACK with that changed.

Michal

No, we can't do this, as it could be the last reference to
closeCallbacks and after derefencing it, virObjectUnlock would touch
freed memory.


Ah, good point. Stick to the original then.

Michal


Pushed now. Thanks.

Maxim

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

Re: [libvirt] [PATCH] qemu: guest agent: introduce new error code VIR_ERR_AGENT_UNSYNCED

[Maxim Nestratov]

06-Sep-16 18:03, Michal Privoznik пишет:


On 05.09.2016 18:42, Maxim Nestratov wrote:

From: Yuri Pudgorodskiy <y...@virtuozzo.com>

A separate error code will help recognize real failures from
necessity to try again

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  include/libvirt/virterror.h | 2 ++
  src/qemu/qemu_agent.c   | 6 +++---
  src/util/virerror.c | 6 ++
  3 files changed, 11 insertions(+), 3 deletions(-)

ACK

Just curious - have you experienced this behaviour?

Michal


Pushed now. Thanks.

Maxim

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

Re: [libvirt] [PATCH] util: fix crash in virClassIsDerivedFrom for CloseCallbacks objects

[Maxim Nestratov]

06-Sep-16 17:42, Michal Privoznik пишет:


On 05.09.2016 18:42, Maxim Nestratov wrote:

There is a possibility that qemu driver frees by unreferencing its
closeCallbacks pointer as it has the only reference to the object,
while in fact not all users of CloseCallbacks called thier
virCloseCallbacksUnset.

Backtrace is the following:
Thread #1:
0  in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
1  in virCondWait (c=, m=)
 at util/virthread.c:154
2  in virThreadPoolFree (pool=0x7f0810110b50)
 at util/virthreadpool.c:266
3  in qemuStateCleanup () at qemu/qemu_driver.c:1116
4  in virStateCleanup () at libvirt.c:808
5  in main (argc=, argv=)
 at libvirtd.c:1660

Thread #2:
0  in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7f0837c694d0) at 
util/virobject.c:169
1  in virObjectIsClass (anyobj=anyobj@entry=0x7f08101d4760, klass=) at util/virobject.c:365
2  in virObjectLock (anyobj=0x7f08101d4760) at util/virobject.c:317
3  in virCloseCallbacksUnset (closeCallbacks=0x7f08101d4760, 
vm=vm@entry=0x7f08101d47b0, cb=cb@entry=0x7f081d078fc0 
) at util/virclosecallbacks.c:163
4  in qemuProcessAutoDestroyRemove (driver=driver@entry=0x7f081018be50, 
vm=vm@entry=0x7f08101d47b0) at qemu/qemu_process.c:6368
5  in qemuProcessStop (driver=driver@entry=0x7f081018be50, 
vm=vm@entry=0x7f08101d47b0, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN, 
asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=flags@entry=0) at 
qemu/qemu_process.c:5854
6  in processMonitorEOFEvent (vm=0x7f08101d47b0, driver=0x7f081018be50) at 
qemu/qemu_driver.c:4585
7  qemuProcessEventHandler (data=, opaque=0x7f081018be50) at 
qemu/qemu_driver.c:4629
8  in virThreadPoolWorker (opaque=opaque@entry=0x7f0837c4f820) at 
util/virthreadpool.c:145
9  in virThreadHelper (data=) at util/virthread.c:206
10 in start_thread () from /lib64/libpthread.so.0

Let's reference CloseCallbacks object in virCloseCallbacksSet and
unreference in virCloseCallbacksUnset.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  src/util/virclosecallbacks.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
index 82d4071..891a92b 100644
--- a/src/util/virclosecallbacks.c
+++ b/src/util/virclosecallbacks.c
@@ -141,6 +141,7 @@ virCloseCallbacksSet(virCloseCallbacksPtr closeCallbacks,
  virObjectRef(vm);
  }
  
+virObjectRef(closeCallbacks);

  ret = 0;
   cleanup:
  virObjectUnlock(closeCallbacks);
@@ -180,6 +181,8 @@ virCloseCallbacksUnset(virCloseCallbacksPtr closeCallbacks,
  ret = 0;
   cleanup:
  virObjectUnlock(closeCallbacks);
+if (!ret)
+virObjectUnref(closeCallbacks);

Might as well put this a few lines above, just before 'ret = 0' line.

ACK with that changed.

Michal


No, we can't do this, as it could be the last reference to closeCallbacks and after derefencing it, virObjectUnlock 
would touch freed memory.


Maxim

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

Re: [libvirt] [PATCH] qemu: guest agent: introduce new error code VIR_ERR_AGENT_UNSYNCED

[Maxim Nestratov]

06-Sep-16 18:03, Michal Privoznik пишет:


On 05.09.2016 18:42, Maxim Nestratov wrote:

From: Yuri Pudgorodskiy <y...@virtuozzo.com>

A separate error code will help recognize real failures from
necessity to try again

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
  include/libvirt/virterror.h | 2 ++
  src/qemu/qemu_agent.c   | 6 +++---
  src/util/virerror.c | 6 ++
  3 files changed, 11 insertions(+), 3 deletions(-)

ACK

Just curious - have you experienced this behaviour?

Michal


Yeah, pretty often.

Maxim

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

[libvirt] [PATCH] util: fix crash in virClassIsDerivedFrom for CloseCallbacks objects

[Maxim Nestratov]

There is a possibility that qemu driver frees by unreferencing its
closeCallbacks pointer as it has the only reference to the object,
while in fact not all users of CloseCallbacks called thier
virCloseCallbacksUnset.

Backtrace is the following:
Thread #1:
0  in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
1  in virCondWait (c=, m=)
at util/virthread.c:154
2  in virThreadPoolFree (pool=0x7f0810110b50)
at util/virthreadpool.c:266
3  in qemuStateCleanup () at qemu/qemu_driver.c:1116
4  in virStateCleanup () at libvirt.c:808
5  in main (argc=, argv=)
at libvirtd.c:1660

Thread #2:
0  in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7f0837c694d0) at 
util/virobject.c:169
1  in virObjectIsClass (anyobj=anyobj@entry=0x7f08101d4760, klass=) at util/virobject.c:365
2  in virObjectLock (anyobj=0x7f08101d4760) at util/virobject.c:317
3  in virCloseCallbacksUnset (closeCallbacks=0x7f08101d4760, 
vm=vm@entry=0x7f08101d47b0, cb=cb@entry=0x7f081d078fc0 
) at util/virclosecallbacks.c:163
4  in qemuProcessAutoDestroyRemove (driver=driver@entry=0x7f081018be50, 
vm=vm@entry=0x7f08101d47b0) at qemu/qemu_process.c:6368
5  in qemuProcessStop (driver=driver@entry=0x7f081018be50, 
vm=vm@entry=0x7f08101d47b0, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN, 
asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=flags@entry=0) at 
qemu/qemu_process.c:5854
6  in processMonitorEOFEvent (vm=0x7f08101d47b0, driver=0x7f081018be50) at 
qemu/qemu_driver.c:4585
7  qemuProcessEventHandler (data=, opaque=0x7f081018be50) at 
qemu/qemu_driver.c:4629
8  in virThreadPoolWorker (opaque=opaque@entry=0x7f0837c4f820) at 
util/virthreadpool.c:145
9  in virThreadHelper (data=) at util/virthread.c:206
10 in start_thread () from /lib64/libpthread.so.0

Let's reference CloseCallbacks object in virCloseCallbacksSet and
unreference in virCloseCallbacksUnset.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/util/virclosecallbacks.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
index 82d4071..891a92b 100644
--- a/src/util/virclosecallbacks.c
+++ b/src/util/virclosecallbacks.c
@@ -141,6 +141,7 @@ virCloseCallbacksSet(virCloseCallbacksPtr closeCallbacks,
 virObjectRef(vm);
 }
 
+virObjectRef(closeCallbacks);
 ret = 0;
  cleanup:
 virObjectUnlock(closeCallbacks);
@@ -180,6 +181,8 @@ virCloseCallbacksUnset(virCloseCallbacksPtr closeCallbacks,
 ret = 0;
  cleanup:
 virObjectUnlock(closeCallbacks);
+if (!ret)
+virObjectUnref(closeCallbacks);
 return ret;
 }
 
-- 
2.4.11

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

[libvirt] [PATCH 0/3] vz: nettype=bridge and other corrections

[Maxim Nestratov]

Maxim Nestratov (3):
  vz: support type=bridge network interface type correctly
  vz: remove Bridged network name and rename Routed
  vz: add MIGRATION_V3 capability

 src/vz/vz_driver.c |   1 +
 src/vz/vz_sdk.c| 100 -
 src/vz/vz_utils.h  |   3 +-
 3 files changed, 16 insertions(+), 88 deletions(-)

-- 
2.4.11

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

[libvirt] [PATCH 2/3] vz: remove Bridged network name and rename Routed

[Maxim Nestratov]

It's funny, but Routed network name was incorrect. We should use
host-routed instead.
---
 src/vz/vz_utils.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h
index 4b407ec..9e02fe0 100644
--- a/src/vz/vz_utils.h
+++ b/src/vz/vz_utils.h
@@ -47,8 +47,7 @@
_("no domain with matching uuid '%s'"), uuidstr); \
 } while (0)
 
-# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME   "Routed"
-# define PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME  "Bridged"
+# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME   "host-routed"
 # define VIRTUOZZO_VER_7 ((unsigned long) 700)
 
 struct _vzCapabilities {
-- 
2.4.11

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

[libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Maxim Nestratov]

Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to  support it.

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_sdk.c | 100 
 1 file changed, 14 insertions(+), 86 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..933b222 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
virDomainNetDefPtr net, bool isCt)
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
 goto cleanup;
 } else {
-char *netid = NULL;
-
-if (!(netid =
+char *netid =
   prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
-  netAdapter)))
-goto cleanup;
+  netAdapter);
 
-/*
- * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters
- * except those whose Virtual Network Id differ from Parallels
- * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME
- * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME
- */
-if (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+if (emulatedType == PNA_BRIDGE) {
 net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-net->data.network.name = netid;
+if (netid)
+net->data.bridge.brname = netid;
 } else {
 net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-net->data.bridge.brname = netid;
+if (netid)
+net->data.network.name = netid;
 }
-
 }
 
 if (!isCt) {
@@ -3175,16 +3167,14 @@ static int prlsdkConfigureGateways(PRL_HANDLE sdknet, 
virDomainNetDefPtr net)
 return ret;
 }
 
-static int prlsdkConfigureNet(vzDriverPtr driver,
-  virDomainObjPtr dom,
+static int prlsdkConfigureNet(vzDriverPtr driver ATTRIBUTE_UNUSED,
+  virDomainObjPtr dom ATTRIBUTE_UNUSED,
   PRL_HANDLE sdkdom,
   virDomainNetDefPtr net,
   bool isCt, bool create)
 {
 PRL_RESULT pret;
 PRL_HANDLE sdknet = PRL_INVALID_HANDLE;
-PRL_HANDLE vnet = PRL_INVALID_HANDLE;
-PRL_HANDLE job = PRL_INVALID_HANDLE;
 PRL_HANDLE addrlist = PRL_INVALID_HANDLE;
 size_t i;
 int ret = -1;
@@ -3291,35 +3281,17 @@ static int prlsdkConfigureNet(vzDriverPtr driver,
 if (STREQ(net->data.network.name, 
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) {
 pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED);
 prlsdkCheckRetGoto(pret, cleanup);
-} else if (STREQ(net->data.network.name, 
PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
-pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET);
+} else {
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_NETWORK);
 prlsdkCheckRetGoto(pret, cleanup);
 
 pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, 
net->data.network.name);
 prlsdkCheckRetGoto(pret, cleanup);
 }
-} else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
-/*
- * For this type of adapter we create a new
- * Virtual Network assuming that bridge with given name exists
- * Failing creating this means domain creation failure
- */
-pret = PrlVirtNet_Create();
-prlsdkCheckRetGoto(pret, cleanup);
-
-pret = PrlVirtNet_SetNetworkId(vnet, net->data.bridge.brname);
-prlsdkCheckRetGoto(pret, cleanup);
 
-pret = PrlVirtNet_SetNetworkType(vnet, PVN_BRIDGED_ETHERNET);
-prlsdkCheckRetGoto(pret, cleanup);
-
-job = PrlSrv_AddVirtualNetwork(driver->server,
-   vnet,
-   PRL_USE_VNET_NAME_FOR_BRIDGE_NAME);
-if (PRL_FAILED(pret = waitDomainJob(job, dom)))
-goto cleanup;
+} else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
 
-pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET);
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGE);
 prlsdkCheckRetGoto(pret, cleanup);
 
 pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, 
net->data.bridge.brname);
@@ -3334,40 +3306,10 @@ static int prlsdkConfigureNet(vzDriverPtr driver,
  cleanup:
 VIR_FREE(addrstr);
 PrlHandle_Free(addrlist);
-PrlHandle_Free(vnet);
 PrlHandle_Free(sdknet);
 return ret;
 }
 
-static void
-prlsdkCleanupBridgedNet(vzDriverPtr driver,
-virDomainObjPtr dom,
-virDomainNetDefPtr net)
-{
-PRL_RESULT pret;
-PRL_HANDLE vnet = PRL_INVALID_HANDLE;
-PRL_HA

[libvirt] [PATCH] qemu: guest agent: introduce new error code VIR_ERR_AGENT_UNSYNCED

[Maxim Nestratov]

From: Yuri Pudgorodskiy <y...@virtuozzo.com>

A separate error code will help recognize real failures from
necessity to try again

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 include/libvirt/virterror.h | 2 ++
 src/qemu/qemu_agent.c   | 6 +++---
 src/util/virerror.c | 6 ++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 2ec560e..efe83aa 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -315,6 +315,8 @@ typedef enum {
 VIR_ERR_AUTH_UNAVAILABLE = 94, /* authentication unavailable */
 VIR_ERR_NO_SERVER = 95, /* Server was not found */
 VIR_ERR_NO_CLIENT = 96, /* Client was not found */
+VIR_ERR_AGENT_UNSYNCED = 97,/* guest agent replies with wrong id
+   to guest-sync command */
 } virErrorNumber;
 
 /**
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index eeede6b..fdc4fed 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -966,21 +966,21 @@ qemuAgentGuestSync(qemuAgentPtr mon)
 goto cleanup;
 
 if (!sync_msg.rxObject) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_AGENT_UNSYNCED, "%s",
_("Missing monitor reply object"));
 goto cleanup;
 }
 
 if (virJSONValueObjectGetNumberUlong(sync_msg.rxObject,
  "return", _ret) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+virReportError(VIR_ERR_AGENT_UNSYNCED, "%s",
_("Malformed return value"));
 goto cleanup;
 }
 
 VIR_DEBUG("Guest returned ID: %llu", id_ret);
 if (id_ret != id) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
+virReportError(VIR_ERR_AGENT_UNSYNCED,
_("Guest agent returned ID: %llu instead of %llu"),
id_ret, id);
 goto cleanup;
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 1177570..2958308 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -1394,6 +1394,12 @@ virErrorMsg(virErrorNumber error, const char *info)
 else
 errmsg = _("Client not found: %s");
 break;
+case VIR_ERR_AGENT_UNSYNCED:
+if (info == NULL)
+errmsg = _("guest agent replied with wrong id to guest-sync 
command");
+else
+errmsg = _("guest agent replied with wrong id to guest-sync 
command: %s");
+break;
 }
 return errmsg;
 }
-- 
2.4.11

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

[libvirt] [PATCH 3/3] vz: add MIGRATION_V3 capability

[Maxim Nestratov]

Signed-off-by: Maxim Nestratov <mnestra...@virtuozzo.com>
---
 src/vz/vz_driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 819dad7..b971add 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3071,6 +3071,7 @@ vzConnectSupportsFeature(virConnectPtr conn 
ATTRIBUTE_UNUSED, int feature)
 return -1;
 
 switch (feature) {
+case VIR_DRV_FEATURE_MIGRATION_V3:
 case VIR_DRV_FEATURE_MIGRATION_PARAMS:
 case VIR_DRV_FEATURE_MIGRATION_P2P:
 return 1;
-- 
2.4.11

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

Re: [libvirt] [PATCH] vz: update domain cache after device updates

[Maxim Nestratov]

25-Aug-16 12:25, Mikhail Feoktistov пишет:



On 25.08.2016 11:33, Nikolay Shirokovskiy wrote:

---
  src/vz/vz_driver.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index b34fe33..f223794 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -1694,6 +1694,9 @@ static int vzDomainUpdateDeviceFlags(virDomainPtr domain,
  if (prlsdkUpdateDevice(driver, dom, dev) < 0)
  goto cleanup;
  +if (prlsdkUpdateDomain(driver, dom) < 0)
+goto cleanup;
+
  ret = 0;
   cleanup:


Ack



Pushed now. Thanks.

Maxim

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

Re: [libvirt] [PATCH] vz: fixed race in vzDomainAttach/DettachDevice

[Maxim Nestratov]

18-Aug-16 14:57, Olga Krishtal пишет:


While dettaching/attaching device in OpenStack, nova
calls vzDomainDettachDevice twice, because the update of the internal
configuration of the ct comes a bit latter than the update event.
As the result, we suffer from the second call to dettach the same device.

Signed-off-by: Olga Krishtal 
---
  src/vz/vz_sdk.c | 12 
  1 file changed, 12 insertions(+)


ACKed and pushed. Thanks!

Maxim

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

Re: [libvirt] [PATCH] vz: getting bus type for containers

[Maxim Nestratov]

25-Aug-16 11:13, Nikolay Shirokovskiy пишет:



On 15.08.2016 19:02, Mikhail Feoktistov wrote:

We should query bus type for containers too, like for VM.
In openstack we add volume disk like SCSI, so we can't
hardcode SATA bus.
---
  src/vz/vz_sdk.c | 32 +---
  1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f81b320..c4a1c3d 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c

ACK


Pushed now.

Maxim

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