[libvirt] [PATCH] storage_backend_rbd: continue searching when failing in rbd_diff_iterate

[Chen Hanxiao]

From: Chen Hanxiao 

We try to find a snapshot that had no different between
the current state of RBD image.
If we failed in rbd_diff_iterate, just continue for the
next search iteration.

Signed-off-by: Chen Hanxiao 
---
 src/storage/storage_backend_rbd.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend_rbd.c 
b/src/storage/storage_backend_rbd.c
index 4dd4b24..2756c83 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -831,9 +831,10 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image,
 #endif
 
 if (r < 0) {
-virReportSystemError(-r, _("failed to iterate RBD snapshot %s@%s"),
- imgname, snaps[i].name);
-goto cleanup;
+VIR_DEBUG("failed to iterate RBD snapshot %s@%s,"
+  " rbd_diff return %d",
+  imgname, snaps[i].name, r);
+continue;
 }
 
 /* If diff is still set to zero we found a snapshot without deltas */
-- 
1.8.3.1


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

Re: [libvirt] [Qemu-devel] qapi DEVICE_DELETED event issued *before* instance_finalize?!

[Alex Williamson]

On Mon, 5 Sep 2016 11:36:55 +0200
Paolo Bonzini  wrote:

> On 05/09/2016 11:23, Markus Armbruster wrote:
> >> >
> >> > On the other hand, it is clearly documented that the DEVICE_DELETED
> >> > event is sent as soon as guest acknowledges completion of device
> >> > removal. So libvirt's buggy if we'd follow documentation strictly. But
> >> > then again, I don't see much information value in "guest has detached
> >> > device but qemu hasn't yet" event. Libvirt would ignore such event.  
> > Unless I'm missing something, libvirt needs an event that signals "Guest
> > and QEMU are done with this device".  Current DEVICE_DELETED isn't.
> > 
> > Can we imagine a use for current DEVICE_DELETED, i.e. "Guest is done,
> > but QEMU isn't"?
> > 
> > Would anything break if we changed semantics of DEVICE_DELETED to what
> > libvirt actually needs?
> > 
> > If the answers are "no" and "no", let's do it.  
> 
> There is a subtle aspect of this.  After the current DEVICE_DELETED, the
> device id is not used any more.  So technically you could have
> 
>device_add bar,id=foo
>device_del foo
> 
>// something in QEMU prevents the device from going away?
>// for example there is a storage issue that blocks completion
>// of a read(), and bar is a storage device
> 
>device_add bar,id=foo
>device_del foo
> 
>// which foo is being deleted?  The old one or the new one?
>event DEVICE_DELETED
> 
> DEVICE_DELETED does have a meaning: management cannot talk to the device
> anymore in QMP once it is raised.

It seems like this is just pointing out another flaw in the semantics
of DEVICE_DELETED, a device can linger without a device id, so there's
no way to reference it via QMP.  QEMU can't signal anything more about
the device, nor can the VM admin perform any further operations on it.
It's like detecting planets around distant stars, libvirt can't actually
see the device, it can only monitor the affects the device has on the
VM.  This is broken and it seems like the fix is to push both the
release of the device id and the DEVICE_DELETED notification until
after the instance_finalize callback.  Doesn't that solve the nuance
you've identified here as well?

> Technically what libvirt wants to know for VFIO is not whether the
> device is gone; it's whether the device's _backend_ (the VFIO file
> descriptor) is gone.  The device backend could have been a separate QOM
> object, but it isn't.
> 
> So perhaps we need a new event that is specific to VFIO?

This immediately sounds like the wrong path.  A) Why is this vfio
specific?  B) Without a device id, how are we going to signal an
event?  It seems that nobody actually cares about this interim event in
QEMU and releasing the device id prior to the actual device itself is
just as problematic as the premature signal itself.  Thanks,

Alex

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

Re: [libvirt] [Qemu-devel] qapi DEVICE_DELETED event issued *before* instance_finalize?!

[Laine Stump]

On 09/05/2016 05:36 AM, Paolo Bonzini wrote:


On 05/09/2016 11:23, Markus Armbruster wrote:

On the other hand, it is clearly documented that the DEVICE_DELETED
event is sent as soon as guest acknowledges completion of device
removal. So libvirt's buggy if we'd follow documentation strictly. But
then again, I don't see much information value in "guest has detached
device but qemu hasn't yet" event. Libvirt would ignore such event.

Unless I'm missing something, libvirt needs an event that signals "Guest
and QEMU are done with this device".  Current DEVICE_DELETED isn't.

Can we imagine a use for current DEVICE_DELETED, i.e. "Guest is done,
but QEMU isn't"?

Would anything break if we changed semantics of DEVICE_DELETED to what
libvirt actually needs?

If the answers are "no" and "no", let's do it.

There is a subtle aspect of this.  After the current DEVICE_DELETED, the
device id is not used any more.  So technically you could have

device_add bar,id=foo
device_del foo

// something in QEMU prevents the device from going away?
// for example there is a storage issue that blocks completion
// of a read(), and bar is a storage device

device_add bar,id=foo
device_del foo

// which foo is being deleted?  The old one or the new one?
event DEVICE_DELETED

DEVICE_DELETED does have a meaning: management cannot talk to the device
anymore in QMP once it is raised.

Technically what libvirt wants to know for VFIO is not whether the
device is gone; it's whether the device's _backend_ (the VFIO file
descriptor) is gone.  The device backend could have been a separate QOM
object, but it isn't.

So perhaps we need a new event that is specific to VFIO?


Sigh.

I always hate adding more knobs...

The original reason libvirt asked for the DEVICE_DELETED event was 
because there were cases where libvirt was attempting to re-use the 
device id when it was still in use by qemu, so attempts to attach new 
devices were failing. When it was provided we just assumed that 
"DEVICE_DELETED" meant "everybody is finished with this device, and it's 
safe to recycle all the resources now". I guess we generalized just a 
bit too much.


From libvirt's point of view, I don't see any problem with widening the 
definition of the existing DEVICE_DELETED event. But if that doesn't 
make sense from QEMU's point of view, or if anyone can come up with a 
practical reason for wanting both events, we can of course modify our 
event handling accordingly (the simplest way would be to just ignore 
DEVICE_DELETED in the case of vfio devices, and wait for the new event 
to trigger both freeing of the device ID and re-attaching the device to 
its host driver; trying to release the device ID in response to 
DEVICE_DELETED, and then re-attach the device to the host driver in 
response to a separate event would just be adding an extra layer of 
waiting for no perceptible gain).


Oh, or are you saying that for vfio devices it would have this new new 
event *instead of* DEVICE_DELETED for vfio devices? I don't really see 
the point of that...




Thanks,

Paolo

--
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] [libvirt-php] libvirt_connect not reading out credential info on 0.5.2

[Fernando Casas Schössow]

Hi again Michal,

I have more information to share.
I think I can confirm that the patches are actually fixing the 
credentials problem and they are passed along.

This is the content of test.log with libvirt-php output:

[2016-09-05 22:06:05 libvirt-php/core ]: libvirt_connect: credentials 
index 2
[2016-09-05 22:06:05 libvirt-php/core ]: libvirt_connect: credentials 
index 5
[2016-09-05 22:06:05 libvirt-php/core ]: libvirt_connect: Found 2 
elements for credentials
[2016-09-05 22:06:05 libvirt-php/core ]: 
libvirt_virConnectAuthCallback: cred 0, type 5, prompt Enter 
Administrator's password for 192.168.7.2 challenge 192.168.7.2
[2016-09-05 22:06:05 libvirt-php/core ]: 
libvirt_virConnectAuthCallback: result somepass (16)


Of course then the process is crashing and the connection is aborted so 
I believe that the patches work fine (they fix the connection 
credentials not passed along issue) but once this is fixed the other 
problem is uncovered.


I also took a trace of virsh to compare the behavior and I found that 
right after trying to open the file 
"/etc/openwsman/openwsman_client.conf" which doesn't exist, the network 
connection to the Hyper-V host starts:


open("/etc/openwsman/openwsman_client.conf", O_RDONLY) = -1 ENOENT (No 
such file or directory)

