On 11/28/2010 06:27 AM, Michael S. Tsirkin wrote:
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@xxxxxxxxxx>
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.
My guess is this comes from the fact that you are rewriting large pieces
of code from scratch so it suits your personal style perfectly :)
That's probably a good observations.
If history teaches us anything, as with most projects in qemu, what we will
end up with is a half done conversion of maybe 30% of the codebase.
The result might be anything: safer, more correct - but it won't be prettier.
This is where things need to be different. I'm not at all interested in
"introducing" C++ to QEMU because of exactly what you describe above.
I think the only viable approach is one where we have a segregated code
base that is correct with an incremental movement of code from the "old"
code base to the new way of doing things.
I've always thought that the device model should be a library and I
think that's the way to structure it. Have a libqemuhw and only move
devices into it as they are converted properly.
The fact that objects are easily created on the stack and on the
heap is also pretty significant.
Significant how?
To create an object on the stack, you must have the class definition in
a public header and a public constructor/destructor.
This is exactly the same in C.
It's really more of a design statement than a statement about C++ vs. C.
In qdev today, we mix object initialization with a user-exposed
factory. This means that you cannot do something simple like:
struct i440fx {
struct piix3 piix;
};
void i440fx_init(struct i440fx_init *obj) {
piix3_init(&obj->piix);
}
But rather need to use ugly factory functions with all sorts of
DO_UPCAST. This is really unfriendly especially for writing test cases.
But this isn't C vs. C++, this is just about device model design. I
think C++ makes it quite a bit more obvious though how to design
correctly though.
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;
// ...
};
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;
};
We can have the same thing today. In fact, getting rid of the UP_CAST
and opaque pointers should be a priority.
I find that you end up writing a lot more boiler plate code trying to do
this properly in C. I think gobject is probably the best example of
this. If you've ever written a GTK widget from scratch in C and then
written one in gtkmm, the difference is night and day.
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.
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.
Of course, this can be wrapped in a factory to make it drivable via
an API or config file.
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;
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.
Except that you get used to the fact that [] is safe,
and then forget to check the value in a dynamically
sized array access. Boom.
I don't think the fact that you get deterministic vs. non-deterministic
behavior changes the approach people take to using arrays. If it simply
threw away invalid accesses, I might buy your argument. But anyone that
catches an out of bound exception as a normal error handling mechanism
deserves to get beaten with a three day old trout.
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
At least with VMSTATE_INT16 I can grep and find the definition.
grep 'vmstate(.*, int16_t' *.hpp
Works perfectly fine.
Regards,
Anthony Liguori
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
