[libvirt] [PATCH 1/5]virsh: disable config readonly and shareable in virsh command

[Chen Hanxiao]

From: Chen Hanxiao 

Daneil's suggestion about flag  and  as follow:
- Exclusive read-write. This is the default
- Shared read-write. This is the  flag
- Shared read-only. This is the  flag

So we should disable config both readonly and shareable
in virsh command to solve the confliction.
For backwards compatibility we keep the code about ''.
In this patch, '--mode' option will have high priority.
If set, it will screen the '--shareable' option.

Signed-off-by: Chen Hanxiao 
---
 tools/virsh-domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6d241db..2aed9f9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -609,7 +609,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
 if (wwn)
 virBufferAsprintf(&buf, "  %s\n", wwn);
 
-if (vshCommandOptBool(cmd, "shareable"))
+if (!mode && vshCommandOptBool(cmd, "shareable"))
 virBufferAddLit(&buf, "  \n");
 
 if (straddr) {
-- 
1.8.2.1

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

[libvirt] [PATCH 4/5]virsh: If VSH_OFLAG_IGNORE set, don't auto complete it in virsh

[Chen Hanxiao]

From: Chen Hanxiao 

If VSH_OFLAG_IGNORE set, don't auto complete in virsh.

Signed-off-by: Chen Hanxiao 
---
 tools/virsh.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/virsh.c b/tools/virsh.c
index f772794..42d79f1 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2677,6 +2677,9 @@ vshReadlineOptionsGenerator(const char *text, int state)
 /* ignore non --option */
 continue;
 
+if (opt->flags & VSH_OFLAG_IGNORE)
+continue;
+
 if (len > 2) {
 if (STRNEQLEN(name, text + 2, len - 2))
 continue;
-- 
1.8.2.1

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

[libvirt] [PATCH 3/5]virsh: if VSH_OFLAG_IGNORE set, don't show it in '--help'

[Chen Hanxiao]

From: Chen Hanxiao 

If VSH_OFLAG_IGNORE set, don't show it in '--help'.

Signed-off-by: Chen Hanxiao 
---
 tools/virsh.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tools/virsh.c b/tools/virsh.c
index 6842ed8..f772794 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1004,6 +1004,8 @@ vshCmddefOptParse(const vshCmdDef *cmd, uint32_t 
*opts_need_arg,
 
 if (i > 31)
 return -1; /* too many options */
+if (opt->flags & VSH_OFLAG_IGNORE)
+continue;
 if (opt->type == VSH_OT_BOOL) {
 if (opt->flags & VSH_OFLAG_REQ)
 return -1; /* bool options can't be mandatory */
@@ -1213,6 +1215,8 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
 if (def->opts) {
 const vshCmdOptDef *opt;
 for (opt = def->opts; opt->name; opt++) {
+if (opt->flags & VSH_OFLAG_IGNORE)
+continue;
 const char *fmt = "%s";
 switch (opt->type) {
 case VSH_OT_BOOL:
@@ -1267,6 +1271,8 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname)
 const vshCmdOptDef *opt;
 fputs(_("\n  OPTIONS\n"), stdout);
 for (opt = def->opts; opt->name; opt++) {
+if (opt->flags & VSH_OFLAG_IGNORE)
+continue;
 switch (opt->type) {
 case VSH_OT_BOOL:
 snprintf(buf, sizeof(buf), "--%s", opt->name);
-- 
1.8.2.1

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

[libvirt] [PATCH 0/5]virsh: solutions for hiding '--shareable' option

[Chen Hanxiao]

From: Chen Hanxiao 

As Daniel mentioned, '--shareable' should be hidden from users.
But we had to take care of backwards compatibility.
These patches give solutions to solve these issues.

Chen Hanxiao (5):
  [libvirt]virsh: disable config readonly and shareable in virsh command
  [libvirt]virsh: introduce flags VSH_OFLAG_IGNORE
  [libvirt]virsh: if VSH_OFLAG_IGNORE set, don't show it in '--help'
  [libvirt]virsh: If VSH_OFLAG_IGNORE set, don't auto complete in virsh
  [libvirt]virsh: mark '--shareable' as VSH_OFLAG_IGNORE

 tools/virsh-domain.c | 3 ++-
 tools/virsh.c| 9 +
 tools/virsh.h| 1 +
 tools/virsh.pod  | 3 +--
 4 files changed, 13 insertions(+), 3 deletions(-)

-- 
1.8.2.1

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

[libvirt] [PATCH 5/5]virsh: mark '--shareable' as VSH_OFLAG_IGNORE

[Chen Hanxiao]

From: Chen Hanxiao 

Mark '--shareable' as VSH_OFLAG_IGNORE and
delete related description in docs.

Signed-off-by: Chen Hanxiao 
---
 tools/virsh-domain.c | 1 +
 tools/virsh.pod  | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2aed9f9..b5b9006 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -313,6 +313,7 @@ static const vshCmdOptDef opts_attach_disk[] = {
 },
 {.name = "shareable",
  .type = VSH_OT_BOOL,
+ .flags = VSH_OFLAG_IGNORE,
  .help = N_("shareable between domains")
 },
 {.name = "rawio",
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 7af5503..cb273e0 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1923,7 +1923,7 @@ expected.
 [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
 [I<--driver driver>] [I<--subdriver subdriver>] [I<--cache cache>]
 [I<--type type>] [I<--mode mode>] [I<--config>] [I<--sourcetype soucetype>]
-[I<--serial serial>] [I<--wwn wwn>] [I<--shareable>] [I<--rawio>]
+[I<--serial serial>] [I<--wwn wwn>] [I<--rawio>]
 [I<--address address>] [I<--multifunction>] [I<--print-xml>]
 
 Attach a new disk device to the domain.
@@ -1945,7 +1945,6 @@ I can indicate the type of source (block|file)
 I can be one of "default", "none", "writethrough", "writeback",
 "directsync" or "unsafe".
 I is the serial of disk device. I is the wwn of disk device.
-I indicates the disk device is shareable between domains.
 I indicates the disk needs rawio capability.
 I is the address of disk device in the form of 
pci:domain.bus.slot.function,
 scsi:controller.bus.unit or ide:controller.bus.unit.
-- 
1.8.2.1

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

[libvirt] [PATCH 2/5]virsh: introduce flags VSH_OFLAG_IGNORE

[Chen Hanxiao]

From: Chen Hanxiao 

If some command option should be phased out,
we need to hide it. But for backwards compatibility,
we had to keep those obsolete codes.
With this flag, we could keep most of the old codes
and let tools like virsh know don't show it.

Signed-off-by: Chen Hanxiao 
---
 tools/virsh.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/virsh.h b/tools/virsh.h
index f978d94..029ae9f 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -136,6 +136,7 @@ enum {
 VSH_OFLAG_REQ  = (1 << 0), /* option required */
 VSH_OFLAG_EMPTY_OK = (1 << 1), /* empty string option allowed */
 VSH_OFLAG_REQ_OPT  = (1 << 2), /* --optionname required */
+VSH_OFLAG_IGNORE   = (1 << 3), /* keep this option but don't show it to 
users */
 };
 
 /* forward declarations */
-- 
1.8.2.1

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

Re: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies too quickly

[Michal Privoznik]

On 18.10.2013 08:22, Wangyufei (A) wrote:
> I'm sorry. I didn't get what you mean.
> 
> In virQEMUCapsInitQMP
> 
> if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL)) ||
> !(vm = virDomainObjNew(xmlopt)))
> goto cleanup;
> 
> vm->pid = pid;   //Apparently vm is not NULL here.
> 
> if (!(mon = qemuMonitorOpen(vm, &config, true, &callbacks, NULL))) {  
> //If qemuMonitorOpen returns NULL here, but not do mon->vm = 
> virObjectRef(vm); in qemuMonitorOpenInternal.
> ret = 0;
> goto cleanup;  // We go to cleanup here.
> }
> 
> virObjectLock(mon);
> 
> if (virQEMUCapsInitQMPMonitor(qemuCaps, mon) < 0)
> goto cleanup;
> 
> ret = 0;
> 
> cleanup:
> if (mon)
> virObjectUnlock(mon);
> qemuMonitorClose(mon);
> virCommandAbort(cmd);
> virCommandFree(cmd);
> VIR_FREE(monarg);
> VIR_FREE(monpath);
> virObjectUnref(vm);//vm is not NULL here, and we'll do something 
> about vm->refs, right?

Yes. In fact we dispose @vm as we are the only one holding reference to
it and we don't longer need it. If qemuMonitorOpenInternal would do
virObjectRef(vm), then vm->refs = 2 before executing this line. After
the execution, the refs is decremented to 1 as @mon is the only one
holding reference to @vm.

> virObjectUnref(xmlopt);

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

Re: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies too quickly

[Wangyufei (A)]

I'm sorry. I didn't get what you mean.

In virQEMUCapsInitQMP

if (!(xmlopt = virDomainXMLOptionNew(NULL, NULL, NULL)) ||
!(vm = virDomainObjNew(xmlopt)))
goto cleanup;

vm->pid = pid;   //Apparently vm is not NULL here.

if (!(mon = qemuMonitorOpen(vm, &config, true, &callbacks, NULL))) {  //If 
qemuMonitorOpen returns NULL here, but not do mon->vm = virObjectRef(vm); in 
qemuMonitorOpenInternal.
ret = 0;
goto cleanup;  // We go to cleanup here.
}

virObjectLock(mon);

if (virQEMUCapsInitQMPMonitor(qemuCaps, mon) < 0)
goto cleanup;

ret = 0;

cleanup:
if (mon)
virObjectUnlock(mon);
qemuMonitorClose(mon);
virCommandAbort(cmd);
virCommandFree(cmd);
VIR_FREE(monarg);
VIR_FREE(monpath);
virObjectUnref(vm);//vm is not NULL here, and we'll do something about 
vm->refs, right?
virObjectUnref(xmlopt);

> -Original Message-
> From: Michal Privoznik [mailto:mpriv...@redhat.com]
> Sent: Friday, October 18, 2013 1:12 PM
> To: Wangyufei (A)
> Cc: libvir-list@redhat.com; Wangrui (K)
> Subject: Re: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain
> dies too quickly
> 
> On 18.10.2013 06:06, Wangyufei (A) wrote:
> > Thanks at first, this patch some kinda solve my problem until now. But I 
> > still
> have a doubt about this patch.
> >
> >> -Original Message-
> >> From: libvir-list-boun...@redhat.com
> >> [mailto:libvir-list-boun...@redhat.com] On Behalf Of Michal Privoznik
> >> Sent: Friday, October 11, 2013 8:15 PM
> >> To: libvir-list@redhat.com
> >> Subject: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain
> dies
> >> too quickly
> 
> >> @@ -2673,6 +2677,8 @@ cleanup:
> >>  virCommandFree(cmd);
> >>  VIR_FREE(monarg);
> >>  VIR_FREE(monpath);
> >> +virObjectUnref(vm);
> >
> > Is this virObjectUnref(vm) corresponding to mon->vm = virObjectRef(vm)
> added in qemuMonitorOpenInternal?
> > If it is, how can we confirm virObjectRef(vm) has been done in
> qemuMonitorOpenInternal? If an error (anyone follows)happened in
> qemuMonitorOpenInternal is before mon->vm = virObjectRef(vm),
> > then we still goto cleanup and do virObjectUnref(vm), the refs will be
> wrong. Am I right?
> >
> 
> Unfortunately, you've cut off the chunk above that allocates @mon.
> Anyway, on initialization, @mon is filled with zeros. So until somebody
> sets mon->vm [1] mon->vm is effectively NULL. And virObjectUnref() acts
> like NOP on NULL.
> 
> > if (!cb->eofNotify) {
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >_("EOF notify callback must be supplied"));
> > return NULL;
> > }
> > if (!cb->errorNotify) {
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >_("Error notify callback must be supplied"));
> > return NULL;
> > }
> >
> > if (qemuMonitorInitialize() < 0)
> > return NULL;
> >
> > if (!(mon = virObjectLockableNew(qemuMonitorClass)))
> > return NULL;
> >
> > mon->fd = -1;
> > mon->logfd = -1;
> > if (virCondInit(&mon->notify) < 0) {
> > virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >_("cannot initialize monitor condition"));
> > goto cleanup;
> > }
> > mon->fd = fd;
> > mon->hasSendFD = hasSendFD;
> > mon->vm = virObjectRef(vm);
> 
> 1: ^^ until after this line
> >
> >> +virObjectUnref(xmlopt);
> >>
> >>  if (pid != 0) {
> >>  char ebuf[1024];
> 
> I hope it makes things a bit clearer.
> 
> Michal

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

Re: [libvirt] [PATCH]daemon: don't free domain if it's null

[Ján Tomko]

On 10/18/2013 04:12 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> If we fail to get domain, we had to judge whether
> it's null or not when doing 'cleanup'.
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  daemon/remote.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

ACK and pushed.

Jan




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

Re: [libvirt] [PATCH] Fix typo s/SASL_CONF_DIR/SASL_CONF_PATH/ in QEMU VNC code

[Ján Tomko]

On 10/17/2013 05:05 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" 
> 
> The QEMU VNC client arg code has a long standing typo
> of SASL_CONF_DIR when it should be SASL_CONFIG_PATH for

s/SASL_CONFIG_PATH/SASL_CONF_PATH

> the env variable name.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  src/qemu/qemu_command.c| 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args | 2 +-
>  tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 

Jan

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

Re: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies too quickly

[Michal Privoznik]

On 18.10.2013 06:06, Wangyufei (A) wrote:
> Thanks at first, this patch some kinda solve my problem until now. But I 
> still have a doubt about this patch.
> 
>> -Original Message-
>> From: libvir-list-boun...@redhat.com
>> [mailto:libvir-list-boun...@redhat.com] On Behalf Of Michal Privoznik
>> Sent: Friday, October 11, 2013 8:15 PM
>> To: libvir-list@redhat.com
>> Subject: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies
>> too quickly

>> @@ -2673,6 +2677,8 @@ cleanup:
>>  virCommandFree(cmd);
>>  VIR_FREE(monarg);
>>  VIR_FREE(monpath);
>> +virObjectUnref(vm);
> 
> Is this virObjectUnref(vm) corresponding to mon->vm = virObjectRef(vm) added 
> in qemuMonitorOpenInternal?
> If it is, how can we confirm virObjectRef(vm) has been done in 
> qemuMonitorOpenInternal? If an error (anyone follows)happened in 
> qemuMonitorOpenInternal is before mon->vm = virObjectRef(vm),
> then we still goto cleanup and do virObjectUnref(vm), the refs will be wrong. 
> Am I right?
> 

Unfortunately, you've cut off the chunk above that allocates @mon.
Anyway, on initialization, @mon is filled with zeros. So until somebody
sets mon->vm [1] mon->vm is effectively NULL. And virObjectUnref() acts
like NOP on NULL.

> if (!cb->eofNotify) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>_("EOF notify callback must be supplied"));
> return NULL;
> }
> if (!cb->errorNotify) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>_("Error notify callback must be supplied"));
> return NULL;
> }
> 
> if (qemuMonitorInitialize() < 0)
> return NULL;
> 
> if (!(mon = virObjectLockableNew(qemuMonitorClass)))
> return NULL;
> 
> mon->fd = -1;
> mon->logfd = -1;
> if (virCondInit(&mon->notify) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>_("cannot initialize monitor condition"));
> goto cleanup;
> }
> mon->fd = fd;
> mon->hasSendFD = hasSendFD;
> mon->vm = virObjectRef(vm);

