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

Re: [libvirt] [PATCH] Convert 'raw MAC address' usages to use virMacAddr



On 06/03/2012 07:08 PM, Stefan Berger wrote:
> Introduce new members in the virMacAddr 'class':
> - virMacAddrSet: set virMacAddr from a virMacAddr
> - virMacAddrSetRaw: setting virMacAddr from raw 6 byte MAC address buffer
> - virMacAddrGetRaw: writing virMacAddr into raw 6 byte MAC address buffer
> - virMacAddrCmp: comparing two virMacAddr
> - virMacAddrCmpRaw: comparing a virMacAddr with a raw 6 byte MAC address
> buffer
> 
> then replace raw MAC addresses by replacing
> 
> - 'unsigned char *' with virMacAddrPtr
> - 'unsigned char ... [VIR_MAC_BUFLEN]' with virMacAddr
> 
> and introduce usage of above functions where necessary.

Unfortunately, I tried applying this patch, but it failed:

$ git am -3 ../virMacAddr.eml
Applying: Convert 'raw MAC address' usages to use virMacAddr
fatal: corrupt patch at line 91
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Convert 'raw MAC address' usages to use virMacAddr
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

Can you investigate what might have happened on your end?

> 
> ---
>  src/conf/capabilities.c                   |    4 -
>  src/conf/capabilities.h                   |    4 -
>  src/conf/domain_audit.c                   |    6 -
>  src/conf/domain_conf.c                    |   34 ++++-----
>  src/conf/domain_conf.h                    |    6 -
>  src/conf/domain_nwfilter.c                |    2
>  src/conf/domain_nwfilter.h                |    4 -
>  src/conf/network_conf.c                   |   14 ++--
>  src/conf/network_conf.h                   |    2
>  src/conf/nwfilter_conf.c                  |   17 +---
>  src/conf/nwfilter_conf.h                  |   14 +---
>  src/conf/nwfilter_params.c                |    2
>  src/conf/nwfilter_params.h                |    3
>  src/libvirt_private.syms                  |    5 +
>  src/libxl/libxl_conf.c                    |    2
>  src/lxc/lxc_driver.c                      |    6 -
>  src/network/bridge_driver.c               |    2
>  src/nwfilter/nwfilter_dhcpsnoop.c         |   18 ++---
>  src/nwfilter/nwfilter_dhcpsnoop.h         |    4 -
>  src/nwfilter/nwfilter_driver.c            |    2
>  src/nwfilter/nwfilter_ebiptables_driver.c |    6 -
>  src/nwfilter/nwfilter_gentech_driver.c    |   10 +-
>  src/nwfilter/nwfilter_gentech_driver.h    |    4 -
>  src/nwfilter/nwfilter_learnipaddr.c       |   29 +++-----
>  src/nwfilter/nwfilter_learnipaddr.h       |    4 -
>  src/openvz/openvz_conf.c                  |    2
>  src/openvz/openvz_driver.c                |    8 +-
>  src/qemu/qemu_bridge_filter.c             |    4 -
>  src/qemu/qemu_bridge_filter.h             |    4 -
>  src/qemu/qemu_command.c                   |   22 +++---
>  src/qemu/qemu_driver.c                    |   16 ++--
>  src/qemu/qemu_hostdev.c                   |    8 +-
>  src/qemu/qemu_hotplug.c                   |   14 ++--
>  src/qemu/qemu_migration.c                 |    6 -
>  src/qemu/qemu_process.c                   |    4 -
>  src/uml/uml_conf.c                        |    6 -
>  src/util/ebtables.c                       |   16 ++--
>  src/util/ebtables.h                       |    6 +
>  src/util/virmacaddr.c                     |  104
> +++++++++++++++++++++++++-----

This is an example of the corruption - the diffstat is showing up with
line-wrapping.