gettimeofday({1473108181, 503801}, NULL) = 0
socket(PF_INET, SOCK_DGRAM, IPPROTO_IP) = 6
ioctl(6, SIOCGIFCONF, {80, {{"lo", {AF_INET, inet_addr("127.0.0.1")}}, 
{"eth0", {AF_INET, inet_addr("192.168.0.24")) = 0
ioctl(6, SIOCGIFFLAGS, {ifr_name="lo", 
ifr_flags=IFF_UP|IFF_LOOPBACK|IFF_RUNNING}) = 0
ioctl(6, SIOCGIFFLAGS, {ifr_name="eth0", 
ifr_flags=IFF_UP|IFF_BROADCAST|IFF_RUNNING|IFF_MULTICAST}) = 0
ioctl(6, SIOCGIFHWADDR, {ifr_name="eth0", 
ifr_hwaddr=00:15:5d:07:32:03}) = 0

close(6) = 0
futex(0x7fa9b602fd48, FUTEX_WAKE_PRIVATE, 2147483647) = 0
open("/etc/openwsman/openwsman_client.conf", O_RDONLY) = -1 ENOENT (No 
such file or directory)

brk(0) = 0x7fa9b881f000
brk(0x7fa9b884) = 0x7fa9b884
clock_gettime(CLOCK_MONOTONIC, {1530071, 524608469}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 524644770}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 524680171}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 524715271}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 524753172}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 524791573}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 524823274}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 524875775}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 524976977}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 525034178}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 525103179}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 525135580}) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 525176581}) = 0
socket(PF_INET, SOCK_STREAM, IPPROTO_IP) = 6
fcntl(6, F_GETFL) = 0x2 (flags O_RDWR)
fcntl(6, F_SETFL, O_RDWR|O_NONBLOCK) = 0
clock_gettime(CLOCK_MONOTONIC, {1530071, 525334084}) = 0
connect(6, {sa_family=AF_INET, sin_port=htons(5985), 
sin_addr=inet_addr("192.168.0.2")}, 16) = -1 EINPROGRESS (Operation now 
in progress)

clock_gettime(CLOCK_MONOTONIC, {1530071, 525458286}) = 0

While in the case of libvirt-php, it will crash and abort the 
connection. If you want me to send you the full virsh trace, just let 
me know.
Also, not sure if it worth mention it and I guess you are aware that 
libvirt depends on openwsman to connect to a Hyper-V host.


Thanks in advance for your help.

Fer

On lun, sep 5, 2016 at 9:38 , Fernando Casas Schössow 
 wrote:

Thanks for your reply Michal.

Actually you don't need a Hyper-V server to reproduce the problem 
since the connection to the server is never initiated because the 
process crash before that stage (got wireshark on the network to 
confirm this).
So you should be able to reproduce the problem without a Hyper-V 
server, just using the same PHP code I'm using (or any other hyperv 
URI for that matter).


==

Following your advice I attached strace to the PHP processes and 
reproduced the issue. This is the trace result:


