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

["Michael S. Tsirkin" <mst@redhat.com>]

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.

> ---
>  api/kvmxx.cc |  168 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  api/kvmxx.h  |   80 +++++++++++++++++++++++++++
>  2 files changed, 248 insertions(+), 0 deletions(-)
>  create mode 100644 api/kvmxx.cc
>  create mode 100644 api/kvmxx.h
> 
> diff --git a/api/kvmxx.cc b/api/kvmxx.cc
> new file mode 100644
> index 0000000..2f8fc27
> --- /dev/null
> +++ b/api/kvmxx.cc
> @@ -0,0 +1,168 @@
> +#include "kvmxx.h"
> +#include <fcntl.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <memory>
> +#include <algorithm>
> +
> +namespace kvm {
> +
> +static long check_error(long r)
> +{
> +    if (r == -1) {
> +	throw errno;
> +    }
> +    return r;
> +}
> +
> +fd::fd(int fd)
> +    : _fd(fd)
> +{
> +}
> +
> +fd::fd(const fd& other)
> +    : _fd(::dup(other._fd))
> +{
> +    check_error(_fd);
> +}
> +
> +fd::fd(std::string device_node, int flags)
> +    : _fd(::open(device_node.c_str(), flags))
> +{
> +    check_error(_fd);
> +}
> +
> +long fd::ioctl(unsigned nr, long arg)
> +{
> +    return check_error(::ioctl(_fd, nr, arg));
> +}
> +
> +vcpu::vcpu(vm& vm, int id)
> +    : _vm(vm), _fd(vm._fd.ioctl(KVM_CREATE_VCPU, id)), _shared(NULL)
> +{
> +    unsigned mmap_size = _vm._system._fd.ioctl(KVM_GET_VCPU_MMAP_SIZE, 0);
> +    kvm_run *shared = static_cast<kvm_run*>(::mmap(NULL, mmap_size,
> +						   PROT_READ | PROT_WRITE,
> +						   MAP_SHARED,
> +						   _fd.get(), 0));
> +    if (shared == MAP_FAILED) {
> +	throw errno;
> +    }
> +    _shared = shared;
> +}
> +
> +vcpu::~vcpu()
> +{
> +    unsigned mmap_size = _vm._system._fd.ioctl(KVM_GET_VCPU_MMAP_SIZE, 0);

This might throw an exception on destructor path, if this happens
because an exception was thrown terminate is called.

> +    munmap(_shared, mmap_size);
> +}
> +
> +void vcpu::run()
> +{
> +    _fd.ioctl(KVM_RUN, 0);
> +}
> +
> +kvm_regs vcpu::regs()
> +{
> +    kvm_regs regs;
> +    _fd.ioctlp(KVM_GET_REGS, &regs);
> +    return regs;
> +}
> +
> +void vcpu::set_regs(const kvm_regs& regs)
> +{
> +    _fd.ioctlp(KVM_SET_REGS, const_cast<kvm_regs*>(&regs));
> +}
> +
> +kvm_sregs vcpu::sregs()
> +{
> +    kvm_sregs sregs;
> +    _fd.ioctlp(KVM_GET_SREGS, &sregs);
> +    return sregs;
> +}
> +
> +void vcpu::set_sregs(const kvm_sregs& sregs)
> +{
> +    _fd.ioctlp(KVM_SET_SREGS, const_cast<kvm_sregs*>(&sregs));
> +}
> +
> +kvm_msrs* vcpu::alloc_msr_list(size_t nmsrs)
> +{
> +    size_t size = sizeof(kvm_msrs) + sizeof(kvm_msr_entry) * nmsrs;
> +    kvm_msrs* ret = static_cast<kvm_msrs*>(malloc(size));
> +    if (!ret) {
> +	throw ENOMEM;
> +    }
> +    return ret;
> +}
> +
> +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.

> +    msrs->nmsrs = indices.size();
> +    for (unsigned i = 0; i < msrs->nmsrs; ++i) {
> +	msrs->entries[i].index = indices[i];
> +    }
> +    _fd.ioctlp(KVM_GET_MSRS, msrs.get());
> +    return std::vector<kvm_msr_entry>(msrs->entries,
> +				      msrs->entries + msrs->nmsrs);
> +}
> +
> +void vcpu::set_msrs(const std::vector<kvm_msr_entry>& msrs)
> +{
> +    std::auto_ptr<kvm_msrs> _msrs(alloc_msr_list(msrs.size()));

As above.

> +    _msrs->nmsrs = msrs.size();
> +    std::copy(msrs.begin(), msrs.end(), _msrs->entries);
> +    _fd.ioctlp(KVM_SET_MSRS, _msrs.get());
> +}
> +
> +void vcpu::set_debug(uint64_t dr[8], bool enabled, bool singlestep)
> +{
> +    kvm_guest_debug gd;
> +
> +    gd.control = 0;
> +    if (enabled) {
> +	gd.control |= KVM_GUESTDBG_ENABLE;
> +    }
> +    if (singlestep) {
> +	gd.control |= KVM_GUESTDBG_SINGLESTEP;
> +    }
> +    for (int i = 0; i < 8; ++i) {
> +	gd.arch.debugreg[i] = dr[i];
> +    }
> +    _fd.ioctlp(KVM_SET_GUEST_DEBUG, &gd);
> +}
> +
> +vm::vm(system& system)
> +    : _system(system), _fd(system._fd.ioctl(KVM_CREATE_VM, 0))
> +{
> +}
> +
> +void vm::set_memory_region(int slot, void *addr, uint64_t gpa, size_t len)
> +{
> +    struct kvm_userspace_memory_region umr;
> +
> +    umr.slot = slot;
> +    umr.flags = 0;
> +    umr.guest_phys_addr = gpa;
> +    umr.memory_size = len;
> +    umr.userspace_addr = reinterpret_cast<uint64_t>(addr);
> +    _fd.ioctlp(KVM_SET_USER_MEMORY_REGION, &umr);
> +}
> +
> +void vm::set_tss_addr(uint32_t addr)
> +{
> +    _fd.ioctl(KVM_SET_TSS_ADDR, addr);
> +}
> +
> +system::system(std::string device_node)
> +    : _fd(device_node, O_RDWR)
> +{
> +}
> +
> +bool system::check_extension(int extension)
> +{
> +    return _fd.ioctl(KVM_CHECK_EXTENSION, extension);
> +}
> +
> +};
> diff --git a/api/kvmxx.h b/api/kvmxx.h
> new file mode 100644
> index 0000000..716e400
> --- /dev/null
> +++ b/api/kvmxx.h
> @@ -0,0 +1,80 @@
> +#ifndef KVMXX_H
> +#define KVMXX_H
> +
> +#include <string>
> +#include <signal.h>
> +#include <unistd.h>
> +#include <vector>
> +#include <errno.h>
> +#include <linux/kvm.h>
> +#include <stdint.h>
> +
> +namespace kvm {
> +
> +class system;
> +class vm;
> +class vcpu;
> +class fd;
> +
> +class fd {
> +public:
> +    explicit fd(int n);
> +    explicit fd(std::string path, int flags);
> +    fd(const fd& other);
> +    ~fd() { ::close(_fd); }
> +    int get() { return _fd; }
> +    long ioctl(unsigned nr, long arg);
> +    long ioctlp(unsigned nr, void *arg) {
> +	return ioctl(nr, reinterpret_cast<long>(arg));
> +    }
> +private:
> +    int _fd;
> +};
> +
> +class vcpu {
> +public:
> +    vcpu(vm& vm, int fd);
> +    ~vcpu();
> +    void run();
> +    kvm_run *shared();
> +    kvm_regs regs();
> +    void set_regs(const kvm_regs& regs);
> +    kvm_sregs sregs();
> +    void set_sregs(const kvm_sregs& sregs);
> +    std::vector<kvm_msr_entry> msrs(std::vector<uint32_t> indices);
> +    void set_msrs(const std::vector<kvm_msr_entry>& msrs);
> +    void set_debug(uint64_t dr[8], bool enabled, bool singlestep);
> +private:
> +    static kvm_msrs* alloc_msr_list(size_t nmsrs);
> +private:
> +    vm& _vm;
> +    fd _fd;
> +    kvm_run *_shared;
> +    friend class vm;
> +};
> +
> +class vm {
> +public:
> +    explicit vm(system& system);
> +    void set_memory_region(int slot, void *addr, uint64_t gpa, size_t len);
> +    void set_tss_addr(uint32_t addr);
> +private:
> +    system& _system;
> +    fd _fd;
> +    friend class system;
> +    friend class vcpu;
> +};
> +
> +class system {
> +public:
> +    explicit system(std::string device_node = "/dev/kvm");
> +    bool check_extension(int extension);
> +private:
> +    fd _fd;
> +    friend class vcpu;
> +    friend class vm;
> +};
> +
> +};
> +
> +#endif
> -- 
> 1.7.1
> 
> --
> 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