Re: [libvirt] [PATCH 3/3] PHYP: create, destroy and other network functions
On 11/19/2010 07:55 AM, Eduardo Otubo wrote:
Adding networkCreateXML, networkDestroy, networkIsActive and
networkLookupByName.
In the function phypCreateNetwork I just use the def->domain information to create
the new network interface because of the behaviour of the HMC and the hypervisor:
* HMC can't simply create a network interface without assigning it to a specific
LPAR.
* I also can't assign an IP addr or any other information from the HMC or VIOS
side, but I can control in which vlan or vswitch it will be attached - but
thought just in the simplest case scenarion now, I'll make some improvements
s/scenarion/scenario/
+++ b/src/phyp/phyp_driver.c
@@ -1,7 +1,7 @@
/*
* Copyright (C) 2010 Red Hat, Inc.
- * Copyright IBM Corp. 2009
+ * Copyright IBM Corp. 2010
You should never remove copyright years; this would be okay as 2009-2010.
+
+ virBufferAddLit(&buf, "lshwres ");
+ if (system_type == HMC)
+ virBufferVSprintf(&buf, "-m %s ", managed_system);
+
+ virBufferVSprintf(&buf,
+ " -r virtualio --rsubtype slot --level slot "
+ " -F drc_name,lpar_id|grep %s|"
+ " sed -e 's/^.*,//g'", net->name);
Another grep | sed you can simplify:
sed -n '/%s/ s/^.*,//p'
+ virBufferVSprintf(&buf,
+ " -r virtualio --rsubtype eth --level lpar "
+ " -F mac_addr,slot_num|grep %lld|"
+ " sed -e 's/^.*,//g'", mac);
MACs are usually represented as hex, not decimal. And if it really is
decimal, wouldn't you want unsigned?
Simplify:
sed -n '/%lld/ /^.*,//p'
+
+ if (!def->domain) {
+ VIR_ERROR0(_("Domain can't be NULL, you must especify in which"
s/especify/specify/
+
+ ret = phypExec(session, cmd, &exit_status, conn);
+
+ if (exit_status < 0 || ret != NULL)
+ goto err;
+
+ /* Need to sleep a little while to wait for the HMC to
+ * complete the execution of the command.
+ * */
+ sleep(1);
This seems racy, and 1 second is a long pause. Is there something more
reliable you can use to tell whether HMC is done? Can you set up a
retry loop and sleep for shorter periods of time with retries until it
works to avoid a long pause?
+
+ /* Getting the new interface name */
+ virBufferAddLit(&buf, "lshwres ");
+ if (system_type == HMC)
+ virBufferVSprintf(&buf, "-m %s ", managed_system);
+
+ virBufferVSprintf(&buf,
+ " -r virtualio --rsubtype slot --level slot"
+ " |grep lpar_id=%d|grep slot_num=%d|"
+ " sed -e 's/^.*drc_name=//g'", lpar_id, slot);
sed '/lpar_id=%d/!d; /slot_num=%d/!d; s/^.*drc_name=//'
+ /* Getting the new interface mac addr */
+ virBufferAddLit(&buf, "lshwres ");
+ if (system_type == HMC)
+ virBufferVSprintf(&buf, "-m %s ", managed_system);
+
+ virBufferVSprintf(&buf,
+ "-r virtualio --rsubtype eth --level lpar "
+ "|grep lpar_id=%d|grep slot_num=%d|"
+ " sed -e 's/^.*mac_addr=//g'", lpar_id, slot);
Similar.
+ virBufferAddLit(&buf, "lshwres ");
+ if (system_type == HMC)
+ virBufferVSprintf(&buf, "-m %s ", managed_system);
+
+ virBufferVSprintf(&buf,
+ "-r virtualio --rsubtype eth --level lpar "
+ "-F mac_addr,state |grep %lld|"
+ "sed -e 's/^.*,//g'", mac);
Another instance where decimal MAC seems odd.
sed -n '/%lld/ s/^.*,//p'
diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h
index 603d048..34ad84b 100644
--- a/src/phyp/phyp_driver.h
+++ b/src/phyp/phyp_driver.h
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2010 Red Hat, Inc.
- * Copyright IBM Corp. 2009
+ * Copyright IBM Corp. 2010
This hunk seems completely random. Should it be rebased into another
patch that actually touches phyp_driver.h? And should it be 2009-2010?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)