1: ^^ until after this line
> 
>> +virObjectUnref(xmlopt);
>>
>>  if (pid != 0) {
>>  char ebuf[1024];

I hope it makes things a bit clearer.

Michal

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

Re: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies too quickly

[Wangyufei (A)]

Thanks at first, this patch some kinda solve my problem until now. But I still 
have a doubt about this patch.

> -Original Message-
> From: libvir-list-boun...@redhat.com
> [mailto:libvir-list-boun...@redhat.com] On Behalf Of Michal Privoznik
> Sent: Friday, October 11, 2013 8:15 PM
> To: libvir-list@redhat.com
> Subject: [libvirt] [PATCH v2] qemu_migration: Avoid crashing if domain dies
> too quickly
> 
> I've noticed a SIGSEGV-ing libvirtd on the destination when the qemu
> died too quickly = in Prepare phase. What is happening here is:
> 
> 1) [Thread 3493] We are in qemuMigrationPrepareAny() and calling
> qemuProcessStart() which subsequently calls qemuProcessWaitForMonitor()
> and qemuConnectMonitor(). So far so good. The qemuMonitorOpen()
> succeeds, however switching monitor to QMP mode fails as qemu died
> meanwhile. That is qemuMonitorSetCapabilities() returns -1.
> 
> 2013-10-08 15:54:10.629+: 3493: debug :
> qemuMonitorSetCapabilities:1356 : mon=0x14a53da0
> 2013-10-08 15:54:10.630+: 3493: debug :
> qemuMonitorJSONCommandWithFd:262 : Send command
> '{"execute":"qmp_capabilities","id":"libvirt-1"}' for write with FD -1
> 2013-10-08 15:54:10.630+: 3493: debug :
> virEventPollUpdateHandle:147 : EVENT_POLL_UPDATE_HANDLE: watch=17
> events=13
> ...
> 2013-10-08 15:54:10.631+: 3493: debug : qemuMonitorSend:956 :
> QEMU_MONITOR_SEND_MSG: mon=0x14a53da0
> msg={"execute":"qmp_capabilities","id":"libvirt-1"}
>  fd=-1
> 2013-10-08 15:54:10.631+: 3262: debug : virEventPollRunOnce:641 :
> Poll got 1 event(s)
> 
> 2) [Thread 3262] The event loop is trying to do the talking to monitor.
> However, qemu is dead already, remember?
> 
> 2013-10-08 15:54:13.436+: 3262: error : qemuMonitorIORead:551 :
> Unable to read from monitor: Connection reset by peer
> 2013-10-08 15:54:13.516+: 3262: debug : virFileClose:90 : Closed fd 25
> ...
> 2013-10-08 15:54:13.533+: 3493: debug : qemuMonitorSend:968 : Send
> command resulted in error internal error: early end of file from monitor:
> possible problem:
> 
> 3) [Thread 3493] qemuProcessStart() failed. No big deal. Go to the
> 'endjob' label and subsequently to the 'cleanup'. Since the domain is
> not persistent and ret is -1, the qemuDomainRemoveInactive() is called.
> This has an (unpleasant) effect of virObjectUnref()-in the @vm object.
> Unpleasant because the event loop which is about to trigger EOF callback
> still holds a pointer to the @vm (not the reference). See the valgrind
> output below.
> 
> 4) [Thread 3262] So the even loop starts triggering EOF:
> 
> 2013-10-08 15:54:13.542+: 3262: debug : qemuMonitorIO:729 :
> Triggering EOF callback
> 2013-10-08 15:54:13.543+: 3262: debug :
> qemuProcessHandleMonitorEOF:294 : Received EOF on 0x14549110 'migt10'
> 
> And the monitor is cleaned up. This results in calling
> qemuProcessHandleMonitorEOF with the @vm pointer passed. The pointer
> is
> kept in qemuMonitor struct.
> 
> ==3262== Thread 1:
> ==3262== Invalid read of size 4
> ==3262==at 0x77ECCAA: pthread_mutex_lock (in
> /lib64/libpthread-2.15.so)
> ==3262==by 0x52FAA06: virMutexLock (virthreadpthread.c:85)
> ==3262==by 0x52E3891: virObjectLock (virobject.c:320)
> ==3262==by 0x11626743: qemuProcessHandleMonitorEOF
> (qemu_process.c:296)
> ==3262==by 0x11642593: qemuMonitorIO (qemu_monitor.c:730)
> ==3262==by 0x52BD526: virEventPollDispatchHandles
> (vireventpoll.c:501)
> ==3262==by 0x52BDD49: virEventPollRunOnce (vireventpoll.c:648)
> ==3262==by 0x52BBC68: virEventRunDefaultImpl (virevent.c:274)
> ==3262==by 0x542D3D9: virNetServerRun (virnetserver.c:1112)
> ==3262==by 0x11F368: main (libvirtd.c:1513)
> ==3262==  Address 0x14549128 is 24 bytes inside a block of size 136 free'd
> ==3262==at 0x4C2AF5C: free (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==3262==by 0x529B1FF: virFree (viralloc.c:580)
> ==3262==by 0x52E3703: virObjectUnref (virobject.c:270)
> ==3262==by 0x531557E: virDomainObjListRemove
> (domain_conf.c:2355)
> ==3262==by 0x1160E899: qemuDomainRemoveInactive
> (qemu_domain.c:2061)
> ==3262==by 0x1163A0C6: qemuMigrationPrepareAny
> (qemu_migration.c:2450)
> ==3262==by 0x1163A923: qemuMigrationPrepareDirect
> (qemu_migration.c:2626)
> ==3262==by 0x11682D71: qemuDomainMigratePrepare3Params
> (qemu_driver.c:10309)
> ==3262==by 0x53B0976: virDomainMigratePrepare3Params
> (libvirt.c:7266)
> ==3262==by 0x1502D3:
> remoteDispatchDomainMigratePrepare3Params (remote.c:4797)
> ==3262==by 0x12DECA:
> remoteDispatchDomainMigratePrepare3ParamsHelper
> (remote_dispatch.h:5741)
> ==3262==by 0x54322EB: virNetServerProgramDispatchCall
> (virnetserverprogram.c:435)
> 
> The mon->vm is set in qemuMonitorOpenInternal() which is the correct
> place to increase @vm ref counter. The correct place to decrease the ref
> counter is then qemuMonitorDispose().
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> It turned out the hack 

[libvirt] When vm's status file being left over, some persistent but inactive vms will be lost by libvirtd after libvirtd rebooting.

[Wangyufei (A)]

Hello,
  I found a problem that:
  vm's status file may be left over in the path /var/run/libvirt/qemu under 
some situation, such as host reboot. When vm's status file is left over, some 
persistent but inactive vms will be lost by libvirtd after it is rebooted. And 
you can do as follows to reproduce the problem:
  1、Create a vm and start it by the commands: virsh define vm-xml and virsh 
start vm-name.
  2、Stop the libvirtd by the command: service libvirtd stop.
  3、Kill the qemu process related to the vm, and make the vm's status file left 
over.
  4、Start libvirtd.
  After starting the libvirtd service, we find that the vm has been lost by 
libvirtd with command"virsh list --all". 
What we expect is that the vm is shown with shutoff status, should we?

The reason for the problem is that:
  During libvirtd startup, it first loads status files of vms under the path 
/var/run/libvirt/qemu, creates virDomainObj for each vm and adds it to 
driver->domains list.  
  Then it creates a thread to connect related qemu process for each 
virDomainObj in the domains list.Because the qemu process has been killed, so 
connecting to 
qemu will be failed. When connecting to qemu failed, connection-thread will do 
the follows: 
  1、Check if vm->persistent is 1. 
  2、If vm->persistent is not 1, then qemuDomainRemoveInactive() is called to 
remove the virDomainObj.
  3、Then the following calling sequence will occur:qemuDomainRemoveInactive() 
-->virDomainObjListRemove()-->virHashRemoveEntry(). Around 
virHashRemoveEntry(), 
  domlist and dom will be locked and unlocked sequencely.
  The problem of the above steps is that vm->persistent maybe has been set to 1 
by libvirtd main-thread when connection-thread calling virHashRemoveEntry() to 
remove the dom. That is a persistent virDomainObj is removed during libvirtd 
startup.

Two ways can resolve the above problem:
  1、expending the range of locking virDomainObj and virDomainObjList, lock the 
object of virDomainObj and virDomainObjList in connection-thread before 
checking vm->persistent.
  2、checking vm->persistent again before calling virHashRemoveEntry().

  Do you think it is a problem described above and which way listed above is 
more suitable to resolve the problem, or is there any other better idea? Any 
suggestions?

Best Regards,
-WangYufei



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

[libvirt] [PATCH]daemon: don't free domain if it's null

[Chen Hanxiao]

From: Chen Hanxiao 

If we fail to get domain, we had to judge whether
it's null or not when doing 'cleanup'.

Signed-off-by: Chen Hanxiao 
---
 daemon/remote.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index f3de6a0..decaecc 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4643,7 +4643,8 @@ lxcDispatchDomainOpenNamespace(virNetServerPtr server 
ATTRIBUTE_UNUSED,
 cleanup:
 if (rv < 0)
 virNetMessageSaveError(rerr);
-virDomainFree(dom);
+if (dom)
+virDomainFree(dom);
 return rv;
 }
 
-- 
1.8.2.1

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

Re: [libvirt] [PATCH] remote:Fix the parameter passed to remoteDispatchConnectDomainEventDeregisterAny() should be eventID

[Wangyufei (A)]

Thanks for your reply. Rewiting is fine.

> -Original Message-
> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Thursday, October 17, 2013 10:28 PM
> To: Wangyufei (A); libvir-list@redhat.com
> Cc: zhouyimin Zhou(Yimin); mpriv...@redhat.com; Wangrui (K);
> jdene...@redhat.com
> Subject: Re: [libvirt] [PATCH] remote:Fix the parameter passed to
> remoteDispatchConnectDomainEventDeregisterAny() should be eventID
> 
> On 10/17/2013 03:37 AM, Wangyufei (A) wrote:
> >>From 0832ab83685e20580c8128f5505096e71e747b8a Mon Sep 17
> 00:00:00 2001
> > From: zhouyimin 
> > Date: Thu, 17 Oct 2013 15:59:21 +0800
> > Subject: [PATCH] remote:Fix the parameter passed to
> > remoteDispatchConnectDomainEventDeregisterAny() should be eventID
> 
> Subject line is too long - 'git shortlog -30' will give you a hint of typical
> subjects, which we try to keep at 60 chars or less.  Better would be:
> 
> remote: fix regression in event deregistration
> 
> >
> > Introduced by 7b87a3
> 
> Ouch - present since the 0.9.9 release.
> 
> > When I quit the process which only register
> VIR_DOMAIN_EVENT_ID_REBOOT, I got error like:
> > "libvirt: XML-RPC error : internal error: domain event 0 not registered".
> > Then I add the following code, it fixed.
> >
> > Signed-off-by: zhouyimin 
> 
> Thanks for the patch.  However, we prefer to have the Signed-off-by use a
> legal name, rather than a login alias.  Given your cc: line, is it okay if I
> rewrite this patch to use the following authorship:
> 
> From: Zhou Yimin 
> 
> Or would you prefer yet another legal spelling?  UTF-8 is fine, if you'd like
> to represent your name in native characters; some people even choose to
> represent their name natively followed by a Latin form in ().
> 
> Once we've sorted that out, I can go ahead and push this.
> 
> > ---
> >  src/remote/remote_driver.c |2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> > index 87ef5a9..115d0bc 100644
> > --- a/src/remote/remote_driver.c
> > +++ b/src/remote/remote_driver.c
> > @@ -5137,7 +5137,7 @@ static int
> remoteConnectDomainEventDeregisterAny(virConnectPtr conn,
> >  /* If that was the last callback for this eventID, we need to disable
> >   * events on the server */
> >  if (count == 0) {
> > -args.eventID = callbackID;
> > +args.eventID = eventID;
> >
> >  if (call(conn, priv, 0,
> REMOTE_PROC_CONNECT_DOMAIN_EVENT_DEREGISTER_ANY,
> >   (xdrproc_t)
> > xdr_remote_connect_domain_event_deregister_any_args, (char *) &args,
> >
> 
> --
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org


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

Re: [libvirt] [PATCH]virsh: improve usability of '--print-xml' flag for attach-disk command

[Chen Hanxiao]

> -Original Message-
> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Friday, October 18, 2013 5:45 AM
> To: Chen Hanxiao; libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH]virsh: improve usability of '--print-xml' flag 
> for
> attach-disk command
> 
> Just code motion - makes sense to delay probing for the domain until you
> know you need it.
> 
Wow

> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index 2aed9f9..565966d 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -528,13 +528,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd
> *cmd)
> >  if (live)
> >  flags |= VIR_DOMAIN_AFFECT_LIVE;
> >
> > -if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> > -return false;
> > -
> > -if (persistent &&
> > -virDomainIsActive(dom) == 1)
> > -flags |= VIR_DOMAIN_AFFECT_LIVE;
> > -
> >  if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 ||
> >  vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 ||
> >  vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 ||
> > @@ -672,6 +665,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd
> *cmd)
> >  goto cleanup;
> 
> Ouch - you can now go to cleanup while dom is still NULL.
> 
> >  }
> >
> > +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> > +return false;
> 
> and this is wrong - if you get here, you've allocated resources that
> need the cleanup label.
> 
> > +
> > +if (persistent &&
> > +virDomainIsActive(dom) == 1)
> > +flags |= VIR_DOMAIN_AFFECT_LIVE;
> > +
> >  if (flags)
> >  ret = virDomainAttachDeviceFlags(dom, xml, flags);
> >  else
> >
> 
> ACK with this squashed in, and pushed.

Thanks for your kindly help.

> 
> diff --git i/tools/virsh-domain.c w/tools/virsh-domain.c
> index 45c4823..b75f331 100644
> --- i/tools/virsh-domain.c
> +++ w/tools/virsh-domain.c
> @@ -666,7 +666,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>  }
> 
>  if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> -return false;
> +goto cleanup;
> 
>  if (persistent &&
>  virDomainIsActive(dom) == 1)
> @@ -686,7 +686,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
> 
>   cleanup:
>  VIR_FREE(xml);
> -virDomainFree(dom);
> +if (dom)
> +virDomainFree(dom);
>  virBufferFreeAndReset(&buf);
>  return functionReturn;
>  }
> 
> 
> 
> --
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org




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

Re: [libvirt] [PATCH] remote:Fix the parameter passed to remoteDispatchConnectDomainEventDeregisterAny() should be eventID

[Wangyufei (A)]

Because there're so many patches a day, sometimes you may not focus on your 
interests. So I Cc to the guys who are familiar with the patch issue to get 
their focus(not random). If you feel not good about it, I will try another way 
to get your attention. That will be fine. Thanks for your suggestion. 

> -Original Message-
> From: libvir-list-boun...@redhat.com
> [mailto:libvir-list-boun...@redhat.com] On Behalf Of Jiri Denemark
> Sent: Friday, October 18, 2013 12:51 AM
> To: Wangyufei (A)
> Cc: libvir-list@redhat.com; zhouyimin Zhou(Yimin); Wangrui (K)
> Subject: Re: [libvirt] [PATCH] remote:Fix the parameter passed to
> remoteDispatchConnectDomainEventDeregisterAny() should be eventID
> 
> In addition to comments from Eric, please stop adding random libvirt
> developers to Cc: unless there's a real reason for that (which is quite
> rare). We are all subscribed to this list.
> 
> Jirka
> 
> --
> 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]virsh: improve usability of '--print-xml' flag for attach-disk command

[Eric Blake]

On 10/16/2013 10:05 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> '--print-xml' option is very useful for doing some test.
> But we had to specify a real domain for it.
> This patch could enable us to specify a fake domain
> when using --print-xml option.
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  tools/virsh-domain.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Just code motion - makes sense to delay probing for the domain until you
know you need it.

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 2aed9f9..565966d 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -528,13 +528,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>  if (live)
>  flags |= VIR_DOMAIN_AFFECT_LIVE;
>  
> -if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> -return false;
> -
> -if (persistent &&
> -virDomainIsActive(dom) == 1)
> -flags |= VIR_DOMAIN_AFFECT_LIVE;
> -
>  if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 ||
>  vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 ||
>  vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 ||
> @@ -672,6 +665,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>  goto cleanup;

Ouch - you can now go to cleanup while dom is still NULL.

>  }
>  
> +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> +return false;

and this is wrong - if you get here, you've allocated resources that
need the cleanup label.

> +
> +if (persistent &&
> +virDomainIsActive(dom) == 1)
> +flags |= VIR_DOMAIN_AFFECT_LIVE;
> +
>  if (flags)
>  ret = virDomainAttachDeviceFlags(dom, xml, flags);
>  else
> 

ACK with this squashed in, and pushed.

diff --git i/tools/virsh-domain.c w/tools/virsh-domain.c
index 45c4823..b75f331 100644
--- i/tools/virsh-domain.c
+++ w/tools/virsh-domain.c
@@ -666,7 +666,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
 }

 if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
-return false;
+goto cleanup;

 if (persistent &&
 virDomainIsActive(dom) == 1)
@@ -686,7 +686,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)

  cleanup:
 VIR_FREE(xml);
-virDomainFree(dom);
+if (dom)
+virDomainFree(dom);
 virBufferFreeAndReset(&buf);
 return functionReturn;
 }



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



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

Re: [libvirt] [PATCH 0/3] esx: Remove unnecessary NULL comparisons

[Geoff Hickey]

On Thu, Oct 17, 2013 at 1:32 PM, Eric Blake  wrote:

> > Geoff Hickey (3):
> >   esx: Remove unnecessary NULL comparisons (1/3)
> >   esx: Remove unnecessary NULL comparisons (2/3)
> >   esx: Remove unnecessary NULL comparisons (3/3)
>
> I didn't read every line of the diff, but it looked fairly mechanical
> and the places where I did spot check were correct.  ACK and pushed.


Thanks!

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

Re: [libvirt] [PATCH 0/3] esx: Remove unnecessary NULL comparisons

[Eric Blake]

On 10/17/2013 11:04 AM, Geoff Hickey wrote:
> In reply to my last submit, Eric Blake suggested removing an explicit NULL
> comparison, and instead to simply use the pointer in a boolean context, as
> in: if (ptr) instead of if (ptr != NULL). Since the second form was used
> thoughout the esx code, making this change in one place wouldn't have
> advanced the cause of consistency in the code. This series of patches
> makes this change throughout the esx code. There are no logic changes. The
> result is (arguably) easier to read.
> 
> Geoff Hickey (3):
>   esx: Remove unnecessary NULL comparisons (1/3)
>   esx: Remove unnecessary NULL comparisons (2/3)
>   esx: Remove unnecessary NULL comparisons (3/3)

I didn't read every line of the diff, but it looked fairly mechanical
and the places where I did spot check were correct.  ACK and pushed.

> 
>  src/esx/esx_driver.c| 244 +-
>  src/esx/esx_interface_driver.c  |  10 +-
>  src/esx/esx_network_driver.c|  64 ++---
>  src/esx/esx_storage_backend_iscsi.c |  44 ++--
>  src/esx/esx_storage_backend_vmfs.c  |  86 +++
>  src/esx/esx_storage_driver.c|   6 +-
>  src/esx/esx_util.c  |  48 ++--
>  src/esx/esx_vi.c| 475 
> ++--
>  src/esx/esx_vi_methods.c|  10 +-
>  src/esx/esx_vi_types.c  |  88 +++
>  10 files changed, 535 insertions(+), 540 deletions(-)
> 

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



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

[libvirt] [PATCH 3/3] esx: Remove unnecessary NULL comparisons (3/3)

[Geoff Hickey]

Code cleanup: remove explicit NULL comparisons like ptr == NULL and
ptr != NULL from the ESX code, replacing them with the simpler ptr
and !ptr.

Part three of three.
---
 src/esx/esx_storage_backend_iscsi.c | 44 +--
 src/esx/esx_storage_backend_vmfs.c  | 86 ++---
 src/esx/esx_storage_driver.c|  6 +--
 3 files changed, 68 insertions(+), 68 deletions(-)

diff --git a/src/esx/esx_storage_backend_iscsi.c 
b/src/esx/esx_storage_backend_iscsi.c
index 6d80f90..66b91d1 100644
--- a/src/esx/esx_storage_backend_iscsi.c
+++ b/src/esx/esx_storage_backend_iscsi.c
@@ -67,7 +67,7 @@ esxConnectNumOfStoragePools(virConnectPtr conn)
 }
 
 /* FIXME: code looks for software iSCSI adapter only */
