Re: [libvirt] [PATCH 10/10] linkstate: qemu: Add link state modification support to qemu driver.
On 08/11/2011 09:27 AM, Peter Krempa wrote:
Adds support for the new API. While starting a qemu link state
cannot be set with an command line argument and therefore is done
by entering monitor.
---
src/qemu/qemu_driver.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_process.c | 37 ++++++++
2 files changed, 265 insertions(+), 0 deletions(-)
Needs rework to be called as part of virDomainUpdateDevice, but you can
reuse a lot of the non-boilerplate code for that purpose.
+
+ /* Check the path is one of the domain's network interfaces. */
+ /* Qemu does not support reading the link state, report saved state */
Don't you just love write-only interfaces :) How does link state fare
across migration?
+ if (flags& VIR_DOMAIN_AFFECT_LIVE) {
+ for (i = 0; i< def->nnets; i++) {
+ if (memcmp(def->nets[i]->mac, mac, VIR_MAC_BUFLEN) == 0) {
+ if (def->nets[i]->info.alias) {
+
+ if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_LINK_SET)) {
+ qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _("This qemu doesn't support setting
link state"));
+ goto endjob;
+ }
+
+ qemuDomainObjEnterMonitor(driver, vm);
+ ret = qemuMonitorSetLink(priv->mon,
def->nets[i]->info.alias, state);
+ qemuDomainObjExitMonitor(driver, vm);
+ } else {
+ qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _("Device alias not found. (QEMU probably
doesn't support device naming)"));
Don't we have a QEMU_CAPS that says whether we have -device (and
therefore device naming)? In fact, if settable link state was
introduced the same time as -device, then we might not need a new
QEMU_CAPS_LINK_SET (but I'd have to research when things appeared).
+
+ /* one or both configurations successfuly altered */
s/successfuly/successfully/; although if you hook into
virDomainUpdateDevice code properly, this is probably boilerplate that
you don't have to worry about.
+/* set link states to down on interfaces at qemu start */
+static int
+qemuProcessSetLinkStates(virDomainObjPtr vm)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virDomainDefPtr def = vm->def;
+ int i;
+ int ret = 0;
+
+ for (i = 0; i< def->nnets; i++) {
+ if (def->nets[i]->linkstate == VIR_LINK_STATE_DOWN) {
+ ret = qemuMonitorSetLink(priv->mon,
+ def->nets[i]->info.alias,
+ VIR_LINK_STATE_DOWN);
+
+ if (ret != 0)
+ break;
+ }
+ }
+
+ return ret;
+}
+
/* Set CPU affinites for vcpus if vcpupin xml provided. */
As long as you are touching near this code: s/affinites/affinities/
+ /* set default link states */
+ /* qemu doesn't support setting this on the command line, so
+ * enter the monitor */
Oh, well that answers my earlier question about why you didn't change
qemu_command.c.
+ if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_LINK_SET)) {
+ VIR_DEBUG("Setting network link states");
+ qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ if (qemuProcessSetLinkStates(vm)< 0) {
+ qemuDomainObjExitMonitorWithDriver(driver, vm);
+ goto cleanup;
+ }
I don't think you quite got the logic right. If qemu lacks the
capability, we still need to inspect all interfaces, and issue an error
if any of them requested link down (if they all lacked <link>, or
requested link up, then things are okay), rather than silently ignoring
requests for link down when unsupported by qemu.
Overall, I think the feature addition of being able to simulate link
down will be nice to have, but the series needs some polish first :)
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org