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
signature.asc
Description: Digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
