Re: [PATCH kvm-unit-tests 2/4] Introduce a C++ wrapper for the kvm APIs

Gleb Natapov
2010-11-24T15:40:12Z

On Wed, Nov 24, 2010 at 08:41:26AM -0600, Anthony Liguori wrote:
> On 11/24/2010 06:59 AM, Alexander Graf wrote:
> >On 24.11.2010, at 11:52, Avi Kivity wrote:
> >
> >>Introduce exception-safe objects for calling system, vm, and vcpu ioctls.
> >>
> >>Signed-off-by: Avi Kivity<avi@redhat.com>
> >
> >FWIW, I still disagree with C++ and believe this code to be hardly readable.
>
> There's a general prettiness that well written C++ code will have
> over C when there's heavy object modelling. This can be subjective
> but for me, it's fairly significant.
>
> The fact that objects are easily created on the stack and on the
> heap is also pretty significant. When considering device models, we
> struggle today with device composition.
>
> In real hardware, the i8042 (keyboard controller) is actually
> implemented in the PIIX3 which is a chip that is part of the i440fx.
> The i440fx acts as both the memory controller and as the PCI Host
> controller. So you get something that looks like:
>
> class PIIX3 : public PCIDevice
> {
> private:
> I8042 i8042;
> RTC rtc;
> // ...
> };
>
The fact that in physical implementation they sit in the same silicon
does not mean that logically they belong to the same class. PIIX3
is ISA bridge. It doesn't mean it owns devices on the ISA bus it
provides. The information that you are trying to convey here belongs to
configuration file. Here I go factory design pattern.

> class I440FX : public PCIHostController
> {
> I440FX(void) {
> this->slots[1].plug(&this->piix3); // piix3 is always in slot 1
> }
>
> private:
> Plug<PCIDevice *> slots[32]; // slot 0 is the PMC
> PIIX3 piix3;
> };
>
> So whereas we have this very complicate machine create function that
> attempts to create and composite all of these devices after the
> fact, when written in C++, partially due to good design, but
> partially due to the fact that the languages forces you to think a
> certain way, you get a tremendous simplification.
>
Forcing to think you in certain way does not make you suddenly make good
design decisions (if only programming was so easy). But it makes it even
harder to fix bad decision since suddenly all you design depends on it.

> A proper C++ device model turns a vast majority of our device
> creation complexity into a single new I440FX. Then it's just a
> matter of instantiating and plugging the appropriate set of PCI
> devices.
That is if you are using code as data. With other design you actually
read I440FX configuration from file and build it from smaller parts.
You see C++ doesn't stop us from arguing what design is correct.

>
> Of course, this can be wrapped in a factory to make it drivable via
> an API or config file.
Exactly.

>
> Another area that C++ shines is safety. C++ enables you to inject
> safe versions of things that you really can't do in C. For
> instance, the PIT has three channels but the mask to select a
> channel is two bits. There was a kernel exploit that found a way to
> trick selection of a forth channel because of a missing check.
>
> In C++, you can convert:
>
> PITChannel channnels[3];
>
> Into:
>
> Array<PITChannel, 3> channels;
>
Any sane modern language gives you that. Why C++?

> It behaves in every other way just like a normal array. The memory
> is stack allocated, the type has a fixed size. The only difference
> is that you can overload the [] operators and implement bounds
> checking for array accesses. This means that as long as you use
> Array<>, array overflows disappear from the code base. That's a big
> deal.
>
> Another area C++ shines is generating metacode. Consider the
> ugliness around VMState. The crux of the problem is that it's not
> possible to write type-neutral code in C. This all gets simplified
> with C++. Instead of having a bunch of macros like:
>
> VMSTATE_INT8(val0, ...)
> VMSTATE_INT16(val1, ...)
>
> You can just have:
>
> vmstate(val0)
> vmstate(val1)
>
> And use type overloading to implement different behaviors. Combined
> with template specialization and an Array wrapper, the same thing
> works for arrays too.
>
> Regards,
>
> Anthony Liguori
>
> Regards,
>
> Anthony Liguori
>
> >Alex
> >
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe kvm" in
> >the body of a message to majordomo@vger.kernel.org
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Gleb.