Process 34978 attached
Process 34979 attached
Process 34981 attached
Process 34983 attached
Process 34984 attached
Process 34986 attached
Process 34987 attached
Process 34989 attached
[pid 34989] accept(0, 
[pid 34987] wait4(-1, 
[pid 34986] accept(0, 
[pid 34981] accept(0, 
[pid 34979] wait4(-1, 
[pid 34978] wait4(-1, 
[pid 34984] wait4(-1, 
[pid 34983] accept(0,
 
[pid 34989] <... accept resumed> {sa_family=AF_LOCAL, NULL}, [2]) = 4
[pid 34989] poll([{fd=4, events=POLLIN}], 1, 5000) = 1 ([{fd=4, 
revents=POLLIN}])

[pid 34989] read(4, "\1\1\0\1\0\10\0\0", 8) = 8
[pid 34989] read(4, "\0\1\0\0\0\0\0\0", 8) = 8
[pid 34989] read(4, "\1\4\0\1\2\351\0\0", 8) = 8
[pid 34989] read(4, "\17\10SERVER_SOFTWARElighttpd\v\fSERVE"..., 745) 
= 745

[pid 34989] read(4, "\1\4\0\1\0\0\0\0", 8) = 8
[pid 34989] 

Re: [libvirt] [libvirt-php] libvirt_connect not reading out credential info on 0.5.2

[Fernando Casas Schössow]

Thanks for your reply Michal.

Actually you don't need a Hyper-V server to reproduce the problem since 
the connection to the server is never initiated because the process 
crash before that stage (got wireshark on the network to confirm this).
So you should be able to reproduce the problem without a Hyper-V 
server, just using the same PHP code I'm using (or any other hyperv URI 
for that matter).


==

Following your advice I attached strace to the PHP processes and 
reproduced the issue. This is the trace result:


Process 34978 attached
Process 34979 attached
Process 34981 attached
Process 34983 attached
Process 34984 attached
Process 34986 attached
Process 34987 attached
Process 34989 attached
[pid 34989] accept(0, 
[pid 34987] wait4(-1, 
[pid 34986] accept(0, 
[pid 34981] accept(0, 
[pid 34979] wait4(-1, 
[pid 34978] wait4(-1, 
[pid 34984] wait4(-1, 
[pid 34983] accept(0,

[pid 34989] <... accept resumed> {sa_family=AF_LOCAL, NULL}, [2]) = 4
[pid 34989] poll([{fd=4, events=POLLIN}], 1, 5000) = 1 ([{fd=4, 
revents=POLLIN}])

[pid 34989] read(4, "\1\1\0\1\0\10\0\0", 8) = 8
[pid 34989] read(4, "\0\1\0\0\0\0\0\0", 8) = 8
[pid 34989] read(4, "\1\4\0\1\2\351\0\0", 8) = 8
[pid 34989] read(4, "\17\10SERVER_SOFTWARElighttpd\v\fSERVE"..., 745) = 
745

[pid 34989] read(4, "\1\4\0\1\0\0\0\0", 8) = 8
[pid 34989] lstat("/storage/lighttpd/wwwroot/libvirt.php", 
{st_mode=S_IFREG|0755, st_size=633, ...}) = 0
[pid 34989] lstat("/storage/lighttpd/wwwroot", {st_mode=S_IFDIR|0755, 
st_size=4096, ...}) = 0
[pid 34989] lstat("/storage/lighttpd", {st_mode=S_IFDIR|0755, 
st_size=0, ...}) = 0
[pid 34989] lstat("/storage", {st_mode=S_IFDIR|0755, st_size=31, ...}) 
= 0

[pid 34989] gettimeofday({1473102621, 318809}, NULL) = 0
[pid 34989] stat("/storage/lighttpd/wwwroot/.user.ini", 0x7ffc541fcbe0) 
= -1 ENOENT (No such file or directory)
[pid 34989] setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={60, 
0}}, NULL) = 0
[pid 34989] rt_sigaction(SIGPROF, {0x7f9772bbb970, [PROF], 
SA_RESTORER|SA_RESTART, 0x7f976f07b670}, {SIG_DFL, [], 0}, 8) = 0

[pid 34989] rt_sigprocmask(SIG_UNBLOCK, [PROF], NULL, 8) = 0
[pid 34989] open("/storage/lighttpd/wwwroot/libvirt.php", O_RDONLY) = 5
[pid 34989] fstat(5, {st_mode=S_IFREG|0755, st_size=633, ...}) = 0
[pid 34989] fstat(5, {st_mode=S_IFREG|0755, st_size=633, ...}) = 0
[pid 34989] fstat(5, {st_mode=S_IFREG|0755, st_size=633, ...}) = 0
[pid 34989] mmap(NULL, 633, PROT_READ, MAP_SHARED, 5, 0) = 
0x7f9772959000

[pid 34989] getcwd("/usr/bin", 4095) = 9
[pid 34989] chdir("/storage/lighttpd/wwwroot") = 0
[pid 34989] setitimer(ITIMER_PROF, {it_interval={0, 0}, 
it_value={14400, 0}}, NULL) = 0

[pid 34989] munmap(0x7f9772959000, 633) = 0
[pid 34989] close(5) = 0
[pid 34989] unlink("test.log") = 0
[pid 34989] access("test.log", F_OK) = -1 ENOENT (No such file or 
directory)

[pid 34989] open("test.log", O_WRONLY|O_CREAT|O_APPEND, 0666) = 5
[pid 34989] lseek(5, 0, SEEK_END) = 0
[pid 34989] dup3(5, 2, 0) = 2
[pid 34989] close(5) = 0
[pid 34989] stat("/etc/localtime", {st_mode=S_IFREG|0644, st_size=2593, 
...}) = 0

[pid 34989] fstat(2, {st_mode=S_IFREG|0755, st_size=0, ...}) = 0
[pid 34989] mmap(NULL, 16384, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f9772956000

[pid 34989] write(2, "[2016-09-05 21:10:21 libvirt-php"..., 80) = 80
[pid 34989] stat("/etc/localtime", {st_mode=S_IFREG|0644, st_size=2593, 
...}) = 0

[pid 34989] write(2, "[2016-09-05 21:10:21 libvirt-php"..., 80) = 80
[pid 34989] stat("/etc/localtime", {st_mode=S_IFREG|0644, st_size=2593, 
...}) = 0

[pid 34989] write(2, "[2016-09-05 21:10:21 libvirt-php"..., 93) = 93
[pid 34989] futex(0x7f976ae3cb18, FUTEX_WAKE_PRIVATE, 2147483647) = 0
[pid 34989] futex(0x7f976ae3d078, FUTEX_WAKE_PRIVATE, 2147483647) = 0
[pid 34989] geteuid() = 997
[pid 34989] geteuid() = 997
[pid 34989] socket(PF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0) 
= 5
[pid 34989] connect(5, {sa_family=AF_LOCAL, 
sun_path="/var/run/nscd/socket"}, 110) = -1 ENOENT (No such file or 
directory)

[pid 34989] close(5) = 0
[pid 34989] socket(PF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, 0) 
= 5
[pid 34989] connect(5, {sa_family=AF_LOCAL, 
sun_path="/var/run/nscd/socket"}, 110) = -1 ENOENT (No such file or 
directory)

[pid 34989] close(5) = 0
[pid 34989] open("/etc/passwd", O_RDONLY|O_CLOEXEC) = 5
[pid 34989] fstat(5, {st_mode=S_IFREG|0644, st_size=1998, ...}) = 0
[pid 34989] mmap(NULL, 4096, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f9772955000

[pid 34989] read(5, "root:x:0:0:root:/root:/bin/bash\n"..., 4096) = 1998
[pid 34989] close(5) = 0
[pid 34989] munmap(0x7f9772955000, 4096) = 0
[pid 34989] access("/var/www/lighttpd/.config/libvirt/libvirt.conf", 
F_OK) = -1 ENOENT (No such file or directory)

[pid 34989] geteuid() = 997
[pid 34989] open("/etc/passwd", O_RDONLY|O_CLOEXEC) = 5
[pid 34989] fstat(5, 

Re: [libvirt] virt-admin commands aliases

[Daniel P. Berrange]

On Mon, Sep 05, 2016 at 05:37:07PM +0200, Erik Skultety wrote:
> Hi there,
> 
> after my presentation at KVM Forum, it was pointed out from the audience
> that we might think about doing something about the naming of the
> virt-admin's comands, since there is some sort of inconsistency: srv-
> vs. client- vs. dmn- (not merged yet). When I sent patches to upstream I
> already knew that the naming was not optimal, but I didn't come up with
> anything better so I hoped that the reviewer might think of something
> better which unfortunately did not happen.
> 
> Anyway, there are multiple options how this can be approached but I'm
> not 100% satisfied with neither of them:
> 
> 1) rename the commands completely
> Although clean, obviously this is the non-preferred option because this
> would break any backwards compatibility however, I think there is a fair
> chance that people haven't actually started using it yet (although that
> might change between 7.3 and 7.4).
> 
> 2) create aliases for non-abbreviated forms of the commands
> That way, srv- would become server- and dmn- would become daemon-.
> However, by doing this we'll end up with 6 almost identical entries in
> the commands structure which might be error-prone once we decide to
> add/create a flag to the command primitive, since the flag would
> have to be added both to the alias and to the original (unlikely, but
> possible that someone might forget about that)
> 
> 3) abbreviate client- to something like clnt-
> Identical to the above except for the amount of duplicate entries which
> would be reduced to 2
> 
> 4) leave it as is if such a consensus is reached and accepted
> I guess this does no need any additional comments.

I just vote for 4.

In retrospect it would have been nice to use 'server' instead of
'srv', but ultimately it isn't a functional problem.  The "solutions"
create extra code and/or inconsitency and/or break back-compat so just
aren't worth it IMHO.

IOW, admit 'srv' sucks but don't change it, and ensure new server
commands continue to use 'srv' for consistency.

We can of couse use 'daemon-' as prefix for new commands, since we
have not yet released any versions using 'dmn-' as prefix


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

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

Re: [libvirt] virt-admin commands aliases

[Daniel P. Berrange]

On Mon, Sep 05, 2016 at 07:40:30PM +0200, Andrea Bolognani wrote:
> On Mon, 2016-09-05 at 17:37 +0200, Erik Skultety wrote:
> > Hi there,
> > 
> > after my presentation at KVM Forum, it was pointed out from the audience
> 
> Disclaimer: I'm the audience in question :)

There's always one trouble-maker in the audience ;-P

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

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

Re: [libvirt] virt-admin commands aliases

[Andrea Bolognani]

On Mon, 2016-09-05 at 17:37 +0200, Erik Skultety wrote:
> Hi there,
> 
> after my presentation at KVM Forum, it was pointed out from the audience

Disclaimer: I'm the audience in question :)

> that we might think about doing something about the naming of the
> virt-admin's comands, since there is some sort of inconsistency: srv-
> vs. client- vs. dmn- (not merged yet). When I sent patches to upstream I
> already knew that the naming was not optimal, but I didn't come up with
> anything better so I hoped that the reviewer might think of something
> better which unfortunately did not happen.

I'm sorry for not paying enough attention at the time and
for not raising the issue while the series was still undergoing
review. If I had, we wouldn't need to have this discussion :(

> Anyway, there are multiple options how this can be approached but I'm
> not 100% satisfied with neither of them:
> 
> 1) rename the commands completely
> Although clean, obviously this is the non-preferred option because this
> would break any backwards compatibility however, I think there is a fair
> chance that people haven't actually started using it yet (although that
> might change between 7.3 and 7.4).

This is very tempting, but I'm not sure we can actually get
away with it.

> 2) create aliases for non-abbreviated forms of the commands
> That way, srv- would become server- and dmn- would become daemon-.
> However, by doing this we'll end up with 6 almost identical entries in
> the commands structure which might be error-prone once we decide to
> add/create a flag to the command primitive, since the flag would
> have to be added both to the alias and to the original (unlikely, but
> possible that someone might forget about that)

This seems like a fair solution, but note that I haven't
looked at the patch implementing it yet - I might change my
mind once I did that ;)

> 3) abbreviate client- to something like clnt-
> Identical to the above except for the amount of duplicate entries which
> would be reduced to 2

I feel very strongly against this option.

Not only "clnt" is four letters long, as opposed to the three
letters of both "srv" and "dmn", I also think both "clnt" and
"dmn" are absolutely unsightly and have no place in a private
API - much less in a public one.

Not to mention that we will probably end up adding more
entitites, that will have to be shortened in the same way,
quite possibly with suboptimal results.

> 4) leave it as is if such a consensus is reached and accepted
> I guess this does no need any additional comments.

I feel *even more strongly* about this one :)

With all I've written above agains "clnt" and "dmn", I'd
rather have those instead of the current inconsistent naming
convention.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

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

[Maxim Nestratov]

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

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

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

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

Signed-off-by: Maxim Nestratov 
---
 src/util/virclosecallbacks.c | 3 +++
 1 file changed, 3 insertions(+)

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

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

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

[Maxim Nestratov]

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

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

-- 
2.4.11

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

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

[Maxim Nestratov]

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

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

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

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

[Maxim Nestratov]

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

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

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

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

[Maxim Nestratov]

From: Yuri Pudgorodskiy 

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

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

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

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

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

[Maxim Nestratov]

Signed-off-by: Maxim Nestratov 
---
 src/vz/vz_driver.c | 1 +
 1 file changed, 1 insertion(+)

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

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

[libvirt] [PATCH 2/2] util: storage: Add json pseudo protocol support for legacy RBD strings

[Peter Krempa]

RBD in qemu still uses only the legacy 'filename' syntax.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1371758
---
 src/util/virstoragefile.c | 23 +++
 tests/virstoragetest.c|  6 ++
 2 files changed, 29 insertions(+)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 02cae66..41827f0 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2940,6 +2940,28 @@ virStorageSourceParseBackingJSONSSH(virStorageSourcePtr 
src,
 }


+static int
+virStorageSourceParseBackingJSONRBD(virStorageSourcePtr src,
+virJSONValuePtr json,
+int opaque ATTRIBUTE_UNUSED)
+{
+const char *filename;
+
+src->type = VIR_STORAGE_TYPE_NETWORK;
+src->protocol = VIR_STORAGE_NET_PROTOCOL_RBD;
+
+/* legacy syntax passed via 'filename' option */
+if ((filename = virJSONValueObjectGetString(json, "filename")))
+return virStorageSourceParseRBDColonString(filename, src);
+
+/* RBD currently supports only URI syntax passed in as filename */
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("missing RBD filename in JSON backing volume 
definition"));
+
+return -1;
+}
+
+
 struct virStorageSourceJSONDriverParser {
 const char *drvname;
 int (*func)(virStorageSourcePtr src, virJSONValuePtr json, int opaque);
@@ -2960,6 +2982,7 @@ static const struct virStorageSourceJSONDriverParser 
jsonParsers[] = {
 {"nbd", virStorageSourceParseBackingJSONNbd, 0},
 {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0},
 {"ssh", virStorageSourceParseBackingJSONSSH, 0},
+{"rbd", virStorageSourceParseBackingJSONRBD, 0},
 };


diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index fd79abb..f766df1 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1486,6 +1486,12 @@ mymain(void)
"\n"
"  \n"
"\n");
+TEST_BACKING_PARSE("json:{\"file.driver\":\"rbd\","
+ 
"\"file.filename\":\"rbd:testshare:id=asdf:mon_host=example.com\""
+"}",
+   "\n"
+   "  \n"
+   "\n");

  cleanup:
 /* Final cleanup */
-- 
2.9.2

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

[libvirt] [PATCH 0/2] util: storage: Fixes for the JSON pseudo protocol parser

[Peter Krempa]

Gluster protocol type was not set properly and the RBD protocol was missing.

Peter Krempa (2):
  util: storage: Properly set protocol type when parsing gluster json
string
  util: storage: Add json pseudo protocol support for legacy RBD strings

 src/util/virstoragefile.c | 26 ++
 tests/virstoragetest.c| 10 --
 2 files changed, 34 insertions(+), 2 deletions(-)

-- 
2.9.2

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

[libvirt] [PATCH 1/2] util: storage: Properly set protocol type when parsing gluster json string

[Peter Krempa]

Commit 2ed772cd forgot to set proper protocol. This was also present in
the test data.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1372251
---
 src/util/virstoragefile.c | 3 +++
 tests/virstoragetest.c| 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index feeb061..02cae66 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2786,6 +2786,9 @@ 
virStorageSourceParseBackingJSONGluster(virStorageSourcePtr src,
 return -1;
 }

+src->type = VIR_STORAGE_TYPE_NETWORK;
+src->protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER;
+
 if (VIR_STRDUP(src->volume, volume) < 0 ||
 virAsprintf(>path, "/%s", path) < 0)
 return -1;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index f7f5030..fd79abb 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1412,7 +1412,7 @@ mymain(void)
   "]"
   "}"
  "}",
-"\n"
+"\n"
 "  \n"
 "  \n"
 "  \n"
@@ -1432,7 +1432,7 @@ mymain(void)
"}"
  "]"
 "}",
-"\n"
+"\n"
 "  \n"
 "  \n"
 "  \n"
-- 
2.9.2

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

[libvirt] [PATCH 1/2] qemu: domain: Clear startup policy for dropped removable media

[Peter Krempa]

When a source image is dropped when missing due to startup policy the
policy needs to be cleared since it was relevant only for the given
storage source. New sources need to update it if needed.
---
 src/qemu/qemu_domain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 7fce2fc..37b7b44 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4334,6 +4334,8 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver,
disk->info.alias,

VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START);
 ignore_value(virDomainDiskSetSource(disk, NULL));
+/* keeping the old startup policy would be invalid for new images */
+disk->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_DEFAULT;
 } else {
 event = virDomainEventDiskChangeNewFromObj(vm, src, NULL,
disk->info.alias,
-- 
2.9.2

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

[libvirt] [PATCH 0/2] qemu: hotplug: Fix two issues with drives made empty by using starup policy

[Peter Krempa]

If a cdrom drive is emptied by 'requisite' startup policy the policy setting 
would
not be cleared and due to a bug in qemu the cdrom image could not be changed.

The first patch clears the startup policy setting and the second one fixes
ejection of cdroms if --force is specified.

Also I've reported the issue in qemu 
https://bugzilla.redhat.com/show_bug.cgi?id=1373264

Peter Krempa (2):
  qemu: domain: Clear startup policy for dropped removable media
  qemu: hotplug: Don't wait if cdrom tray is opened forcibly

 src/qemu/qemu_domain.c  | 2 ++
 src/qemu/qemu_hotplug.c | 9 -
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.9.2

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

[libvirt] [PATCH 2/2] qemu: hotplug: Don't wait if cdrom tray is opened forcibly

[Peter Krempa]

Qemu always opens the tray if forced to. Skip the waiting step in such
case.

This also helps if qemu does not report the tray change event when
opening the cdrom forcibly (the documentation says that the event will
not be sent although qemu in fact does trigger it even if @force is
selceted).

This is a workaround for a qemu issue where qemu does not send the tray
change event in some cases (after migration with empty closed locked
drive) and thus renders the cdrom useless from libvirt's point of view.

Partially resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1368368
---
 src/qemu/qemu_hotplug.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d13474a..48108fb 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -149,8 +149,7 @@ static int
 qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
 virDomainDiskDefPtr disk,
-const char *driveAlias,
-bool force)
+const char *driveAlias)
 {
 unsigned long long now;
 int rc;
@@ -175,7 +174,7 @@ qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,

 /* re-issue ejection command to pop out the media */
 qemuDomainObjEnterMonitor(driver, vm);
-rc = qemuMonitorEjectMedia(qemuDomainGetMonitor(vm), driveAlias, force);
+rc = qemuMonitorEjectMedia(qemuDomainGetMonitor(vm), driveAlias, false);
 if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
 return -1;

@@ -238,9 +237,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 goto cleanup;

 /* If the tray is present and tray change event is supported wait for it 
to open. */
-if (diskPriv->tray &&
+if (!force && diskPriv->tray &&
 virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_TRAY_MOVED)) {
-rc = qemuHotplugWaitForTrayEject(driver, vm, disk, driveAlias, force);
+rc = qemuHotplugWaitForTrayEject(driver, vm, disk, driveAlias);
 if (rc < 0)
 goto error;
 } else  {
-- 
2.9.2

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

Re: [libvirt] [libvirt-glib/libvirt-gconfig 06/17] gconfig: Implement gvir_config_domain_graphics_spice_get_tls_port()

[Christophe Fergeau]

On Thu, Apr 21, 2016 at 12:12:19PM +0200, Christophe Fergeau wrote:
> For patches up to this one:
> 
> Acked-by: Christophe Fergeau 

On second thought, the API added by the patches before this one becomes
redundant and deprecated once the Remote/Local classes are introduced,
so adding these now does not sound very useful.

Christophe


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

[libvirt] virt-admin commands aliases

[Erik Skultety]

Hi there,

after my presentation at KVM Forum, it was pointed out from the audience
that we might think about doing something about the naming of the
virt-admin's comands, since there is some sort of inconsistency: srv-
vs. client- vs. dmn- (not merged yet). When I sent patches to upstream I
already knew that the naming was not optimal, but I didn't come up with
anything better so I hoped that the reviewer might think of something
better which unfortunately did not happen.

Anyway, there are multiple options how this can be approached but I'm
not 100% satisfied with neither of them:

1) rename the commands completely
Although clean, obviously this is the non-preferred option because this
would break any backwards compatibility however, I think there is a fair
chance that people haven't actually started using it yet (although that
might change between 7.3 and 7.4).

2) create aliases for non-abbreviated forms of the commands
That way, srv- would become server- and dmn- would become daemon-.
However, by doing this we'll end up with 6 almost identical entries in
the commands structure which might be error-prone once we decide to
add/create a flag to the command primitive, since the flag would
have to be added both to the alias and to the original (unlikely, but
possible that someone might forget about that)

