Re: [libvirt-glib] Add gvir_domain_open_graphics_fd()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 19, 2014 at 11:44 AM, Daniel P. Berrange
<berrange@xxxxxxxxxx> wrote:
> On Wed, Nov 19, 2014 at 11:38:48AM +0000, Zeeshan Ali (Khattak) wrote:
>> On Wed, Nov 19, 2014 at 10:01 AM, Marc-André Lureau
>> <marcandre.lureau@xxxxxxxxx> wrote:
>> > This API was added recently, in libvirt v1.2.8. You should either
>> > change libvirt-glib dependency or add a fallback using
>> > virDomainOpenGraphics, dating back from v0.9.7.
>>
>> Thanks for pointing out. I would just bump the dep since libvirt-glib
>> and libvirt are so related but I'll need Daniel's approval for that.
>
> Bumping to 1.2.8 is probably a bit too new since we don't have that
> version in Fedora 20, nor in RHEL-7.
>
> It isn't unreasonable to #ifdef #else it to use the old API as an
> alternative impl here. The only difference is with the old API you
> had to call 'socketpair' to create the socket you pass in, instead
> of getting it straight back. We can hide that difference in the
> gvir_domain_open_graphics_fd method impl without trouble though.

Too many #ifdefs makes code very unreadable so I tend to avoid them
but I guess in this case its justified.

> For that matter, I'd suggest dropping the '_fd' suffix of the
> method name. That's only there in libvirt because the desired
> name was already taken by the old method, so we don't need to
> carry over that naming.

As Christophe pointed out, its the same in libvirt-glib since already
have gvir_domain_open_graphics.

-- 
Regards,

Zeeshan Ali (Khattak)
________________________________________
Befriend GNOME: http://www.gnome.org/friends/

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





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]