[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 15/18] qemu: Support input model=virtio-{non-}transitional



On Wed, 2019-01-23 at 13:30 -0500, Cole Robinson wrote:
> On 01/22/2019 12:32 PM, Cole Robinson wrote:
> > On 01/21/2019 11:20 AM, Andrea Bolognani wrote:
> > > On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
> > > > @@ -917,6 +917,15 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
> > > >      case VIR_DOMAIN_DEVICE_INPUT:
> > > >          switch ((virDomainInputBus) dev->data.input->bus) {
> > > >          case VIR_DOMAIN_INPUT_BUS_VIRTIO:
> > > > +            switch ((virDomainInputModel) dev->data.input->model) {
> > > > +            case VIR_DOMAIN_INPUT_MODEL_VIRTIO_TRANSITIONAL:
> > > > +                return pciFlags;
> > > > +            case VIR_DOMAIN_INPUT_MODEL_VIRTIO:
> > > > +            case VIR_DOMAIN_INPUT_MODEL_VIRTIO_NON_TRANSITIONAL:
> > > > +            case VIR_DOMAIN_INPUT_MODEL_DEFAULT:
> > > > +            case VIR_DOMAIN_INPUT_MODEL_LAST:
> > > > +                break;
> > > > +            }
> > > >              return virtioFlags;
> > > 
> > > VIR_DOMAIN_INPUT_MODEL_DEFAULT and VIR_DOMAIN_INPUT_MODEL_LAST
> > > should result in 0 rather than virtioFlags being returned.
> > 
> > Oops good catch
> 
> Actually this missed a case: at least DEFAULT (meaning no model=
> specified) should return virtioFlags, since this block is already
> conditional on bus='virtio'. I'll have DEFAULT return virtioFlags and
> LAST return 0;

Now that you mention it, for devices such as <input> and <disk>,
where we're introducing the model attribute just now, we should make
sure that we expand the default correctly, such that eg.

  <disk type='file' device='disk'>
    <target dev='vda' bus='virtio'/>
  </disk>

is formatted back as

  <disk type='file' device='disk' model='virtio'>
    <target dev='vda' bus='virtio'/>
  </disk>

We might have to be careful when formatting the XML for migration
and scrub model='virtio' in that case in order not to break
backwards migration, as we already do for plenty of other cases.

Once that's done, then you'll be guaranteed the model above is
never going to be DEFAULT.

-- 
Andrea Bolognani / Red Hat / Virtualization


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]