On 02/17/2013 08:56 PM, Doug Goldstein wrote:
> Mechanical move to break up udevIfaceGetIfaceDef() into different
> helpers for each of the interface types to hopefully make the code
> easier to follow. This moves the vlan code to
> udevIfaceGetIfaceDefVlan().
> ---
> src/interface/interface_backend_udev.c | 81 ++++++++++++++++++++++------------
> 1 file changed, 54 insertions(+), 27 deletions(-)
>
> diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
> index abd9895..c144978 100644
> --- a/src/interface/interface_backend_udev.c
> +++ b/src/interface/interface_backend_udev.c
> @@ -627,6 +627,59 @@ cleanup:
> return -1;
> }
>
> +static int
> +udevIfaceGetIfaceDefVlan(struct udev *udev ATTRIBUTE_UNUSED,
> + struct udev_device *dev ATTRIBUTE_UNUSED,
> + const char *name,
> + virInterfaceDef *ifacedef)
> + ATTRIBUTE_NONNULL(1)
> + ATTRIBUTE_NONNULL(2)
> + ATTRIBUTE_NONNULL(3)
> + ATTRIBUTE_NONNULL(4)
> + ATTRIBUTE_RETURN_CHECK;
Same comment applies here as in bridge case, about combining the forward
declaration with the definition, and putting the ATTRIBUTES directly in
the definition.
ACK with that change.
> +static int
> +udevIfaceGetIfaceDefVlan(struct udev *udev ATTRIBUTE_UNUSED,
> + struct udev_device *dev ATTRIBUTE_UNUSED,
> + const char *name,
> + virInterfaceDef *ifacedef)
> +{
> + char *vid;
> + char *vlan_parent_dev = NULL;
> +
> + vlan_parent_dev = strdup(name);
> + if (!vlan_parent_dev) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + /* Find the DEVICE.VID again */
> + vid = strrchr(vlan_parent_dev, '.');
> + if (!vid) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to find the VID for the VLAN device '%s'"),
> + name);
> + goto cleanup;
> + }
> +
> + /* Replace the dot with a NULL so we can have the device and VID */
> + vid[0] = '\0';
> + vid++;
> +
> + /* Set our type to VLAN */
> + ifacedef->type = VIR_INTERFACE_TYPE_VLAN;
> +
> + /* Set the VLAN specifics */
> + ifacedef->data.vlan.tag = vid;
> + ifacedef->data.vlan.devname = vlan_parent_dev;
> +
> + return 0;
> +
> +cleanup:
> + VIR_FREE(vlan_parent_dev);
> +
> + return -1;
> +}
> +
> static virInterfaceDef * ATTRIBUTE_NONNULL(1)
> udevIfaceGetIfaceDef(struct udev *udev, const char *name)
> {
> @@ -685,34 +738,8 @@ udevIfaceGetIfaceDef(struct udev *udev, const char *name)
> * other devices */
> vlan_parent_dev = strrchr(name, '.');
> if (vlan_parent_dev) {
> - /* Found the VLAN dot */
> - char *vid;
> -
> - vlan_parent_dev = strdup(name);
> - if (!vlan_parent_dev) {
> - virReportOOMError();
> - goto cleanup;
> - }
> -
> - /* Find the DEVICE.VID separator again */
> - vid = strrchr(vlan_parent_dev, '.');
> - if (!vid) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("failed to find the VID for the VLAN device '%s'"),
> - name);
> + if (udevIfaceGetIfaceDefVlan(udev, dev, name, ifacedef))
> goto cleanup;
> - }
> -
> - /* Replace the dot with a NULL so we can have the device and VID */
> - vid[0] = '\0';
> - vid++;
> -
> - /* Set our type to VLAN */
> - ifacedef->type = VIR_INTERFACE_TYPE_VLAN;
> -
> - /* Set the VLAN specifics */
> - ifacedef->data.vlan.tag = vid;
> - ifacedef->data.vlan.devname = vlan_parent_dev;
> } else if (STREQ_NULLABLE(udev_device_get_devtype(dev), "bridge")) {
> if (udevIfaceGetIfaceDefBridge(udev, dev, name, ifacedef))
> goto cleanup;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list