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

Avi Kivity
2010-11-28T09:31:13Z

On 11/28/2010 10:58 AM, Michael S. Tsirkin wrote:
> On Sat, Nov 27, 2010 at 11:12:58AM +0200, Avi Kivity wrote:
> > On 11/26/2010 12:16 PM, Michael S. Tsirkin wrote:
> > >On Wed, Nov 24, 2010 at 12:52:11PM +0200, Avi Kivity wrote:
> > >> Introduce exception-safe objects for calling system, vm, and vcpu ioctls.
> > >>
> > >> Signed-off-by: Avi Kivity<avi@redhat.com>
> > >
> > >ioctlp calls below ignore possible errors.
> > >Somre more comments below.
> > >
> >
> > Can you elaborate? The simply propagate the exception.
>
> I was confused by this:
>
> + long ioctlp(unsigned nr, void *arg) {
> + return ioctl(nr, reinterpret_cast<long>(arg));
> + }
>
>
> ioctl here is not the C ioctl function. It's the local method that
> throws exceptions on errors. This will likely confuse others
> as well.

Could do this->ioctl(), though I don't much like it.

> > >> +
> > >> +std::vector<kvm_msr_entry> vcpu::msrs(std::vector<uint32_t> indices)
> > >> +{
> > >> + std::auto_ptr<kvm_msrs> msrs(alloc_msr_list(indices.size()));
> > >
> > >This looks wrong. auto_ptr frees memory with delete,
> > >alloc_msr_list allocates it with malloc.
> >
> > Anthony pointed this out as well.
>
> Another problem is that there seem to be two memory allocations and a
> copy here, apparently just to simplify error handling. It might be fine
> for this test but won't scale for when performance matters.

When it matters, we can fix it. I don't see msr read/write becoming a
hot path.

> > Fixed by replacing
> > alloc_msr_list() by an object.
>
> It seems that any action which has side effects which needs to be
> undone on error, we will have to have a new class with constructor doing
> the work. This will likely create much more lines of code
> than a simple goto end strategy.

It creates correctness. The equivalent in qemu is to create a constant
size array on the stack, because people can't be bothered with error
checking.

The lines of code pay back as soon as there is some reuse (2x in this case).

> One also wonders how well will the compiler be able to optimize
> such complex code, and by how much will compile times go up.
>
> Any input on that?

The compiler should optimize it away completely. There's been a lot of
work in gcc on that.

About compile times, I don't care much.

--
error compiling committee.c: too many arguments to function