On 05/25/2013 09:24 AM, Roman Bogorodskiy wrote:
> This method is useful not only in virnetdev.c.
> ---
> src/libvirt_private.syms | 1 +
> src/util/virnetdev.c | 15 +++++++++++++--
> src/util/virnetdev.h | 11 +++++++++++
> src/util/virnetdevmacvlan.c | 2 +-
> src/util/virnetdevvportprofile.c | 2 +-
> 5 files changed, 27 insertions(+), 4 deletions(-)
>
You don't have 'cppi' installed, or 'make syntax-check' would have
pointed out these nits:
preprocessor_indentation
cppi: src/util/virnetdev.h: line 33: not properly indented
cppi: src/util/virnetdev.h: line 37: not properly indented
cppi: src/util/virnetdev.h: line 40: not properly indented
maint.mk: incorrect preprocessor indentation
> +#else
> +int
> +virNetDevSetupControl(const char *ifname ATTRIBUTE_UNUSED,
> + void *ifr ATTRIBUTE_UNUSED)
> +{
> + virReportSystemError(ENOSYS,
> + _("%s is not supported on this platform"),
> + __func__);
I liked Dan's suggestion for how to word this:
>> How about
>>
>> "Network device configuration is not supported on this platform"
> +#ifdef HAVE_STRUCT_IFREQ
> +int virNetDevSetupControl(const char *ifname,
> + struct ifreq *ifr)
> + ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +#else
> +int virNetDevSetupControl(const char *ifname,
> + void *ifr);
> +#endif
I'm not sure I'm a fan of changing the compile-time attributes based on
whether the struct exists; I'm proposing an alternate solution below.
> +++ b/src/util/virnetdevmacvlan.c
> @@ -49,7 +49,7 @@ VIR_ENUM_IMPL(virNetDevMacVLanMode, VIR_NETDEV_MACVLAN_MODE_LAST,
> # include <sys/socket.h>
> # include <sys/ioctl.h>
>
> -# include <linux/if.h>
> +# include <net/if.h>
> # include <linux/if_tun.h>
Independently useful. I might split your patch into two, and apply the
<net/if.h> usage right away while awaiting feedback on my proposed tweaks.
diff --git i/src/util/virnetdev.c w/src/util/virnetdev.c
index f658c6d..1316bc4 100644
--- i/src/util/virnetdev.c
+++ w/src/util/virnetdev.c
@@ -99,9 +99,9 @@ int
virNetDevSetupControl(const char *ifname ATTRIBUTE_UNUSED,
void *ifr ATTRIBUTE_UNUSED)
{
- virReportSystemError(ENOSYS,
- _("%s is not supported on this platform"),
- __func__);
+ virReportSystemError(ENOSYS, "%s",
+ _("Network device configuration is not supported "
+ "on this platform"));
return -1;
}
#endif
diff --git i/src/util/virnetdev.h w/src/util/virnetdev.h
index 97369c1..3faff48 100644
--- i/src/util/virnetdev.h
+++ w/src/util/virnetdev.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007-2012 Red Hat, Inc.
+ * Copyright (C) 2007-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -30,14 +30,15 @@
# include "virmacaddr.h"
# include "virpci.h"
-#ifdef HAVE_STRUCT_IFREQ
+# ifdef HAVE_STRUCT_IFREQ
+typedef struct ifreq virIfreq;
+# else
+struct virIfreq { };
+# endif
+
int virNetDevSetupControl(const char *ifname,
- struct ifreq *ifr)
+ virIfreq *ifr)
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-#else
-int virNetDevSetupControl(const char *ifname,
- void *ifr);
-#endif
int virNetDevExists(const char *brname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list