Re: [Qemu-devel] Qemu-devel Digest, Vol 112, Issue 513

[Paulo Arcinas]

address@hidden
On Jul 22, 2012 11:38 PM, <address@hidden> wrote:
>
> Send Qemu-devel mailing list submissions to
>         address@hidden
>
> To subscribe or unsubscribe via the World Wide Web, visit
>         https://lists.nongnu.org/mailman/listinfo/qemu-devel
> or, via email, send a message with subject or body 'help' to
>         address@hidden
>
> You can reach the person managing the list at
>         address@hidden
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Qemu-devel digest..."
>
>
> Today's Topics:
>
>    1. [PATCH] configure: fix libcacard check (Alon Levy)
>    2. Re: [PATCH] configure: fix libcacard check (Peter Maydell)
>    3. Re: [PATCH v7.5] kvm: notify host when the guest is panicked
>       (Sasha Levin)
>    4. Re: [PATCH] configure: fix libcacard check (Alon Levy)
>    5. Re: [RFC PATCH 0/2] GlusterFS support in QEMU - v2
>       (Stefan Hajnoczi)
>    6. Re: question about qemu migrate. (Sheldon)
>    7. Re: [RFC PATCH 2/2] block: gluster as block backend
>       (Stefan Hajnoczi)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Sun, 22 Jul 2012 14:00:29 +0300
> From: Alon Levy <address@hidden>
> To: address@hidden
> Subject: [Qemu-devel] [PATCH] configure: fix libcacard check
> Message-ID: <address@hidden>
>
> SRC_PATH is undefined during configure run time, it is only defined in
> makefiles generated by it. Replace with source_path which is defined.
>
> Signed-off-by: Alon Levy <address@hidden>
> ---
>  configure |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 5fcd315..227c7d4 100755
> --- a/configure
> +++ b/configure
> @@ -2653,7 +2653,7 @@ if test "$smartcard" != "no" ; then
>  #include <pk11pub.h>
>  int main(void) { PK11_FreeSlot(0); return 0; }
>  EOF
> -        smartcard_cflags="-I\$(SRC_PATH)/libcacard"
> +        smartcard_cflags="-I$source_path/libcacard"
>          libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
>          libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
>          if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
> --
> 1.7.10.1
>
>
>
>
> ------------------------------
>
> Message: 2
> Date: Sun, 22 Jul 2012 12:30:27 +0100
> From: Peter Maydell <address@hidden>
> To: Alon Levy <address@hidden>
> Cc: address@hidden
> Subject: Re: [Qemu-devel] [PATCH] configure: fix libcacard check
> Message-ID:
>         <address@hidden>
> Content-Type: text/plain; charset=UTF-8
>
> On 22 July 2012 12:00, Alon Levy <address@hidden> wrote:
> > SRC_PATH is undefined during configure run time, it is only defined in
> > makefiles generated by it. Replace with source_path which is defined.
>
> There's a (slightly different) patch to address this issue in my
> configure series I posted earlier this week:
> http://patchwork.ozlabs.org/patch/171698/
>
> -- PMM
>
>
>
> ------------------------------
>
> Message: 3
> Date: Sun, 22 Jul 2012 13:39:57 +0200
> From: Sasha Levin <address@hidden>
> To: Wen Congyang <address@hidden>
> Cc: Gleb Natapov <address@hidden>, kvm list <address@hidden>,
>         Jan Kiszka <address@hidden>, qemu-devel
>         <address@hidden>,        "address@hidden"
>         <address@hidden>, Avi Kivity <address@hidden>,    KAMEZAWA
>         Hiroyuki <address@hidden>
> Subject: Re: [Qemu-devel] [PATCH v7.5] kvm: notify host when the guest
>         is panicked
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=ISO-8859-1
>
> On 07/21/2012 10:44 AM, Wen Congyang wrote:
> > We can know the guest is panicked when the guest runs on xen.
> > But we do not have such feature on kvm.
> >
> > Another purpose of this feature is: management app(for example:
> > libvirt) can do auto dump when the guest is panicked. If management
> > app does not do auto dump, the guest's user can do dump by hand if
> > he sees the guest is panicked.
> >
> > We have three solutions to implement this feature:
> > 1. use vmcall
> > 2. use I/O port
> > 3. use virtio-serial.
> >
> > We have decided to avoid touching hypervisor. The reason why I choose
> > choose the I/O port is:
> > 1. it is easier to implememt
> > 2. it does not depend any virtual device
> > 3. it can work when starting the kernel
>
> Was the option of implementing a virtio-watchdog driver considered?
>
> You're basically re-implementing a watchdog, a guest-host interface and a set of protocols for guest-host communications.
>
> Why can't we re-use everything we have now, push a virtio watchdog driver into drivers/watchdog/, and gain a more complete solution to detecting hangs inside the guest.
>
>
>
> ------------------------------
>
> Message: 4
> Date: Sun, 22 Jul 2012 15:05:08 +0300
> From: Alon Levy <address@hidden>
> To: Peter Maydell <address@hidden>
> Cc: address@hidden
> Subject: Re: [Qemu-devel] [PATCH] configure: fix libcacard check
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=us-ascii
>
> On Sun, Jul 22, 2012 at 12:30:27PM +0100, Peter Maydell wrote:
> > On 22 July 2012 12:00, Alon Levy <address@hidden> wrote:
> > > SRC_PATH is undefined during configure run time, it is only defined in
> > > makefiles generated by it. Replace with source_path which is defined.
> >
> > There's a (slightly different) patch to address this issue in my
> > configure series I posted earlier this week:
> > http://patchwork.ozlabs.org/patch/171698/
> >
>
> Your fix looks fine. I appologize for not checking - wasn't sure if this
> happens when not requesting libcacard compilation (via
> --enable-smartcard-nss), so didn't think it was already addressed on the
> list, my bad.
>
> > -- PMM
> >
>
>
>
> ------------------------------
>
> Message: 5
> Date: Sun, 22 Jul 2012 15:42:28 +0100
> From: Stefan Hajnoczi <address@hidden>
> To: address@hidden
> Cc: Amar Tumballi <address@hidden>, Anand Avati
>         <address@hidden>,    address@hidden, Vijay Bellur
>         <address@hidden>
> Subject: Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU -
>         v2
> Message-ID:
>         <CAJSP0QX6SJMC_i3bJO2gYFHag+QxPo5zsVhKo2OjZYLt+8x2=address@hidden>
> Content-Type: text/plain; charset=ISO-8859-1
>
> On Sat, Jul 21, 2012 at 9:29 AM, Bharata B Rao
> <address@hidden> wrote:
> > -drive file=gluster:address@hidden:volname:image
> >
> > - Here 'gluster' is the protocol.
> > - 'address@hidden' specifies the server where the volume file specification for
> >   the given volume resides. 'port' is the port number on which gluster
> >   management daemon (glusterd) is listening. This is optional and if not
> >   specified, QEMU will send 0 which will make libgfapi to use the default
> >   port.
>
> 'address@hidden' is weird notation.  Normally it is 'server:port' (e.g.
> URLs).  Can you change it?
>
> What about the other transports supported by libgfapi: UNIX domain
> sockets and RDMA?  My reading of glfs.h is that there are 3 connection
> options:
> 1. 'transport': 'socket' (default), 'unix', 'rdma'
> 2. 'host': server hostname for 'socket', path to UNIX domain socket
> for 'unix', or something else for 'rdma'
> 3. 'port': TCP port when 'socket' is used.  Ignored otherwise.
>
> Unfortunately QEMU block drivers cannot take custom options yet.  That
> would make it possible to cleanly map these connection options and
> save you from inventing syntax which doesn't expose all options.
>
> In the meantime it would be nice if the syntax exposed all options.
>
> > Note that we are no longer using volfiles directly and use volume names
> > instead. For this to work, gluster management daemon (glusterd) needs to
> > be running on the QEMU node. This limits the QEMU user to access the volumes by
> > the default volfiles that are generated by gluster CLI. This should be
> > fine as long as gluster CLI provides the capability to generate or regenerate
> > volume files for a given volume with the xlator set that QEMU user is
> > interested in. GlusterFS developers tell me that this can be provided with
> > some enhancements to Gluster CLI/glusterd. Note that the custom volume files
> > is typically needed when GlusterFS server is co-located with QEMU in
> > which case it would  be beneficial to get rid of client-server overhead and
> > RPC communication overhead.
>
> My knowledge of GlusterFS is limited.  Here is what I am thinking:
>
> 1. The user cannot specify a local configuration file, you require
> that there is a glusterd running which provides configuration
> information.
> 2. It is currently not possible to bypass RPC because the glusterd
> managed configuration file doesn't support that.
>
> I'm not sure if these statements are true?
>
> Would you support local volfiles in the future again?  Why force users
> to run glusterd?
>
> > - As mentioned above, the VM image on gluster volume can be specified like
> >   this:
> >         -drive file=gluster:localhost:testvol:/F17,format=gluster
> >
> >   Note that format=gluster is not needed ideally and its a work around I have
> >   until libgfapi provides a working connection cleanup routine (glfs_fini()).
> >   When the format isn't specified, QEMU figures out the format by doing
> >   find_image_format that results in one open and close before opening the
> >   image file long term for standard read and write. Gluster connection
> >   initialization is done from open and connection termination is done from
> >   close. But since glfs_fini() isn't working yet, I am bypassing
> >   find_image_format by specifying format=gluster directly which results in
> >   just one open and hence I am not limited by glfs_fini().
>
> Has libgfapi been released yet?  Does it have versioning which will
> allow the QEMU GlusterFS block driver to build against different
> versions?  I'm just wondering how the pieces will fit together once
> distros start shipping them.
>
> Stefan
>
>
>
> ------------------------------
>
> Message: 6
> Date: Sun, 22 Jul 2012 22:59:43 +0800
> From: Sheldon <address@hidden>
> To: Amos Kong <address@hidden>
> Cc: address@hidden, Stefan Hajnoczi
>         <address@hidden>
> Subject: Re: [Qemu-devel] question about qemu migrate.
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset="iso-8859-1"; Format="flowed"
>
> On 07/22/2012 12:49 PM, Amos Kong wrote:
> > On Sun, Jul 22, 2012 at 12:30 PM, Sheldon <address@hidden
> > <mailto:address@hidden>> wrote:
> >
> >     I find there are four kinds of uri for qmp_migrate
> >     "tcp:", "exec:", "unix:", and "fd:"
> >
> >
> >     now I want to migrate the vm to a local file with "exec:" uri by
> >     hmp-command.
> >
> >     How should I do the migration?
> >
> >
> > (qemu) migrate -d exec:cat>/tmp/vm.out
> > .... -incoming fd:</tmp/vm.out
>
> thank you. it can work now.
> there are still some questions.
> 1. the migration file vm.out  is  a QEMU suspend to disk image.
> How can I reboot vm.out by QEMU?
>
> 2. I execute "migrate" with -b option, seems that the "migrate" runs at
> background.
> I cat get the "migrate" information by qmp , there is a { "execute":
> "query-migrate" } command in qmp-command.
> But I can not get the the "migrate" information by hmp,  there is not a
> "query-migrate" command in hmp-command.
>
>
>
>
>
>
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.nongnu.org/archive/html/qemu-devel/attachments/20120722/bb1ee297/attachment.html>
>
> ------------------------------
>
> Message: 7
> Date: Sun, 22 Jul 2012 16:38:00 +0100
> From: Stefan Hajnoczi <address@hidden>
> To: address@hidden
> Cc: Amar Tumballi <address@hidden>, Anand Avati
>         <address@hidden>,    address@hidden, Vijay Bellur
>         <address@hidden>
> Subject: Re: [Qemu-devel] [RFC PATCH 2/2] block: gluster as block
>         backend
> Message-ID:
>         <CAJSP0QXv9-0-_dvC1kPz0JoMCs7T80nGyuYo=VH3eRge4ODi=address@hidden>
> Content-Type: text/plain; charset=ISO-8859-1
>
> On Sat, Jul 21, 2012 at 9:31 AM, Bharata B Rao
> <address@hidden> wrote:
> > +typedef struct GlusterAIOCB {
> > +    BlockDriverAIOCB common;
> > +    QEMUIOVector *qiov;
>
> The qiov field is unused.
>
> > +    char *bounce;
>
> Unused.
>
> > +    struct BDRVGlusterState *s;
>
> You can get this through common.bs->opaque, but if you like having a
> shortcut, that's fine.
>
> > +    int cancelled;
>
> bool
>
> > +} GlusterAIOCB;
> > +
> > +typedef struct GlusterCBKData {
> > +    GlusterAIOCB *acb;
> > +    struct BDRVGlusterState *s;
> > +    int64_t size;
> > +    int ret;
> > +} GlusterCBKData;
>
> I think GlusterCBKData could just be part of GlusterAIOCB.  That would
> simplify the code a little and avoid some malloc/free.
>
> > +
> > +typedef struct BDRVGlusterState {
> > +    struct glfs *glfs;
> > +    int fds[2];
> > +    int open_flags;
> > +    struct glfs_fd *fd;
> > +    int qemu_aio_count;
> > +    int event_reader_pos;
> > +    GlusterCBKData *event_gcbk;
> > +} BDRVGlusterState;
> > +
> > +#define GLUSTER_FD_READ 0
> > +#define GLUSTER_FD_WRITE 1
> > +
> > +static void qemu_gluster_complete_aio(GlusterCBKData *gcbk)
> > +{
> > +    GlusterAIOCB *acb = gcbk->acb;
> > +    int ret;
> > +
> > +    if (acb->cancelled) {
>
> Where does cancelled get set?
>
> > +        qemu_aio_release(acb);
> > +        goto done;
> > +    }
> > +
> > +    if (gcbk->ret == gcbk->size) {
> > +        ret = 0; /* Success */
> > +    } else if (gcbk->ret < 0) {
> > +        ret = gcbk->ret; /* Read/Write failed */
> > +    } else {
> > +        ret = -EINVAL; /* Partial read/write - fail it */
>
> EINVAL is for invalid arguments.  EIO would be better.
>
> > +/*
> > + * file=protocol:address@hidden:volname:image
> > + */
> > +static int qemu_gluster_parsename(GlusterConf *c, const char *filename)
> > +{
> > +    char *file = g_strdup(filename);
> > +    char *token, *next_token, *saveptr;
> > +    char *token_s, *next_token_s, *saveptr_s;
> > +    int ret = -EINVAL;
> > +
> > +    /* Discard the protocol */
> > +    token = strtok_r(file, ":", &saveptr);
> > +    if (!token) {
> > +        goto out;
> > +    }
> > +
> > +    /* address@hidden */
> > +    next_token = strtok_r(NULL, ":", &saveptr);
> > +    if (!next_token) {
> > +        goto out;
> > +    }
> > +    if (strchr(next_token, '@')) {
> > +        token_s = strtok_r(next_token, "@", &saveptr_s);
> > +        if (!token_s) {
> > +            goto out;
> > +        }
> > +        strncpy(c->server, token_s, HOST_NAME_MAX);
>
> strncpy(3) will not NUL-terminate when token_s is HOST_NAME_MAX
> characters long.  QEMU has cutils.c:pstrcpy().
>
> When the argument is too long we should probably report an error
> instead of truncating.
>
> Same below.
>
> > +        next_token_s = strtok_r(NULL, "@", &saveptr_s);
> > +        if (!next_token_s) {
> > +            goto out;
> > +        }
> > +        c->port = atoi(next_token_s);
>
> No error checking.  If the input is invalid an error message would
> help the user here.
>
> > +static struct glfs *qemu_gluster_init(GlusterConf *c, const char *filename)
> > +{
> > +    struct glfs *glfs = NULL;
> > +    int ret;
> > +
> > +    ret = qemu_gluster_parsename(c, filename);
> > +    if (ret < 0) {
> > +        errno = -ret;
> > +        goto out;
> > +    }
> > +
> > +    glfs = glfs_new(c->volname);
> > +    if (!glfs) {
> > +        goto out;
> > +    }
> > +
> > +    ret = glfs_set_volfile_server(glfs, "socket", c->server, c->port);
> > +    if (ret < 0) {
> > +        goto out;
> > +    }
> > +
> > +    /*
> > +     * TODO: Logging is not necessary but instead nice to have.
> > +     * Can QEMU optionally log into a standard place ?
>
> QEMU prints to stderr, can you do that here too?  The global log file
> is not okay, especially when multiple QEMU instances are running.
>
> > +     * Need to use defines like gf_loglevel_t:GF_LOG_INFO instead of
> > +     * hard coded values like 7 here.
> > +     */
> > +    ret = glfs_set_logging(glfs, "/tmp/qemu-gluster.log", 7);
> > +    if (ret < 0) {
> > +        goto out;
> > +    }
> > +
> > +    ret = glfs_init(glfs);
> > +    if (ret < 0) {
> > +        goto out;
> > +    }
> > +    return glfs;
> > +
> > +out:
> > +    if (glfs) {
> > +        (void)glfs_fini(glfs);
> > +    }
> > +    return NULL;
> > +}
> > +
> > +static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
> > +    int bdrv_flags)
> > +{
> > +    BDRVGlusterState *s = bs->opaque;
> > +    GlusterConf *c = g_malloc(sizeof(GlusterConf));
>
> Can this be allocated on the stack?
>
> > +    int ret;
> > +
> > +    s->glfs = qemu_gluster_init(c, filename);
> > +    if (!s->glfs) {
> > +        ret = -errno;
> > +        goto out;
> > +    }
> > +
> > +    s->open_flags |=  O_BINARY;
>
> Can open_flags be a local variable?
>
> > +static int qemu_gluster_create(const char *filename,
> > +        QEMUOptionParameter *options)
> > +{
> > +    struct glfs *glfs;
> > +    struct glfs_fd *fd;
> > +    GlusterConf *c = g_malloc(sizeof(GlusterConf));
> > +    int ret = 0;
> > +    int64_t total_size = 0;
> > +
> > +    glfs = qemu_gluster_init(c, filename);
> > +    if (!glfs) {
> > +        ret = -errno;
> > +        goto out;
> > +    }
> > +
> > +    /* Read out options */
> > +    while (options && options->name) {
> > +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> > +            total_size = options->value.n / BDRV_SECTOR_SIZE;
> > +        }
> > +        options++;
> > +    }
> > +
> > +    fd = glfs_creat(glfs, c->image, O_WRONLY|O_CREAT|O_TRUNC|O_BINARY, S_IRWXU);
>
> Why set the execute permission bit?
>
> > +static void qemu_gluster_close(BlockDriverState *bs)
> > +{
> > +    BDRVGlusterState *s = bs->opaque;
> > +
> > +    if (s->fd) {
> > +        glfs_close(s->fd);
> > +        s->fd = NULL;
> > +    }
>
> Why not call glfs_fini() here?
>
>
>
> ------------------------------
>
> _______________________________________________
> Qemu-devel mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/qemu-devel
>
>
> End of Qemu-devel Digest, Vol 112, Issue 513
> ********************************************