Hi,

On Wed, Sep 2, 2015 at 2:21 PM, Dimitris Aragiorgis <
[email protected]> wrote:

> Hi,
>
> It seems that the first round of review is over. I am working on v2 of
> the patch-set that will include all comments made in the thread. Just to
> be sure I am not missing anything important (besides the nits and epydoc
> stuff), I should:
>
>  - Modify design doc and patches to handle the 32 slots of the PCI bus
> properly
>  - Use Constants.hs where necessary
>  - Move SCSI controller stuff from patch 2 to patch 6
>  - Move comment for drive_add_fn to patch 5 to patch 4
>  - Add a unittest for _GenerateDeviceHVInfo()
>

After seeing the runtime issues, I do have an additional comment, but it is
nothing huge, and can be added as a separate patch by me later. I'll
respond in the appropriate patch to hear your opinion, without delaying the
process further.


>
> Unresolved:
>
>   - I think we cannot handle the upgrade-reboot-downgrade corner
>     case seamlessly.
>

There is one option - SSH'ing into all available nodes and changing the
values manually upon a downgrade. Perhaps not the best solution, and one
best handled in additional patches.


>
> Are you OK with the above?
>

Yes!


>
> Thanks a lot for the review,
> dimara
>

Thanks a lot for the contribution,
Riba


>
>
> * Dimitris Aragiorgis <[email protected]> [2015-08-18
> 13:32:36 +0300]:
>
> > This patch-set targets stable-2.16 as the cover letter implies.
> > Please ignore the [PATCH stable-2.14] on the patch titles.
> > It was a typo while using git-format-patch :(
> >
> > Sorry for that,
> > dimara
> >
> > * Dimitris Aragiorgis <[email protected]> [2015-08-18
> 13:06:55 +0300]:
> >
> > > Hi,
> > >
> > > This is actually a resend of a previous patch-set. It contains some
> > > major fixes regarding:
> > >
> > >  1) Hotplug support for QEMU 2.2 and later which is now broken
> > >  2) Buggy behavior when using hotplug on instances with non
> paravirtual disks
> > >  3) The way we construct the QEMU command line which is error-prone
> > >
> > > At the end of the day we get:
> > >
> > >  1) Robust hotplug support
> > >  2) Full, generic SCSI support
> > >
> > > Thanks,
> > > dimara
> > >
> > > PS: The patch-set passes coverage and distcheck. qa-kvm-tiny on
> external
> > > buildbot is broken. Fixing it would be really helpful :)
> > >
> > > Dimitris Aragiorgis (6):
> > >   Add design doc for SCSI support in KVM
> > >   kvm: Refactor device option handling
> > >   monitor: Use hvinfo in QMP methods
> > >   kvm: Work around QEMU commit 48f364dd
> > >   kvm: Use the new interface during hotplug actions
> > >   kvm: Add new scsi_controller_type hvparam
> > >
> > >  Makefile.am                                  |    1 +
> > >  doc/design-draft.rst                         |    1 +
> > >  doc/design-scsi-kvm.rst                      |  220 +++++++++++++++
> > >  lib/hypervisor/hv_kvm/__init__.py            |  371
> ++++++++++++++++++++------
> > >  lib/hypervisor/hv_kvm/monitor.py             |   88 ++++--
> > >  lib/objects.py                               |    3 +-
> > >  man/gnt-instance.rst                         |   11 +
> > >  src/Ganeti/Constants.hs                      |   40 ++-
> > >  test/data/kvm_runtime.json                   |   21 +-
> > >  test/py/ganeti.hypervisor.hv_kvm_unittest.py |    9 +-
> > >  10 files changed, 642 insertions(+), 123 deletions(-)
> > >  create mode 100644 doc/design-scsi-kvm.rst
> > >
> > > --
> > > 1.7.10.4
>
>
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Geschäftsführer: Graham Law, Christine Elizabeth Flores
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Reply via email to