-if (hostInternetScsiHba == NULL) {
+if (!hostInternetScsiHba) {
 /* iSCSI adapter may not be enabled for this host */
 return 0;
 }
@@ -80,7 +80,7 @@ esxConnectNumOfStoragePools(virConnectPtr conn)
  * return iSCSI names for all static targets to avoid duplicate names.
  */
 for (target = hostInternetScsiHba->configuredStaticTarget;
- target != NULL; target = target->_next) {
+ target; target = target->_next) {
 ++count;
 }
 
@@ -117,7 +117,7 @@ esxConnectListStoragePools(virConnectPtr conn, char **const 
names,
 }
 
 /* FIXME: code looks for software iSCSI adapter only */
-if (hostInternetScsiHba == NULL) {
+if (!hostInternetScsiHba) {
 /* iSCSI adapter may not be enabled for this host */
 return 0;
 }
@@ -130,7 +130,7 @@ esxConnectListStoragePools(virConnectPtr conn, char **const 
names,
  * return iSCSI names for all static targets to avoid duplicate names.
  */
 for (target = hostInternetScsiHba->configuredStaticTarget;
- target != NULL && count < maxnames; target = target->_next) {
+ target && count < maxnames; target = target->_next) {
 if (VIR_STRDUP(names[count], target->iScsiName) < 0)
 goto cleanup;
 
@@ -173,7 +173,7 @@ esxStoragePoolLookupByName(virConnectPtr conn,
 goto cleanup;
 }
 
-if (target == NULL) {
+if (!target) {
 /* pool not found, error handling done by the base driver */
 goto cleanup;
 }
@@ -214,13 +214,13 @@ esxStoragePoolLookupByUUID(virConnectPtr conn,
 }
 
 /* FIXME: code just looks for software iSCSI adapter */
-if (hostInternetScsiHba == NULL) {
+if (!hostInternetScsiHba) {
 /* iSCSI adapter may not be enabled for this host */
 return NULL;
 }
 
 for (target = hostInternetScsiHba->configuredStaticTarget;
- target != NULL; target = target->_next) {
+ target; target = target->_next) {
 md5_buffer(target->iScsiName, strlen(target->iScsiName), md5);
 
 if (memcmp(uuid, md5, VIR_UUID_STRING_BUFLEN) == 0) {
@@ -228,7 +228,7 @@ esxStoragePoolLookupByUUID(virConnectPtr conn,
 }
 }
 
-if (target == NULL) {
+if (!target) {
 /* pool not found, error handling done by the base driver */
 goto cleanup;
 }
@@ -310,13 +310,13 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned 
int flags)
 }
 
 for (target = hostInternetScsiHba->configuredStaticTarget;
- target != NULL; target = target->_next) {
+ target; target = target->_next) {
 if (STREQ(target->iScsiName, pool->name)) {
 break;
 }
 }
 
-if (target == NULL) {
+if (!target) {
 /* pool not found */
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not find storage pool with name '%s'"),
@@ -339,7 +339,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned 
int flags)
 
 def.source.hosts[0].name = target->address;
 
-if (target->port != NULL) {
+if (target->port) {
 def.source.hosts[0].port = target->port->value;
 }
 
@@ -369,7 +369,7 @@ esxStoragePoolNumOfVolumes(virStoragePoolPtr pool)
 }
 
 for (hostScsiTopologyLun = hostScsiTopologyLunList;
- hostScsiTopologyLun != NULL;
+ hostScsiTopologyLun;
  hostScsiTopologyLun = hostScsiTopologyLun->_next) {
 ++count;
 }
@@ -399,7 +399,7 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char 
**const names,
 goto cleanup;
 }
 
-if (hostScsiTopologyLunList == NULL) {
+if (!hostScsiTopologyLunList) {
 /* iSCSI adapter may not be enabled on ESX host */
 return 0;
 }
@@ -408,10 +408,10 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char 
**const names,
 goto cleanup;
 }
 
-for (scsiLun = scsiLunList; scsiLun != NULL && count < maxnames;
+for (scsiLun = scsiLunList; scsiLun && count < maxnames;
  scsiLun = scsiLun->_next) {
 for (hostScsiTopologyLun = hostScsiTopologyLunList;
- hostScsiTopologyLun != NULL && count < maxnames;
+ hostScsiTopologyLun && count < maxnames;

[libvirt] [PATCH 0/3] esx: Remove unnecessary NULL comparisons

[Geoff Hickey]

In reply to my last submit, Eric Blake suggested removing an explicit NULL
comparison, and instead to simply use the pointer in a boolean context, as
in: if (ptr) instead of if (ptr != NULL). Since the second form was used
thoughout the esx code, making this change in one place wouldn't have
advanced the cause of consistency in the code. This series of patches
makes this change throughout the esx code. There are no logic changes. The
result is (arguably) easier to read.

Geoff Hickey (3):
  esx: Remove unnecessary NULL comparisons (1/3)
  esx: Remove unnecessary NULL comparisons (2/3)
  esx: Remove unnecessary NULL comparisons (3/3)

 src/esx/esx_driver.c| 244 +-
 src/esx/esx_interface_driver.c  |  10 +-
 src/esx/esx_network_driver.c|  64 ++---
 src/esx/esx_storage_backend_iscsi.c |  44 ++--
 src/esx/esx_storage_backend_vmfs.c  |  86 +++
 src/esx/esx_storage_driver.c|   6 +-
 src/esx/esx_util.c  |  48 ++--
 src/esx/esx_vi.c| 475 ++--
 src/esx/esx_vi_methods.c|  10 +-
 src/esx/esx_vi_types.c  |  88 +++
 10 files changed, 535 insertions(+), 540 deletions(-)

-- 
1.8.1.2

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

[libvirt] [PATCH 2/3] esx: Remove unnecessary NULL comparisons (2/3)

[Geoff Hickey]

Code cleanup: remove explicit NULL comparisons like ptr == NULL and
ptr != NULL from the ESX code, replacing them with the simpler ptr
and !ptr.

Part two of three.
---
 src/esx/esx_interface_driver.c | 10 +++
 src/esx/esx_network_driver.c   | 64 +-
 src/esx/esx_util.c | 48 +++
 3 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/src/esx/esx_interface_driver.c b/src/esx/esx_interface_driver.c
index 2cee3b7..dcb9f03 100644
--- a/src/esx/esx_interface_driver.c
+++ b/src/esx/esx_interface_driver.c
@@ -82,7 +82,7 @@ esxConnectNumOfInterfaces(virConnectPtr conn)
 return -1;
 }
 
-for (physicalNic = physicalNicList; physicalNic != NULL;
+for (physicalNic = physicalNicList; physicalNic;
  physicalNic = physicalNic->_next) {
 ++count;
 }
@@ -113,7 +113,7 @@ esxConnectListInterfaces(virConnectPtr conn, char **const 
names, int maxnames)
 return -1;
 }
 
-for (physicalNic = physicalNicList; physicalNic != NULL;
+for (physicalNic = physicalNicList; physicalNic;
  physicalNic = physicalNic->_next) {
 if (VIR_STRDUP(names[count], physicalNic->device) < 0)
 goto cleanup;
@@ -237,15 +237,15 @@ esxInterfaceGetXMLDesc(virInterfacePtr iface, unsigned 
int flags)
 def.startmode = VIR_INTERFACE_START_ONBOOT;
 
 /* FIXME: Add support for IPv6, requires to use vSphere API 4.0 */
-if (physicalNic->spec->ip != NULL) {
+if (physicalNic->spec->ip) {
 protocol.family = (char *)"ipv4";
 
 if (physicalNic->spec->ip->dhcp == esxVI_Boolean_True) {
 protocol.dhcp = 1;
 }
 
-if (physicalNic->spec->ip->ipAddress != NULL &&
-physicalNic->spec->ip->subnetMask != NULL &&
+if (physicalNic->spec->ip->ipAddress &&
+physicalNic->spec->ip->subnetMask &&
 strlen(physicalNic->spec->ip->ipAddress) > 0 &&
 strlen(physicalNic->spec->ip->subnetMask) > 0) {
 hasAddress = true;
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 24059c1..c8b53b1 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -89,7 +89,7 @@ esxConnectNumOfNetworks(virConnectPtr conn)
 return -1;
 }
 
-for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch != NULL;
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
  hostVirtualSwitch = hostVirtualSwitch->_next) {
 ++count;
 }
@@ -121,7 +121,7 @@ esxConnectListNetworks(virConnectPtr conn, char **const 
names, int maxnames)
 return -1;
 }
 
-for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch != NULL;
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
  hostVirtualSwitch = hostVirtualSwitch->_next) {
 if (VIR_STRDUP(names[count], hostVirtualSwitch->name) < 0)
 goto cleanup;
@@ -183,7 +183,7 @@ esxNetworkLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 return NULL;
 }
 
-for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch != NULL;
+for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch;
  hostVirtualSwitch = hostVirtualSwitch->_next) {
 md5_buffer(hostVirtualSwitch->key, strlen(hostVirtualSwitch->key), 
md5);
 
@@ -192,7 +192,7 @@ esxNetworkLookupByUUID(virConnectPtr conn, const unsigned 
char *uuid)
 }
 }
 
-if (hostVirtualSwitch == NULL) {
+if (!hostVirtualSwitch) {
 virUUIDFormat(uuid, uuid_string);
 
 virReportError(VIR_ERR_NO_NETWORK,
@@ -252,12 +252,12 @@ esxBandwidthToShapingPolicy(virNetDevBandwidthPtr 
bandwidth,
 {
 int result = -1;
 
-if (shapingPolicy == NULL || *shapingPolicy != NULL) {
+if (!shapingPolicy || *shapingPolicy) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
 return -1;
 }
 
-if (bandwidth->in == NULL || bandwidth->out == NULL ||
+if (!bandwidth->in || !bandwidth->out ||
 bandwidth->in->average != bandwidth->out->average ||
 bandwidth->in->peak != bandwidth->out->peak ||
 bandwidth->in->burst != bandwidth->out->burst) {
@@ -341,7 +341,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 /* Parse network XML */
 def = virNetworkDefParseString(xml);
 
-if (def == NULL) {
+if (!def) {
 return NULL;
 }
 
@@ -352,7 +352,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 goto cleanup;
 }
 
-if (hostVirtualSwitch != NULL) {
+if (hostVirtualSwitch) {
 /* FIXME */
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("HostVirtualSwitch already exists, editing existing "
@@ -383,7 +383,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
 }
 
 for (i = 0; i < def->nPortGroups; ++i) {
-  

Re: [libvirt] [PATCH] remote:Fix the parameter passed to remoteDispatchConnectDomainEventDeregisterAny() should be eventID

[Jiri Denemark]

In addition to comments from Eric, please stop adding random libvirt
developers to Cc: unless there's a real reason for that (which is quite
rare). We are all subscribed to this list.

Jirka

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

[libvirt] [PATCH 3/4] nodeinfo: Avoid forward declarations of static functions

[Peter Krempa]

linuxNodeGetCPUStats() and linuxNodeGetMemoryStats() are static and
don't need a forward declaration. Forward declaration of
nodeGetCellMemory() can be avoided by moving the function to the right
place.
---
 src/nodeinfo.c | 113 +++--
 1 file changed, 54 insertions(+), 59 deletions(-)

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index d4bb792..8646a50 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -89,15 +89,6 @@ appleFreebsdNodeGetCPUCount(void)
 # define LINUX_NB_MEMORY_STATS_ALL 4
 # define LINUX_NB_MEMORY_STATS_CELL 2

-static int linuxNodeGetCPUStats(FILE *procstat,
-int cpuNum,
-virNodeCPUStatsPtr params,
-int *nparams);
-static int linuxNodeGetMemoryStats(FILE *meminfo,
-   int cellNum,
-   virNodeMemoryStatsPtr params,
-   int *nparams);
-
 /* Return the positive decimal contents of the given
  * DIR/cpu%u/FILE, or -1 on error.  If DEFAULT_VALUE is non-negative
  * and the file could not be found, return that instead of an error;
@@ -586,10 +577,11 @@ cleanup:

 # define TICK_TO_NSEC (1000ull * 1000ull * 1000ull / sysconf(_SC_CLK_TCK))

-int linuxNodeGetCPUStats(FILE *procstat,
- int cpuNum,
- virNodeCPUStatsPtr params,
- int *nparams)
+static int
+linuxNodeGetCPUStats(FILE *procstat,
+ int cpuNum,
+ virNodeCPUStatsPtr params,
+ int *nparams)
 {
 int ret = -1;
 char line[1024];
@@ -689,10 +681,11 @@ cleanup:
 return ret;
 }

-int linuxNodeGetMemoryStats(FILE *meminfo,
-int cellNum,
-virNodeMemoryStatsPtr params,
-int *nparams)
+static int
+linuxNodeGetMemoryStats(FILE *meminfo,
+int cellNum,
+virNodeMemoryStatsPtr params,
+int *nparams)
 {
 int ret = -1;
 size_t i = 0, j = 0, k = 0;
@@ -1534,7 +1527,6 @@ nodeGetFreeMemoryFake(void)
 # define MASK_CPU_ISSET(mask, cpu) \
   (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask & 1)

-static unsigned long long nodeGetCellMemory(int cell);

 static virBitmapPtr
 virNodeGetSiblingsList(const char *dir, int cpu_id)
@@ -1584,6 +1576,50 @@ virNodeCapsFillCPUInfo(int cpu_id, 
virCapsHostNUMACellCPUPtr cpu)
 return 0;
 }

+
+/**
+ * nodeGetCellMemory
+ * @cell: The number of the numa cell to get memory info for.
+ *
+ * Will call the numa_node_size64() function from libnuma to get
+ * the amount of total memory in bytes. It is then converted to
+ * KiB and returned.
+ *
+ * Returns 0 if unavailable, amount of memory in KiB on success.
+ */
+static unsigned long long
+nodeGetCellMemory(int cell)
+{
+long long mem;
+unsigned long long memKiB = 0;
+int maxCell;
+
+/* Make sure the provided cell number is valid. */
+maxCell = numa_max_node();
+if (cell > maxCell) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("cell %d out of range (0-%d)"),
+   cell, maxCell);
+goto cleanup;
+}
+
+/* Get the amount of memory(bytes) in the node */
+mem = numa_node_size64(cell, NULL);
+if (mem < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to query NUMA total memory for node: %d"),
+   cell);
+goto cleanup;
+}
+
+/* Convert the memory from bytes to KiB */
+memKiB = mem >> 10;
+
+cleanup:
+return memKiB;
+}
+
+
 int
 nodeCapsInitNUMA(virCapsPtr caps)
 {
@@ -1718,47 +1754,6 @@ nodeGetFreeMemory(void)
 return freeMem;
 }

-/**
- * nodeGetCellMemory
- * @cell: The number of the numa cell to get memory info for.
- *
- * Will call the numa_node_size64() function from libnuma to get
- * the amount of total memory in bytes. It is then converted to
- * KiB and returned.
- *
- * Returns 0 if unavailable, amount of memory in KiB on success.
- */
-static unsigned long long nodeGetCellMemory(int cell)
-{
-long long mem;
-unsigned long long memKiB = 0;
-int maxCell;
-
-/* Make sure the provided cell number is valid. */
-maxCell = numa_max_node();
-if (cell > maxCell) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("cell %d out of range (0-%d)"),
-   cell, maxCell);
-goto cleanup;
-}
-
-/* Get the amount of memory(bytes) in the node */
-mem = numa_node_size64(cell, NULL);
-if (mem < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Failed to query NUMA total memory for node: %d"),
-   cell);
-goto cleanup;
-}
-
-/* Convert the memory from bytes to KiB */
-memKiB = mem >> 10;
-
-cleanup:

[libvirt] [PATCH 4/4] numa: Introduce virNumaGetMaxNode and use it instead of numa_max_node

[Peter Krempa]

Avoid necessary checks for the numa library with this helper.
---
 src/libvirt_private.syms |  1 +
 src/nodeinfo.c   | 37 ++---
 src/util/virnuma.c   | 37 +
 src/util/virnuma.h   |  2 ++
 4 files changed, 62 insertions(+), 15 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 48d3da7..c8959ad 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1520,6 +1520,7 @@ virNodeSuspendGetTargetMask;
 virDomainNumatuneMemModeTypeFromString;
 virDomainNumatuneMemModeTypeToString;
 virNumaGetAutoPlacementAdvice;
+virNumaGetMaxNode;
 virNumaIsAvailable;
 virNumaSetupMemoryPolicy;
 virNumaTuneMemPlacementModeTypeFromString;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 8646a50..2a78778 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -966,25 +966,21 @@ int nodeGetMemoryStats(int cellNum ATTRIBUTE_UNUSED,
 int ret;
 char *meminfo_path = NULL;
 FILE *meminfo;
+int max_node;

 if (cellNum == VIR_NODE_MEMORY_STATS_ALL_CELLS) {
 if (VIR_STRDUP(meminfo_path, MEMINFO_PATH) < 0)
 return -1;
 } else {
-if (!virNumaIsAvailable()) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("NUMA not supported on this host"));
+if ((max_node = virNumaGetMaxNode()) < 0)
 return -1;
-}

-# if WITH_NUMACTL
-if (cellNum > numa_max_node()) {
+if (cellNum > max_node) {
 virReportInvalidArg(cellNum,
 _("cellNum in %s must be less than or 
equal to %d"),
-__FUNCTION__, numa_max_node());
+__FUNCTION__, max_node);
 return -1;
 }
-# endif

 if (virAsprintf(&meminfo_path, "%s/node/node%d/meminfo",
 SYSFS_SYSTEM_PATH, cellNum) < 0)
@@ -1595,7 +1591,9 @@ nodeGetCellMemory(int cell)
 int maxCell;

 /* Make sure the provided cell number is valid. */
-maxCell = numa_max_node();
+if ((maxCell = virNumaGetMaxNode()) < 0)
+return 0;
+
 if (cell > maxCell) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("cell %d out of range (0-%d)"),
@@ -1630,31 +1628,35 @@ nodeCapsInitNUMA(virCapsPtr caps)
 virCapsHostNUMACellCPUPtr cpus = NULL;
 int ret = -1;
 int max_n_cpus = NUMA_MAX_N_CPUS;
+int mask_n_bytes = max_n_cpus / 8;
 int ncpus = 0;
 bool topology_failed = false;
+int max_node;

 if (!virNumaIsAvailable())
 return nodeCapsInitNUMAFake(caps);

-int mask_n_bytes = max_n_cpus / 8;
+if ((max_node = virNumaGetMaxNode()) < 0)
+goto cleanup;
+
 if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof(*mask)) < 0)
 goto cleanup;
 if (VIR_ALLOC_N(allonesmask, mask_n_bytes / sizeof(*mask)) < 0)
 goto cleanup;
 memset(allonesmask, 0xff, mask_n_bytes);

-for (n = 0; n <= numa_max_node(); n++) {
+for (n = 0; n <= max_node; n++) {
 size_t i;
 /* The first time this returns -1, ENOENT if node doesn't exist... */
 if (numa_node_to_cpus(n, mask, mask_n_bytes) < 0) {
 VIR_WARN("NUMA topology for cell %d of %d not available, ignoring",
- n, numa_max_node()+1);
+ n, max_node + 1);
 continue;
 }
 /* second, third... times it returns an all-1's mask */
 if (memcmp(mask, allonesmask, mask_n_bytes) == 0) {
 VIR_DEBUG("NUMA topology for cell %d of %d is all ones, ignoring",
-  n, numa_max_node()+1);
+  n, max_node + 1);
 continue;
 }

@@ -1709,7 +1711,9 @@ nodeGetCellsFreeMemory(unsigned long long *freeMems,
 return nodeGetCellsFreeMemoryFake(freeMems,
   startCell, maxCells);

-maxCell = numa_max_node();
+if ((maxCell = virNumaGetMaxNode()) < 0)
+return 0;
+
 if (startCell > maxCell) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("start cell %d out of range (0-%d)"),
@@ -1737,13 +1741,16 @@ unsigned long long
 nodeGetFreeMemory(void)
 {
 unsigned long long freeMem = 0;
+int max_node;
 int n;

 if (!virNumaIsAvailable())
 return nodeGetFreeMemoryFake();

+if ((max_node = virNumaGetMaxNode()) < 0)
+return 0;

-for (n = 0; n <= numa_max_node(); n++) {
+for (n = 0; n <= max_node; n++) {
 long long mem;
 if (numa_node_size64(n, &mem) < 0)
 continue;
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 11078a1..2d8d0e9 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -174,6 +174,34 @@ virNumaIsAvailable(void)
 {
 return numa_available() != -1;
 }
+
+
+/**
+ * virNuma

[libvirt] [PATCH 2/4] numa: Introduce virNumaIsAvailable and use it instead of numa_available

[Peter Krempa]

All functions from libnuma must be protected with ifdefs. Avoid this by
using our own wrapper.
---
 src/libvirt_private.syms |  1 +
 src/nodeinfo.c   | 13 +
 src/util/virnuma.c   | 14 ++
 src/util/virnuma.h   |  2 ++
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 84c1c28..48d3da7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1520,6 +1520,7 @@ virNodeSuspendGetTargetMask;
 virDomainNumatuneMemModeTypeFromString;
 virDomainNumatuneMemModeTypeToString;
 virNumaGetAutoPlacementAdvice;
+virNumaIsAvailable;
 virNumaSetupMemoryPolicy;
 virNumaTuneMemPlacementModeTypeFromString;
 virNumaTuneMemPlacementModeTypeToString;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 232b465..d4bb792 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -55,6 +55,7 @@
 #include "virfile.h"
 #include "virtypedparam.h"
 #include "virstring.h"
+#include "virnuma.h"

 #define VIR_FROM_THIS VIR_FROM_NONE

@@ -977,15 +978,11 @@ int nodeGetMemoryStats(int cellNum ATTRIBUTE_UNUSED,
 if (VIR_STRDUP(meminfo_path, MEMINFO_PATH) < 0)
 return -1;
 } else {
-# if WITH_NUMACTL
-if (numa_available() < 0) {
-# endif
+if (!virNumaIsAvailable()) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("NUMA not supported on this host"));
 return -1;
-# if WITH_NUMACTL
 }
-# endif

 # if WITH_NUMACTL
 if (cellNum > numa_max_node()) {
@@ -1600,7 +1597,7 @@ nodeCapsInitNUMA(virCapsPtr caps)
 int ncpus = 0;
 bool topology_failed = false;

-if (numa_available() < 0)
+if (!virNumaIsAvailable())
 return nodeCapsInitNUMAFake(caps);

 int mask_n_bytes = max_n_cpus / 8;
@@ -1672,7 +1669,7 @@ nodeGetCellsFreeMemory(unsigned long long *freeMems,
 int ret = -1;
 int maxCell;

-if (numa_available() < 0)
+if (!virNumaIsAvailable())
 return nodeGetCellsFreeMemoryFake(freeMems,
   startCell, maxCells);

@@ -1706,7 +1703,7 @@ nodeGetFreeMemory(void)
 unsigned long long freeMem = 0;
 int n;

-if (numa_available() < 0)
+if (!virNumaIsAvailable())
 return nodeGetFreeMemoryFake();


diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index ecf7ede..11078a1 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -167,6 +167,13 @@ virNumaSetupMemoryPolicy(virNumaTuneDef numatune,
 cleanup:
 return ret;
 }
+
+
+bool
+virNumaIsAvailable(void)
+{
+return numa_available() != -1;
+}
 #else
 int
 virNumaSetupMemoryPolicy(virNumaTuneDef numatune,
@@ -181,4 +188,11 @@ virNumaSetupMemoryPolicy(virNumaTuneDef numatune,

 return 0;
 }
+
+
+bool
+virNumaIsAvailable(void)
+{
+return false;
+}
 #endif
diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index 9ff8e69..5f9c38b 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -55,4 +55,6 @@ char *virNumaGetAutoPlacementAdvice(unsigned short vcups,

 int virNumaSetupMemoryPolicy(virNumaTuneDef numatune,
  virBitmapPtr nodemask);
+
+bool virNumaIsAvailable(void);
 #endif /* __VIR_NUMA_H__ */
-- 
1.8.3.2

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

[libvirt] [PATCH 1/4] nodeinfo: Rename linuxNodeInfoCPUPopulate and export it properly

[Peter Krempa]

Call it virNodeInfoLinuxPopulateCPU and use the header file to export
it instead of extern definition in the test file.
---
 src/libvirt_linux.syms |  2 +-
 src/nodeinfo.c | 14 +-
 src/nodeinfo.h |  5 +
 tests/nodeinfotest.c   |  6 +-
 4 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms
index 3500898..bfef3ec 100644
--- a/src/libvirt_linux.syms
+++ b/src/libvirt_linux.syms
@@ -3,7 +3,7 @@
 #

 # nodeinfo.h
-linuxNodeInfoCPUPopulate;
+virNodeInfoLinuxPopulateCPU;

 # util/virstatslinux.h
 linuxDomainInterfaceStats;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 70814c2..232b465 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -88,11 +88,6 @@ appleFreebsdNodeGetCPUCount(void)
 # define LINUX_NB_MEMORY_STATS_ALL 4
 # define LINUX_NB_MEMORY_STATS_CELL 2

-/* NB, this is not static as we need to call it from the testsuite */
-int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- const char *sysfs_dir,
- virNodeInfoPtr nodeinfo);
-
 static int linuxNodeGetCPUStats(FILE *procstat,
 int cpuNum,
 virNodeCPUStatsPtr params,
@@ -376,9 +371,10 @@ cleanup:
 return ret;
 }

-int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
- const char *sysfs_dir,
- virNodeInfoPtr nodeinfo)
+int
+virNodeInfoLinuxPopulateCPU(FILE *cpuinfo,
+const char *sysfs_dir,
+virNodeInfoPtr nodeinfo)
 {
 char line[1024];
 DIR *nodedir = NULL;
@@ -872,7 +868,7 @@ int nodeGetInfo(virNodeInfoPtr nodeinfo)
 return -1;
 }

-ret = linuxNodeInfoCPUPopulate(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo);
+ret = virNodeInfoLinuxPopulateCPU(cpuinfo, SYSFS_SYSTEM_PATH, nodeinfo);
 if (ret < 0)
 goto cleanup;

diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 413fddd..a13cf28 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -57,4 +57,9 @@ int nodeGetCPUMap(unsigned char **cpumap,
   unsigned int *online,
   unsigned int flags);

+
+int virNodeInfoLinuxPopulateCPU(FILE *cpuinfo,
+const char *sysfs_dir,
+virNodeInfoPtr nodeinfo);
+
 #endif /* __VIR_NODEINFO_H__*/
diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c
index 74f6d4d..9bb7adb 100644
--- a/tests/nodeinfotest.c
+++ b/tests/nodeinfotest.c
@@ -27,10 +27,6 @@ main(void)

 #else

-extern int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
-char *sysfs_dir,
-virNodeInfoPtr nodeinfo);
-
 static int
 linuxTestCompareFiles(const char *cpuinfofile,
   char *sysfs_dir,
@@ -50,7 +46,7 @@ linuxTestCompareFiles(const char *cpuinfofile,
 goto fail;

 memset(&nodeinfo, 0, sizeof(nodeinfo));
-if (linuxNodeInfoCPUPopulate(cpuinfo, sysfs_dir, &nodeinfo) < 0) {
+if (virNodeInfoLinuxPopulateCPU(cpuinfo, sysfs_dir, &nodeinfo) < 0) {
 if (virTestGetDebug()) {
 virErrorPtr error = virSaveLastError();
 if (error && error->code != VIR_ERR_OK)
-- 
1.8.3.2

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

[libvirt] [PATCH 0/4] Start cleaning up nodeinfo.c

[Peter Krempa]

Some parts of nodeinfo.c are a mess. This series is a start of cleanup
effort on that file that should result also into tests being written
to test the nodeinfo code as it was shown to be fragile.

To avoid sending a huge series I will send it continuously as I progress.

Peter Krempa (4):
  nodeinfo: Rename linuxNodeInfoCPUPopulate and export it properly
  numa: Introduce virNumaIsAvailable and use it instead of
numa_available
  nodeinfo: Avoid forward declarations of static functions
  numa: Introduce virNumaGetMaxNode and use it instead of numa_max_node

 src/libvirt_linux.syms   |   2 +-
 src/libvirt_private.syms |   2 +
 src/nodeinfo.c   | 173 +++
 src/nodeinfo.h   |   5 ++
 src/util/virnuma.c   |  51 ++
 src/util/virnuma.h   |   4 ++
 tests/nodeinfotest.c |   6 +-
 7 files changed, 148 insertions(+), 95 deletions(-)

-- 
1.8.3.2

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

Re: [libvirt] [PATCH] Remove ATTRIBUTE_NONNULL(3) from qemuMonitorJSONDrivePivot

[Eric Blake]

On 10/17/2013 09:10 AM, John Ferlan wrote:
> The header definition didn't match the function declaration, so adjusted
> header to reflect the definition.
> 
> Found during a Coverity build where STATIC_ANALYSIS is enabled resulting
> in the internal.h adding __nonnull__ handling to arguments.
> 
> Commit '6d264c91' added support for the qemuMonitorJSONDrivePivot() and
> commit 'fbc3adc9' added a corresponding test which ended up triggering
> the build failure which I didn't notice until today!
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_monitor_json.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index 51cf19c..a1a7548 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -250,7 +250,7 @@ int qemuMonitorJSONDrivePivot(qemuMonitorPtr mon,
>const char *device,
>const char *file,
>const char *format)
> -ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

Hmm.  The 'file' argument is marked ATTRIBUTE_UNUSED upstream, but
actually used downstream in RHEL 6 where it targets a different
downstream-only QMP command.  But as we don't need to cater to
downstream-only usage, I'm fine with this.

ACK.

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



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

Re: [libvirt] [PATCH] Fix typo s/SASL_CONF_DIR/SASL_CONF_PATH/ in QEMU VNC code

[Christophe Fergeau]

On Thu, Oct 17, 2013 at 04:05:55PM +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" 
> 
> The QEMU VNC client arg code has a long standing typo
> of SASL_CONF_DIR when it should be SASL_CONFIG_PATH for
> the env variable name.

ACK

Christophe


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

[libvirt] [PATCH] Remove ATTRIBUTE_NONNULL(3) from qemuMonitorJSONDrivePivot

[John Ferlan]

The header definition didn't match the function declaration, so adjusted
header to reflect the definition.

Found during a Coverity build where STATIC_ANALYSIS is enabled resulting
in the internal.h adding __nonnull__ handling to arguments.

Commit '6d264c91' added support for the qemuMonitorJSONDrivePivot() and
commit 'fbc3adc9' added a corresponding test which ended up triggering
the build failure which I didn't notice until today!

Signed-off-by: John Ferlan 
---
 src/qemu/qemu_monitor_json.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 51cf19c..a1a7548 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -250,7 +250,7 @@ int qemuMonitorJSONDrivePivot(qemuMonitorPtr mon,
   const char *device,
   const char *file,
   const char *format)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
 int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
const char *device,
-- 
1.8.3.1

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

[libvirt] [PATCH] Fix typo s/SASL_CONF_DIR/SASL_CONF_PATH/ in QEMU VNC code

[Daniel P. Berrange]

From: "Daniel P. Berrange" 

The QEMU VNC client arg code has a long standing typo
of SASL_CONF_DIR when it should be SASL_CONFIG_PATH for
the env variable name.

Signed-off-by: Daniel P. Berrange 
---
 src/qemu/qemu_command.c| 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args | 2 +-
 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 814f368..5e6b0c0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7125,7 +7125,7 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr 
cfg,
 virBufferAddLit(&opt, ",sasl");
 
 if (cfg->vncSASLdir)
-virCommandAddEnvPair(cmd, "SASL_CONF_DIR", cfg->vncSASLdir);
+virCommandAddEnvPair(cmd, "SASL_CONF_PATH", cfg->vncSASLdir);
 
 /* TODO: Support ACLs later */
 }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args 
b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args
index 67ef88f..239fde1 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.args
@@ -1,5 +1,5 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
-SASL_CONF_DIR=/root/.sasl2 QEMU_AUDIO_DRV=none \
+SASL_CONF_PATH=/root/.sasl2 QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 214 \
 -smp 1 -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb 
-hda \
 /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -vnc \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args 
b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args
index d71a998..c681b1b 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.args
@@ -1,5 +1,5 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
-SASL_CONF_DIR=/root/.sasl2 QEMU_AUDIO_DRV=none \
+SASL_CONF_PATH=/root/.sasl2 QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 214 \
 -smp 1 -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb 
-hda \
 /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -vnc \
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] remote:Fix the parameter passed to remoteDispatchConnectDomainEventDeregisterAny() should be eventID

[Eric Blake]

On 10/17/2013 03:37 AM, Wangyufei (A) wrote:
>>From 0832ab83685e20580c8128f5505096e71e747b8a Mon Sep 17 00:00:00 2001
> From: zhouyimin 
> Date: Thu, 17 Oct 2013 15:59:21 +0800
> Subject: [PATCH] remote:Fix the parameter passed to 
> remoteDispatchConnectDomainEventDeregisterAny() should be eventID

Subject line is too long - 'git shortlog -30' will give you a hint of
typical subjects, which we try to keep at 60 chars or less.  Better
would be:

remote: fix regression in event deregistration

> 
> Introduced by 7b87a3

Ouch - present since the 0.9.9 release.

> When I quit the process which only register VIR_DOMAIN_EVENT_ID_REBOOT, I got 
> error like:
> "libvirt: XML-RPC error : internal error: domain event 0 not registered".
> Then I add the following code, it fixed.
> 
> Signed-off-by: zhouyimin 

Thanks for the patch.  However, we prefer to have the Signed-off-by use
a legal name, rather than a login alias.  Given your cc: line, is it
okay if I rewrite this patch to use the following authorship:

From: Zhou Yimin 

Or would you prefer yet another legal spelling?  UTF-8 is fine, if you'd
like to represent your name in native characters; some people even
choose to represent their name natively followed by a Latin form in ().

Once we've sorted that out, I can go ahead and push this.

> ---
>  src/remote/remote_driver.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 87ef5a9..115d0bc 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -5137,7 +5137,7 @@ static int 
> remoteConnectDomainEventDeregisterAny(virConnectPtr conn,
>  /* If that was the last callback for this eventID, we need to disable
>   * events on the server */
>  if (count == 0) {
> -args.eventID = callbackID;
> +args.eventID = eventID;
>  
>  if (call(conn, priv, 0, 
> REMOTE_PROC_CONNECT_DOMAIN_EVENT_DEREGISTER_ANY,
>   (xdrproc_t) 
> xdr_remote_connect_domain_event_deregister_any_args, (char *) &args,
> 

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



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

Re: [libvirt] [PATCH] Add support for SPICE SASL

[Christophe Fergeau]

On Thu, Oct 17, 2013 at 02:46:46PM +0100, Daniel P. Berrange wrote:
> On Thu, Oct 17, 2013 at 03:38:04PM +0200, Christophe Fergeau wrote:
> > This adds a spice_sasl boolean option to qemu.conf to mimic what is
> > donc for VNC SASL support.
> 
> This is a dup of what I've already posted, but not yet pushed
> 
> https://www.redhat.com/archives/libvir-list/2013-September/msg00319.html
> 
> Now I've remembered, I'll push it :-)

Ah indeed, ACK to your patch fwiw. Feel free to squash the additions I made
to tests/ in your patch.

> 
> > I did not add a spice_sasl_dir option as the corresponding
> > vnc_sasl_dir option sets a SASL_CONF_DIR environment variable, but
> > I could not find any reference to that variable in either QEMU or
> > cyrus-sasl, and Google was not helpful either.
> 
> It is in the 'sasl_getconfpath_t' man page
> 
>   DESCRIPTION
>sasl_getconfpath_t is used if the application wishes to use a  
> different
>location  for the SASL configuration files. If this callback is not 
> used
>SASL  will  either  use  the  location  in  the   environment   
> variable
>SASL_CONF_PATH  (provided  we  are  not  SUID  or SGID) or /etc/sasl2 
> by
>default.
> 
> except we typoed  s/PATH/DIR/ so I'll fix that :-)

Ah, I indeed have more hits with that name! :)

Christophe


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

Re: [libvirt] [PATCH] netcf: Don't complain when cleanup is called before init

[Daniel P. Berrange]

On Thu, Oct 17, 2013 at 03:57:19PM +0200, Christophe Fergeau wrote:
> netcfStateInitialize() initializes the driverState variable,
> and when netcfStateCleanup is called, it will call virReportError()
> if driverState is NULL.
> This is not consistent with what other state objects are doing,
> they return -1 without reporting an error in such cases.
> 
> See also
> https://www.redhat.com/archives/libvir-list/2013-October/msg00809.html:
> 
> On Thu, Oct 17, 2013 at 01:40:19PM +0100, Daniel P. Berrange wrote:
> > We don't want virStateCleanup to skip execution if virStateInitialize
> > has failed though - every callback in virStateCleanup should be written
> > to be safe if its corresponding init function hasn't run.
> ---
>  src/interface/interface_backend_netcf.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/src/interface/interface_backend_netcf.c 
> b/src/interface/interface_backend_netcf.c
> index fac059d..c4e18c4 100644
> --- a/src/interface/interface_backend_netcf.c
> +++ b/src/interface/interface_backend_netcf.c
> @@ -100,11 +100,8 @@ netcfStateInitialize(bool privileged ATTRIBUTE_UNUSED,
>  static int
>  netcfStateCleanup(void)
>  {
> -if (!driverState) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("Attempt to close netcf state driver already 
> closed"));
> +if (!driverState)
>  return -1;
> -}
>  
>  if (virObjectUnref(driverState)) {
>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

ACK


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

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

[libvirt] [PATCH] netcf: Don't complain when cleanup is called before init

[Christophe Fergeau]

netcfStateInitialize() initializes the driverState variable,
and when netcfStateCleanup is called, it will call virReportError()
if driverState is NULL.
This is not consistent with what other state objects are doing,
they return -1 without reporting an error in such cases.

See also
https://www.redhat.com/archives/libvir-list/2013-October/msg00809.html:

On Thu, Oct 17, 2013 at 01:40:19PM +0100, Daniel P. Berrange wrote:
> We don't want virStateCleanup to skip execution if virStateInitialize
> has failed though - every callback in virStateCleanup should be written
> to be safe if its corresponding init function hasn't run.
---
 src/interface/interface_backend_netcf.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/interface/interface_backend_netcf.c 
b/src/interface/interface_backend_netcf.c
index fac059d..c4e18c4 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -100,11 +100,8 @@ netcfStateInitialize(bool privileged ATTRIBUTE_UNUSED,
 static int
 netcfStateCleanup(void)
 {
-if (!driverState) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("Attempt to close netcf state driver already 
closed"));
+if (!driverState)
 return -1;
-}
 
 if (virObjectUnref(driverState)) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-- 
1.8.3.1

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

Re: [libvirt] [PATCH] Add support for SPICE SASL

[Daniel P. Berrange]

On Thu, Oct 17, 2013 at 03:38:04PM +0200, Christophe Fergeau wrote:
> This adds a spice_sasl boolean option to qemu.conf to mimic what is
> donc for VNC SASL support.

This is a dup of what I've already posted, but not yet pushed

https://www.redhat.com/archives/libvir-list/2013-September/msg00319.html

Now I've remembered, I'll push it :-)

> I did not add a spice_sasl_dir option as the corresponding
> vnc_sasl_dir option sets a SASL_CONF_DIR environment variable, but
> I could not find any reference to that variable in either QEMU or
> cyrus-sasl, and Google was not helpful either.

It is in the 'sasl_getconfpath_t' man page

  DESCRIPTION
   sasl_getconfpath_t is used if the application wishes to use a  different
   location  for the SASL configuration files. If this callback is not used
   SASL  will  either  use  the  location  in  the   environment   variable
   SASL_CONF_PATH  (provided  we  are  not  SUID  or SGID) or /etc/sasl2 by
   default.

except we typoed  s/PATH/DIR/ so I'll fix that :-)

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

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

Re: [libvirt] [PATCH] qemuDomainCleanupRemove: s/memmove/VIR_DELETE_ELEMENT_INPLACE/

[Daniel P. Berrange]

On Thu, Oct 17, 2013 at 03:33:52PM +0200, Michal Privoznik wrote:
> The last argument of memmove is the amount of bytes to be moved. The
> amount is in Bytes. We are moving some void pointers around. However,
> since sizeof(void *) is not Byte on any architecture, we've got the
> arithmetic wrong.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/qemu/qemu_domain.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index d054d64..b8aec2d 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2235,12 +2235,9 @@ qemuDomainCleanupRemove(virDomainObjPtr vm,
>  VIR_DEBUG("vm=%s, cb=%p", vm->def->name, cb);
>  
>  for (i = 0; i < priv->ncleanupCallbacks; i++) {
> -if (priv->cleanupCallbacks[i] == cb) {
> -memmove(priv->cleanupCallbacks + i,
> -priv->cleanupCallbacks + i + 1,
> -priv->ncleanupCallbacks - i - 1);
> -priv->ncleanupCallbacks--;
> -}
> +if (priv->cleanupCallbacks[i] == cb)
> +VIR_DELETE_ELEMENT_INPLACE(priv->cleanupCallbacks,
> +   i, priv->ncleanupCallbacks);
>  }
>  
>  VIR_SHRINK_N(priv->cleanupCallbacks,

ACK

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

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

Re: [libvirt] [PATCH] vircgroupmock.c: Avoid crashing if lstat() not found

[Daniel P. Berrange]

On Thu, Oct 17, 2013 at 03:33:36PM +0200, Michal Privoznik wrote:
> In init_syms() we admit that stat() may not exists (in favor of existing
> __lxstat). However, if previously an library checked for existence of
> these two symbols and wisely has chosen the existing one, now, that we've
> mocked the both symbols, we are confusing the library. Moreover, such
> library will get SIGSEGV immediately after wrong decision, since
> reallstat is NULL.

I don't really get what you're trying to say here. If lstat() doesn't
exist in the libraries we're linking to, then our lstat() wrapper should
not be called either. Also, what platform do you see a crash on.

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

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

[libvirt] [PATCH] Add support for SPICE SASL

[Christophe Fergeau]

This adds a spice_sasl boolean option to qemu.conf to mimic what is
donc for VNC SASL support.

I did not add a spice_sasl_dir option as the corresponding
vnc_sasl_dir option sets a SASL_CONF_DIR environment variable, but
I could not find any reference to that variable in either QEMU or
cyrus-sasl, and Google was not helpful either.
---
 src/qemu/libvirtd_qemu.aug |  1 +
 src/qemu/qemu.conf | 11 ++
 src/qemu/qemu_command.c|  2 +
 src/qemu/qemu_conf.c   |  1 +
 src/qemu/qemu_conf.h   |  1 +
 src/qemu/test_libvirtd_qemu.aug.in |  1 +
 .../qemuxml2argv-graphics-spice-sasl.args  | 12 ++
 .../qemuxml2argv-graphics-spice-sasl.xml   | 45 ++
 tests/qemuxml2argvtest.c   |  6 +++
 9 files changed, 80 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.xml

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 32db983..3dc1b43 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -38,6 +38,7 @@ module Libvirtd_qemu =
  | bool_entry "spice_tls"
  | str_entry  "spice_tls_x509_cert_dir"
  | str_entry "spice_password"
+ | bool_entry "spice_sasl"
 
let nogfx_entry = bool_entry "nographics_allow_host_audio"
 
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index bf57b9c..7b128aa 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -140,6 +140,17 @@
 #spice_password = "XYZ12345"
 
 
+# Enable use of SASL encryption on the SPICE server. This requires
+# a SPICE client which supports the SASL protocol extension.
+# Examples include vinagre, virt-viewer and virt-manager
+# itself.
+#
+# It is necessary to configure /etc/sasl2/qemu.conf to choose
+# the desired SASL plugin (eg, GSSPI for Kerberos)
+#
+#spice_sasl = 1
+
+
 # By default, if no graphical front end is configured, libvirt will disable
 # QEMU audio output since directly talking to alsa/pulseaudio may not work
 # with various security settings. If you know what you're doing, enable
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index abb62e9..ea5cfcb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7340,6 +7340,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr 
cfg,
  * migration algorithm silently. */
 virBufferAddLit(&opt, ",seamless-migration=on");
 }
+if (cfg->spiceSASL)
+virBufferAddLit(&opt, ",sasl");
 
 virCommandAddArg(cmd, "-spice");
 virCommandAddArgBuffer(cmd, &opt);
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 44a2296..532b0ff 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -399,6 +399,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 GET_VALUE_STR("spice_tls_x509_cert_dir", cfg->spiceTLSx509certdir);
 GET_VALUE_STR("spice_listen", cfg->spiceListen);
 GET_VALUE_STR("spice_password", cfg->spicePassword);
+GET_VALUE_BOOL("spice_sasl", cfg->spiceSASL);
 
 
 GET_VALUE_LONG("remote_websocket_port_min", cfg->webSocketPortMin);
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index ea3c691..35a2515 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -110,6 +110,7 @@ struct _virQEMUDriverConfig {
 char *vncSASLdir;
 
 bool spiceTLS;
+bool spiceSASL;
 char *spiceTLSx509certdir;
 char *spiceListen;
 char *spicePassword;
diff --git a/src/qemu/test_libvirtd_qemu.aug.in 
b/src/qemu/test_libvirtd_qemu.aug.in
index 7af3f64..f759db5 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -15,6 +15,7 @@ module Test_libvirtd_qemu =
 { "spice_tls" = "1" }
 { "spice_tls_x509_cert_dir" = "/etc/pki/libvirt-spice" }
 { "spice_password" = "XYZ12345" }
+{ "spice_sasl" = "1" }
 { "nographics_allow_host_audio" = "1" }
 { "remote_display_port_min" = "5900" }
 { "remote_display_port_max" = "65535" }
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args 
b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args
new file mode 100644
index 000..4fe78a5
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-sasl.args
@@ -0,0 +1,12 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice 
\
+/usr/bin/qemu -S -M pc -m 214 -smp 1 -nodefaults -monitor \
+unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
+/dev/HostVG/QEMUGuest1 -spice port=5903,tls-port=5904,addr=127.0.0.1,\
+x509-dir=/etc/pki/libvirt-spice,tls-channel=default,tls-channel=main,\
+plaintext-channel=inputs,\
+image-compression=auto_glz,jpeg-wan-compression=auto,\
+zlib-glz-wan-compression=auto,\
+playback-compression=on,streaming-video=filter,disable

[libvirt] [PATCH] qemuDomainCleanupRemove: s/memmove/VIR_DELETE_ELEMENT_INPLACE/

[Michal Privoznik]

The last argument of memmove is the amount of bytes to be moved. The
amount is in Bytes. We are moving some void pointers around. However,
since sizeof(void *) is not Byte on any architecture, we've got the
arithmetic wrong.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d054d64..b8aec2d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2235,12 +2235,9 @@ qemuDomainCleanupRemove(virDomainObjPtr vm,
 VIR_DEBUG("vm=%s, cb=%p", vm->def->name, cb);
 
 for (i = 0; i < priv->ncleanupCallbacks; i++) {
-if (priv->cleanupCallbacks[i] == cb) {
-memmove(priv->cleanupCallbacks + i,
-priv->cleanupCallbacks + i + 1,
-priv->ncleanupCallbacks - i - 1);
-priv->ncleanupCallbacks--;
-}
+if (priv->cleanupCallbacks[i] == cb)
+VIR_DELETE_ELEMENT_INPLACE(priv->cleanupCallbacks,
+   i, priv->ncleanupCallbacks);
 }
 
 VIR_SHRINK_N(priv->cleanupCallbacks,
-- 
1.8.1.5

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

[libvirt] [PATCH] vircgroupmock.c: Avoid crashing if lstat() not found

[Michal Privoznik]

In init_syms() we admit that stat() may not exists (in favor of existing
__lxstat). However, if previously an library checked for existence of
these two symbols and wisely has chosen the existing one, now, that we've
mocked the both symbols, we are confusing the library. Moreover, such
library will get SIGSEGV immediately after wrong decision, since
reallstat is NULL.

Signed-off-by: Michal Privoznik 
---
 tests/vircgroupmock.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
index adc1718..0b44fda 100644
--- a/tests/vircgroupmock.c
+++ b/tests/vircgroupmock.c
@@ -542,10 +542,16 @@ int lstat(const char *path, struct stat *sb)
 errno = ENOMEM;
 return -1;
 }
-ret = reallstat(newpath, sb);
+if (reallstat)
+ret = reallstat(newpath, sb);
+else
+ret = real__lxstat(_STAT_VER, newpath, sb);
 free(newpath);
 } else {
-ret = reallstat(path, sb);
+if (reallstat)
+ret = reallstat(path, sb);
+else
+ret = real__lxstat(_STAT_VER, path, sb);
 }
 return ret;
 }
-- 
1.8.1.5

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

Re: [libvirt] [PATCH] qemu: Avoid assigning unavailable migration ports

[Ján Tomko]

On 10/15/2013 02:42 PM, Jiri Denemark wrote:
> From: WangYufei 
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1019053
> 
> When we migrate vms concurrently, there's a chance that libvirtd on
> destination assigns the same port for different migrations, which will
> lead to migration failure during prepare phase on destination. So we use
> virPortAllocator here to solve the problem.
> 
> Signed-off-by: WangYufei 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_command.h   |  3 +++
>  src/qemu/qemu_conf.h  |  6 +++---
>  src/qemu/qemu_domain.h|  1 +
>  src/qemu/qemu_driver.c|  6 ++
>  src/qemu/qemu_migration.c | 39 +--
>  5 files changed, 38 insertions(+), 17 deletions(-)
> 


> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 38edadb..a77aeb7 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2550,8 +2556,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>   * to be a correct hostname which refers to the target machine).
>   */
>  if (uri_in == NULL) {
> -this_port = QEMUD_MIGRATION_FIRST_PORT + port++;
> -if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0;
> +if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0)
> +goto cleanup;
>  
>  /* Get hostname */
>  if ((hostname = virGetHostname()) == NULL)

If all the ports are occupied, virPortAllocatorAcquire will return 0, but set
the port to 0. We need to report an error if (port == 0)


> @@ -2607,16 +2613,16 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
>  
>  if (uri->port == 0) {
>  /* Generate a port */
> -this_port = QEMUD_MIGRATION_FIRST_PORT + port++;
> -if (port == QEMUD_MIGRATION_NUM_PORTS)
> -port = 0;
> +if (virPortAllocatorAcquire(driver->migrationPorts, &port) < 0)
> +goto cleanup;
>  

Same here.

Jan






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

Re: [libvirt] Migration issue php-libvirt

[Umar Draz]

Hi Claudio,

One more error,

I tried this migration and its work, But its added a line in the domain xml
(**), now I tried to again migrate
back to the host but this time I got error

[17-Oct-2013 09:03:14 America/New_York] PHP Warning:
 libvirt_domain_migrate_to_uri(): XML error: missing security model when
using multiple labels in /home/www/virtspace/migrate.php on line 35

When I used virsh for migration then virsh inserted only this line (*)*
*
*
*Br.*
*
*
*Umar*



On Thu, Oct 17, 2013 at 2:45 PM, Claudio Bley  wrote:

> [Repeat: please, don't top post on technical lists.]
>
> At Thu, 17 Oct 2013 14:26:33 +0500,
> Umar Draz wrote:
> >
> > Here is the php_error_logs
> > --
> > *[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
> > constant VIR_MIGRATE_UNSAFE - assumed 'VIR_MIGRATE_UNSAFE' in
> > /home/www/virtspace/inc/mig.php on line 21*
> > *[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
> > constant VIR_MIGRATE_OFFLINE - assumed 'VIR_MIGRATE_OFFLINE' in
> > /home/www/virtspace/inc/mig.php on line 21*
> >
> > According to above php_notices, both VIR_OFFLINE AND VIR_UNSAFE options
> are
> > not available.
>
> Indeed, looking at the libvirt-php code
> (
> http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=a149662272016e946d53f4589722d090ceccdbcf;hb=HEAD
> ),
> these constants are not defined yet. The wrapper is lacking the
> following constants, which you can easily define yourself in your PHP
> script:
>
> VIR_MIGRATE_CHANGE_PROTECTION=  256
> VIR_MIGRATE_UNSAFE   =  512
> VIR_MIGRATE_OFFLINE  =  1024
> VIR_MIGRATE_COMPRESSED   = 2048
>
>
> Claudio
> --
> AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
> Phone: +49 341 265 310 19
> Web:
>
> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
> Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
>



-- 
Umar Draz
Network Architect
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 2/2] capabilities: add baselabel per sec driver/virt type to secmodel

[Daniel P. Berrange]

On Fri, Sep 06, 2013 at 06:29:56PM +0200, Giuseppe Scrivano wrote:
> Expand the "secmodel" XML fragment of "host" with a sequence of
> baselabel's which describe the default security context used by
> libvirt with a specific security model and virtualization type:
> 
> 
>   selinux
>   0
>   system_u:system_r:svirt_t:s0
>   system_u:system_r:svirt_t:s0

s/svirt_t/svirt_tcg_t/ for the qemu example just to illustrate
that it is sometimes diferent.

> 
> 
>   dac
>   0
>   0:0
>   0:0

I'd suggest  '107:107' for these examples since that's the usual
ID for Fedora 'qemu' user.

> 
> 
> "baselabel" is driver-specific information, e.g. in the DAC security
> model, it indicates USER_ID:GROUP_ID.
> 
> Signed-off-by: Giuseppe Scrivano 
> ---
>  docs/schemas/capability.rng  |  8 
>  src/conf/capabilities.c  | 60 
> +++-
>  src/conf/capabilities.h  | 14 +++
>  src/libvirt_private.syms |  1 +
>  src/lxc/lxc_conf.c   | 10 -
>  src/qemu/qemu_conf.c | 21 --
>  tests/capabilityschemadata/caps-qemu-kvm.xml |  2 +
>  tests/capabilityschemadata/caps-test3.xml|  2 +
>  8 files changed, 111 insertions(+), 7 deletions(-)
> 

> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> index 1acc936..b0e2ff9 100644
> --- a/src/conf/capabilities.c
> +++ b/src/conf/capabilities.c
> @@ -184,6 +184,20 @@ virCapabilitiesFreeNUMAInfo(virCapsPtr caps)
>  }
>  
>  static void
> +virCapabilitiesFreeSecModel(virCapsHostSecModelPtr secmodel)
> +{
> +size_t i;
> +for (i = 0; i < secmodel->nlabels; i++) {
> +VIR_FREE(secmodel->labels[i].type);
> +VIR_FREE(secmodel->labels[i].label);
> +}
> +
> +VIR_FREE(secmodel->labels);
> +VIR_FREE(secmodel->model);
> +VIR_FREE(secmodel->doi);
> +}

For functions which don't actually free the passed-in pointer
itself, we prefer to use 'Clear' instead of 'Free' in the name,
to make it more obvious to people what the semantics are.

> +
> +static void
>  virCapabilitiesDispose(void *object)
>  {
>  virCapsPtr caps = object;
> @@ -204,8 +218,7 @@ virCapabilitiesDispose(void *object)
>  VIR_FREE(caps->host.migrateTrans);
>  
>  for (i = 0; i < caps->host.nsecModels; i++) {
> -VIR_FREE(caps->host.secModels[i].model);
> -VIR_FREE(caps->host.secModels[i].doi);
> +virCapabilitiesFreeSecModel(&caps->host.secModels[i]);
>  }
>  VIR_FREE(caps->host.secModels);
>  
> @@ -507,6 +520,44 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,

> diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
> index c1cee3f..557191a 100644
> --- a/src/lxc/lxc_conf.c
> +++ b/src/lxc/lxc_conf.c
> @@ -126,10 +126,13 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver)
>  
>  if (driver) {
>  /* Security driver data */
> -const char *doi, *model;
> +const char *doi, *model, *label, *type;
>  
>  doi = virSecurityManagerGetDOI(driver->securityManager);
>  model = virSecurityManagerGetModel(driver->securityManager);
> +label = virSecurityManagerGetBaseLabel(driver->securityManager,
> +   VIR_DOMAIN_VIRT_LXC);

Hmm, the virSecurityManagerGetBaseLabel method can raise a VIR_ERR_NO_SUPPORT
message if unsupported, which would be ignored here. It is none the less
valid for this method to be not-implemented by a driver. Since I don't believe
we have a need to report errors in this method, I think we should remove the
call to virReportError in its impl.


> +type = virDomainVirtTypeToString(VIR_DOMAIN_VIRT_LXC);
>  /* Allocate the primary security driver for LXC. */
>  if (VIR_ALLOC(caps->host.secModels) < 0)
>  goto error;
> @@ -138,6 +141,11 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver)
>  goto error;
>  if (VIR_STRDUP(caps->host.secModels[0].doi, doi) < 0)
>  goto error;
> +if (label &&
> +virCapabilitiesHostSecModelAddBaseLabel(&caps->host.secModels[0],
> +type,
> +label) < 0)
> +goto error;
>  
>  VIR_DEBUG("Initialized caps for security driver \"%s\" with "
>"DOI \"%s\"", model, doi);
> diff --git a/tests/capabilityschemadata/caps-qemu-kvm.xml 
> b/tests/capabilityschemadata/caps-qemu-kvm.xml
> index 1fbc22b..066ec71 100644
> --- a/tests/capabilityschemadata/caps-qemu-kvm.xml
> +++ b/tests/capabilityschemadata/caps-qemu-kvm.xml
> @@ -25,6 +25,8 @@
>  
>selinux
>0
> +  system_u:system_r:svirt_t:s0
> +  system_u:system_r:svirt_t:s0

s/svirt_t/svirt_tcg_t/ in this example

>  
>
>  
> diff --git a/tests/capabilityschemadata/caps-test3.xml 
> b/tests/capabilityschemadata/caps-test3.xml
> index e6c56c5..d359f2

Re: [libvirt] [PATCH v3 1/2] security: add new internal function "virSecurityManagerGetBaseLabel"

[Daniel P. Berrange]

On Fri, Sep 06, 2013 at 06:29:55PM +0200, Giuseppe Scrivano wrote:
> virSecurityManagerGetBaseLabel queries the default settings used by
> a security model.
> 
> Signed-off-by: Giuseppe Scrivano 
> ---
>  src/libvirt_private.syms |  1 +
>  src/security/security_apparmor.c |  8 
>  src/security/security_dac.c  | 34 --
>  src/security/security_dac.h  |  7 +++
>  src/security/security_driver.h   |  4 
>  src/security/security_manager.c  | 22 --
>  src/security/security_manager.h  |  2 ++
>  src/security/security_nop.c  | 10 ++
>  src/security/security_selinux.c  | 12 
>  src/security/security_stack.c|  9 +
>  10 files changed, 93 insertions(+), 16 deletions(-)
> diff --git a/src/security/security_manager.c b/src/security/security_manager.c
> index 92fb504..c4b8f10 100644
> --- a/src/security/security_manager.c
> +++ b/src/security/security_manager.c
> @@ -273,6 +275,22 @@ virSecurityManagerGetModel(virSecurityManagerPtr mgr)
>  return NULL;
>  }
>  
> +/* return NULL if a base label is not present */
> +const char *
> +virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr, int virtType)
> +{
> +if (mgr->drv->getBaseLabel) {
> +const char *ret;
> +virObjectLock(mgr);
> +ret = mgr->drv->getBaseLabel(mgr, virtType);
> +virObjectUnlock(mgr);
> +return ret;
> +}
> +
> +virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +return NULL;

Per my reply to the 2nd patch, we need to remove thie virReportError
method call. It is valid to not implement this method if no baselabel
is required.

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

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

[libvirt] [PATCHv2 6/8] conf: Refactor storing and usage of feature flags

[Peter Krempa]

Currently we were storing domain feature flags in a bit field as the
they were either enabled or disabled. New features such as paravirtual
spinlocks however can be tri-state as the default option may depend on
hypervisor version.

To allow storing tri-state feature state in the same place instead of
having to declare dedicated variables for each feature this patch
refactors the bit field to an array.
---

Notes:
Version 2:
- clarified the intent of this patch in the commit message

 src/conf/domain_conf.c | 159 ++---
 src/conf/domain_conf.h |   2 +-
 src/libxl/libxl_conf.c |   9 ++-
 src/lxc/lxc_container.c|   6 +-
 src/qemu/qemu_command.c|  15 ++---
 src/vbox/vbox_tmpl.c   |  45 +++--
 src/xenapi/xenapi_driver.c |  10 +--
 src/xenapi/xenapi_utils.c  |  22 +++
 src/xenxs/xen_sxpr.c   |  20 +++---
 src/xenxs/xen_xm.c |  30 -
 10 files changed, 187 insertions(+), 131 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 562d98b..6a99e3f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11411,10 +11411,10 @@ virDomainDefParseXML(xmlDocPtr xml,
_("unexpected feature '%s'"), nodes[i]->name);
 goto error;
 }
-def->features |= (1 << val);
-if (val == VIR_DOMAIN_FEATURE_APIC) {
-tmp = virXPathString("string(./features/apic/@eoi)", ctxt);
-if (tmp) {
+
+switch ((enum virDomainFeature) val) {
+case VIR_DOMAIN_FEATURE_APIC:
+if ((tmp = virXPathString("string(./features/apic/@eoi)", ctxt))) {
 int eoi;
 if ((eoi = virDomainFeatureStateTypeFromString(tmp)) <= 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -11425,11 +11425,23 @@ virDomainDefParseXML(xmlDocPtr xml,
 def->apic_eoi = eoi;
 VIR_FREE(tmp);
 }
+/* fallthrough */
+case VIR_DOMAIN_FEATURE_ACPI:
+case VIR_DOMAIN_FEATURE_PAE:
+case VIR_DOMAIN_FEATURE_HAP:
+case VIR_DOMAIN_FEATURE_VIRIDIAN:
+case VIR_DOMAIN_FEATURE_PRIVNET:
+case VIR_DOMAIN_FEATURE_HYPERV:
+def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
+break;
+
+case VIR_DOMAIN_FEATURE_LAST:
+break;
 }
 }
 VIR_FREE(nodes);

-if (def->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) {
+if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == 
VIR_DOMAIN_FEATURE_STATE_ON) {
 int feature;
 int value;
 node = ctxt->node;
@@ -13401,12 +13413,16 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 {
 size_t i;

-/* basic check */
-if (src->features != dst->features) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("Target domain features %d does not match source %d"),
-   dst->features, src->features);
-return false;
+for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
+if (src->features[i] != dst->features[i]) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("State of feature '%s' differs: "
+ "source: '%s', destination: '%s'"),
+   virDomainFeatureTypeToString(i),
+   virDomainFeatureStateTypeToString(src->features[i]),
+   
virDomainFeatureStateTypeToString(dst->features[i]));
+return false;
+}
 }

 /* APIC EOI */
@@ -13420,7 +13436,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 }

 /* hyperv */
-if (src->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) {
+if (src->features[VIR_DOMAIN_FEATURE_HYPERV] == 
VIR_DOMAIN_FEATURE_STATE_ON) {
 for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
 switch ((enum virDomainHyperv) i) {
 case VIR_DOMAIN_HYPERV_RELAXED:
@@ -16743,58 +16759,99 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 virBufferAddLit(buf, "  \n");
 }

+for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
+if (def->features[i] != VIR_DOMAIN_FEATURE_STATE_DEFAULT)
+break;
+}

-if (def->features) {
+if (i != VIR_DOMAIN_FEATURE_LAST) {
 virBufferAddLit(buf, "  \n");
+
 for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
-if (def->features & (1 << i) && i != VIR_DOMAIN_FEATURE_HYPERV) {
-const char *name = virDomainFeatureTypeToString(i);
-if (!name) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("unexpected feature %zu"), i);
-goto error;
-}
-virBufferAsprintf(buf, "<%s", name);
-if (i == VIR_DOMAIN_FEATURE_APIC && def->apic_eoi) {
-virBufferAsprintf(buf,
-   

[libvirt] [PATCHv2 0/8] Introduce paravirtual spinlock support

[Peter Krempa]

Version 2 fixes stuff pointed out in Dan's review
Jiri Denemark (2):
  cpu: Export few x86-specific APIs
  qemu: Add monitor APIs to fetch CPUID data from QEMU

Peter Krempa (6):
  cpu_x86: Refactor storage of CPUID data to add support for KVM
features
  cpu: x86: Parse the CPU feature map only once
  cpu: x86: Add internal CPUID features support and KVM feature bits
  conf: Refactor storing and usage of feature flags
  qemu: Add support for paravirtual spinlocks in the guest
  qemu: process: Validate specific CPUID flags of a guest

 docs/formatdomain.html.in  |   8 +
 docs/schemas/domaincommon.rng  |  10 +-
 src/conf/domain_conf.c | 195 +---
 src/conf/domain_conf.h |   3 +-
 src/cpu/cpu_x86.c  | 350 +++--
 src/cpu/cpu_x86.h  |   9 +
 src/cpu/cpu_x86_data.h |  20 +-
 src/libvirt_private.syms   |   6 +
 src/libxl/libxl_conf.c |   9 +-
 src/lxc/lxc_container.c|   6 +-
 src/qemu/qemu_command.c|  28 +-
 src/qemu/qemu_monitor.c|  35 +++
 src/qemu/qemu_monitor.h|   5 +
 src/qemu/qemu_monitor_json.c   | 145 +
 src/qemu/qemu_monitor_json.h   |   2 +
 src/qemu/qemu_process.c|  53 
 src/vbox/vbox_tmpl.c   |  45 ++-
 src/xenapi/xenapi_driver.c |  10 +-
 src/xenapi/xenapi_utils.c  |  22 +-
 src/xenxs/xen_sxpr.c   |  20 +-
 src/xenxs/xen_xm.c |  30 +-
 tests/Makefile.am  |   1 +
 .../qemumonitorjson-getcpu-full.data   |   5 +
 .../qemumonitorjson-getcpu-full.json   |  46 +++
 .../qemumonitorjson-getcpu-host.data   |   6 +
 .../qemumonitorjson-getcpu-host.json   |  45 +++
 tests/qemumonitorjsontest.c|  76 +
 .../qemuxml2argv-pv-spinlock-disabled.args |   5 +
 .../qemuxml2argv-pv-spinlock-disabled.xml  |  26 ++
 .../qemuxml2argv-pv-spinlock-enabled.args  |   5 +
 .../qemuxml2argv-pv-spinlock-enabled.xml   |  26 ++
 tests/qemuxml2argvtest.c   |   2 +
 tests/qemuxml2xmltest.c|   2 +
 33 files changed, 952 insertions(+), 304 deletions(-)
 create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data
 create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json
 create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data
 create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml

-- 
1.8.3.2

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

[libvirt] [PATCHv2 4/8] cpu: x86: Parse the CPU feature map only once

[Peter Krempa]

Until now the map was loaded from the XML definition file every time a
operation on the flags was requested. With the introduciton of one shot
initializers we can store the definition forever (as it will never
change) instead of parsing it over and over again.
---

Notes:
Version 2:
- kept the map loading function separate so that tests can use it in the 
future

 src/cpu/cpu_x86.c | 67 +--
 1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 1b1f2b4..ba6a2b0 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -70,6 +70,10 @@ struct x86_map {
 struct x86_model *models;
 };

+static struct x86_map* virCPUx86Map = NULL;
+int virCPUx86MapOnceInit(void);
+VIR_ONCE_GLOBAL_INIT(virCPUx86Map);
+

 enum compare_result {
 SUBSET,
@@ -1065,7 +1069,7 @@ x86MapLoadCallback(enum cpuMapElement element,


 static struct x86_map *
-x86LoadMap(void)
+virCPUx86LoadMap(void)
 {
 struct x86_map *map;

@@ -1083,6 +1087,26 @@ error:
 }


+int
+virCPUx86MapOnceInit(void)
+{
+if (!(virCPUx86Map = virCPUx86LoadMap()))
+return -1;
+
+return 0;
+}
+
+
+static const struct x86_map *
+virCPUx86GetMap(void)
+{
+if (virCPUx86MapInitialize() < 0)
+return NULL;
+
+return virCPUx86Map;
+}
+
+
 static char *
 x86CPUDataFormat(const virCPUData *data)
 {
@@ -1194,7 +1218,7 @@ x86Compute(virCPUDefPtr host,
virCPUDataPtr *guest,
char **message)
 {
-struct x86_map *map = NULL;
+const struct x86_map *map = NULL;
 struct x86_model *host_model = NULL;
 struct x86_model *cpu_force = NULL;
 struct x86_model *cpu_require = NULL;
@@ -1247,7 +1271,7 @@ x86Compute(virCPUDefPtr host,
 return VIR_CPU_COMPARE_INCOMPATIBLE;
 }

-if (!(map = x86LoadMap()) ||
+if (!(map = virCPUx86GetMap()) ||
 !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE)) ||
 !(cpu_force = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_FORCE)) ||
 !(cpu_require = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_REQUIRE)) ||
@@ -1323,7 +1347,6 @@ x86Compute(virCPUDefPtr host,
 }

 cleanup:
-x86MapFree(map);
 x86ModelFree(host_model);
 x86ModelFree(diff);
 x86ModelFree(cpu_force);
@@ -1362,7 +1385,7 @@ x86GuestData(virCPUDefPtr host,

 static int
 x86AddFeatures(virCPUDefPtr cpu,
-   struct x86_map *map)
+   const struct x86_map *map)
 {
 const struct x86_model *candidate;
 const struct x86_feature *feature = map->features;
@@ -1398,7 +1421,7 @@ x86Decode(virCPUDefPtr cpu,
   unsigned int flags)
 {
 int ret = -1;
-struct x86_map *map;
+const struct x86_map *map;
 const struct x86_model *candidate;
 virCPUDefPtr cpuCandidate;
 virCPUDefPtr cpuModel = NULL;
@@ -1406,7 +1429,7 @@ x86Decode(virCPUDefPtr cpu,

 virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);

-if (data == NULL || (map = x86LoadMap()) == NULL)
+if (!data || !(map = virCPUx86GetMap()))
 return -1;

 candidate = map->models;
@@ -1490,7 +1513,6 @@ x86Decode(virCPUDefPtr cpu,
 ret = 0;

 out:
-x86MapFree(map);
 virCPUDefFree(cpuModel);

 return ret;
@@ -1537,14 +1559,13 @@ x86Encode(virArch arch,
   virCPUDataPtr *forbidden,
   virCPUDataPtr *vendor)
 {
-struct x86_map *map = NULL;
+const struct x86_map *map = NULL;
 virCPUx86Data *data_forced = NULL;
 virCPUx86Data *data_required = NULL;
 virCPUx86Data *data_optional = NULL;
 virCPUx86Data *data_disabled = NULL;
 virCPUx86Data *data_forbidden = NULL;
 virCPUx86Data *data_vendor = NULL;
-int ret = -1;

 if (forced)
 *forced = NULL;
@@ -1559,7 +1580,7 @@ x86Encode(virArch arch,
 if (vendor)
 *vendor = NULL;

-if ((map = x86LoadMap()) == NULL)
+if ((map = virCPUx86GetMap()) == NULL)
 goto error;

 if (forced) {
@@ -1627,12 +1648,7 @@ x86Encode(virArch arch,
 !(*vendor = virCPUx86MakeData(arch, &data_vendor)))
 goto error;

-ret = 0;
-
-cleanup:
-x86MapFree(map);
-
-return ret;
+return 0;

 error:
 virCPUx86DataFree(data_forced);
@@ -1653,7 +1669,7 @@ error:
 x86FreeCPUData(*forbidden);
 if (vendor)
 x86FreeCPUData(*vendor);
-goto cleanup;
+return -1;
 }


@@ -1748,7 +1764,7 @@ x86Baseline(virCPUDefPtr *cpus,
 unsigned int nmodels,
 unsigned int flags)
 {
-struct x86_map *map = NULL;
+const struct x86_map *map = NULL;
 struct x86_model *base_model = NULL;
 virCPUDefPtr cpu = NULL;
 size_t i;
@@ -1756,7 +1772,7 @@ x86Baseline(virCPUDefPtr *cpus,
 struct x86_model *model = NULL;
 bool outputVendor = true;

-if (!(map = x86LoadMap()))
+if (!(map = virCPUx86GetMap()))
 goto error;

 if (!(base_model = x86ModelFromCPU(cpus[0], map, VIR_CPU_FEATURE_REQUIRE)))
@@ -1837,7 +1853

[libvirt] [PATCHv2 5/8] cpu: x86: Add internal CPUID features support and KVM feature bits

[Peter Krempa]

Some of the emulator features are presented in the  element in
the domain XML although they are virtual CPUID feature bits when
presented to the guest. To avoid confusing the users with these
features, as they are not configurable via the  element, this patch
adds an internal array where those can be stored privately instead of
exposing them in the XML.

Additionaly KVM feature bits are added as example usage of this code.
---

Notes:
Version 2:
-no change, ACK'd in V1, didn't make sense to push

 src/cpu/cpu_x86.c  | 64 ++
 src/cpu/cpu_x86_data.h | 12 ++
 2 files changed, 76 insertions(+)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index ba6a2b0..ce75562 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -56,6 +56,25 @@ struct x86_feature {
 struct x86_feature *next;
 };

+struct x86_kvm_feature {
+const char *name;
+const virCPUx86CPUID cpuid;
+};
+
+static const struct x86_kvm_feature x86_kvm_features[] =
+{
+{VIR_CPU_x86_KVM_CLOCKSOURCE,  { .function = 0x4001, .eax = 0x0001 
}},
+{VIR_CPU_x86_KVM_NOP_IO_DELAY, { .function = 0x4001, .eax = 0x0002 
}},
+{VIR_CPU_x86_KVM_MMU_OP,   { .function = 0x4001, .eax = 0x0004 
}},
+{VIR_CPU_x86_KVM_CLOCKSOURCE2, { .function = 0x4001, .eax = 0x0008 
}},
+{VIR_CPU_x86_KVM_ASYNC_PF, { .function = 0x4001, .eax = 0x0010 
}},
+{VIR_CPU_x86_KVM_STEAL_TIME,   { .function = 0x4001, .eax = 0x0020 
}},
+{VIR_CPU_x86_KVM_PV_EOI,   { .function = 0x4001, .eax = 0x0040 
}},
+{VIR_CPU_x86_KVM_PV_UNHALT,{ .function = 0x4001, .eax = 0x0080 
}},
+{VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT,
+   { .function = 0x4001, .eax = 0x0100 
}},
+};
+
 struct x86_model {
 char *name;
 const struct x86_vendor *vendor;
@@ -1068,6 +1087,48 @@ x86MapLoadCallback(enum cpuMapElement element,
 }


+static int
+x86MapLoadInternalFeatures(struct x86_map *map)
+{
+size_t i;
+struct x86_feature *feature = NULL;
+
+for (i = 0; i < ARRAY_CARDINALITY(x86_kvm_features); i++) {
+const char *name = x86_kvm_features[i].name;
+
+if (x86FeatureFind(map, name)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("CPU feature %s already defined"), name);
+return -1;
+}
+
+if (!(feature = x86FeatureNew()))
+goto error;
+
+if (VIR_STRDUP(feature->name, name) < 0)
+goto error;
+
+if (virCPUx86DataAddCPUID(feature->data, &x86_kvm_features[i].cpuid))
+goto error;
+
+if (map->features == NULL) {
+map->features = feature;
+} else {
+feature->next = map->features;
+map->features = feature;
+}
+
+feature = NULL;
+}
+
+return 0;
+
+error:
+x86FeatureFree(feature);
+return -1;
+}
+
+
 static struct x86_map *
 virCPUx86LoadMap(void)
 {
@@ -1079,6 +1140,9 @@ virCPUx86LoadMap(void)
 if (cpuMapLoad("x86", x86MapLoadCallback, map) < 0)
 goto error;

+if (x86MapLoadInternalFeatures(map) < 0)
+goto error;
+
 return map;

 error:
diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
index 69066f1..88dccf6 100644
--- a/src/cpu/cpu_x86_data.h
+++ b/src/cpu/cpu_x86_data.h
@@ -36,8 +36,20 @@ struct _virCPUx86CPUID {
 };

 # define CPUX86_BASIC0x0
+# define CPUX86_KVM  0x4000
 # define CPUX86_EXTENDED 0x8000

+# define VIR_CPU_x86_KVM_CLOCKSOURCE  "__kvm_clocksource"
+# define VIR_CPU_x86_KVM_NOP_IO_DELAY "__kvm_no_io_delay"
+# define VIR_CPU_x86_KVM_MMU_OP   "__kvm_mmu_op"
+# define VIR_CPU_x86_KVM_CLOCKSOURCE2 "__kvm_clocksource2"
+# define VIR_CPU_x86_KVM_ASYNC_PF "__kvm_async_pf"
+# define VIR_CPU_x86_KVM_STEAL_TIME   "__kvm_steal_time"
+# 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"
+
+
 typedef struct _virCPUx86Data virCPUx86Data;
 struct _virCPUx86Data {
 size_t len;
-- 
1.8.3.2

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

[libvirt] [PATCHv2 2/8] cpu_x86: Refactor storage of CPUID data to add support for KVM features

[Peter Krempa]

The CPUID functions were stored in multiple arrays according to a
specified prefix of those. This made it very hard to add another prefix
to store KVM CPUID features (0x4000). Instead of hardcoding a third
array this patch changes the approach used:

The code is refactored to use a single array where the CPUID functions
are stored ordered by the cpuid function so that they don't depend on
the specific prefix and don't waste memory. The code is also less
complex using this approach. A tradeoff to this is the change from O(N)
complexity to O(N^2) in x86DataAdd and x86DataSubtract. The rest of the
functions were already using O(N^2) algorithms.
---

Notes:
Version 2:
- no change, weak ACK from Daniel
- fixed typo in commit message

 src/cpu/cpu_x86.c  | 213 ++---
 src/cpu/cpu_x86_data.h |   8 +-
 2 files changed, 80 insertions(+), 141 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 539d8b2..1b1f2b4 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -80,14 +80,13 @@ enum compare_result {


 struct virCPUx86DataIterator {
-virCPUx86Data *data;
+const virCPUx86Data *data;
 int pos;
-bool extended;
 };


 #define virCPUx86DataIteratorInit(data) \
-{ data, -1, false }
+{ data, -1 }


 static bool
@@ -116,6 +115,9 @@ static void
 x86cpuidSetBits(virCPUx86CPUID *cpuid,
 const virCPUx86CPUID *mask)
 {
+if (!mask)
+return;
+
 cpuid->eax |= mask->eax;
 cpuid->ebx |= mask->ebx;
 cpuid->ecx |= mask->ecx;
@@ -127,6 +129,9 @@ static void
 x86cpuidClearBits(virCPUx86CPUID *cpuid,
   const virCPUx86CPUID *mask)
 {
+if (!mask)
+return;
+
 cpuid->eax &= ~mask->eax;
 cpuid->ebx &= ~mask->ebx;
 cpuid->ecx &= ~mask->ecx;
@@ -138,42 +143,45 @@ static void
 x86cpuidAndBits(virCPUx86CPUID *cpuid,
 const virCPUx86CPUID *mask)
 {
+if (!mask)
+return;
+
 cpuid->eax &= mask->eax;
 cpuid->ebx &= mask->ebx;
 cpuid->ecx &= mask->ecx;
 cpuid->edx &= mask->edx;
 }

+static int
+virCPUx86CPUIDSorter(const void *a, const void *b)
+{
+virCPUx86CPUID *da = (virCPUx86CPUID *) a;
+virCPUx86CPUID *db = (virCPUx86CPUID *) b;
+
+if (da->function > db->function)
+return 1;
+else if (da->function < db->function)
+return -1;
+
+return 0;
+}
+

 /* skips all zero CPUID leafs */
 static virCPUx86CPUID *
 x86DataCpuidNext(struct virCPUx86DataIterator *iterator)
 {
-virCPUx86CPUID *ret;
-virCPUx86Data *data = iterator->data;
+const virCPUx86Data *data = iterator->data;

 if (!data)
 return NULL;

-do {
-ret = NULL;
-iterator->pos++;
-
-if (!iterator->extended) {
-if (iterator->pos < data->basic_len)
-ret = data->basic + iterator->pos;
-else {
-iterator->extended = true;
-iterator->pos = 0;
-}
-}
-
-if (iterator->extended && iterator->pos < data->extended_len) {
-ret = data->extended + iterator->pos;
-}
-} while (ret && x86cpuidMatch(ret, &cpuidNull));
+while (++iterator->pos < data->len) {
+if (!x86cpuidMatch(data->data + iterator->pos, &cpuidNull))
+return data->data + iterator->pos;
+}

-return ret;
+return NULL;
 }


@@ -181,36 +189,23 @@ static virCPUx86CPUID *
 x86DataCpuid(const virCPUx86Data *data,
  uint32_t function)
 {
-virCPUx86CPUID *cpuids;
-int len;
 size_t i;

-if (function < CPUX86_EXTENDED) {
-cpuids = data->basic;
-len = data->basic_len;
-i = function;
-}
-else {
-cpuids = data->extended;
-len = data->extended_len;
-i = function - CPUX86_EXTENDED;
+for (i = 0; i < data->len; i++) {
+if (data->data[i].function == function)
+return data->data + i;
 }

-if (i < len && !x86cpuidMatch(cpuids + i, &cpuidNull))
-return cpuids + i;
-else
-return NULL;
+return NULL;
 }

-
 void
 virCPUx86DataFree(virCPUx86Data *data)
 {
 if (data == NULL)
 return;

-VIR_FREE(data->basic);
-VIR_FREE(data->extended);
+VIR_FREE(data->data);
 VIR_FREE(data);
 }

@@ -247,77 +242,36 @@ x86DataCopy(const virCPUx86Data *data)
 virCPUx86Data *copy = NULL;
 size_t i;

-if (VIR_ALLOC(copy) < 0
-|| VIR_ALLOC_N(copy->basic, data->basic_len) < 0
-|| VIR_ALLOC_N(copy->extended, data->extended_len) < 0) {
+if (VIR_ALLOC(copy) < 0 ||
+VIR_ALLOC_N(copy->data, data->len) < 0) {
 virCPUx86DataFree(copy);
 return NULL;
 }

-copy->basic_len = data->basic_len;
-for (i = 0; i < data->basic_len; i++)
-copy->basic[i] = data->basic[i];
-
-copy->extended_len = data->extended_len;
-for (i = 0; i < data->extended_len; i++)
-copy->extended[i] = data->extended[

[libvirt] [PATCHv2 3/8] qemu: Add monitor APIs to fetch CPUID data from QEMU

[Peter Krempa]

From: Jiri Denemark 

The qemu monitor supports retrieval of actual CPUID bits presented to
the guest using QMP monitor. Add APIs to extract these information and
tests for them.

Signed-off-by: Peter Krempa 
---

Notes:
Version 2:
- unified global and JSON monitor func signatures
- changed return values so that errors can be differenitated from lack of 
support
- code is conditionally run only when architecture matches

 src/qemu/qemu_monitor.c|  35 +
 src/qemu/qemu_monitor.h|   5 +
 src/qemu/qemu_monitor_json.c   | 145 +
 src/qemu/qemu_monitor_json.h   |   2 +
 tests/Makefile.am  |   1 +
 .../qemumonitorjson-getcpu-full.data   |   5 +
 .../qemumonitorjson-getcpu-full.json   |  46 +++
 .../qemumonitorjson-getcpu-host.data   |   6 +
 .../qemumonitorjson-getcpu-host.json   |  45 +++
 tests/qemumonitorjsontest.c|  76 +++
 10 files changed, 366 insertions(+)
 create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.data
 create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-full.json
 create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.data
 create mode 100644 tests/qemumonitorjsondata/qemumonitorjson-getcpu-host.json

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2bafe28..e2339cd 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3926,3 +3926,38 @@ qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd)

 return 0;
 }
+
+
+/**
+ * qemuMonitorJSONGetGuestCPU:
+ * @mon: Pointer to the monitor
+ * @arch: arch of the guest
+ * @cpuData: Returned cpu definition of the gues
+ *
+ * Retrieve the definition of the guest CPU from a running qemu instance.
+ *
+ * Returns 0 on success; -1 on error; -2 if error may be caused by qemu not
+ * supporting reporting of CPU definition.
+ */
+int
+qemuMonitorGetGuestCPU(qemuMonitorPtr mon,
+   virArch arch,
+   virCPUDataPtr *cpuData)
+{
+VIR_DEBUG("mon=%p, arch='%s' cpuData=%p",
+  mon, virArchToString(arch), cpuData);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("monitor must not be NULL"));
+return -1;
+}
+
+if (!mon->json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("JSON monitor is required"));
+return -2;
+}
+
+return qemuMonitorJSONGetGuestCPU(mon, arch, cpuData);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 06ba7e8..f893b1f 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -32,6 +32,7 @@
 # include "virhash.h"
 # include "virjson.h"
 # include "device_conf.h"
+# include "cpu/cpu.h"

 typedef struct _qemuMonitor qemuMonitor;
 typedef qemuMonitor *qemuMonitorPtr;
@@ -763,6 +764,10 @@ int qemuMonitorGetDeviceAliases(qemuMonitorPtr mon,

 int qemuMonitorSetDomainLog(qemuMonitorPtr mon, int logfd);

+int qemuMonitorGetGuestCPU(qemuMonitorPtr mon,
+   virArch arch,
+   virCPUDataPtr *data);
+
 /**
  * When running two dd process and using <> redirection, we need a
  * shell that will not truncate files.  These two strings serve that
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 05f8aa6..342d8f0 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -42,6 +42,7 @@
 #include "virerror.h"
 #include "virjson.h"
 #include "virstring.h"
+#include "cpu/cpu_x86.h"

 #ifdef WITH_DTRACE_PROBES
 # include "libvirt_qemu_probes.h"
@@ -49,6 +50,7 @@

 #define VIR_FROM_THIS VIR_FROM_QEMU

+#define QOM_CPU_PATH  "/machine/unattached/device[0]"

 #define LINE_ENDING "\r\n"

@@ -5454,3 +5456,146 @@ cleanup:
 VIR_FREE(paths);
 return ret;
 }
+
+
+static int
+qemuMonitorJSONParseCPUx86FeatureWord(virJSONValuePtr data,
+  virCPUx86CPUID *cpuid)
+{
+const char *reg;
+unsigned long long fun;
+unsigned long long features;
+
+memset(cpuid, 0, sizeof(*cpuid));
+
+if (!(reg = virJSONValueObjectGetString(data, "cpuid-register"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing cpuid-register in CPU data"));
+return -1;
+}
+if (virJSONValueObjectGetNumberUlong(data, "cpuid-input-eax", &fun)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing or invalid cpuid-input-eax in CPU data"));
+return -1;
+}
+if (virJSONValueObjectGetNumberUlong(data, "features", &features) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("missing or invalid features in CPU data"));
+return -1;
+}
+
+cpuid->function = fun;
+

[libvirt] [PATCHv2 8/8] qemu: process: Validate specific CPUID flags of a guest

[Peter Krempa]

When starting a VM the qemu process may filter out some requested
features of a domain as it's not supported either by the host or by
qemu. Libvirt didn't check if this happened which might end up in
changing of the guest ABI when migrating.

The proof of concept implementation adds the check for the recently
introduced kvm_pv_unhalt cpuid feature bit. This feature depends on both
qemu and host kernel support and thus increase the possibility of guest
ABI breakage.
---

Notes:
Version 2:
-- adapted to previous changes

 src/qemu/qemu_process.c | 53 +
 1 file changed, 53 insertions(+)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 354e079..ca024a9 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -44,6 +44,7 @@
 #include "qemu_bridge_filter.h"
 #include "qemu_migration.h"

+#include "cpu/cpu.h"
 #include "datatypes.h"
 #include "virlog.h"
 #include "virerror.h"
@@ -3473,6 +3474,54 @@ qemuValidateCpuMax(virDomainDefPtr def, virQEMUCapsPtr 
qemuCaps)
 return true;
 }

+
+static bool
+qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver, virDomainObjPtr vm)
+{
+virDomainDefPtr def = vm->def;
+virArch arch = def->os.arch;
+virCPUDataPtr guestcpu = NULL;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+bool ret = false;
+int rc;
+
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorGetGuestCPU(priv->mon, arch, &guestcpu);
+qemuDomainObjExitMonitor(driver, vm);
+
+if (rc < 0) {
+if (rc == -2) {
+virResetLastError();
+return true;
+}
+
+goto cleanup;
+}
+
+switch (arch) {
+case VIR_ARCH_I686:
+case VIR_ARCH_X86_64:
+if (def->features[VIR_DOMAIN_FEATURE_PVSPINLOCK] == 
VIR_DOMAIN_FEATURE_STATE_ON) {
+if (!cpuHasFeature(guestcpu, VIR_CPU_x86_KVM_PV_UNHALT)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("host doesn't support paravirtual 
spinlocks"));
+goto cleanup;
+}
+}
+break;
+
+default:
+break;
+}
+
+ret = true;
+
+cleanup:
+cpuDataFree(guestcpu);
+return ret;
+}
+
+
 int qemuProcessStart(virConnectPtr conn,
  virQEMUDriverPtr driver,
  virDomainObjPtr vm,
@@ -3940,6 +3989,10 @@ int qemuProcessStart(virConnectPtr conn,
 priv->agentError = true;
 }

+VIR_DEBUG("Detecting if required emulator features are present");
+if (!qemuProcessVerifyGuestCPU(driver, vm))
+goto cleanup;
+
 VIR_DEBUG("Detecting VCPU PIDs");
 if (qemuProcessDetectVcpuPIDs(driver, vm) < 0)
 goto cleanup;
-- 
1.8.3.2

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

[libvirt] [PATCHv2 7/8] qemu: Add support for paravirtual spinlocks in the guest

[Peter Krempa]

The linux kernel recently added support for paravirtual spinlock
handling to avoid performance regressions on overcomitted hosts. This
feature needs to be turned in the hypervisor so that the guest OS is
notified about the possible support.

This patch adds a new feature "paravirt-spinlock" to the XML and
supporting code to enable the "kvm_pv_unhalt" pseudo CPU feature in
qemu.

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

Notes:
Version 2:
- s/paravirt-spinlock/pvspinlock/

 docs/formatdomain.html.in  |  8 +
 docs/schemas/domaincommon.rng  | 10 +-
 src/conf/domain_conf.c | 36 +-
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_command.c| 13 
 .../qemuxml2argv-pv-spinlock-disabled.args |  5 +++
 .../qemuxml2argv-pv-spinlock-disabled.xml  | 26 
 .../qemuxml2argv-pv-spinlock-enabled.args  |  5 +++
 .../qemuxml2argv-pv-spinlock-enabled.xml   | 26 
 tests/qemuxml2argvtest.c   |  2 ++
 tests/qemuxml2xmltest.c|  2 ++
 11 files changed, 132 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c5a3fa8..14fe84e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1184,6 +1184,7 @@
   
   
 
+

   
   ...
@@ -1255,6 +1256,13 @@
 
   
   
+  pvspinlock
+  Notify the guest that the host supports paravirtual spinlocks
+  for example by exposing the pvticketlocks mechanism. This feature
+  can be explicitly disabled by using state='off'
+  attribute.
+  
+
 

 Time keeping
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 14b6700..cc3096d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3561,7 +3561,7 @@
   
   
   
 
@@ -3607,6 +3607,14 @@
   
 
   
+  
+
+  
+
+  
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6a99e3f..adc27e3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -142,7 +142,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
   "hap",
   "viridian",
   "privnet",
-  "hyperv")
+  "hyperv",
+  "pvspinlock")

 VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST,
   "default",
@@ -11435,6 +11436,22 @@ virDomainDefParseXML(xmlDocPtr xml,
 def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
 break;

+case VIR_DOMAIN_FEATURE_PVSPINLOCK:
+node = ctxt->node;
+ctxt->node = nodes[i];
+if ((tmp = virXPathString("string(./@state)", ctxt))) {
+if ((def->features[val] = 
virDomainFeatureStateTypeFromString(tmp)) == -1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown state atribute '%s' of feature 
'%s'"),
+   tmp, virDomainFeatureTypeToString(val));
+goto error;
+}
+} else {
+def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
+}
+ctxt->node = node;
+break;
+
 case VIR_DOMAIN_FEATURE_LAST:
 break;
 }
@@ -16802,6 +16819,23 @@ virDomainDefFormatInternal(virDomainDefPtr def,

 break;

+case VIR_DOMAIN_FEATURE_PVSPINLOCK:
+switch ((enum virDomainFeatureState) def->features[i]) {
+case VIR_DOMAIN_FEATURE_STATE_LAST:
+case VIR_DOMAIN_FEATURE_STATE_DEFAULT:
+break;
+
+case VIR_DOMAIN_FEATURE_STATE_ON:
+   virBufferAsprintf(buf, "<%s state='on'/>\n", name);
+   break;
+
+case VIR_DOMAIN_FEATURE_STATE_OFF:
+   virBufferAsprintf(buf, "<%s state='off'/>\n", name);
+   break;
+}
+
+break;
+
 case VIR_DOMAIN_FEATURE_APIC:
 if (def->features[i] == VIR_DOMAIN_FEATURE_STATE_ON) {
 virBufferAddLit(buf, "features[VIR_DOMAIN_FEATURE_PVSPINLOCK]) {
+

[libvirt] [PATCHv2 1/8] cpu: Export few x86-specific APIs

[Peter Krempa]

From: Jiri Denemark 

This makes virCPUx86DataAddCPUID, virCPUx86DataFree, and
virCPUx86MakeData available for direct usage outside of cpu driver in
tests and the new qemu monitor that will request the actual CPU
definition from a running qemu instance.
---

Notes:
Version 2:
- clarified scope of the export in the commit message

 src/cpu/cpu_x86.c| 6 +++---
 src/cpu/cpu_x86.h| 9 +
 src/libvirt_private.syms | 6 ++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 8427555..539d8b2 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -203,7 +203,7 @@ x86DataCpuid(const virCPUx86Data *data,
 }


-static void
+void
 virCPUx86DataFree(virCPUx86Data *data)
 {
 if (data == NULL)
@@ -215,7 +215,7 @@ virCPUx86DataFree(virCPUx86Data *data)
 }


-static virCPUDataPtr
+virCPUDataPtr
 virCPUx86MakeData(virArch arch, virCPUx86Data **data)
 {
 virCPUDataPtr cpuData;
@@ -295,7 +295,7 @@ x86DataExpand(virCPUx86Data *data,
 }


-static int
+int
 virCPUx86DataAddCPUID(virCPUx86Data *data,
   const virCPUx86CPUID *cpuid)
 {
diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h
index 77965b7..af0fa23 100644
--- a/src/cpu/cpu_x86.h
+++ b/src/cpu/cpu_x86.h
@@ -25,7 +25,16 @@
 # define __VIR_CPU_X86_H__

 # include "cpu.h"
+# include "cpu_x86_data.h"

 extern struct cpuArchDriver cpuDriverX86;

+int virCPUx86DataAddCPUID(virCPUx86Data *data,
+  const virCPUx86CPUID *cpuid);
+
+void virCPUx86DataFree(virCPUx86Data *data);
+
+virCPUDataPtr virCPUx86MakeData(virArch arch,
+virCPUx86Data **data);
+
 #endif /* __VIR_CPU_X86_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 84c1c28..cb80700 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -733,6 +733,12 @@ cpuNodeData;
 cpuUpdate;


+# cpu/cpu_x86.h
+virCPUx86DataAddCPUID;
+virCPUx86DataFree;
+virCPUx86MakeData;
+
+
 # datatypes.h
 virConnectClass;
 virDomainClass;
-- 
1.8.3.2

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

Re: [libvirt] [PATCH v3 1/2] security: add new internal function "virSecurityManagerGetBaseLabel"

[Daniel P. Berrange]

On Fri, Sep 06, 2013 at 06:29:55PM +0200, Giuseppe Scrivano wrote:
> virSecurityManagerGetBaseLabel queries the default settings used by
> a security model.
> 
> Signed-off-by: Giuseppe Scrivano 
> ---
>  src/libvirt_private.syms |  1 +
>  src/security/security_apparmor.c |  8 
>  src/security/security_dac.c  | 34 --
>  src/security/security_dac.h  |  7 +++
>  src/security/security_driver.h   |  4 
>  src/security/security_manager.c  | 22 --
>  src/security/security_manager.h  |  2 ++
>  src/security/security_nop.c  | 10 ++
>  src/security/security_selinux.c  | 12 
>  src/security/security_stack.c|  9 +
>  10 files changed, 93 insertions(+), 16 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 35f0f1b..aea7e94 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1033,6 +1033,7 @@ virSecurityDriverLookup;
>  # security/security_manager.h
>  virSecurityManagerClearSocketLabel;
>  virSecurityManagerGenLabel;
> +virSecurityManagerGetBaseLabel;
>  virSecurityManagerGetDOI;
>  virSecurityManagerGetModel;
>  virSecurityManagerGetMountOptions;
> diff --git a/src/security/security_apparmor.c 
> b/src/security/security_apparmor.c
> index adc9918..2d74cdd 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -931,6 +931,12 @@ AppArmorGetMountOptions(virSecurityManagerPtr mgr 
> ATTRIBUTE_UNUSED,
>  return opts;
>  }
>  
> +static const char *
> +AppArmorGetBaseLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
> + int virtType ATTRIBUTE_UNUSED)
> +{
> +return NULL;
> +}
>  
>  virSecurityDriver virAppArmorSecurityDriver = {
>  .privateDataLen = 0,
> @@ -972,4 +978,6 @@ virSecurityDriver virAppArmorSecurityDriver = {
>  .domainSetSecurityTapFDLabel= AppArmorSetFDLabel,
>  
>  .domainGetSecurityMountOptions  = AppArmorGetMountOptions,
> +
> +.getBaseLabel   = AppArmoryGetBaseLabel,
>  };
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 6876bd5..019c789 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -47,22 +47,25 @@ struct _virSecurityDACData {
>  gid_t *groups;
>  int ngroups;
>  bool dynamicOwnership;
> +char *baselabel;
>  };
>  
> -void
> -virSecurityDACSetUser(virSecurityManagerPtr mgr,
> -  uid_t user)
> +/* returns -1 on error, 0 on success */
> +int
> +virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
> +  uid_t user,
> +  gid_t group)
>  {
>  virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
>  priv->user = user;
> -}
> -
> -void
> -virSecurityDACSetGroup(virSecurityManagerPtr mgr,
> -   gid_t group)
> -{
> -virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
>  priv->group = group;
> +
> +if (virAsprintf(&priv->baselabel, "%u:%u",
> +(unsigned int) user,
> +(unsigned int) group) < 0)
> +return -1;
> +
> +return 0;
>  }
>  
>  void
> @@ -217,6 +220,7 @@ virSecurityDACClose(virSecurityManagerPtr mgr)
>  {
>  virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
>  VIR_FREE(priv->groups);
> +VIR_FREE(priv->baselabel);
>  return 0;
>  }
>  
> @@ -1170,6 +1174,14 @@ virSecurityDACGetMountOptions(virSecurityManagerPtr 
> mgr ATTRIBUTE_UNUSED,
>  return NULL;
>  }
>  
> +static const char *
> +virSecurityDACGetBaseLabel(virSecurityManagerPtr mgr,
> +   int virt ATTRIBUTE_UNUSED)
> +{
> +virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
> +return priv->baselabel;
> +}
> +
>  virSecurityDriver virSecurityDriverDAC = {
>  .privateDataLen = sizeof(virSecurityDACData),
>  .name   = SECURITY_DAC_NAME,
> @@ -1212,4 +1224,6 @@ virSecurityDriver virSecurityDriverDAC = {
>  .domainSetSecurityTapFDLabel= virSecurityDACSetTapFDLabel,
>  
>  .domainGetSecurityMountOptions  = virSecurityDACGetMountOptions,
> +
> +.getBaseLabel   = virSecurityDACGetBaseLabel,
>  };
> diff --git a/src/security/security_dac.h b/src/security/security_dac.h
> index 02432a5..dbcf56f 100644
> --- a/src/security/security_dac.h
> +++ b/src/security/security_dac.h
> @@ -25,10 +25,9 @@
>  
>  extern virSecurityDriver virSecurityDriverDAC;
>  
> -void virSecurityDACSetUser(virSecurityManagerPtr mgr,
> -   uid_t user);
> -void virSecurityDACSetGroup(virSecurityManagerPtr mgr,
> -gid_t group);
> +int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
> +  uid_t user,
> +  

Re: [libvirt] [PATCH v3 0/2] expose baselabel for each sec model/virt type

[Daniel P. Berrange]

On Thu, Oct 17, 2013 at 02:55:04PM +0200, Giuseppe Scrivano wrote:
> Giuseppe Scrivano  writes:
> 
> > Now each security model can define its own base label, that describes
> > the default security context used by libvirt to run an hypervisor
> > process.  This information is exposed to users trough the host
> > capabilities XML.
> >
> > *v3 major changes
> > - support LXC
> > - merge virSecurityDACSetUser and virSecurityDACSetGroup in
> >   virSecurityDACSetUserAndGroup
> > - DAC sets the baselabel in virSecurityDACSetUserAndGroup
> > - Use virDomainVirtTypeToString instead of hardcoding the name
> 
> I've ran a quick smoke test on top of the current HEAD and it seems to
> work, can someone please review it or tell me if it makes sense at all
> to have this information under "capabilities"?

Opps my bad, it was on my todo list to review this, but I totally
forgot. Will do it now.

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

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

Re: [libvirt] [PATCH v3 0/2] expose baselabel for each sec model/virt type

[Giuseppe Scrivano]

Giuseppe Scrivano  writes:

> Now each security model can define its own base label, that describes
> the default security context used by libvirt to run an hypervisor
> process.  This information is exposed to users trough the host
> capabilities XML.
>
> *v3 major changes
> - support LXC
> - merge virSecurityDACSetUser and virSecurityDACSetGroup in
>   virSecurityDACSetUserAndGroup
> - DAC sets the baselabel in virSecurityDACSetUserAndGroup
> - Use virDomainVirtTypeToString instead of hardcoding the name

I've ran a quick smoke test on top of the current HEAD and it seems to
work, can someone please review it or tell me if it makes sense at all
to have this information under "capabilities"?

Thanks,
Giuseppe

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

Re: [libvirt] Migration issue php-libvirt

[Michal Novotny]

Hi,
the interesting thing is that you use constructor as
$conn = new Libvirt('connection-uri');

You should be using libvirt_connect() API instead. See [1] for example.

Not all of the defines are defined in the libvirt-php scope, that is
correct and patches are welcome :-)

Thanks,
Michal

[1]
http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=tests/test-domain-create.phpt;h=1bcccb8871fd908a0cb90cba9a390fb9e87a97f4;hb=HEAD

On 10/17/2013 11:45 AM, Claudio Bley wrote:
> [Repeat: please, don't top post on technical lists.]
>
> At Thu, 17 Oct 2013 14:26:33 +0500,
> Umar Draz wrote:
>> Here is the php_error_logs
>> --
>> *[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
>> constant VIR_MIGRATE_UNSAFE - assumed 'VIR_MIGRATE_UNSAFE' in
>> /home/www/virtspace/inc/mig.php on line 21*
>> *[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
>> constant VIR_MIGRATE_OFFLINE - assumed 'VIR_MIGRATE_OFFLINE' in
>> /home/www/virtspace/inc/mig.php on line 21*
>>
>> According to above php_notices, both VIR_OFFLINE AND VIR_UNSAFE options are
>> not available.
> Indeed, looking at the libvirt-php code
> (http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=a149662272016e946d53f4589722d090ceccdbcf;hb=HEAD),
> these constants are not defined yet. The wrapper is lacking the
> following constants, which you can easily define yourself in your PHP
> script:
>
> VIR_MIGRATE_CHANGE_PROTECTION  =  256 
> VIR_MIGRATE_UNSAFE =  512 
> VIR_MIGRATE_OFFLINE=  1024
> VIR_MIGRATE_COMPRESSED = 2048
>
>
> Claudio

-- 
Michal Novotny , RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

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

Re: [libvirt] Internal error message from netcf when trying to start libvirtd

[Daniel P. Berrange]

On Tue, Oct 15, 2013 at 06:03:27PM +0200, Christophe Fergeau wrote:
> Hey,
> 
> Due to a configuration issue on my system, libvirtd is not starting on my
> system (not complaining about this!):
> 2013-10-15 15:40:51.024+: 10222: info : libvirt version: 1.1.3
> 2013-10-15 15:40:51.024+: 10222: error : 
> virNetTLSContextCheckCertFile:117 : Cannot read CA certificate
> '/home/teuf/usr/etc/pki/CA/cacert.pem': No such file or directory
> 
> However, before libvirtd exits, I get another error message from the netcf
> code, which is unexpected this time:
> 2013-10-15 15:49:18.361+: 10222: error : netcfStateCleanup:105 :
> internal error: Attempt to close netcf state driver already closed
> 
> This message comes from the call of virStateCleanup() at the end of main()
> in libvirtd.c. virStateCleanup() should not be called before
> daemonStateInit() has been called in main.
> After this call, things get more ugly as daemonStateInit() calls
> virStateInitialize() from a thread, so there's probably a small window for
> virStateInitialize() and virStateCleanup() running concurrently if an error
> occurs between the call to daemonStateInit() and the call to
> virNetServerRun().
> 
> I'm sending this email rather than a patch as I'm not sure what is the best
> way to fix it. The easy way would be for virStateCleanup() to be a noop
> when virStateInitialize() hasn't been called (iow remove the error message
> from netcfStateCleanup). However, this would leave this small race
> condition around (which is not that bad as it would only occurs in
> situations when the daemon fails to start). So another approach would be to
> set a vir_state_initialized boolean once the thread has called
> ivrStateInitialize, and only call virStateCleanup() when it's set.
> 
> Or maybe there's a 3rd way to fix this?
> 
> Let me know if you have any guidance into the best way to fix this,

Yeah, I've known about this issue for a while and its a bit horrible
to solve. I think we probably need to have a global mutex to serialize
the virStateInitialize, virStateReload and virStateCleanup calls so
that none of them can run in parallel.

I'd have virGlobalInit create the mutex instance, since we know that
is run from thread safe context.

Then make the virState* calls hold that mutex while executing.

We don't want virStateCleanup to skip execution if virStateInitialize
has failed though - every callback in virStateCleanup should be written
to be safe if its corresponding init function hasn't run. Just the
mutex serialization ought to be sufficient I think.


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

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

Re: [libvirt] Migration issue php-libvirt

[Umar Draz]

Hi Claudio,

Thanks, its working.

Br.

Umar


On Thu, Oct 17, 2013 at 2:45 PM, Claudio Bley  wrote:

> [Repeat: please, don't top post on technical lists.]
>
> At Thu, 17 Oct 2013 14:26:33 +0500,
> Umar Draz wrote:
> >
> > Here is the php_error_logs
> > --
> > *[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
> > constant VIR_MIGRATE_UNSAFE - assumed 'VIR_MIGRATE_UNSAFE' in
> > /home/www/virtspace/inc/mig.php on line 21*
> > *[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
> > constant VIR_MIGRATE_OFFLINE - assumed 'VIR_MIGRATE_OFFLINE' in
> > /home/www/virtspace/inc/mig.php on line 21*
> >
> > According to above php_notices, both VIR_OFFLINE AND VIR_UNSAFE options
> are
> > not available.
>
> Indeed, looking at the libvirt-php code
> (
> http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=a149662272016e946d53f4589722d090ceccdbcf;hb=HEAD
> ),
> these constants are not defined yet. The wrapper is lacking the
> following constants, which you can easily define yourself in your PHP
> script:
>
> VIR_MIGRATE_CHANGE_PROTECTION=  256
> VIR_MIGRATE_UNSAFE   =  512
> VIR_MIGRATE_OFFLINE  =  1024
> VIR_MIGRATE_COMPRESSED   = 2048
>
>
> Claudio
> --
> AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
> Phone: +49 341 265 310 19
> Web:
>
> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
> Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
>



-- 
Umar Draz
Network Architect
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] PATCH: better error checking for LOCAL_PEERCRED

[Brian Candler]

On 17/10/2013 13:29, Eric Blake wrote:

On the other hand, making someone chase a URL is less convenient than
attaching the patch (ideally, sending inline the way 'git send-email'
does things is preferred, but since 'git am' can also handle
attachments, it's still easier on the maintainer to send through the
list).  That said, I went ahead and did the work for you this time around.

Thank you Eric. Sorry for making you jump through hoops!

Regards,

Brian.

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

Re: [libvirt] PATCH: better error checking for LOCAL_PEERCRED

[Eric Blake]

On 10/16/2013 10:08 AM, Brian Candler wrote:
> On 15/10/2013 12:16, Daniel P. Berrange wrote:
>> Unfortunately your patch does not apply since your mail client has
>> messed up line wrapping. Also there have been conflicting changes
>> to the code since your patch. I would fix it myself, but I don't
>> have ability to compile test code on BSD platforms.
>>
>> Can you update your patch & re-send.
> Sorry about that. Updated patch uploaded to
> https://bugzilla.redhat.com/show_bug.cgi?id=1019929 where Thunderbird
> can't mangle it.

On the other hand, making someone chase a URL is less convenient than
attaching the patch (ideally, sending inline the way 'git send-email'
does things is preferred, but since 'git am' can also handle
attachments, it's still easier on the maintainer to send through the
list).  That said, I went ahead and did the work for you this time around.

Here's what I pushed:

From aa0f09929d02ccdbf3ca9502a1fd39d90db0c690 Mon Sep 17 00:00:00 2001
From: Brian Candler 
Date: Thu, 17 Oct 2013 06:21:57 -0600
Subject: [PATCH] better error checking for LOCAL_PEERCRED

This patch improves the error checking in the LOCAL_PEERCRED version
of virNetSocketGetUNIXIdentity, used by FreeBSD and Mac OSX.

1. The error return paths now correctly unlock the socket. This is
implemented in exactly the same way as the SO_PEERCRED version,
using "goto cleanup"

2. cr.cr_ngroups is initialised to -1, and cr.cr_ngroups is checked
for negative and overlarge values.

This means that if the getsockopt() call returns success but doesn't
actually update the xucred structure, this is now caught. This
happened previously when getsockopt was called with SOL_SOCKET
instead of SOL_LOCAL, prior to commit 5a468b3, and resulted in
random uids being accepted.

Signed-off-by: Eric Blake 
---
 src/rpc/virnetsocket.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index e8cdfa6..2e50f8c 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -1174,25 +1174,27 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr
sock,
 {
 struct xucred cr;
 socklen_t cr_len = sizeof(cr);
+int ret = -1;
+
 virObjectLock(sock);

+cr.cr_ngroups = -1;
 if (getsockopt(sock->fd, VIR_SOL_PEERCRED, LOCAL_PEERCRED, &cr,
&cr_len) < 0) {
 virReportSystemError(errno, "%s",
  _("Failed to get client socket identity"));
-virObjectUnlock(sock);
-return -1;
+goto cleanup;
 }

 if (cr.cr_version != XUCRED_VERSION) {
 virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
_("Failed to get valid client socket identity"));
-return -1;
+goto cleanup;
 }

-if (cr.cr_ngroups == 0) {
+if (cr.cr_ngroups <= 0 || cr.cr_ngroups > NGROUPS) {
 virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
_("Failed to get valid client socket identity
groups"));
-return -1;
+goto cleanup;
 }

 /* PID and process creation time are not supported on BSDs */
@@ -1201,8 +1203,11 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr sock,
 *uid = cr.cr_uid;
 *gid = cr.cr_gid;

+ret = 0;
+
+cleanup:
 virObjectUnlock(sock);
-return 0;
+return ret;
 }
 #else
 int virNetSocketGetUNIXIdentity(virNetSocketPtr sock ATTRIBUTE_UNUSED,
-- 
1.8.3.1


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



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

Re: [libvirt] PATCH: better error checking for LOCAL_PEERCRED

[Daniel P. Berrange]

On Thu, Oct 17, 2013 at 06:29:14AM -0600, Eric Blake wrote:
> On 10/16/2013 10:08 AM, Brian Candler wrote:
> > On 15/10/2013 12:16, Daniel P. Berrange wrote:
> >> Unfortunately your patch does not apply since your mail client has
> >> messed up line wrapping. Also there have been conflicting changes
> >> to the code since your patch. I would fix it myself, but I don't
> >> have ability to compile test code on BSD platforms.
> >>
> >> Can you update your patch & re-send.
> > Sorry about that. Updated patch uploaded to
> > https://bugzilla.redhat.com/show_bug.cgi?id=1019929 where Thunderbird
> > can't mangle it.
> 
> On the other hand, making someone chase a URL is less convenient than
> attaching the patch (ideally, sending inline the way 'git send-email'
> does things is preferred, but since 'git am' can also handle
> attachments, it's still easier on the maintainer to send through the
> list).  That said, I went ahead and did the work for you this time around.
> 
> Here's what I pushed:
> 
> From aa0f09929d02ccdbf3ca9502a1fd39d90db0c690 Mon Sep 17 00:00:00 2001
> From: Brian Candler 
> Date: Thu, 17 Oct 2013 06:21:57 -0600
> Subject: [PATCH] better error checking for LOCAL_PEERCRED
> 
> This patch improves the error checking in the LOCAL_PEERCRED version
> of virNetSocketGetUNIXIdentity, used by FreeBSD and Mac OSX.
> 
> 1. The error return paths now correctly unlock the socket. This is
> implemented in exactly the same way as the SO_PEERCRED version,
> using "goto cleanup"
> 
> 2. cr.cr_ngroups is initialised to -1, and cr.cr_ngroups is checked
> for negative and overlarge values.
> 
> This means that if the getsockopt() call returns success but doesn't
> actually update the xucred structure, this is now caught. This
> happened previously when getsockopt was called with SOL_SOCKET
> instead of SOL_LOCAL, prior to commit 5a468b3, and resulted in
> random uids being accepted.
> 
> Signed-off-by: Eric Blake 
> ---
>  src/rpc/virnetsocket.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index e8cdfa6..2e50f8c 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -1174,25 +1174,27 @@ int virNetSocketGetUNIXIdentity(virNetSocketPtr
> sock,
>  {
>  struct xucred cr;
>  socklen_t cr_len = sizeof(cr);
> +int ret = -1;
> +
>  virObjectLock(sock);
> 
> +cr.cr_ngroups = -1;
>  if (getsockopt(sock->fd, VIR_SOL_PEERCRED, LOCAL_PEERCRED, &cr,
> &cr_len) < 0) {

Hehe, your email client is mangling line breaks too Eric :-P

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

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

Re: [libvirt] [PATCH] qemu: Avoid assigning unavailable migration ports

[Ján Tomko]

On 10/15/2013 02:42 PM, Jiri Denemark wrote:
> From: WangYufei 
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1019053
> 
> When we migrate vms concurrently, there's a chance that libvirtd on
> destination assigns the same port for different migrations, which will
> lead to migration failure during prepare phase on destination. So we use
> virPortAllocator here to solve the problem.
> 
> Signed-off-by: WangYufei 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_command.h   |  3 +++
>  src/qemu/qemu_conf.h  |  6 +++---
>  src/qemu/qemu_domain.h|  1 +
>  src/qemu/qemu_driver.c|  6 ++
>  src/qemu/qemu_migration.c | 39 +--
>  5 files changed, 38 insertions(+), 17 deletions(-)
> 

> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -160,6 +160,7 @@ struct _qemuDomainObjPrivate {
>  unsigned long migMaxBandwidth;
>  char *origname;
>  int nbdPort; /* Port used for migration with NBD */
> +int migrationPort;
>  
>  virChrdevsPtr devs;
>  

unsigned short might be nicer here.

ACK

Jan




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

Re: [libvirt] [PATCH] qemu: Make migration port range configurable

[Ján Tomko]

On 10/15/2013 03:27 PM, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu.conf | 11 +++
>  src/qemu/qemu_conf.c   | 21 +
>  src/qemu/qemu_conf.h   |  2 ++
>  src/qemu/qemu_driver.c |  4 ++--
>  src/qemu/test_libvirtd_qemu.aug.in |  2 ++
>  5 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index bf57b9c..a6d7c15 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -436,3 +436,14 @@
>  # Override the listen address for all incoming migrations. Defaults to
>  # 0.0.0.0 or :: in case if both host and qemu are capable of IPv6.
>  #migration_address = "127.0.0.1"
> +
> +
> +# Override the port range used for incoming migrations.
> +#
> +# Minimum must be greater than 0, however when QEMU is not running as root,
> +# setting the minimum to be lower than 1024 will not work.
> +#
> +# Maximum must be lower then 65535.

s/be lower then/not be greater than/

> +#
> +#migration_port_min = 49152
> +#migration_port_max = 49215

ACK

Jan



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

Re: [libvirt] [PATCH 0/6] AArch64 support for libvirt.

[Pranavkumar Sawargaonkar]

Hi Cole,

On 16 October 2013 02:34, Cole Robinson  wrote:
> On 10/08/2013 09:49 AM, Pranavkumar Sawargaonkar wrote:
>> This patchset extends libvirt for AArch64 (armv8a).
>>
>> All patches have been tested on APM X-Gene SoC and we are able
>> to run libvirtd on APM X-Gene SOC and spawn VMs remotely using
>> virsh and virt-manager.
>>
>> Pranavkumar Sawargaonkar (6):
>>   AArch64: Add AArch64 architecture to list of valid arches.
>>   AArch64: CPU Support for AArch64 (ARMv8 64bit).
>>   AArch64: Parse cputopology from /proc/cpuinfo.
>>   Implement minimal sysinfo for AArch64 platforms.
>>   Add parsing of AArch64 qemu capabilities.
>>   AArch64: Add qemu capabilities schemeta for test.
>>
>>  src/Makefile.am  |1 +
>>  src/cpu/cpu.c|2 +
>>  src/cpu/cpu_aarch64.c|   79 
>> ++
>>  src/cpu/cpu_aarch64.h|   31 ++
>>  src/nodeinfo.c   |5 +-
>>  src/qemu/qemu_capabilities.c |4 ++
>>  src/util/virarch.c   |1 +
>>  src/util/virarch.h   |1 +
>>  src/util/virsysinfo.c|3 +-
>>  tests/capabilityschemadata/caps-qemu-kvm.xml |   11 
>>  tests/sysinfodata/aarch64cpuinfo.data|   10 
>>  tests/sysinfodata/aarch64sysinfo.expect  |   10 
>>  tests/sysinfotest.c  |   14 -
>>  13 files changed, 168 insertions(+), 4 deletions(-)
>>  create mode 100644 src/cpu/cpu_aarch64.c
>>  create mode 100644 src/cpu/cpu_aarch64.h
>>  create mode 100644 tests/sysinfodata/aarch64cpuinfo.data
>>  create mode 100644 tests/sysinfodata/aarch64sysinfo.expect
>>
>
> I pushed 1-4 and 6, skipping 5 since as Dan pointed out it shouldn't be 
> required.
>
> 2 needed a small fix due to a recent libvirt refactoring.
> 6 needed a small schema addition, please make sure you have xmllint installed,
> since certain tests need it.

Thanks for pushing the patches.
Sure i will install xmllint.
>
> - Cole

Thanks,
Pranav

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

Re: [libvirt] [PATCHv2 1/2] virsh: move command cpu-baseline from domain group to host group.

[liyang]

On 2013-10-16 10:23, liyang wrote:

Signed-off-by: Li Yang
---
  tools/virsh-domain.c |  116 -
  tools/virsh-host.c   |  117 ++
  2 files changed, 117 insertions(+), 116 deletions(-)

Ping...

--
Regards,
--
Li Yang
TEL：+86+25-86630566-8529
EMail：liyang.f...@cn.fujitsu.com
--



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

Re: [libvirt] Migration issue php-libvirt

[Claudio Bley]

[Repeat: please, don't top post on technical lists.]

At Thu, 17 Oct 2013 14:26:33 +0500,
Umar Draz wrote:
> 
> Here is the php_error_logs
> --
> *[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
> constant VIR_MIGRATE_UNSAFE - assumed 'VIR_MIGRATE_UNSAFE' in
> /home/www/virtspace/inc/mig.php on line 21*
> *[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
> constant VIR_MIGRATE_OFFLINE - assumed 'VIR_MIGRATE_OFFLINE' in
> /home/www/virtspace/inc/mig.php on line 21*
> 
> According to above php_notices, both VIR_OFFLINE AND VIR_UNSAFE options are
> not available.

Indeed, looking at the libvirt-php code
(http://libvirt.org/git/?p=libvirt-php.git;a=blob;f=src/libvirt-php.c;h=a149662272016e946d53f4589722d090ceccdbcf;hb=HEAD),
these constants are not defined yet. The wrapper is lacking the
following constants, which you can easily define yourself in your PHP
script:

VIR_MIGRATE_CHANGE_PROTECTION=  256 
VIR_MIGRATE_UNSAFE   =  512 
VIR_MIGRATE_OFFLINE  =  1024
VIR_MIGRATE_COMPRESSED   = 2048


Claudio
-- 
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:

Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

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

[libvirt] libvirt VF indexing issue

[Niilona]

libvirt points to wrong VF -channel, when trying to change MAC -address.

libvirt v1 1.0.2
linux driver : ixgbe 3.17.3

Symptom description
--


1.) How the VF channels are seen in the Host ( correct in increasing
address order )

command : ls -la  /sys/bus/pci/devices/\:04\:00.0/
..

lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn0 -> ../:04:10.0/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn1 -> ../:04:10.2/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn10 -> ../:04:12.4/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn11 -> ../:04:12.6/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn12 -> ../:04:13.0/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn13 -> ../:04:13.2/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn2 -> ../:04:10.4/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn3 -> ../:04:10.6/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn4 -> ../:04:11.0/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn5 -> ../:04:11.2/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn6 -> ../:04:11.4/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn7 -> ../:04:11.6/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn8 -> ../:04:12.0/
lrwxrwxrwx  1 root root  0 Oct 17 12:25 virtfn9 -> ../:04:12.2/

---

2.) How the libvirt sees them

virsh nodedev-dumpxml pci__04_00_0

  pci__04_00_0
  pci__00_03_0
  
ixgbe
  
  
0
4
0
0
82599EB 10-Gigabit SFI/SFP+ Network
Connection
Intel Corporation

  
  
  
  
  
  
  
  
  
  
  
  
  
  

  


3.) Libvirt sets MAC -address in slot/position 5 instead of 1

ip link show eth10
87: eth10:  mtu 2100 qdisc mq
state UP mode DEFAULT qlen 1000
link/ether 00:1b:21:b9:a5:60 brd ff:ff:ff:ff:ff:ff
vf 0 MAC 92:e2:6b:53:a7:c2, spoof checking off <-- this is the
slot, MAC tried to set
vf 1 MAC 0a:04:38:2c:79:35, spoof checking off
vf 2 MAC b2:46:b6:91:44:2e, spoof checking off
vf 3 MAC de:b6:28:da:20:44, spoof checking off
vf 4 MAC 02:00:00:80:00:01, spoof checking off <- MAC sets
in wrong slot
vf 5 MAC 02:00:00:80:00:02, spoof checking off
vf 6 MAC 02:00:00:80:00:20, spoof checking off
vf 7 MAC 02:00:00:80:00:30, spoof checking off
vf 8 MAC 52:54:00:de:f8:08, spoof checking off
vf 9 MAC 5e:b0:d0:34:d6:cc, spoof checking off
vf 10 MAC d2:88:5c:2d:db:b5, spoof checking off
vf 11 MAC 6a:06:8a:9c:31:8b, spoof checking off
vf 12 MAC 4a:38:be:8f:b3:bf, spoof checking off
vf 13 MAC b6:c1:d4:12:d3:3d, spoof checking off

5.) Question : Why this happens in ixgbe -driver and when we try to
use more than 10 VF's/physical port. ?

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

[libvirt] [PATCH] remote:Fix the parameter passed to remoteDispatchConnectDomainEventDeregisterAny() should be eventID

[Wangyufei (A)]

>From 0832ab83685e20580c8128f5505096e71e747b8a Mon Sep 17 00:00:00 2001
From: zhouyimin 
Date: Thu, 17 Oct 2013 15:59:21 +0800
Subject: [PATCH] remote:Fix the parameter passed to 
remoteDispatchConnectDomainEventDeregisterAny() should be eventID

Introduced by 7b87a3
When I quit the process which only register VIR_DOMAIN_EVENT_ID_REBOOT, I got 
error like:
"libvirt: XML-RPC error : internal error: domain event 0 not registered".
Then I add the following code, it fixed.

Signed-off-by: zhouyimin 
---
 src/remote/remote_driver.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 87ef5a9..115d0bc 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5137,7 +5137,7 @@ static int 
remoteConnectDomainEventDeregisterAny(virConnectPtr conn,
 /* If that was the last callback for this eventID, we need to disable
  * events on the server */
 if (count == 0) {
-args.eventID = callbackID;
+args.eventID = eventID;
 
 if (call(conn, priv, 0, 
REMOTE_PROC_CONNECT_DOMAIN_EVENT_DEREGISTER_ANY,
  (xdrproc_t) 
xdr_remote_connect_domain_event_deregister_any_args, (char *) &args,
-- 
1.7.3.1.msysgit.0


Best Regards,
-WangYufei



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

Re: [libvirt] Migration issue php-libvirt

[Umar Draz]

Hi Claudio

Here is my PHP code

get_domain_by_name("ldap");
if ($dom==false)
{
   echo ("Domain not found\n");
   echo ("Libvirt last error: ".libvirt_get_last_error()."\n");
   exit;
}
echo ("Domain found\n");

echo ("Migrating domain to $duri\n");
$rv=libvirt_domain_migrate_to_uri($dom,$duri, VIR_MIGRATE_PEER2PEER |
VIR_MIGRATE_UNSAFE | VIR_MIGRATE_OFFLINE | VIR_MIGRATE_PERSI
ST_DEST | VIR_MIGRATE_UNDEFINE_SOURCE);
if ($rv==false)
{
   echo ("Failure!");
   echo ("Libvirt last error: ".libvirt_get_last_error()."\n");
}
else
{
   echo ("Success\n");
}


Here is the error message when I tried to migrate (powered off) domain
--
*Looking up test domain*
*Domain found*
*Migrating domain to qemu+tcp://192.168.168.14/system*
*Failure!Libvirt last error: Requested operation is not valid: domain is
not running*


And here is the error message when I tried (powered on) domain
--
*Looking up test domain*
*Domain found*
*Migrating domain to qemu+tcp://192.168.168.14/system*
*Failure!Libvirt last error: Unsafe migration: Migration may lead to data
corruption if disks use cache != none*


Here is the php_error_logs
--
*[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
constant VIR_MIGRATE_UNSAFE - assumed 'VIR_MIGRATE_UNSAFE' in
/home/www/virtspace/inc/mig.php on line 21*
*[17-Oct-2013 05:17:42 America/New_York] PHP Notice:  Use of undefined
constant VIR_MIGRATE_OFFLINE - assumed 'VIR_MIGRATE_OFFLINE' in
/home/www/virtspace/inc/mig.php on line 21*

According to above php_notices, both VIR_OFFLINE AND VIR_UNSAFE options are
not available.

*My libvirt version on kvm hosts is (102), and php-libvirt version is
(0.4.8)*

Br.

Umar


On Thu, Oct 17, 2013 at 11:14 AM, Claudio Bley  wrote:

> [Please, don't top post on technical lists.]
>
> At Thu, 17 Oct 2013 08:40:52 +0500,
> Umar Draz wrote:
> >
> > HI,
> >
> > I am using KVM
>
> Could you tell us a bit more about your setup and show your code?
> Which version of libvirt are you using?
>
> My guess would be that you don't have shared storage and are using
> the VIR_MIGRATE_NON_SHARED_DISK flag when migrating, which only works
> if the domain is running.
>
> In that case, you could start your domain in the 'paused' state,
> migrate and then destroy it.
>
> > On Tue, Oct 15, 2013 at 4:11 PM, Daniel P. Berrange  >wrote:
> >
> > > On Sun, Oct 13, 2013 at 03:20:31PM +0500, Umar Draz wrote:
> > > > Hi All
> > > >
> > > > I am trying to migrate offline domain on other URI but its not
> working
> > > >
> > > > due to this error
> > > >
> > > > Failure!Libvirt last error: Requested operation is not valid: domain
> is
> > > not
> > > > running
> > > >
> > > > I tried to use this option but not working
> > > >
> > > > VIR_MIGRATE_OFFLINE
> > > >
> > > > Please anybody help me?
> > >
> > > You don't mention what hypervisor you're using. IIRC only the QEMU
> driver
> > > supports the VIR_MIGRATE_OFFLINE flag.
> --
> AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
> Phone: +49 341 265 310 19
> Web:
>
> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
> Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
>



-- 
Umar Draz
Network Architect
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH]virsh: fix a typo in virsh-domain

[Ján Tomko]

On 10/17/2013 03:42 AM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> s/it's/its
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  tools/virsh-domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

ACK and pushed.

Jan




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