On Sun, Nov 28, 2010 at 11:31:09AM +0200, Avi Kivity wrote:
> 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<a...@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.
It will be very painful to fix it.
> >> 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.
Should as opposed to does. Want me to try a simple test?
> There's been a lot
> of work in gcc on that.
>
> About compile times, I don't care much.
I do. You will too when we have codebase that can be built as fast as
we commit things, so buildbot breaks.
This is common in C++ based projects.
> --
> error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
