[libvirt] [PATCHv2 3/5] Allocate virtio-serial addresses when starting a domain
Ján Tomko
jtomko at redhat.com
Tue Mar 24 11:41:12 UTC 2015
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150324/db02b940/attachment-0001.sig>
More information about the libvir-list
mailing list