[Libosinfo] [libosinfo] Add 'persistent' parameter to OsinfoMedia

Daniel P. Berrange berrange at redhat.com
Thu Apr 13 09:15:18 UTC 2017


On Thu, Apr 13, 2017 at 11:01:06AM +0200, Christophe Fergeau wrote:
> On Wed, Apr 12, 2017 at 11:09:34AM +0200, Fabiano Fidêncio wrote:
> > If media is an installer, thus specifies whether the media should be
> > persistent accross the final reboot during its installation process.
> > Default value is false.
> > 
> > This is mainly needed for applications like GNOME Boxes (and maybe
> > virt-install) to be able to decide whether the media should be ejected
> > or not when the final reboot happens during its installation process.
> > 
> > The latter case may happen when the installer leaves some packages to be
> > installed after rebooting the OS for the last time.
> > 
> > The situation this patch solves is a corner-case faced when adding the
> > install scripts for SLES, as during its installation only one reboot is
> > performed (so, installer-reboots attribute doesn't help us) and the media
> > must persist after this reboot in order to finish the installation.
> 
> The installer cannot use the network for that? Maybe it would be
> acceptable to not add such a property, and document that libosinfo users
> should remove the iso from the libvirt XML only after the last reboot?
> 
> This would mean the installation ISO would always be visible in the
> guest OS until the first user-initiated reboot, but maybe that's ok?
> We could also add some ejection of the ISO to the post-install scripts,
> but that's getting complex and ugly ;)

A user initiated reboot after install wouldn't magically cause the ISO
to be ejected. You would need the app to keep looking for that reboot
and eject manually which I don't think is viable.

Also, note that most installers will explicitly instruct the user to
eject the CDROM at end of installation, so current app behaviour of
rmoving the CDROM at this time matches that (of course that instruction
is mostly about boot order priority in physical bios).


> > Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> > ---
> >  osinfo/libosinfo.syms  |  5 +++++
> >  osinfo/osinfo_db.c     |  3 +++
> >  osinfo/osinfo_loader.c |  9 +++++++++
> >  osinfo/osinfo_media.c  | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  osinfo/osinfo_media.h  |  2 ++
> >  5 files changed, 67 insertions(+), 1 deletion(-)
> > 
> > diff --git a/osinfo/libosinfo.syms b/osinfo/libosinfo.syms
> > index 68d54ff..e148d50 100644
> > --- a/osinfo/libosinfo.syms
> > +++ b/osinfo/libosinfo.syms
> > @@ -520,6 +520,11 @@ LIBOSINFO_0.2.12 {
> >  	osinfo_media_get_volume_size;
> >  } LIBOSINFO_0.2.11;
> >  
> > +LIBOSINFO_0.2.13 {
> > +    global:
> > +    osinfo_media_get_persistent;
> > +} LIBOSINFO_0.2.12;
> > +
> >  /* Symbols in next release...
> >  
> >    LIBOSINFO_0.0.2 {
> > diff --git a/osinfo/osinfo_db.c b/osinfo/osinfo_db.c
> > index cf86ccc..62d54de 100644
> > --- a/osinfo/osinfo_db.c
> > +++ b/osinfo/osinfo_db.c
> > @@ -628,6 +628,7 @@ static void fill_media(OsinfoDb *db, OsinfoMedia *media,
> >  {
> >      gboolean is_installer;
> >      gboolean is_live;
> > +    gboolean is_persistent;
> >      gint reboots;
> >      const gchar *id;
> >      const gchar *kernel_path;
> > @@ -663,9 +664,11 @@ static void fill_media(OsinfoDb *db, OsinfoMedia *media,
> >          g_object_set(G_OBJECT(media), "initrd_path", initrd_path, NULL);
> >      is_installer = osinfo_media_get_installer(matched_media);
> >      is_live = osinfo_media_get_live(matched_media);
> > +    is_persistent = osinfo_media_get_persistent(matched_media);
> >      g_object_set(G_OBJECT(media),
> >                   "installer", is_installer,
> >                   "live", is_live,
> > +                 "persistent", is_persistent,
> >                   NULL);
> >      if (is_installer) {
> >          reboots = osinfo_media_get_installer_reboots(matched_media);
> > diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
> > index dba567f..d4c811a 100644
> > --- a/osinfo/osinfo_loader.c
> > +++ b/osinfo/osinfo_loader.c
> > @@ -1050,6 +1050,8 @@ static OsinfoMedia *osinfo_loader_media(OsinfoLoader *loader,
> >      xmlChar *installer = xmlGetProp(root, BAD_CAST OSINFO_MEDIA_PROP_INSTALLER);
> >      xmlChar *installer_reboots =
> >              xmlGetProp(root, BAD_CAST OSINFO_MEDIA_PROP_INSTALLER_REBOOTS);
> > +    xmlChar *persistent =
> > +            xmlGetProp(root, BAD_CAST OSINFO_MEDIA_PROP_PERSISTENT);
> >      const OsinfoEntityKey keys[] = {
> >          { OSINFO_MEDIA_PROP_URL, G_TYPE_STRING },
> >          { OSINFO_MEDIA_PROP_KERNEL, G_TYPE_STRING },
> > @@ -1082,6 +1084,13 @@ static OsinfoMedia *osinfo_loader_media(OsinfoLoader *loader,
> >          xmlFree(installer_reboots);
> >      }
> >  
> > +    if (persistent) {
> > +        osinfo_entity_set_param(OSINFO_ENTITY(media),
> > +                                OSINFO_MEDIA_PROP_PERSISTENT,
> > +                                (gchar *)persistent);
> > +        xmlFree(persistent);
> > +    }
> > +
> >      gint nnodes = osinfo_loader_nodeset("./variant", loader, ctxt, &nodes, err);
> >      if (error_is_set(err)) {
> >          g_object_unref(media);
> > diff --git a/osinfo/osinfo_media.c b/osinfo/osinfo_media.c
> > index af4bb14..eac48a8 100644
> > --- a/osinfo/osinfo_media.c
> > +++ b/osinfo/osinfo_media.c
> > @@ -156,7 +156,8 @@ enum {
> >      PROP_INSTALLER_REBOOTS,
> >      PROP_OS,
> >      PROP_LANGUAGES,
> > -    PROP_VOLUME_SIZE
> > +    PROP_VOLUME_SIZE,
> > +    PROP_PERSISTENT
> >  };
> >  
> >  static void
> > @@ -236,6 +237,11 @@ osinfo_media_get_property(GObject    *object,
> >                            osinfo_media_get_volume_size(media));
> >          break;
> >  
> > +    case PROP_PERSISTENT:
> > +        g_value_set_boolean(value,
> > +                            osinfo_media_get_persistent(media));
> > +        break;
> > +
> >      default:
> >          /* We don't have any other property... */
> >          G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> > @@ -332,6 +338,12 @@ osinfo_media_set_property(GObject      *object,
> >                                        g_value_get_int64(value));
> >          break;
> >  
> > +    case PROP_PERSISTENT:
> > +        osinfo_entity_set_param_boolean(OSINFO_ENTITY(media),
> > +                                        OSINFO_MEDIA_PROP_PERSISTENT,
> > +                                        g_value_get_boolean(value));
> > +
> > +        break;
> >      default:
> >          /* We don't have any other property... */
> >          G_OBJECT_WARN_INVALID_PROPERTY_ID(object, property_id, pspec);
> > @@ -574,6 +586,24 @@ osinfo_media_class_init(OsinfoMediaClass *klass)
> >                                 G_PARAM_READWRITE |
> >                                 G_PARAM_STATIC_STRINGS);
> >      g_object_class_install_property(g_klass, PROP_VOLUME_SIZE, pspec);
> > +
> > +    /**
> > +     * OsinfoMedia:persistent:
> > +     *
> > +     * Whether the media should be persistent accross reboots done during
> > +     * the installation process.
> > +     *
> > +     * Some distros need their media to be persistent accross reboots as some
> > +     * packages are installed after the reboot (which may cause the media
> > +     * ejection, depending on the application).
> > +     */
> > +    pspec = g_param_spec_boolean("persistent",
> > +                                 "Persistent",
> > +                                 _("The media persists reboots during its installation process"),
> > +                                 FALSE /* default value */,
> > +                                 G_PARAM_READWRITE |
> > +                                 G_PARAM_STATIC_STRINGS);
> > +    g_object_class_install_property(g_klass, PROP_PERSISTENT, pspec);
> >  }
> >  
> >  static void
> > @@ -1269,6 +1299,21 @@ gint64 osinfo_media_get_volume_size(OsinfoMedia *media)
> >          (OSINFO_ENTITY(media), OSINFO_MEDIA_PROP_VOLUME_SIZE, -1);
> >  }
> >  
> > +/**
> > + * osinfo_media_get_persistent:
> > + * @media: an #OsinfoMedia instance
> > + *
> > + * Whether @media should persist accross reboots done during its installation
> > + * process.
> > + *
> > + * Returns: #TRUE if media should persist, #FALSE otherwise
> > + */
> > +gboolean osinfo_media_get_persistent(OsinfoMedia *media)
> > +{
> > +    return osinfo_entity_get_param_value_boolean_with_default
> > +            (OSINFO_ENTITY(media), OSINFO_MEDIA_PROP_PERSISTENT, FALSE);
> > +}
> > +
> >  /*
> >   * Local variables:
> >   *  indent-tabs-mode: nil
> > @@ -1276,3 +1321,5 @@ gint64 osinfo_media_get_volume_size(OsinfoMedia *media)
> >   *  c-basic-offset: 4
> >   * End:
> >   */
> > +
> > +
> > diff --git a/osinfo/osinfo_media.h b/osinfo/osinfo_media.h
> > index 09fbacd..b400fb5 100644
> > --- a/osinfo/osinfo_media.h
> > +++ b/osinfo/osinfo_media.h
> > @@ -90,6 +90,7 @@ typedef struct _OsinfoMediaPrivate OsinfoMediaPrivate;
> >  #define OSINFO_MEDIA_PROP_LANG_MAP       "l10n-language-map"
> >  #define OSINFO_MEDIA_PROP_VARIANT        "variant"
> >  #define OSINFO_MEDIA_PROP_VOLUME_SIZE    "volume-size"
> > +#define OSINFO_MEDIA_PROP_PERSISTENT     "persistent"
> >  
> >  /* object */
> >  struct _OsinfoMedia
> > @@ -140,6 +141,7 @@ gboolean osinfo_media_get_installer(OsinfoMedia *media);
> >  gboolean osinfo_media_get_live(OsinfoMedia *media);
> >  gint osinfo_media_get_installer_reboots(OsinfoMedia *media);
> >  gint64 osinfo_media_get_volume_size(OsinfoMedia *media);
> > +gboolean osinfo_media_get_persistent(OsinfoMedia *media);
> >  
> >  #endif /* __OSINFO_MEDIA_H__ */
> >  /*
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > Libosinfo mailing list
> > Libosinfo at redhat.com
> > https://www.redhat.com/mailman/listinfo/libosinfo



> _______________________________________________
> Libosinfo mailing list
> Libosinfo at redhat.com
> https://www.redhat.com/mailman/listinfo/libosinfo


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|




More information about the Libosinfo mailing list