diff mbox

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

Message ID 0ccd5fa1764f254dbde6851c0d2139e9198a25e2.1332256180.git.mprivozn@redhat.com
State New
Headers show

Commit Message

Michal Prívozník March 20, 2012, 3:09 p.m. UTC
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(-)

Comments

Michael Roth March 20, 2012, 4:10 p.m. UTC | #1
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 Prívozník March 20, 2012, 4:16 p.m. UTC | #2
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. UTC | #3
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. UTC | #4
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. UTC | #5
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. UTC | #6
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
>
diff mbox

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)
 {