>  src/util/virmacaddr.h                     |   25 +++++--
>  src/util/virnetdev.c                      |   61 ++++++++---------
>  src/util/virnetdev.h                      |   11 +--
>  src/util/virnetdevmacvlan.c               |   38 +++++-----
>  src/util/virnetdevmacvlan.h               |   11 +--
>  src/util/virnetdevopenvswitch.c           |    2
>  src/util/virnetdevopenvswitch.h           |    2
>  src/util/virnetdevtap.c                   |   19 ++---
>  src/util/virnetdevtap.h                   |    2
>  src/util/virnetdevvportprofile.c          |   18 ++---
>  src/util/virnetdevvportprofile.h          |    5 -
>  src/util/virnetlink.c                     |   15 ++--
>  src/util/virnetlink.h                     |    7 +-
>  src/vbox/vbox_tmpl.c                      |   16 ++--
>  src/vmx/vmx.c                             |   10 +-
>  src/xen/xend_internal.c                   |    6 -
>  src/xen/xm_internal.c                     |    4 -
>  src/xenxs/xen_sxpr.c                      |    6 -
>  src/xenxs/xen_xm.c                        |    8 +-
>  tools/virsh.c                             |    8 +-
>  59 files changed, 388 insertions(+), 310 deletions(-)
> 
> Index: libvirt-acl/src/conf/domain_conf.h
> ===================================================================
> --- libvirt-acl.orig/src/conf/domain_conf.h
> +++ libvirt-acl/src/conf/domain_conf.h
> @@ -777,7 +777,7 @@ struct _virDomainActualNetDef {
>  /* Stores the virtual network interface configuration */
>  struct _virDomainNetDef {
>      enum virDomainNetType type;
> -    unsigned char mac[VIR_MAC_BUFLEN];
> +    virMacAddr mac;
>      char *model;
>      union {
>          struct {

Overall, these sorts of changes look reasonable, but I'd really like to
apply and compile the test before approving it.


> +
> +/**
> + * virMacAddrCmp:

virMacAddrCmpRaw

> + * @mac1: pointer to 1st MAC address
> + * @mac2: pointer to 2nd MAC address in plain buffer
> + *
> + * Return 0 if MAC addresses are equal,
> + * < 0 if mac1 < mac2,
> + * > 0 if mac1 > mac2
> + */
> +int
> +virMacAddrCmpRaw(const virMacAddrPtr mac1,
> +                 const unsigned char mac2[VIR_MAC_BUFLEN])
> +{
> +    return memcmp(mac1->addr, mac2, VIR_MAC_BUFLEN);
> +}
> +

> +
> +/**
> + * virMacAddrSetRaw
> + * @dst: pointer to destination to hold MAC address
> + * @src: raw MAC address data
> + *
> + * Set the MAC address to the given value
> + */
> +void
> +virMacAddrSetRaw(virMacAddrPtr dst, const unsigned char
> src[VIR_MAC_BUFLEN])

Again some of the line wrapping that corrupted this patch.

> +{
> +    memcpy(dst->addr, src, VIR_MAC_BUFLEN);
> +}
> +
> +/**
> + * virMacAddrGet

virMacAddrGetRaw

> + * @src: pointer to MAC address
> + * @dst: pointer to raw memory to write MAC address into
> + *
> + * Copies the MAC address into raw memory
> + */
> +void
> +virMacAddrGetRaw(virMacAddrPtr src, unsigned char dst[VIR_MAC_BUFLEN])
> +{
> +    memcpy(dst, src->addr, VIR_MAC_BUFLEN);
> +}
> +

> -void virMacAddrFormat(const unsigned char *addr,
> +void virMacAddrFormat(const virMacAddrPtr addr,
>                        char *str)
>  {
>      snprintf(str, VIR_MAC_STRING_BUFLEN,
>               "%02X:%02X:%02X:%02X:%02X:%02X",
> -             addr[0], addr[1], addr[2],
> -             addr[3], addr[4], addr[5]);
> +             addr->addr[0], addr->addr[1], addr->addr[2],
> +             addr->addr[3], addr->addr[4], addr->addr[5]);
>      str[VIR_MAC_STRING_BUFLEN-1] = '\0';

Pre-existing, but why do we need to set the NUL terminator?  snprintf
already takes care of that.


> Index: libvirt-acl/src/util/virnetdev.h

Everything else looks fairly mechanical given the initial changes I
reviewed above, so I didn't really spend any time looking closely at the
rest of this patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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