3) abbreviate client- to something like clnt-
Identical to the above except for the amount of duplicate entries which
would be reduced to 2

4) leave it as is if such a consensus is reached and accepted
I guess this does no need any additional comments.

Thanks for any ideas.

Erik

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

Re: [libvirt] [PATCH v2 0/3] virsh: Option completers and small improvements/fixes for autocomplete

[Michal Privoznik]

On 05.09.2016 11:45, Nishith Shah wrote:
> This series introduces option completers and adds some minor improvements
> and fixes(not bugs per se, just better/sane behavior) in vshReadlineParse.
> 
> The first patch introduces the usage of option completers to auto-complete
> arguments for a particular option.
> 
> The second and third patches provide small improvements like completing
> the options of type VSH_OT_ARGV or VSH_OT_DATA, and to complete multiple
> options as well, if a previous option requires an argument, and that
> argument has been provided.
> 
> ---
> v2: Fix an infinite while loop bug
> 
> v1: https://www.redhat.com/archives/libvir-list/2016-August/msg01009.html
> 
> Nishith Shah (3):
>   virsh: Introduce usage of option completers to auto-complete arguments
>   virsh: Allow data or argument options to be completed as well
>   virsh: Complete multiple options when any one option requires data
> 
>  tools/vsh.c | 75 
> -
>  1 file changed, 49 insertions(+), 26 deletions(-)
> 

ACKed and pushed. Thanks for fixing that bug.

Michal

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

Re: [libvirt] [PATCH] Prefix major/minor with gnu_dev_

[Daniel P. Berrange]

