[libvirt] [libvirt-glib] Add gvir_domain_open_graphics_fd()
Add binding for virDomainOpenGraphicsFD. --- libvirt-gobject/libvirt-gobject-domain.c | 36 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 4 ++++ libvirt-gobject/libvirt-gobject.sym | 5 +++++ 3 files changed, 45 insertions(+) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8df30d7..9c1aa6e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1222,6 +1222,42 @@ cleanup: } /** + * gvir_domain_open_graphics_fd: + * @dom: the domain + * @idx: the graphics index + * @flags: extra flags, currently unused + * + * This will create a socket pair connected to the graphics backend of @dom. One + * end of the socket will be returned on success, and the other end is handed to + * the hypervisor. If @dom has multiple graphics backends configured, then @idx + * will determine which one is opened, starting from @idx 0. + * + * Returns: An fd on success, -1 on failure. + */ +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx, + unsigned int flags, + GError **err) +{ + GVirDomainPrivate *priv; + int ret = -1; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), -1); + g_return_val_if_fail(err == NULL || *err == NULL, -1); + + priv = dom->priv; + + ret = virDomainOpenGraphicsFD(priv->handle, idx, flags); + if (ret <= 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + "Unable to open graphics"); + } + + return ret; +} + +/** * gvir_domain_suspend: * @dom: the domain to suspend * @err: Place-holder for possible errors diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 47ed784..4fe381e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -332,6 +332,10 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom, int fd, unsigned int flags, GError **err); +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx, + unsigned int flags, + GError **err); gboolean gvir_domain_suspend (GVirDomain *dom, GError **err); diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index d18769b..927cad9 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -260,4 +260,9 @@ LIBVIRT_GOBJECT_0.1.9 { gvir_stream_io_condition_get_type; } LIBVIRT_GOBJECT_0.1.5; +LIBVIRT_GOBJECT_0.2.0 { + global: + gvir_domain_open_graphics_fd; +} LIBVIRT_GOBJECT_0.1.9; + # .... define new API here using predicted next version number .... -- 2.1.0
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. Also, it would be nice to have Since: flags in the docs. Looks good otherwise. On Wed, Nov 19, 2014 at 2:19 AM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
Add binding for virDomainOpenGraphicsFD. --- libvirt-gobject/libvirt-gobject-domain.c | 36 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 4 ++++ libvirt-gobject/libvirt-gobject.sym | 5 +++++ 3 files changed, 45 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8df30d7..9c1aa6e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1222,6 +1222,42 @@ cleanup: }
/** + * gvir_domain_open_graphics_fd: + * @dom: the domain + * @idx: the graphics index + * @flags: extra flags, currently unused + * + * This will create a socket pair connected to the graphics backend of @dom. One + * end of the socket will be returned on success, and the other end is handed to + * the hypervisor. If @dom has multiple graphics backends configured, then @idx + * will determine which one is opened, starting from @idx 0. + * + * Returns: An fd on success, -1 on failure. + */ +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx, + unsigned int flags, + GError **err) +{ + GVirDomainPrivate *priv; + int ret = -1; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), -1); + g_return_val_if_fail(err == NULL || *err == NULL, -1); + + priv = dom->priv; + + ret = virDomainOpenGraphicsFD(priv->handle, idx, flags); + if (ret <= 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + "Unable to open graphics"); + } + + return ret; +} + +/** * gvir_domain_suspend: * @dom: the domain to suspend * @err: Place-holder for possible errors diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 47ed784..4fe381e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -332,6 +332,10 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom, int fd, unsigned int flags, GError **err); +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx, + unsigned int flags, + GError **err);
gboolean gvir_domain_suspend (GVirDomain *dom, GError **err); diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index d18769b..927cad9 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -260,4 +260,9 @@ LIBVIRT_GOBJECT_0.1.9 { gvir_stream_io_condition_get_type; } LIBVIRT_GOBJECT_0.1.5;
+LIBVIRT_GOBJECT_0.2.0 { + global: + gvir_domain_open_graphics_fd; +} LIBVIRT_GOBJECT_0.1.9; + # .... define new API here using predicted next version number .... -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Marc-André Lureau
On Wed, Nov 19, 2014 at 10:01 AM, Marc-André Lureau <marcandre.lureau@gmail.com> 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.
Also, it would be nice to have Since: flags in the docs. Looks good otherwise.
Sure, though I don't think we have been doing that in libvirt-glib.
On Wed, Nov 19, 2014 at 2:19 AM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
Add binding for virDomainOpenGraphicsFD. --- libvirt-gobject/libvirt-gobject-domain.c | 36 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 4 ++++ libvirt-gobject/libvirt-gobject.sym | 5 +++++ 3 files changed, 45 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8df30d7..9c1aa6e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1222,6 +1222,42 @@ cleanup: }
/** + * gvir_domain_open_graphics_fd: + * @dom: the domain + * @idx: the graphics index + * @flags: extra flags, currently unused + * + * This will create a socket pair connected to the graphics backend of @dom. One + * end of the socket will be returned on success, and the other end is handed to + * the hypervisor. If @dom has multiple graphics backends configured, then @idx + * will determine which one is opened, starting from @idx 0. + * + * Returns: An fd on success, -1 on failure. + */ +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx, + unsigned int flags, + GError **err) +{ + GVirDomainPrivate *priv; + int ret = -1; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), -1); + g_return_val_if_fail(err == NULL || *err == NULL, -1); + + priv = dom->priv; + + ret = virDomainOpenGraphicsFD(priv->handle, idx, flags); + if (ret <= 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + "Unable to open graphics"); + } + + return ret; +} + +/** * gvir_domain_suspend: * @dom: the domain to suspend * @err: Place-holder for possible errors diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 47ed784..4fe381e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -332,6 +332,10 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom, int fd, unsigned int flags, GError **err); +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx, + unsigned int flags, + GError **err);
gboolean gvir_domain_suspend (GVirDomain *dom, GError **err); diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index d18769b..927cad9 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -260,4 +260,9 @@ LIBVIRT_GOBJECT_0.1.9 { gvir_stream_io_condition_get_type; } LIBVIRT_GOBJECT_0.1.5;
+LIBVIRT_GOBJECT_0.2.0 { + global: + gvir_domain_open_graphics_fd; +} LIBVIRT_GOBJECT_0.1.9; + # .... define new API here using predicted next version number .... -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Marc-André Lureau
-- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/
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@gmail.com> 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. 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. 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 :|
On Wed, Nov 19, 2014 at 11:44 AM, Daniel P. Berrange <berrange@redhat.com> 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@gmail.com> 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/
On Wed, Nov 19, 2014 at 01:19:31AM +0000, Zeeshan Ali (Khattak) wrote:
Add binding for virDomainOpenGraphicsFD. --- libvirt-gobject/libvirt-gobject-domain.c | 36 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 4 ++++ libvirt-gobject/libvirt-gobject.sym | 5 +++++ 3 files changed, 45 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8df30d7..9c1aa6e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1222,6 +1222,42 @@ cleanup: }
/** + * gvir_domain_open_graphics_fd: + * @dom: the domain + * @idx: the graphics index + * @flags: extra flags, currently unused + * + * This will create a socket pair connected to the graphics backend of @dom. One + * end of the socket will be returned on success, and the other end is handed to + * the hypervisor. If @dom has multiple graphics backends configured, then @idx + * will determine which one is opened, starting from @idx 0. + * + * Returns: An fd on success, -1 on failure. + */ +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx,
Could/should this take a GVirConfigDomainGraphics * instead and infer the index from it? I know gvir_domain-open_graphics() uses 'idx', but maybe we should offer the 2 variants? Christophe
On Wed, Nov 19, 2014 at 11:00 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Nov 19, 2014 at 01:19:31AM +0000, Zeeshan Ali (Khattak) wrote:
Add binding for virDomainOpenGraphicsFD. --- libvirt-gobject/libvirt-gobject-domain.c | 36 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 4 ++++ libvirt-gobject/libvirt-gobject.sym | 5 +++++ 3 files changed, 45 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8df30d7..9c1aa6e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1222,6 +1222,42 @@ cleanup: }
/** + * gvir_domain_open_graphics_fd: + * @dom: the domain + * @idx: the graphics index + * @flags: extra flags, currently unused + * + * This will create a socket pair connected to the graphics backend of @dom. One + * end of the socket will be returned on success, and the other end is handed to + * the hypervisor. If @dom has multiple graphics backends configured, then @idx + * will determine which one is opened, starting from @idx 0. + * + * Returns: An fd on success, -1 on failure. + */ +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx,
Could/should this take a GVirConfigDomainGraphics * instead and infer the index from it? I know gvir_domain-open_graphics() uses 'idx', but maybe we should offer the 2 variants?
Doesn't sound very convenient and also I don't think the API guarantees the order of devices returned from gvir_config_domain_get_devices. -- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/
On Wed, Nov 19, 2014 at 12:08:21PM +0000, Zeeshan Ali (Khattak) wrote:
On Wed, Nov 19, 2014 at 11:00 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Could/should this take a GVirConfigDomainGraphics * instead and infer the index from it? I know gvir_domain-open_graphics() uses 'idx', but maybe we should offer the 2 variants?
Doesn't sound very convenient and also I don't think the API guarantees the order of devices returned from gvir_config_domain_get_devices.
Hmm if it's not guaranteed now, we'll have to make sure it is at some point. Or is it possible to find out the index to pass to that function with the current API when there are multiple graphics devices? Christophe
On Wed, Nov 19, 2014 at 12:36 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Nov 19, 2014 at 12:08:21PM +0000, Zeeshan Ali (Khattak) wrote:
On Wed, Nov 19, 2014 at 11:00 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Could/should this take a GVirConfigDomainGraphics * instead and infer the index from it? I know gvir_domain-open_graphics() uses 'idx', but maybe we should offer the 2 variants?
Doesn't sound very convenient and also I don't think the API guarantees the order of devices returned from gvir_config_domain_get_devices.
Hmm if it's not guaranteed now, we'll have to make sure it is at some point.
Well, I guess its guaranteed if we say that it is since we create list of devices by parsing xml.. However, there is still the issue of this API not being convenient for app if it takes a GVirConfigDomainGraphics *. They'll have to get the GVirConfigDomain pointer and get list of devices and and find the graphics device themselves while most domains have just one graphics device typically.
Or is it possible to find out the index to pass to that function with the current API when there are multiple graphics devices?
No, AFAIK. -- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/
On Wed, Nov 19, 2014 at 02:10:11PM +0000, Zeeshan Ali (Khattak) wrote:
However, there is still the issue of this API not being convenient for app if it takes a GVirConfigDomainGraphics *. They'll have to get the GVirConfigDomain pointer and get list of devices and and find the graphics device themselves while most domains have just one graphics device typically.
We could abuse the meaning of a NULL VirConfigDomainGraphics to mean index 0. Note that I did not suggest not having the 'index' API at all as we probably want to have it for consistency, but that my question was about having an additional API which is probably nicer when you have multiple graphics nodes in your libvirt domain definition. Christophe
On Wed, Nov 19, 2014 at 4:31 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Nov 19, 2014 at 02:10:11PM +0000, Zeeshan Ali (Khattak) wrote:
However, there is still the issue of this API not being convenient for app if it takes a GVirConfigDomainGraphics *. They'll have to get the GVirConfigDomain pointer and get list of devices and and find the graphics device themselves while most domains have just one graphics device typically.
We could abuse the meaning of a NULL VirConfigDomainGraphics to mean index 0. Note that I did not suggest not having the 'index' API at all as we probably want to have it for consistency, but that my question was about having an additional API which is probably nicer when you have multiple graphics nodes in your libvirt domain definition.
Hmm.. sure. -- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/
On Wed, Nov 19, 2014 at 12:08:21PM +0000, Zeeshan Ali (Khattak) wrote:
On Wed, Nov 19, 2014 at 11:00 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Nov 19, 2014 at 01:19:31AM +0000, Zeeshan Ali (Khattak) wrote:
Add binding for virDomainOpenGraphicsFD. --- libvirt-gobject/libvirt-gobject-domain.c | 36 ++++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 4 ++++ libvirt-gobject/libvirt-gobject.sym | 5 +++++ 3 files changed, 45 insertions(+)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 8df30d7..9c1aa6e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -1222,6 +1222,42 @@ cleanup: }
/** + * gvir_domain_open_graphics_fd: + * @dom: the domain + * @idx: the graphics index + * @flags: extra flags, currently unused + * + * This will create a socket pair connected to the graphics backend of @dom. One + * end of the socket will be returned on success, and the other end is handed to + * the hypervisor. If @dom has multiple graphics backends configured, then @idx + * will determine which one is opened, starting from @idx 0. + * + * Returns: An fd on success, -1 on failure. + */ +int gvir_domain_open_graphics_fd(GVirDomain *dom, + guint idx,
Could/should this take a GVirConfigDomainGraphics * instead and infer the index from it? I know gvir_domain-open_graphics() uses 'idx', but maybe we should offer the 2 variants?
Doesn't sound very convenient and also I don't think the API guarantees the order of devices returned from gvir_config_domain_get_devices.
If we don't already, we should *explicitly* guarantee that the order of devices returned by gvir_config_domain_get_devices 100% matches the order they appeared in the XML document. The ordering is sensitive when considering things like BIOS disk boot priority, so we must take extra care not to disturb ordering. 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 :|
participants (4)
-
Christophe Fergeau -
Daniel P. Berrange -
Marc-André Lureau -
Zeeshan Ali (Khattak)