On Mon, Mar 23, 2015 at 05:46:23PM -0400, John Ferlan wrote: > > > On 03/17/2015 07:41 AM, Ján Tomko wrote: > > Instead of always using controller 0 and incrementing port number, > > respect the maximum port numbers of controllers and use all of them. > > > > Ports for virtio consoles are quietly reserved, but not formatted > > (neither in XML nor on QEMU command line). > > > > Also rejects duplicate virtio-serial addresses. > > https://bugzilla.redhat.com/show_bug.cgi?id=890606 > > https://bugzilla.redhat.com/show_bug.cgi?id=1076708 > > --- > > src/conf/domain_conf.c | 29 ---------- > > src/qemu/qemu_command.c | 63 ++++++++++++++++++++++ > > src/qemu/qemu_domain.c | 1 + > > src/qemu/qemu_domain.h | 1 + > > src/qemu/qemu_process.c | 2 + > > .../qemuxml2argv-channel-virtio-auto.args | 8 +-- > > .../qemuxml2argv-channel-virtio-autoassign.args | 10 ++-- > > .../qemuxml2xmlout-channel-virtio-auto.xml | 10 ++-- > > 8 files changed, 81 insertions(+), 43 deletions(-) > > > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > > index 02105c3..e7f86e1 100644 > > --- a/src/qemu/qemu_command.c > > +++ b/src/qemu/qemu_command.c > > @@ -1399,6 +1399,65 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info, > > return 0; > > } > > > > + > > +static int > > +qemuDomainAssignVirtioSerialAddresses(virDomainDefPtr def, > > + virDomainObjPtr obj) > > +{ > > + int ret = -1; > > + size_t i; > > + virDomainVirtioSerialAddrSetPtr addrs = NULL; > > + qemuDomainObjPrivatePtr priv = NULL; > > + > > + if (!(addrs = virDomainVirtioSerialAddrSetCreate())) > > + goto cleanup; > > Coverity determines addrs != NULL, but > > > + > > + if (virDomainVirtioSerialAddrSetAddControllers(addrs, def) < 0) > > + goto cleanup; > > + > > + if (virDomainDeviceInfoIterate(def, virDomainVirtioSerialAddrReserve, > > + addrs) < 0) > > + goto cleanup; > > + > > + VIR_DEBUG("Finished reserving existing ports"); > > + > > + for (i = 0; i < def->nconsoles; i++) { > > + virDomainChrDefPtr chr = def->consoles[i]; > > + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && > > + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { > > + if (virDomainVirtioSerialAddrAssign(addrs, &chr->info, true) < 0) > > + goto cleanup; > > + } > > + } > > + > > + for (i = 0; i < def->nchannels; i++) { > > + virDomainChrDefPtr chr = def->channels[i]; > > + if (chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL && > > + chr->info.addr.vioserial.port == 0 && > > + virDomainVirtioSerialAddrAssign(addrs, &chr->info, false) < 0) > > + goto cleanup; > > + } > > + > > + if (obj && obj->privateData) { > > + priv = obj->privateData; > > + if (addrs) { > > There's a check here where the "else" is DEADCODE > Right. The address set should only be generated if there is a virtio-serial controller present. > > + /* if this is the live domain object, we persist the addresses */ > > + virDomainVirtioSerialAddrSetFree(priv->vioserialaddrs); > > + priv->persistentAddrs = 1; > > + priv->vioserialaddrs = addrs; > > + addrs = NULL; > > + } else { > > + priv->persistentAddrs = 0; > > + } > > + } > > + ret = 0; > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > > index ae315df..5589f22 100644 > > --- a/src/qemu/qemu_process.c > > +++ b/src/qemu/qemu_process.c > > @@ -5205,6 +5205,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, > > virDomainDefClearCCWAddresses(vm->def); > > virDomainCCWAddressSetFree(priv->ccwaddrs); > > priv->ccwaddrs = NULL; > > + virDomainVirtioSerialAddrSetFree(priv->vioserialaddrs); > > + priv->vioserialaddrs = NULL; > > } > > > > qemuDomainReAttachHostDevices(driver, vm->def); > > Mostly for my edification... These examples previously although > indicating they were "auto-assign" (of sorts) really weren't - they were > more force-assign for each example. Force-assign after this patch? Otherwise I don't understand. > The way to auto-assign is by setting port='0', correct? > Yes. > However, I'm still missing something from the *auto.args output. It > seems the controller='#' is ignored and I guess I don't understand > that... I must've overlooked that. It shouldn't be a problem to take it as a hint in virDomainVirtioSerialAddrAssign. > Sure "port='0'" (meaning first available port on the > controller), but I would have expected to see : > > kvm.port.0 use "virtio.serial.0.0,nr=1..." (which it does) > kvm.port.foo use "virtio.serial.1.0,nr=1..." (on controller 0?, but XML > has controller='1') > kvm.port.bar use "virtio.serial.1.0,nr=3..." (which it does) > kvm.port.wizz use "virtio.serial.0.0,nr=2" (incorrect due to others) > kvm.port.ooh use "virtio.serial.1.0,nr=2", (on controller 0?, but XML > has controller='1' > kvm.port.lla use "virtio.serial.2.0,nr=1" (on controller 0?, but XML has > controller='2') > > Now if been if "lla" used controller='0', then I assume would nr=4 be > chosen since 1,2 were auto-assigned, 3 was specifically assigned, thus 4 > would be the next one. > > Continuing with that same logic, the *-autoassign example could have > shown that if controller='0',port='2' and 'controller='1',port='1' were > preassigned, then the controllers/ports assigned would be 0.0,nr=1, > 0.0,nr=3, 0.0,nr=4, 1.0,nr=2 (since only 4 ports on controller='0' can > be used w/ 2 be static and controller='1' having port '1' already in use). > nr=4 is out of bounds for a controller with 4 ports. The ports are numbered from 0, but port number 0 can only be used for virtconsoles, not channels. Jan
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list