On Mon, Sep 05, 2016 at 03:33:36PM +0200, Michal Privoznik wrote:
> In the latest glibc, major() and minor() functions are marked as
> deprecated (glibc commit dbab6577):
> 
>   CC   util/libvirt_util_la-vircgroup.lo
> util/vircgroup.c: In function 'virCgroupGetBlockDevString':
> util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated:
>   In the GNU C Library, `major' is defined by .
>   For historical compatibility, it is currently defined by
>as well, but we plan to remove this soon.
>   To use `major', include  directly.
>   If you did not intend to use a system-defined macro `major',
>   you should #undef it after including .
>   [-Werror=deprecated-declarations]
>  if (virAsprintf(, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 
> 0)
>  ^~
> In file included from /usr/include/features.h:397:0,
>  from /usr/include/bits/libc-header-start.h:33,
>  from /usr/include/stdio.h:28,
>  from ../gnulib/lib/stdio.h:43,
>  from util/vircgroup.c:26:
> /usr/include/sys/sysmacros.h:87:1: note: declared here
>  __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)
>  ^
> 
> Applications are supposed to use gnu_dev_major() or
> gnu_dev_minor() respectively.

How does this work on non-gnu systems like *BSD / OS-X / Mingw[1] ?


Regards,
Daniel

[1] Presumably our mingw setup doesn't build the files affected ?
-- 
|: 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] Call for mentors: Outreachy December - March

[Stefan Hajnoczi]

On Mon, Sep 5, 2016 at 9:39 AM, Stefan Hajnoczi  wrote:
> The Outreachy open source internship programme is running a December -
> March round.  Outreachy promotes participation of underrepresented
> groups in open source.
>
> If you would like to be a mentor please reply.

Project ideas can be proposed here:
http://qemu-project.org/Outreachy_2016_DecemberMarch

Stefan

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

[libvirt] Call for mentors: Outreachy December - March

[Stefan Hajnoczi]

The Outreachy open source internship programme is running a December -
March round.  Outreachy promotes participation of underrepresented
groups in open source.

If you would like to be a mentor please reply.

What is a mentor?
Mentors help interns with their project and evaluate their progress.
The mentor is the point of contact for the intern and advises them on
the development process, technical challenges, etc.  Mentors propose a
list of project ideas which they are willing to mentor at the start of
the application phase.

What are the requirements for mentors?
You must be a regular contributor to QEMU, KVM, or libvirt.  You must
be willing to spend ~5 hours/week between 6 Dec - 6 Mar.

Timeline:
Internship candidates apply: 12 Sep - 17 Oct
Coding period: 6 Dec - 6 Mar

Outreachy website: https://www.gnome.org/outreachy/
Overview of open source internship programmes:
http://vmsplice.net/~stefan/stefanha-kvm-forum-2016.pdf

Please let me know if you have any questions.

Stefan

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

[libvirt] [PATCH] Prefix major/minor with gnu_dev_

[Michal Privoznik]

In the latest glibc, major() and minor() functions are marked as
deprecated (glibc commit dbab6577):

  CC   util/libvirt_util_la-vircgroup.lo
util/vircgroup.c: In function 'virCgroupGetBlockDevString':
util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated:
  In the GNU C Library, `major' is defined by .
  For historical compatibility, it is currently defined by
   as well, but we plan to remove this soon.
  To use `major', include  directly.
  If you did not intend to use a system-defined macro `major',
  you should #undef it after including .
  [-Werror=deprecated-declarations]
 if (virAsprintf(, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0)
 ^~
In file included from /usr/include/features.h:397:0,
 from /usr/include/bits/libc-header-start.h:33,
 from /usr/include/stdio.h:28,
 from ../gnulib/lib/stdio.h:43,
 from util/vircgroup.c:26:
/usr/include/sys/sysmacros.h:87:1: note: declared here
 __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)
 ^

Applications are supposed to use gnu_dev_major() or
gnu_dev_minor() respectively.

Signed-off-by: Michal Privoznik 
---
 src/conf/domain_audit.c  |  4 ++--
 src/lxc/lxc_controller.c | 10 +-
 src/lxc/lxc_driver.c | 26 +-
 src/util/vircgroup.c | 10 +-
 src/util/virutil.c   |  6 +++---
 tests/vircgroupmock.c| 16 
 6 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 53a58ac..d0bfa58 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -52,8 +52,8 @@ virDomainAuditGetRdev(const char *path)
 
 if (stat(path, ) == 0 &&
 (S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode))) {
-int maj = major(sb.st_rdev);
-int min = minor(sb.st_rdev);
+int maj = gnu_dev_major(sb.st_rdev);
+int min = gnu_dev_minor(sb.st_rdev);
 ignore_value(virAsprintfQuiet(, "%02X:%02X", maj, min));
 }
 return ret;
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 825b4d4..3bf7eb2 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1520,7 +1520,7 @@ static int 
virLXCControllerPopulateDevices(virLXCControllerPtr ctrl)
 LXC_STATE_DIR, ctrl->def->name, devs[i].path) < 0)
 goto cleanup;
 
