Re: [libvirt] [PATCH 2/4] interface: Refactor interface vlan to helper func
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;