Re: [PATCH V3 1/2] migration: file URI

[Peter Xu]

On Wed, Jun 28, 2023 at 01:07:24PM -0400, Peter Xu wrote:
> On Thu, Jun 22, 2023 at 01:37:30PM -0700, Steve Sistare wrote:
> > Extend the migration URI to support file:<filename>.  This can be used for
> > any migration scenario that does not require a reverse path.  It can be
> > used as an alternative to 'exec:cat > file' in minimized containers that
> > do not contain /bin/sh, and it is easier to use than the fd:<fdname> URI.
> > It can be used in HMP commands, and as a qemu command-line parameter.
> > 
> > For best performance, guest ram should be shared and x-ignore-shared
> > should be true, so guest pages are not written to the file, in which case
> > the guest may remain running.  If ram is not so configured, then the user
> > is advised to stop the guest first.  Otherwise, a busy guest may re-dirty
> > the same page, causing it to be appended to the file multiple times,
> > and the file may grow unboundedly.  That issue is being addressed in the
> > "fixed-ram" patch series.
> > 
> > Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> > Reviewed-by: Fabiano Rosas <farosas@suse.de>
> > ---
> >  migration/file.c       | 62 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  migration/file.h       | 14 ++++++++++++
> >  migration/meson.build  |  1 +
> >  migration/migration.c  |  5 ++++
> >  migration/trace-events |  4 ++++
> >  qemu-options.hx        |  6 ++++-
> >  6 files changed, 91 insertions(+), 1 deletion(-)
> >  create mode 100644 migration/file.c
> >  create mode 100644 migration/file.h
> > 
> > diff --git a/migration/file.c b/migration/file.c
> > new file mode 100644
> > index 0000000..8e35827
> > --- /dev/null
> > +++ b/migration/file.c
> > @@ -0,0 +1,62 @@
> > +/*
> > + * Copyright (c) 2021-2023 Oracle and/or its affiliates.
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "channel.h"
> > +#include "file.h"
> > +#include "migration.h"
> > +#include "io/channel-file.h"
> > +#include "io/channel-util.h"
> > +#include "trace.h"
> > +
> > +void file_start_outgoing_migration(MigrationState *s, const char *filename,
> > +                                   Error **errp)
> > +{
> > +    g_autoptr(QIOChannelFile) fioc = NULL;
> > +    QIOChannel *ioc;
> > +
> > +    trace_migration_file_outgoing(filename);
> > +
> > +    fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | 
> > O_TRUNC,
> > +                                     0600, errp);
> > +    if (!fioc) {
> > +        return;
> > +    }
> > +
> > +    ioc = QIO_CHANNEL(fioc);
> > +    qio_channel_set_name(ioc, "migration-file-outgoing");
> > +    migration_channel_connect(s, ioc, NULL, NULL);
> 
> Miss one object_unref(ioc)?

Never mind, I overlooked the g_autoptr.. all fine:

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu