Patchwork qemu-ga: Make guest-network-get-interfaces Linux only

login
register
mail settings
Submitter Michal Privoznik
Date March 20, 2012, 3:09 p.m.
Message ID <0ccd5fa1764f254dbde6851c0d2139e9198a25e2.1332256180.git.mprivozn@redhat.com>
Download mbox | patch
Permalink /patch/147808/
State New
Headers show

Comments

Michal Privoznik - March 20, 2012, 3:09 p.m.
Currently, the implementation of that command is full of
Linux specific code. Before any brave man will step into
and port it to other OSes, make this function Linux only.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 qga/commands-posix.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
Michael Roth - March 20, 2012, 4:10 p.m.
On Tue, Mar 20, 2012 at 04:09:53PM +0100, Michal Privoznik wrote:
> Currently, the implementation of that command is full of
> Linux specific code. Before any brave man will step into
> and port it to other OSes, make this function Linux only.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Which aspect is linux-specific? getifaddrs() and getnameinfo() seem to
be available on BSD and OpenSolaris, and we're already stubbed for w32.
Are you seeing breakage?

> ---
>  qga/commands-posix.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 7b2be2f..89dde92 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -729,6 +729,7 @@ void qmp_guest_suspend_hybrid(Error **err)
>      guest_suspend("pm-suspend-hybrid", NULL, err);
>  }
> 
> +#if defined(__linux__)
>  static GuestNetworkInterfaceList *
>  guest_find_interface(GuestNetworkInterfaceList *head,
>                       const char *name)
> @@ -904,6 +905,16 @@ error:
>      return NULL;
>  }
> 
> +#else /* defined(linux) */
> +
> +GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **err)
> +{
> +    error_set(err, QERR_UNSUPPORTED);
> +    return NULL;
> +}
> +
> +#endif /* defined(linux) */
> +
>  /* register init/cleanup routines for stateful command groups */
>  void ga_command_state_init(GAState *s, GACommandState *cs)
>  {
> -- 
> 1.7.8.5
> 
>
Michal Privoznik - March 20, 2012, 4:16 p.m.
On 20.03.2012 17:10, Michael Roth wrote:
> On Tue, Mar 20, 2012 at 04:09:53PM +0100, Michal Privoznik wrote:
>> Currently, the implementation of that command is full of
>> Linux specific code. Before any brave man will step into
>> and port it to other OSes, make this function Linux only.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> 
> Which aspect is linux-specific? getifaddrs() and getnameinfo() seem to
> be available on BSD and OpenSolaris, and we're already stubbed for w32.
> Are you seeing breakage?

Personally, I don't; but there has been this report:

http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03951.html

which happens on OpenBSD, I suspect.

Michal
Michael Roth - March 20, 2012, 6:19 p.m.
On Tue, Mar 20, 2012 at 05:16:41PM +0100, Michal Privoznik wrote:
> On 20.03.2012 17:10, Michael Roth wrote:
> > On Tue, Mar 20, 2012 at 04:09:53PM +0100, Michal Privoznik wrote:
> >> Currently, the implementation of that command is full of
> >> Linux specific code. Before any brave man will step into
> >> and port it to other OSes, make this function Linux only.
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > 
> > Which aspect is linux-specific? getifaddrs() and getnameinfo() seem to
> > be available on BSD and OpenSolaris, and we're already stubbed for w32.
> > Are you seeing breakage?
> 
> Personally, I don't; but there has been this report:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03951.html
> 
> which happens on OpenBSD, I suspect.

Ew, okay, well it looks like guest-suspend has a role as well so this
patch wouldn't be sufficient. Let me fire up a FreeBSD image I have
laying around and play around with it a bit.

> 
> Michal
>
Michael Roth - March 21, 2012, 12:47 a.m.
On Tue, Mar 20, 2012 at 01:19:25PM -0500, Michael Roth wrote:
> On Tue, Mar 20, 2012 at 05:16:41PM +0100, Michal Privoznik wrote:
> > On 20.03.2012 17:10, Michael Roth wrote:
> > > On Tue, Mar 20, 2012 at 04:09:53PM +0100, Michal Privoznik wrote:
> > >> Currently, the implementation of that command is full of
> > >> Linux specific code. Before any brave man will step into
> > >> and port it to other OSes, make this function Linux only.
> > >>
> > >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > > 
> > > Which aspect is linux-specific? getifaddrs() and getnameinfo() seem to
> > > be available on BSD and OpenSolaris, and we're already stubbed for w32.
> > > Are you seeing breakage?
> > 
> > Personally, I don't; but there has been this report:
> > 
> > http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03951.html
> > 
> > which happens on OpenBSD, I suspect.
> 
> Ew, okay, well it looks like guest-suspend has a role as well so this
> patch wouldn't be sufficient. Let me fire up a FreeBSD image I have
> laying around and play around with it a bit.
> 

Bleh.. hit a gcc bug on the build then ran out of disk space trying to
upgrade to the latest gcc. Let's stub guest-network* and guest-suspend*
for now. I'll send a patch for the guest-suspend side and to get rid
of the warnings.

> > 
> > Michal
> >
Andreas Färber - March 26, 2012, 4:35 p.m.
Am 20.03.2012 17:10, schrieb Michael Roth:
> On Tue, Mar 20, 2012 at 04:09:53PM +0100, Michal Privoznik wrote:
>> Currently, the implementation of that command is full of
>> Linux specific code. Before any brave man will step into
>> and port it to other OSes, make this function Linux only.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> 
> Which aspect is linux-specific? getifaddrs() and getnameinfo() seem to
> be available on BSD and OpenSolaris, and we're already stubbed for w32.
> Are you seeing breakage?

The two OpenBSD buildbots were reporting breakages:

http://buildbot.b1-systems.de/qemu/buildslaves/brad_openbsd_current
http://buildbot.b1-systems.de/qemu/buildslaves/kraxel_openbsd49

Unfortunately they rarely nag the people to blame ;) and the reports on
qemu-devel are admittedly a bit brief.