-dev_t dev = makedev(devs[i].maj, devs[i].min);
+dev_t dev = gnu_dev_makedev(devs[i].maj, devs[i].min);
 if (mknod(path, S_IFCHR, dev) < 0 ||
 chmod(path, devs[i].mode)) {
 virReportSystemError(errno,
@@ -1592,7 +1592,7 @@ virLXCControllerSetupHostdevSubsysUSB(virDomainDefPtr 
vmDef,
 }
 
 VIR_DEBUG("Creating dev %s (%d,%d)",
-  dstfile, major(sb.st_rdev), minor(sb.st_rdev));
+  dstfile, gnu_dev_major(sb.st_rdev), gnu_dev_minor(sb.st_rdev));
 if (mknod(dstfile, mode, sb.st_rdev) < 0) {
 virReportSystemError(errno,
  _("Unable to create device %s"),
@@ -1672,7 +1672,7 @@ virLXCControllerSetupHostdevCapsStorage(virDomainDefPtr 
vmDef,
 mode = 0700 | S_IFBLK;
 
 VIR_DEBUG("Creating dev %s (%d,%d)", dst,
-  major(sb.st_rdev), minor(sb.st_rdev));
+  gnu_dev_major(sb.st_rdev), gnu_dev_minor(sb.st_rdev));
 if (mknod(dst, mode, sb.st_rdev) < 0) {
 virReportSystemError(errno,
  _("Unable to create device %s"),
@@ -1751,7 +1751,7 @@ virLXCControllerSetupHostdevCapsMisc(virDomainDefPtr 
vmDef,
 mode = 0700 | S_IFCHR;
 
 VIR_DEBUG("Creating dev %s (%d,%d)", dst,
-  major(sb.st_rdev), minor(sb.st_rdev));
+  gnu_dev_major(sb.st_rdev), gnu_dev_minor(sb.st_rdev));
 if (mknod(dst, mode, sb.st_rdev) < 0) {
 virReportSystemError(errno,
  _("Unable to create device %s"),
@@ -1911,7 +1911,7 @@ static int virLXCControllerSetupDisk(virLXCControllerPtr 
ctrl,
  * to that normally implied by the device name
  */
 VIR_DEBUG("Creating dev %s (%d,%d) from %s",
-  dst, major(sb.st_rdev), minor(sb.st_rdev), tmpsrc);
+  dst, gnu_dev_major(sb.st_rdev), gnu_dev_minor(sb.st_rdev), 
tmpsrc);
 if (mknod(dst, mode, sb.st_rdev) < 0) {
 virReportSystemError(errno,
  _("Unable to create device %s"),
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index da98b38..3fe79fd 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3714,7 +3714,7 @@ lxcDomainAttachDeviceMknodHelper(pid_t pid 
ATTRIBUTE_UNUSED,
  * to that normally implied by the device name
  */
 VIR_DEBUG("Creating dev %s (%d,%d)",
-  data->file, major(data->dev), minor(data->dev));
+  data->file, gnu_dev_major(data->dev), gnu_dev_minor(data->dev));
 if (mknod(data->file, data->mode, data->dev) < 0) {
 

Re: [libvirt] [libvirt-perl][PATCH 0/2] Add more PERF_PARAM_* constants

[Daniel P. Berrange]

On Sun, Sep 04, 2016 at 10:53:12PM +0200, Michal Privoznik wrote:
> Now that libvirt-2.2.0 is out, we should:
> a) do the libvirt-perl release too
> b) adapt to new constants pushed to 2.3.0
> 
> Michal Privoznik (2):
>   Bump version to 2.3.0
>   Add more PERF_PARAM_* constants
> 
>  Changes|  5 +
>  Makefile.PL|  2 +-
>  README |  2 +-
>  Virt.xs|  4 
>  lib/Sys/Virt.pm|  2 +-
>  lib/Sys/Virt/Domain.pm | 27 +++
>  6 files changed, 39 insertions(+), 3 deletions(-)

ACK, I've just pushed changes for the 2.2.0 release, so you can
go ahead and push this series.

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

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

Re: [libvirt] [PATCH 0/2] Report block jobs more wisely

[Kashyap Chamarthy]

On Mon, Sep 05, 2016 at 01:48:21PM +0200, Michal Privoznik wrote:
> There's been a discussion recently (even here on the list [1])
> that Nova is sometimes unable to fetch block job stats properly.
> Basically, we may report job.cur == job.end == 0 in some cases
> tricking Nova into thinking job is done when in fact it hasn't
> even started yet.
> 
> Michal Privoznik (2):
>   qemuDomainGetBlockJobInfo: Move info translation into separate func
>   virDomainGetBlockJobInfo: Fix corner case when qemu reports no info
> 
>  src/libvirt-domain.c   |  7 ++
>  src/qemu/qemu_driver.c | 58 
> --
>  2 files changed, 49 insertions(+), 16 deletions(-)

I've tested the obvious case and reported it in this thread:

http://www.redhat.com/archives/libvir-list/2016-September/msg00042.html

FWIW:

Tested-by: Kashyap Chamarthy 


-- 
/kashyap

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

[libvirt] [PATCH 0/2] Report block jobs more wisely

[Michal Privoznik]

There's been a discussion recently (even here on the list [1])
that Nova is sometimes unable to fetch block job stats properly.
Basically, we may report job.cur == job.end == 0 in some cases
tricking Nova into thinking job is done when in fact it hasn't
even started yet.

Michal Privoznik (2):
  qemuDomainGetBlockJobInfo: Move info translation into separate func
  virDomainGetBlockJobInfo: Fix corner case when qemu reports no info

 src/libvirt-domain.c   |  7 ++
 src/qemu/qemu_driver.c | 58 --
 2 files changed, 49 insertions(+), 16 deletions(-)

-- 
2.8.4

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

[libvirt] [PATCH 1/2] qemuDomainGetBlockJobInfo: Move info translation into separate func

[Michal Privoznik]

Even though we merely just pass to users whatever qemu provided
on the monitor, we still do some translation. For instance we
turn bytes into mebibytes, or fix job type if needed. However, in
the future there is more fixing to be done so this code deserves
its own function.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 46 ++
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 807e06d..4535eb8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16330,6 +16330,34 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
 
 
 static int
+qemuBlockJobInfoTranslate(qemuMonitorBlockJobInfoPtr rawInfo,
+  virDomainBlockJobInfoPtr info,
+  virDomainDiskDefPtr disk,
+  bool reportBytes)
+{
+info->cur = rawInfo->cur;
+info->end = rawInfo->end;
+
+info->type = rawInfo->type;
+if (info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
+disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+info->type = disk->mirrorJob;
+
+if (rawInfo->bandwidth && !reportBytes)
+rawInfo->bandwidth = VIR_DIV_UP(rawInfo->bandwidth, 1024 * 1024);
+info->bandwidth = rawInfo->bandwidth;
+if (info->bandwidth != rawInfo->bandwidth) {
+virReportError(VIR_ERR_OVERFLOW,
+   _("bandwidth %llu cannot be represented in result"),
+   rawInfo->bandwidth);
+return -1;
+}
+
+return 0;
+}
+
+
+static int
 qemuDomainGetBlockJobInfo(virDomainPtr dom,
   const char *path,
   virDomainBlockJobInfoPtr info,
@@ -16376,22 +16404,8 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
 if (ret <= 0)
 goto endjob;
 
-info->cur = rawInfo.cur;
-info->end = rawInfo.end;
-
-info->type = rawInfo.type;
-if (info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
-disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
-info->type = disk->mirrorJob;
-
-if (rawInfo.bandwidth &&
-!(flags & VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES))
-rawInfo.bandwidth = VIR_DIV_UP(rawInfo.bandwidth, 1024 * 1024);
-info->bandwidth = rawInfo.bandwidth;
-if (info->bandwidth != rawInfo.bandwidth) {
-virReportError(VIR_ERR_OVERFLOW,
-   _("bandwidth %llu cannot be represented in result"),
-   rawInfo.bandwidth);
+if (qemuBlockJobInfoTranslate(, info, disk,
+  flags & 
VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES) < 0) {
 ret = -1;
 goto endjob;
 }
-- 
2.8.4

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

[libvirt] [PATCH 2/2] virDomainGetBlockJobInfo: Fix corner case when qemu reports no info

[Michal Privoznik]

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

Apparently, some management applications use the following code
pattern when waiting for a block job to finish:

  while (1) {
virDomainGetBlockJobInfo(dom, disk, info, flags);

if (info.cur == info.end)
break;

sleep(1);
  }

Problem with this approach is in its corner cases. In case of
QEMU, libvirt merely pass what has been reported on the monitor.
However, if the block job hasn't started yet, qemu reports cur ==
end == 0 which tricks mgmt apps into thinking job is complete.

The solution is to mangle cur/end values as described here [1].

1: https://www.redhat.com/archives/libvir-list/2016-September/msg00017.html

Signed-off-by: Michal Privoznik 
---
 src/libvirt-domain.c   |  7 +++
 src/qemu/qemu_driver.c | 12 
 2 files changed, 19 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 46f0318..fa82e49 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -9896,6 +9896,13 @@ virDomainBlockJobAbort(virDomainPtr dom, const char 
*disk,
  * can be found by calling virDomainGetXMLDesc() and inspecting
  * elements within //domain/devices/disk.
  *
+ * As a corner case underlying hypervisor may report cur == 0 and
+ * end == 0 when the block job hasn't been started yet. In this
+ * case libvirt reports cur = 0 and end = 1. However, hypervisor
+ * may return cur == 0 and end == 0 if the block job has finished
+ * and was no-op. In this case libvirt reports cur = 1 and end = 1.
+ * Since 2.3.0.
+ *
  * Returns -1 in case of failure, 0 when nothing found, 1 when info was found.
  */
 int
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4535eb8..df0d916 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16338,6 +16338,18 @@ qemuBlockJobInfoTranslate(qemuMonitorBlockJobInfoPtr 
rawInfo,
 info->cur = rawInfo->cur;
 info->end = rawInfo->end;
 
+/* Fix job completeness reporting. If cur == end mgmt
+ * applications think job is completed. Except when both cur
+ * and end are zero, in which case qemu hasn't started the
+ * job yet. */
+if (!info->cur && !info->end) {
+if (rawInfo->ready > 0) {
+info->cur = info->end = 1;
+} else if (rawInfo->ready < 0) {
+info->end = 1;
+}
+}
+
 info->type = rawInfo->type;
 if (info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
 disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
-- 
2.8.4

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

[libvirt] [PATCH v2 1/3] virsh: Introduce usage of option completers to auto-complete arguments

[Nishith Shah]

Call option completers if argument completion is requested using the
corresponding option completer, if it is defined.

Signed-off-by: Nishith Shah 
---
v2: Fix a infinite while loop in vshReadlineParse when using option completers

 tools/vsh.c | 66 ++---
 1 file changed, 46 insertions(+), 20 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 6823ab5..b101645 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2636,14 +2636,16 @@ vshReadlineParse(const char *text, int state)
 vshCommandToken tk;
 static const vshCmdDef *cmd;
 const vshCmdOptDef *opt;
-char *tkdata, *optstr, *const_tkdata;
+char *tkdata, *optstr, *const_tkdata, *completed_name;
 char *res = NULL;
 static char *ctext, *sanitized_text;
+static char **completed_list;
+static unsigned int completed_list_index;
 static uint64_t const_opts_need_arg, const_opts_required, const_opts_seen;
 uint64_t opts_need_arg, opts_seen;
 size_t opt_index;
 static bool cmd_exists, opts_filled, opt_exists;
-static bool non_bool_opt_exists;
+static bool non_bool_opt_exists, data_complete;
 
 if (!state) {
 parser.pos = rl_line_buffer;
@@ -2658,6 +2660,9 @@ vshReadlineParse(const char *text, int state)
 sanitized_text = NULL;
 optstr = NULL;
 
+completed_list = NULL;
+completed_list_index = 0;
+
 /* Sanitize/de-quote the autocomplete text */
 tk = sanitizer.getNextArg(NULL, , _text, false);
 
@@ -2687,6 +2692,7 @@ vshReadlineParse(const char *text, int state)
 cmd_exists = false;
 opts_filled = false;
 non_bool_opt_exists = false;
+data_complete = false;
 
 const_opts_need_arg = 0;
 const_opts_required = 0;
@@ -2707,12 +2713,12 @@ vshReadlineParse(const char *text, int state)
 if (vshCmddefOptFill(cmd, _opts_need_arg,
  _opts_required) < 0)
 goto error;
+opts_need_arg = const_opts_need_arg;
+opts_seen = const_opts_seen;
 opts_filled = true;
 } else if (tkdata[0] == '-' && tkdata[1] == '-' &&
c_isalnum(tkdata[2])) {
 /* Command retrieved successfully, move to options */
-opts_need_arg = const_opts_need_arg;
-opts_seen = const_opts_seen;
 optstr = strchr(tkdata + 2, '=');
 opt_index = 0;
 
@@ -2728,6 +2734,7 @@ vshReadlineParse(const char *text, int state)
 VIR_FREE(optstr);
 goto error;
 }
+
 opt_exists = true;
 VIR_FREE(const_tkdata);
 if (opt->type != VSH_OT_BOOL) {
@@ -2745,10 +2752,11 @@ vshReadlineParse(const char *text, int state)
 goto error;
 
 tkdata = const_tkdata;
-if (STREQ(tkdata, sanitized_text)) {
-/* auto-complete non-bool option */
-break;
-}
+}
+if (STREQ(tkdata, sanitized_text)) {
+/* auto-complete non-bool option arg */
+data_complete = true;
+break;
 }
 if (opt->type != VSH_OT_ARGV)
 opts_need_arg &= ~(1ULL << opt_index);
@@ -2760,17 +2768,19 @@ vshReadlineParse(const char *text, int state)
 goto error;
 }
 }
-} else {
-/* No -- option provided and some other token given */
-if (!opt_exists) {
+} else if (!opt_exists) {
+/* No -- option provided and some other token given
+ * Try to find the default option.
+ */
+if (!(opt = vshCmddefGetData(cmd, _need_arg, _seen))
+&& opt->type == VSH_OT_BOOL)
 goto error;
-} else if (non_bool_opt_exists) {
-/* TODO
- * -- non bool option present, so parse the next arg
- * or call completer on it if it is to be completed
- */
-return NULL;
-}
+opt_exists = true;
+opts_need_arg = const_opts_need_arg;
+opts_seen = const_opts_seen;
+} else {
+/* In every other case, return NULL */
+goto error;
 }
 
 VIR_FREE(const_tkdata);
@@ -2795,10 +2805,26 @@ vshReadlineParse(const char *text, int state)
 VIR_FREE(const_tkdata);
 }
 
-if (!cmd_exists)
+if (!cmd_exists) {
 res = vshReadlineCommandGenerator(sanitized_text, state);

[libvirt] [PATCH v2 2/3] virsh: Allow data or argument options to be completed as well

[Nishith Shah]

Signed-off-by: Nishith Shah 
---
 tools/vsh.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index b101645..45f55d9 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2607,10 +2607,6 @@ vshReadlineOptionsGenerator(const char *text, int state, 
const vshCmdDef *cmd_pa
 
 list_index++;
 
-if (opt->type == VSH_OT_DATA || opt->type == VSH_OT_ARGV)
-/* ignore non --option */
-continue;
-
 if (len > 2) {
 /* provide auto-complete only when the text starts with -- */
 if (STRNEQLEN(text, "--", 2))
-- 
2.1.4

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

[libvirt] [PATCH v2 3/3] virsh: Complete multiple options when any one option requires data

[Nishith Shah]

Before this patch:
virsh # start --domain dom1 [TAB][TAB] <- offers filename completion
virsh # start --domain [TAB][TAB] <- offers filename completion

After this patch:
virsh # start --domain dom1 [TAB][TAB] <- offers command completion
virsh # start --domain [TAB][TAB] <- calls domain completer if
defined, otherwise falls back to filename completion

Signed-off-by: Nishith Shah 
---
 tools/vsh.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 45f55d9..3157922 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2731,6 +2731,7 @@ vshReadlineParse(const char *text, int state)
 goto error;
 }
 
+opts_seen = const_opts_seen;
 opt_exists = true;
 VIR_FREE(const_tkdata);
 if (opt->type != VSH_OT_BOOL) {
@@ -2748,14 +2749,14 @@ vshReadlineParse(const char *text, int state)
 goto error;
 
 tkdata = const_tkdata;
+virSkipSpaces((const char **));
 }
 if (STREQ(tkdata, sanitized_text)) {
 /* auto-complete non-bool option arg */
 data_complete = true;
 break;
 }
-if (opt->type != VSH_OT_ARGV)
-opts_need_arg &= ~(1ULL << opt_index);
+non_bool_opt_exists = false;
 } else {
 tkdata = NULL;
 /* opt type is BOOL */
-- 
2.1.4

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

[libvirt] [PATCH v2 0/3] virsh: Option completers and small improvements/fixes for autocomplete

[Nishith Shah]

This series introduces option completers and adds some minor improvements
and fixes(not bugs per se, just better/sane behavior) in vshReadlineParse.

The first patch introduces the usage of option completers to auto-complete
arguments for a particular option.

The second and third patches provide small improvements like completing
the options of type VSH_OT_ARGV or VSH_OT_DATA, and to complete multiple
options as well, if a previous option requires an argument, and that
argument has been provided.

---
v2: Fix an infinite while loop bug

v1: https://www.redhat.com/archives/libvir-list/2016-August/msg01009.html

Nishith Shah (3):
  virsh: Introduce usage of option completers to auto-complete arguments
  virsh: Allow data or argument options to be completed as well
  virsh: Complete multiple options when any one option requires data

 tools/vsh.c | 75 -
 1 file changed, 49 insertions(+), 26 deletions(-)

-- 
2.1.4

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

Re: [libvirt] [Qemu-devel] qapi DEVICE_DELETED event issued *before* instance_finalize?!

[Paolo Bonzini]

On 05/09/2016 11:23, Markus Armbruster wrote:
>> >
>> > On the other hand, it is clearly documented that the DEVICE_DELETED
>> > event is sent as soon as guest acknowledges completion of device
>> > removal. So libvirt's buggy if we'd follow documentation strictly. But
>> > then again, I don't see much information value in "guest has detached
>> > device but qemu hasn't yet" event. Libvirt would ignore such event.
> Unless I'm missing something, libvirt needs an event that signals "Guest
> and QEMU are done with this device".  Current DEVICE_DELETED isn't.
> 
> Can we imagine a use for current DEVICE_DELETED, i.e. "Guest is done,
> but QEMU isn't"?
> 
> Would anything break if we changed semantics of DEVICE_DELETED to what
> libvirt actually needs?
> 
> If the answers are "no" and "no", let's do it.

There is a subtle aspect of this.  After the current DEVICE_DELETED, the
device id is not used any more.  So technically you could have

   device_add bar,id=foo
   device_del foo

   // something in QEMU prevents the device from going away?
   // for example there is a storage issue that blocks completion
   // of a read(), and bar is a storage device

   device_add bar,id=foo
   device_del foo

   // which foo is being deleted?  The old one or the new one?
   event DEVICE_DELETED

DEVICE_DELETED does have a meaning: management cannot talk to the device
anymore in QMP once it is raised.

Technically what libvirt wants to know for VFIO is not whether the
device is gone; it's whether the device's _backend_ (the VFIO file
descriptor) is gone.  The device backend could have been a separate QOM
object, but it isn't.

So perhaps we need a new event that is specific to VFIO?

Thanks,

Paolo

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

Re: [libvirt] [Qemu-devel] qapi DEVICE_DELETED event issued *before* instance_finalize?!

[Markus Armbruster]

Adding Paolo.

Michal Privoznik  writes:

> On 02.09.2016 01:11, Alex Williamson wrote:
>> Hey,
>> 
>> I'm out of my QOM depth, so I'll just beg for help in advance.  I
>> noticed in testing vfio-pci hotunplug that the host seems to be trying
>> to reclaim the device before QEMU is actually done with it, there's a
>> very short race where libvirt has seen the DEVICE_DELETED event and
>> tries to unbind the physical device from vfio-pci, the use count is
>> clearly non-zero because the host driver tries to send a device
>> request, but that event channel has already been torn down.  Nearly
>> immediately after, QEMU finally releases the device, but we can't do a
>> proper reset due to some issues with device references in the kernel.
>> 
>> When I run gdb on QEMU with breakpoints at
>> qapi_event_send_device_deleted() and vfio_instance_finalize(),  the
>> QAPI even happens first.  Clearly this is horribly wrong, right?  I
>> can't unmap my references to the vfio device file until my
>> instance_finalize is called, so I'm always going to have that open when
>> libvirt takes the DEVICE_DELETED event as a cue to return the device to
>> host drivers.  The call chains look like this:
>> 
>> #0  qapi_event_send_device_deleted (has_device=true, 
>> device=0x7f5ca3e36fb0 "hostdev0", 
>> path=0x7f5c89e84fe0 "/machine/peripheral/hostdev0", 
>> errp=0x7f5ca241f9e8 ) at qapi-event.c:412
>> #1  0x7f5ca1701608 in device_unparent (obj=0x7f5ca43ffc00)
>> at hw/core/qdev.c:1115
>> #2  0x7f5ca18b7891 in object_finalize_child_property 
>> (obj=0x7f5ca380f500, 
>> name=0x7f5ca3f21da0 "hostdev0", opaque=0x7f5ca43ffc00) at 
>> qom/object.c:1362
>> #3  0x7f5ca18b56b2 in object_property_del_child (obj=0x7f5ca380f500, 
>> child=0x7f5ca43ffc00, errp=0x0) at qom/object.c:422
>> #4  0x7f5ca18b5790 in object_unparent (obj=0x7f5ca43ffc00)
>> at qom/object.c:441
>> #5  0x7f5ca16c1f31 in acpi_pcihp_eject_slot (s=0x7f5ca4c41268, bsel=0, 
>> slots=4) at hw/acpi/pcihp.c:139
>> 
>> 
>> #0  vfio_instance_finalize (obj=0x7f5ca43ffc00)
>> at /net/gimli/home/alwillia/Work/qemu.git/hw/vfio/pci.c:2731
>> #1  0x7f5ca18b57c0 in object_deinit (obj=0x7f5ca43ffc00, 
>> type=0x7f5ca376f490) at qom/object.c:448
>> #2  0x7f5ca18b5831 in object_finalize (data=0x7f5ca43ffc00)
>> at qom/object.c:462
>> #3  0x7f5ca18b6782 in object_unref (obj=0x7f5ca43ffc00) at 
>> qom/object.c:896
>> #4  0x7f5ca1550cc0 in memory_region_unref (mr=0x7f5ca43fff00)
>> at /net/gimli/home/alwillia/Work/qemu.git/memory.c:1476
>> #5  0x7f5ca1553886 in do_address_space_destroy (as=0x7f5ca43ffe10)
>> at /net/gimli/home/alwillia/Work/qemu.git/memory.c:2272
>> 
>> 
>> It appears that DEVICE_DELETED only means the VM is done with the
>> device but libvirt is interpreting it as QEMU is done with the device.
>> Which is correct?  Do we need a new event or do we need to fix the
>> ordering of this event?  An ordering fix would be more compatible with
>> existing libvirt.  Thanks,
>
> What an interesting race. I think the even should be sent only after
> both guest and qemu are done with the device. Having two events looks
> like too much granularity to me. I mean, even if libvirt learns that
> guest has detached device, it still can't do anything until qemu clears
> its internal state.
>
> On the other hand, it is clearly documented that the DEVICE_DELETED
> event is sent as soon as guest acknowledges completion of device
> removal. So libvirt's buggy if we'd follow documentation strictly. But
> then again, I don't see much information value in "guest has detached
> device but qemu hasn't yet" event. Libvirt would ignore such event.

Unless I'm missing something, libvirt needs an event that signals "Guest
and QEMU are done with this device".  Current DEVICE_DELETED isn't.

Can we imagine a use for current DEVICE_DELETED, i.e. "Guest is done,
but QEMU isn't"?

Would anything break if we changed semantics of DEVICE_DELETED to what
libvirt actually needs?

If the answers are "no" and "no", let's do it.

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

Re: [libvirt] [Qemu-devel] [PATCH v7 0/4] Add Mediated device support

[Paolo Bonzini]

On 03/09/2016 13:57, John Ferlan wrote:
 After creating the vGPU, if required by the host driver, all the other
 type ids would disappear from "virsh nodedev-dumpxml pci__86_00_0" too.
>>>
>>> Not wanting to make assumptions, but this reads as if I create one type
>>> 11 vGPU, then I can create no others on the host.  Maybe I'm reading it
>>> wrong - it's been a long week.
>>
>> Correct, at least for NVIDIA.
>>
> 
> OK, but so what am I missing vis-a-vis the groups conversation?  Sounds
> like multiple vGPU's are being combined, but if only one can be created.
> I think this is where I got confused while reading...

Oh, I read that as "then I can create no other _types_ on the host".
For NVIDIA you can create other vGPUs but they all have to be of the
same type (type 11 in your example).

Paolo

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

Re: [libvirt] [Qemu-devel] [PATCH v7 0/4] Add Mediated device support

[Paolo Bonzini]

On 03/09/2016 01:57, Laine Stump wrote:
>>
>> mdevs do not exist on the host (they do not have a driver on the host
>> because they are not PCI devices) so they do need any management.  At
>> least I hope that's good news. :)
> 
> What's your definition of "management"? They don't need the same type of
> management as a traditional hostdev, but they certainly don't just
> appear by magic! :-)
>
> For standard PCI devices, the managed attribute says whether or not the
> device needs to be detached from the host driver and attached to
> vfio-pci. For other kinds of hostdev devices, we could decide that it
> meant something different. In this case, perhaps managed='yes' could
> mean that the vGPU will be created as needed, and destroyed when the
> guest is finished with it, and managed='no' could mean that we expect a
> vGPU to already exist, and just need starting.

Yes, you're 100% right.  vGPUs have to be created through sysfs, and
that is indeed a kind of management.  My point is that for now, given
there is no support in libvirt for persistent nodedevs, it is safe to
let the user do that and reject managed='yes' for mdev-based .

If later you want to add nodedev-define, then managed='yes' might mean
"create and destroy the nodedev automatically" based on a persistent
definition.  But for now, you can enforce managed='no' (it's the default
anyway) and have the user create a transient nodedev manually before the
domain.  More features can be added incrementally on top.

Thanks,

Paolo

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

Re: [libvirt] [libvirt-php] libvirt_connect not reading out credential info on 0.5.2

[Michal Privoznik]

On 02.09.2016 11:11, Fernando Casas Schössow wrote:
> I'm running libvirt-php 0.5.2 on CentOS 7 with libvirt 2.1.0.
> Using virsh I'm able to connect to hyper-v hosts correctly but using
> libvirt-php it fails during authentication since it seems that the
> credentials are not being passed along.
> 
> This is the php code I'm using:
> 
>  $logfile = 'test.log';
> 
> unlink($logfile);
> if (!libvirt_logfile_set($logfile))
> die('Cannot set the log file');
> 
> $connstr = 'hyperv://user@hyperv-host/?transport=http';
> $credentials =
> array(VIR_CRED_AUTHNAME=>'Administrator',VIR_CRED_PASSPHRASE=>'somepass');
> $conn = libvirt_connect($connstr, false, $credentials);
> echo libvirt_get_last_error();
> unset($conn);
> 
> $fp = fopen($logfile, 'r');
>$str = fread($fp, filesize($logfile));
>fclose($fp);
> 
> echo '';
> echo $str;
> print_r($credentials);
> echo '';
> ?>
> 

Unfortunately, I don't have a hyperv instance to try this out, but for
other connection URIs (like qemu:///system) this works for me.

> And this is the output:
> 
> 
> authentication failed: Password request failed
> 
> [2016-09-02 11:10:02 libvirt-php/core ]: libvirt_connect: Found 0
> elements for credentials
> [2016-09-02 11:10:02 libvirt-php/core ]: libvirt_virConnectAuthCallback:
> cred 0, type 5, prompt Enter Administrator's password for hyperv-host
> challenge hyperv-host
> [2016-09-02 11:10:02 libvirt-php/core ]: libvirt_virConnectAuthCallback:
> result (null) (0)
> [2016-09-02 11:10:02 libvirt-php/core ]: libvirt_connect: Cannot
> establish connection to hyperv://Administrator@hyperv-host/?transport=http
> Array
> (
>[2] => Administrator
>[5] => somepass
> )
> 
> 
> Note the "Found 0 elements for credentials".
> After doing some googling I found an email thread "[libvirt]
> [libvirt-php PATCH 0/3] Fix PHP5 compatibilty issues." that I think is
> related to my problem, especially patch 2/3.
> I also checked github project and saw that those patches are already
> merged in the code so I went ahead and apply them to 0.5.2 source code
> and rebuild:
> 
> use VIRT_FOREACH macros everywhere - Commit: f4b760d
> libvirt_connect: use loop macros to read cred info - Commit: d704106
> Define macros for looping php hash tables - Commit: 673a0bf
> 
> The build went fine but now when I'm trying to connect to the hyper-v
> host using libvirt-php I get an internal server error (500) so it seems
> PHP is crashing. From the system log I see:
> 
> kernel: traps: php-cgi[43452] general protection ip:7f63907f74a0
> sp:7fff12d61f68 error:0 in libvirt.so.0.2001.0[7f6390735000+385000]

If you could attach a debugger and get a stack trace where this is
happening that'd be great.

> 
> Is there any other patch that I'm missing?

Doesn't look like it, but if you could try the current git HEAD that
would help. I mean, if you see the error even with that the bug is still
there.

> Any ideas on how to fix the libvirt_connect credentials issue without
> applying the patches above?

I should probably do a release soon. Not that there was a much movement
since the last one though.

Michal

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