Maybe it might make sense to let the buildbots build your qemu-ga
staging branch as well, Michael? That could catch this in advance.

Andreas
Michael Roth - March 26, 2012, 6:47 p.m.
On Mon, Mar 26, 2012 at 06:35:49PM +0200, Andreas Färber wrote:
> Am 20.03.2012 17:10, schrieb Michael Roth:
> > On Tue, Mar 20, 2012 at 04:09:53PM +0100, Michal Privoznik wrote:
> >> Currently, the implementation of that command is full of
> >> Linux specific code. Before any brave man will step into
> >> and port it to other OSes, make this function Linux only.
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > 
> > Which aspect is linux-specific? getifaddrs() and getnameinfo() seem to
> > be available on BSD and OpenSolaris, and we're already stubbed for w32.
> > Are you seeing breakage?
> 
> The two OpenBSD buildbots were reporting breakages:
> 
> http://buildbot.b1-systems.de/qemu/buildslaves/brad_openbsd_current
> http://buildbot.b1-systems.de/qemu/buildslaves/kraxel_openbsd49
> 
> Unfortunately they rarely nag the people to blame ;) and the reports on
> qemu-devel are admittedly a bit brief.
> 
> Maybe it might make sense to let the buildbots build your qemu-ga
> staging branch as well, Michael? That could catch this in advance.

That would great! Is there any process in place for having a repo added? I've
been staging directly to pull branches, but started a single staging
branch at:

git://github.com/mdroth/qemu.git qga

> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

Patch

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 7b2be2f..89dde92 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -729,6 +729,7 @@  void qmp_guest_suspend_hybrid(Error **err)
     guest_suspend("pm-suspend-hybrid", NULL, err);
 }
 
+#if defined(__linux__)
 static GuestNetworkInterfaceList *
 guest_find_interface(GuestNetworkInterfaceList *head,
                      const char *name)
@@ -904,6 +905,16 @@  error:
     return NULL;
 }
 
+#else /* defined(linux) */
+
+GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **err)
+{
+    error_set(err, QERR_UNSUPPORTED);
+    return NULL;
+}
+
+#endif /* defined(linux) */
+
 /* register init/cleanup routines for stateful command groups */
 void ga_command_state_init(GAState *s, GACommandState *cs)
 {