[rhel5-branch] New version of the gfs support patch and one more

[Vratislav Podzimek]

First!

PATCH 1/2 is the same as the previous attempt for the gfs support. However,
the previous version had self.check set to 1 that caused problems on reboot
because fsck.gfs failed. I believe 0 is the right value here.

PATCH 2/2 adds the unknownFileSystem class that allows us to just comment
out the fstab lines with unknown (not supported) filesystem on upgrade. We
could just copy such lines, but that may result in non-booting system,
because of for example fsck problems (fsck for such filesystem may be not
installed or whatever). Ending up in a shell during boot process is a lot
worse then fixing the configuration from the running system with all utilities
available.

--
Vratislav Podzimek

[PATCH 1/2] Add gfs support (#754213)

[Vratislav Podzimek]

---
 fsset.py             |   30 ++++++++++++++++++++++++++++++
 loader2/loader.c     |    2 +-
 partitions.py        |    6 +++---
 scripts/mk-images    |    2 +-
 scripts/upd-instroot |    5 +++--
 5 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/fsset.py b/fsset.py
index e157519..9636003 100644
--- a/fsset.py
+++ b/fsset.py
@@ -539,6 +539,36 @@ class jfsFileSystem(FileSystemType):

fileSystemTypeRegister(jfsFileSystem())

+class gfsFileSystem(FileSystemType):
+    def __init__(self):
+        FileSystemType.__init__(self)
+        self.partedFileSystemType = None
+        self.formattable = 1
+        self.checked = 0
+        self.linuxnativefs = 1
+        if flags.cmdline.has_key("gfs"):
+            self.supported = -1
+        else:
+            self.supported = 0
+
+        self.name = "gfs"
+        self.packages = [ "gfs-utils" ]
+
+        self.maxSizeMB = 8 * 1024 * 1024
+
+    def formatDevice(self, entry, progress, chroot='/'):
+        devicePath = entry.device.setupDevice(chroot)
+        rc = iutil.execWithRedirect("/usr/sbin/mkfs.gfs",
+                                    ["-j", "1", "-p", "lock_nolock",
+                                     "-O", devicePath],
+                                    stdout = "/dev/tty5",
+                                    stderr = "/dev/tty5")
+
+        if rc:
+            raise SystemError
+
+fileSystemTypeRegister(gfsFileSystem())
+
 class gfs2FileSystem(FileSystemType):
     def __init__(self):
         FileSystemType.__init__(self)
diff --git a/loader2/loader.c b/loader2/loader.c
index 84e9088..b68601a 100644
--- a/loader2/loader.c
+++ b/loader2/loader.c
@@ -1810,7 +1810,7 @@ int main(int argc, char ** argv) {
     else if (FL_UPDATES(flags))
         loadUpdates(&loaderData);

-    mlLoadModuleSet("md:raid0:raid1:raid10:raid5:raid6:raid456:dm-raid45:fat:msdos:jbd2:crc16:ext4:jbd:ext3:lock_nolock:gfs2:reiserfs:jfs:xfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-emc:dm-crypt:dm-mem-cache:dm-region_hash:dm-message", modLoaded, modDeps, modInfo);
+    mlLoadModuleSet("md:raid0:raid1:raid10:raid5:raid6:raid456:dm-raid45:fat:msdos:jbd2:crc16:ext4:jbd:ext3:lock_nolock:gfs2:gfs:reiserfs:jfs:xfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-emc:dm-crypt:dm-mem-cache:dm-region_hash:dm-message", modLoaded, modDeps, modInfo);

/* crypto modules */
     mlLoadModuleSet("aead:aes_generic:rng:ansi_cprng:krng:anubis:crypto_blkcipher:authenc:blowfish:cast5:cast6:cbc:ccm:chainiv:crypto_hash:cryptomgr:ctr:zlib_deflate:deflate:crypto_null:des:ecb:eseqiv:gf128mul:hmac:khazad:md4:md5:michael_mic:seqiv:serpent:sha256:sha512:tea:tgr192:twofish:wp512:xcbc:xts", modLoaded, modDeps, modInfo);
diff --git a/partitions.py b/partitions.py
index 52d0b8c..208a8d5 100644
--- a/partitions.py
+++ b/partitions.py
@@ -1224,9 +1224,9 @@ class Partitions:

# no gfs support in grub
                 if (bootreq and bootreq.fstype and
-                    bootreq.fstype.getName() == "gfs2"):
-                    errors.append("Bootable partitions cannot be on a GFS2 "
-                                  "filesystem.")
+                    bootreq.fstype.getName() in ["gfs", "gfs2"]):
+                    errors.append("Bootable partitions cannot be on a GFS "
+                                  "nor a GFS2 filesystem.")

# no ext4 support in grub
                 if (bootreq and bootreq.fstype and
diff --git a/scripts/mk-images b/scripts/mk-images
index ef29709..95429f5 100755
--- a/scripts/mk-images
+++ b/scripts/mk-images
@@ -42,7 +42,7 @@ USBMODS="ohci-hcd uhci-hcd ehci-hcd hid mousedev usb-storage sd_mod sr_mod ub"
 FIREWIREMODS="ieee1394 ohci1394 sbp2"
 IDEMODS="ide-cd ide-cs"
 SCSIMODS="sr_mod sg st sd_mod scsi_mod iscsi_tcp iscsi_ibft"
-FSMODS="fat msdos vfat ext3 ext4 reiserfs jfs xfs gfs2 lock_nolock"
+FSMODS="fat msdos vfat ext3 ext4 reiserfs jfs xfs gfs gfs2 lock_nolock"
 LVMMODS="dm-mod dm-zero dm-snapshot dm-mirror dm-multipath dm-round-robin dm-emc dm-crypt"
 RAIDMODS="md raid0 raid1 raid10 raid5 raid6 raid456 dm-raid45 dm-mem-cache dm-region_hash dm-message"
 SECSTAGE="$RAIDMODS $LVMMODS $FSMODS $IDEMODS $SCSIMODS"
diff --git a/scripts/upd-instroot b/scripts/upd-instroot
index 09cf644..d83d375 100755
--- a/scripts/upd-instroot
+++ b/scripts/upd-instroot
@@ -118,8 +118,9 @@ PACKAGES="glibc glibc-common setup openssl python python-libs newt slang libseli
     libstdc++ expat libgcc readline ncurses yum python-sqlite
     python-elementtree pykickstart e2fsprogs-libs iscsi-initiator-utils
     db4 rhpxl xorg-x11-server-Xorg libuser system-config-date
-         yum-metadata-parser gfs2-utils libvolume_id rhel-instnum yum-kmod
-	 libdhcp libnl libdhcp6client libdhcp4client device-mapper-multipath
+         yum-metadata-parser gfs-utils kmod-gfs gfs2-utils libvolume_id
+         rhel-instnum yum-kmod libdhcp libnl libdhcp6client libdhcp4client
+         device-mapper-multipath
     kpartx dmraid python-pyblock mkinitrd libbdevid libbdevid-python
     libselinux-python nss udev keyutils-libs nspr python-iniparse
     cryptsetup-luks e4fsprogs fipscheck fipscheck-lib libdrm libmlx4

[PATCH 1/2] Add gfs support (#754213)

[David Cantrell]

On Tue, Jun 26, 2012 at 02:11:18PM +0200, Vratislav Podzimek wrote:
> fsset.py             |   30 ++++++++++++++++++++++++++++++
>  loader2/loader.c     |    2 +-
>  partitions.py        |    6 +++---
>  scripts/mk-images    |    2 +-
>  scripts/upd-instroot |    5 +++--
>  5 files changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/fsset.py b/fsset.py
> index e157519..9636003 100644
> --- a/fsset.py
> +++ b/fsset.py
> @@ -539,6 +539,36 @@ class jfsFileSystem(FileSystemType):
>
> fileSystemTypeRegister(jfsFileSystem())
>
> +class gfsFileSystem(FileSystemType):
> def __init__(self):
>    FileSystemType.__init__(self)
>    self.partedFileSystemType = None
>    self.formattable = 1
>    self.checked = 0
>    self.linuxnativefs = 1
>    if flags.cmdline.has_key("gfs"):
>        self.supported = -1
>    else:
>        self.supported = 0

Does support for this filesystem at install time really need to require the
'gfs' command line argument?
>    self.name = "gfs"
>    self.packages = [ "gfs-utils" ]
>    self.maxSizeMB = 8 * 1024 * 1024

We are sure this is the actual GFS limit?
> def formatDevice(self, entry, progress, chroot='/'):
>    devicePath = entry.device.setupDevice(chroot)
>    rc = iutil.execWithRedirect("/usr/sbin/mkfs.gfs",
>                                ["-j", "1", "-p", "lock_nolock",
>                                 "-O", devicePath],
>                                stdout = "/dev/tty5",
>                                stderr = "/dev/tty5")

Syntax-wise, this looks fine.  But I don't know if the arguments are
correct.  I assume you've verified that.
>    if rc:
>        raise SystemError
>
> +fileSystemTypeRegister(gfsFileSystem())
>
> class gfs2FileSystem(FileSystemType):
>      def __init__(self):
>          FileSystemType.__init__(self)
> diff --git a/loader2/loader.c b/loader2/loader.c
> index 84e9088..b68601a 100644
> --- a/loader2/loader.c
> +++ b/loader2/loader.c
> @@ -1810,7 +1810,7 @@ int main(int argc, char ** argv) {
>      else if (FL_UPDATES(flags))
>          loadUpdates(&loaderData);
> mlLoadModuleSet("md:raid0:raid1:raid10:raid5:raid6:raid456:dm-raid45:fat:msdos:jbd2:crc16:ext4:jbd:ext3:lock_nolock:gfs2:reiserfs:jfs:xfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-emc:dm-crypt:dm-mem-cache:dm-region_hash:dm-message", modLoaded, modDeps, modInfo);
>
> mlLoadModuleSet("md:raid0:raid1:raid10:raid5:raid6:raid456:dm-raid45:fat:msdos:jbd2:crc16:ext4:jbd:ext3:lock_nolock:gfs2:gfs:reiserfs:jfs:xfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-emc:dm-crypt:dm-mem-cache:dm-region_hash:dm-message", modLoaded, modDeps, modInfo);
>
> /* crypto modules */
> mlLoadModuleSet("aead:aes_generic:rng:ansi_cprng:krng:anubis:crypto_blkcipher:authenc:blowfish:cast5:cast6:cbc:ccm:chainiv:crypto_hash:cryptomgr:ctr:zlib_deflate:deflate:crypto_null:des:ecb:eseqiv:gf128mul:hmac:khazad:md4:md5:michael_mic:seqiv:serpent:sha256:sha512:tea:tgr192:twofish:wp512:xcbc:xts", modLoaded, modDeps, modInfo);
>
> diff --git a/partitions.py b/partitions.py
> index 52d0b8c..208a8d5 100644
> --- a/partitions.py
> +++ b/partitions.py
> @@ -1224,9 +1224,9 @@ class Partitions:
>              # no gfs support in grub
>              if (bootreq and bootreq.fstype and
>                bootreq.fstype.getName() == "gfs2"):
>                errors.append("Bootable partitions cannot be on a GFS2 "
>                              "filesystem.")
>                bootreq.fstype.getName() in ["gfs", "gfs2"]):
>                errors.append("Bootable partitions cannot be on a GFS "
>                              "nor a GFS2 filesystem.")
>
>            # no ext4 support in grub
>            if (bootreq and bootreq.fstype and
>
> diff --git a/scripts/mk-images b/scripts/mk-images
> index ef29709..95429f5 100755
> --- a/scripts/mk-images
> +++ b/scripts/mk-images
> @@ -42,7 +42,7 @@ USBMODS="ohci-hcd uhci-hcd ehci-hcd hid mousedev usb-storage sd_mod sr_mod ub"
>  FIREWIREMODS="ieee1394 ohci1394 sbp2"
>  IDEMODS="ide-cd ide-cs"
>  SCSIMODS="sr_mod sg st sd_mod scsi_mod iscsi_tcp iscsi_ibft"
> -FSMODS="fat msdos vfat ext3 ext4 reiserfs jfs xfs gfs2 lock_nolock"
> +FSMODS="fat msdos vfat ext3 ext4 reiserfs jfs xfs gfs gfs2 lock_nolock"
>  LVMMODS="dm-mod dm-zero dm-snapshot dm-mirror dm-multipath dm-round-robin dm-emc dm-crypt"
>  RAIDMODS="md raid0 raid1 raid10 raid5 raid6 raid456 dm-raid45 dm-mem-cache dm-region_hash dm-message"
>  SECSTAGE="$RAIDMODS $LVMMODS $FSMODS $IDEMODS $SCSIMODS"
> diff --git a/scripts/upd-instroot b/scripts/upd-instroot
> index 09cf644..d83d375 100755
> --- a/scripts/upd-instroot
> +++ b/scripts/upd-instroot
> @@ -118,8 +118,9 @@ PACKAGES="glibc glibc-common setup openssl python python-libs newt slang libseli
>     libstdc++ expat libgcc readline ncurses yum python-sqlite
>     python-elementtree pykickstart e2fsprogs-libs iscsi-initiator-utils
>     db4 rhpxl xorg-x11-server-Xorg libuser system-config-date
>     yum-metadata-parser gfs2-utils libvolume_id rhel-instnum yum-kmod
> libdhcp libnl libdhcp6client libdhcp4client device-mapper-multipath
>     yum-metadata-parser gfs-utils kmod-gfs gfs2-utils libvolume_id
>     rhel-instnum yum-kmod libdhcp libnl libdhcp6client libdhcp4client
>     device-mapper-multipath
> kpartx dmraid python-pyblock mkinitrd libbdevid libbdevid-python
>   libselinux-python nss udev keyutils-libs nspr python-iniparse
>   cryptsetup-luks e4fsprogs fipscheck fipscheck-lib libdrm libmlx4

The rest of this looks fine.

[PATCH 1/2] Add gfs support (#754213)

[Vratislav Podzimek]

On Tue, 2012-06-26 at 09:28 -0400, David Cantrell wrote:
> On Tue, Jun 26, 2012 at 02:11:18PM +0200, Vratislav Podzimek wrote:
> > fsset.py             |   30 ++++++++++++++++++++++++++++++
> >  loader2/loader.c     |    2 +-
> >  partitions.py        |    6 +++---
> >  scripts/mk-images    |    2 +-
> >  scripts/upd-instroot |    5 +++--
> >  5 files changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/fsset.py b/fsset.py
> > index e157519..9636003 100644
> > --- a/fsset.py
> > +++ b/fsset.py
> > @@ -539,6 +539,36 @@ class jfsFileSystem(FileSystemType):
> >
> > fileSystemTypeRegister(jfsFileSystem())
> >
> > +class gfsFileSystem(FileSystemType):
> > def __init__(self):
> >    FileSystemType.__init__(self)
> >    self.partedFileSystemType = None
> >    self.formattable = 1
> >    self.checked = 0
> >    self.linuxnativefs = 1
> >    if flags.cmdline.has_key("gfs"):
> >        self.supported = -1
> >    else:
> >        self.supported = 0
>
> Does support for this filesystem at install time really need to require the
> 'gfs' command line argument?

I've used the same as for the gfs2. If I understand it correctly,
self.supported is used only when creating a new filesystem.
> >    self.name = "gfs"
> >    self.packages = [ "gfs-utils" ]
> >    self.maxSizeMB = 8 * 1024 * 1024
>
> We are sure this is the actual GFS limit?

Again, used the same as for the gfs2. I will try to get more info about
it.
> > def formatDevice(self, entry, progress, chroot='/'):
> >    devicePath = entry.device.setupDevice(chroot)
> >    rc = iutil.execWithRedirect("/usr/sbin/mkfs.gfs",
> >                                ["-j", "1", "-p", "lock_nolock",
> >                                 "-O", devicePath],
> >                                stdout = "/dev/tty5",
> >                                stderr = "/dev/tty5")
>
> Syntax-wise, this looks fine.  But I don't know if the arguments are
> correct.  I assume you've verified that.

Yeah, verified and get the tip from the gfs-utils maintainer. They are
again the same as for the gfs2.
> >    if rc:
> >        raise SystemError
> >
> > +fileSystemTypeRegister(gfsFileSystem())
> >
> > class gfs2FileSystem(FileSystemType):
> >      def __init__(self):
> >          FileSystemType.__init__(self)
> > diff --git a/loader2/loader.c b/loader2/loader.c
> > index 84e9088..b68601a 100644
> > --- a/loader2/loader.c
> > +++ b/loader2/loader.c
> > @@ -1810,7 +1810,7 @@ int main(int argc, char ** argv) {
> >      else if (FL_UPDATES(flags))
> >          loadUpdates(&loaderData);
> > mlLoadModuleSet("md:raid0:raid1:raid10:raid5:raid6:raid456:dm-raid45:fat:msdos:jbd2:crc16:ext4:jbd:ext3:lock_nolock:gfs2:reiserfs:jfs:xfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-emc:dm-crypt:dm-mem-cache:dm-region_hash:dm-message", modLoaded, modDeps, modInfo);
> >
> > mlLoadModuleSet("md:raid0:raid1:raid10:raid5:raid6:raid456:dm-raid45:fat:msdos:jbd2:crc16:ext4:jbd:ext3:lock_nolock:gfs2:gfs:reiserfs:jfs:xfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-emc:dm-crypt:dm-mem-cache:dm-region_hash:dm-message", modLoaded, modDeps, modInfo);
> >
> > /* crypto modules */
> > mlLoadModuleSet("aead:aes_generic:rng:ansi_cprng:krng:anubis:crypto_blkcipher:authenc:blowfish:cast5:cast6:cbc:ccm:chainiv:crypto_hash:cryptomgr:ctr:zlib_deflate:deflate:crypto_null:des:ecb:eseqiv:gf128mul:hmac:khazad:md4:md5:michael_mic:seqiv:serpent:sha256:sha512:tea:tgr192:twofish:wp512:xcbc:xts", modLoaded, modDeps, modInfo);
> >
> > diff --git a/partitions.py b/partitions.py
> > index 52d0b8c..208a8d5 100644
> > --- a/partitions.py
> > +++ b/partitions.py
> > @@ -1224,9 +1224,9 @@ class Partitions:
> >              # no gfs support in grub
> >              if (bootreq and bootreq.fstype and
> >                bootreq.fstype.getName() == "gfs2"):
> >                errors.append("Bootable partitions cannot be on a GFS2 "
> >                              "filesystem.")
> >                bootreq.fstype.getName() in ["gfs", "gfs2"]):
> >                errors.append("Bootable partitions cannot be on a GFS "
> >                              "nor a GFS2 filesystem.")
> >
> >            # no ext4 support in grub
> >            if (bootreq and bootreq.fstype and
> >
> > diff --git a/scripts/mk-images b/scripts/mk-images
> > index ef29709..95429f5 100755
> > --- a/scripts/mk-images
> > +++ b/scripts/mk-images
> > @@ -42,7 +42,7 @@ USBMODS="ohci-hcd uhci-hcd ehci-hcd hid mousedev usb-storage sd_mod sr_mod ub"
> >  FIREWIREMODS="ieee1394 ohci1394 sbp2"
> >  IDEMODS="ide-cd ide-cs"
> >  SCSIMODS="sr_mod sg st sd_mod scsi_mod iscsi_tcp iscsi_ibft"
> > -FSMODS="fat msdos vfat ext3 ext4 reiserfs jfs xfs gfs2 lock_nolock"
> > +FSMODS="fat msdos vfat ext3 ext4 reiserfs jfs xfs gfs gfs2 lock_nolock"
> >  LVMMODS="dm-mod dm-zero dm-snapshot dm-mirror dm-multipath dm-round-robin dm-emc dm-crypt"
> >  RAIDMODS="md raid0 raid1 raid10 raid5 raid6 raid456 dm-raid45 dm-mem-cache dm-region_hash dm-message"
> >  SECSTAGE="$RAIDMODS $LVMMODS $FSMODS $IDEMODS $SCSIMODS"
> > diff --git a/scripts/upd-instroot b/scripts/upd-instroot
> > index 09cf644..d83d375 100755
> > --- a/scripts/upd-instroot
> > +++ b/scripts/upd-instroot
> > @@ -118,8 +118,9 @@ PACKAGES="glibc glibc-common setup openssl python python-libs newt slang libseli
> >     libstdc++ expat libgcc readline ncurses yum python-sqlite
> >     python-elementtree pykickstart e2fsprogs-libs iscsi-initiator-utils
> >     db4 rhpxl xorg-x11-server-Xorg libuser system-config-date
> >     yum-metadata-parser gfs2-utils libvolume_id rhel-instnum yum-kmod
> > libdhcp libnl libdhcp6client libdhcp4client device-mapper-multipath
> >     yum-metadata-parser gfs-utils kmod-gfs gfs2-utils libvolume_id
> >     rhel-instnum yum-kmod libdhcp libnl libdhcp6client libdhcp4client
> >     device-mapper-multipath
> > kpartx dmraid python-pyblock mkinitrd libbdevid libbdevid-python
> >   libselinux-python nss udev keyutils-libs nspr python-iniparse
> >   cryptsetup-luks e4fsprogs fipscheck fipscheck-lib libdrm libmlx4
>
> The rest of this looks fine.

[PATCH 1/2] Add gfs support (#754213)

[David Cantrell]

On Tue, Jun 26, 2012 at 03:36:38PM +0200, Vratislav Podzimek wrote:
> On Tue, 2012-06-26 at 09:28 -0400, David Cantrell wrote:
> > On Tue, Jun 26, 2012 at 02:11:18PM +0200, Vratislav Podzimek wrote:
> > > fsset.py             |   30 ++++++++++++++++++++++++++++++
> > >  loader2/loader.c     |    2 +-
> > >  partitions.py        |    6 +++---
> > >  scripts/mk-images    |    2 +-
> > >  scripts/upd-instroot |    5 +++--
> > >  5 files changed, 38 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/fsset.py b/fsset.py
> > > index e157519..9636003 100644
> > > --- a/fsset.py
> > > +++ b/fsset.py
> > > @@ -539,6 +539,36 @@ class jfsFileSystem(FileSystemType):
> > >
> > > fileSystemTypeRegister(jfsFileSystem())
> > >
> > > +class gfsFileSystem(FileSystemType):
> > > def __init__(self):
> > >    FileSystemType.__init__(self)
> > >    self.partedFileSystemType = None
> > >    self.formattable = 1
> > >    self.checked = 0
> > >    self.linuxnativefs = 1
> > >    if flags.cmdline.has_key("gfs"):
> > >        self.supported = -1
> > >    else:
> > >        self.supported = 0
> >
> > Does support for this filesystem at install time really need to require the
> > 'gfs' command line argument?
>
> I've used the same as for the gfs2. If I understand it correctly,
> self.supported is used only when creating a new filesystem.

Alright, good enough then.
> > >    self.name = "gfs"
> > >    self.packages = [ "gfs-utils" ]
> > >    self.maxSizeMB = 8 * 1024 * 1024
> >
> > We are sure this is the actual GFS limit?
>
> Again, used the same as for the gfs2. I will try to get more info about
> it.

Definitely verify this.  In fact, it may be worth verifying for gfs2 as
well.  We had ext4 wrong in RHEL, after all.
> > > def formatDevice(self, entry, progress, chroot='/'):
> > >    devicePath = entry.device.setupDevice(chroot)
> > >    rc = iutil.execWithRedirect("/usr/sbin/mkfs.gfs",
> > >                                ["-j", "1", "-p", "lock_nolock",
> > >                                 "-O", devicePath],
> > >                                stdout = "/dev/tty5",
> > >                                stderr = "/dev/tty5")
> >
> > Syntax-wise, this looks fine.  But I don't know if the arguments are
> > correct.  I assume you've verified that.
>
> Yeah, verified and get the tip from the gfs-utils maintainer. They are
> again the same as for the gfs2.

Sounds good.
> > >    if rc:
> > >        raise SystemError
> > >
> > > +fileSystemTypeRegister(gfsFileSystem())
> > >
> > > class gfs2FileSystem(FileSystemType):
> > >      def __init__(self):
> > >          FileSystemType.__init__(self)
> > > diff --git a/loader2/loader.c b/loader2/loader.c
> > > index 84e9088..b68601a 100644
> > > --- a/loader2/loader.c
> > > +++ b/loader2/loader.c
> > > @@ -1810,7 +1810,7 @@ int main(int argc, char ** argv) {
> > >      else if (FL_UPDATES(flags))
> > >          loadUpdates(&loaderData);
> > > mlLoadModuleSet("md:raid0:raid1:raid10:raid5:raid6:raid456:dm-raid45:fat:msdos:jbd2:crc16:ext4:jbd:ext3:lock_nolock:gfs2:reiserfs:jfs:xfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-emc:dm-crypt:dm-mem-cache:dm-region_hash:dm-message", modLoaded, modDeps, modInfo);
> > >
> > > mlLoadModuleSet("md:raid0:raid1:raid10:raid5:raid6:raid456:dm-raid45:fat:msdos:jbd2:crc16:ext4:jbd:ext3:lock_nolock:gfs2:gfs:reiserfs:jfs:xfs:dm-mod:dm-zero:dm-mirror:dm-snapshot:dm-multipath:dm-round-robin:dm-emc:dm-crypt:dm-mem-cache:dm-region_hash:dm-message", modLoaded, modDeps, modInfo);
> > >
> > > /* crypto modules */
> > > mlLoadModuleSet("aead:aes_generic:rng:ansi_cprng:krng:anubis:crypto_blkcipher:authenc:blowfish:cast5:cast6:cbc:ccm:chainiv:crypto_hash:cryptomgr:ctr:zlib_deflate:deflate:crypto_null:des:ecb:eseqiv:gf128mul:hmac:khazad:md4:md5:michael_mic:seqiv:serpent:sha256:sha512:tea:tgr192:twofish:wp512:xcbc:xts", modLoaded, modDeps, modInfo);
> > >
> > > diff --git a/partitions.py b/partitions.py
> > > index 52d0b8c..208a8d5 100644
> > > --- a/partitions.py
> > > +++ b/partitions.py
> > > @@ -1224,9 +1224,9 @@ class Partitions:
> > >              # no gfs support in grub
> > >              if (bootreq and bootreq.fstype and
> > >                bootreq.fstype.getName() == "gfs2"):
> > >                errors.append("Bootable partitions cannot be on a GFS2 "
> > >                              "filesystem.")
> > >                bootreq.fstype.getName() in ["gfs", "gfs2"]):
> > >                errors.append("Bootable partitions cannot be on a GFS "
> > >                              "nor a GFS2 filesystem.")
> > >
> > >            # no ext4 support in grub
> > >            if (bootreq and bootreq.fstype and
> > >
> > > diff --git a/scripts/mk-images b/scripts/mk-images
> > > index ef29709..95429f5 100755
> > > --- a/scripts/mk-images
> > > +++ b/scripts/mk-images
> > > @@ -42,7 +42,7 @@ USBMODS="ohci-hcd uhci-hcd ehci-hcd hid mousedev usb-storage sd_mod sr_mod ub"
> > >  FIREWIREMODS="ieee1394 ohci1394 sbp2"
> > >  IDEMODS="ide-cd ide-cs"
> > >  SCSIMODS="sr_mod sg st sd_mod scsi_mod iscsi_tcp iscsi_ibft"
> > > -FSMODS="fat msdos vfat ext3 ext4 reiserfs jfs xfs gfs2 lock_nolock"
> > > +FSMODS="fat msdos vfat ext3 ext4 reiserfs jfs xfs gfs gfs2 lock_nolock"
> > >  LVMMODS="dm-mod dm-zero dm-snapshot dm-mirror dm-multipath dm-round-robin dm-emc dm-crypt"
> > >  RAIDMODS="md raid0 raid1 raid10 raid5 raid6 raid456 dm-raid45 dm-mem-cache dm-region_hash dm-message"
> > >  SECSTAGE="$RAIDMODS $LVMMODS $FSMODS $IDEMODS $SCSIMODS"
> > > diff --git a/scripts/upd-instroot b/scripts/upd-instroot
> > > index 09cf644..d83d375 100755
> > > --- a/scripts/upd-instroot
> > > +++ b/scripts/upd-instroot
> > > @@ -118,8 +118,9 @@ PACKAGES="glibc glibc-common setup openssl python python-libs newt slang libseli
> > >     libstdc++ expat libgcc readline ncurses yum python-sqlite
> > >     python-elementtree pykickstart e2fsprogs-libs iscsi-initiator-utils
> > >     db4 rhpxl xorg-x11-server-Xorg libuser system-config-date
> > >     yum-metadata-parser gfs2-utils libvolume_id rhel-instnum yum-kmod
> > > libdhcp libnl libdhcp6client libdhcp4client device-mapper-multipath
> > >     yum-metadata-parser gfs-utils kmod-gfs gfs2-utils libvolume_id
> > >     rhel-instnum yum-kmod libdhcp libnl libdhcp6client libdhcp4client
> > >     device-mapper-multipath
> > > kpartx dmraid python-pyblock mkinitrd libbdevid libbdevid-python
> > >   libselinux-python nss udev keyutils-libs nspr python-iniparse
> > >   cryptsetup-luks e4fsprogs fipscheck fipscheck-lib libdrm libmlx4
> >
> > The rest of this looks fine.

[PATCH 2/2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[Vratislav Podzimek]

When doing upgrade, there is no need to ignore the fstab lines with filesystem
Anaconda does not support. We can leave the lines as they are and just comment
them out to prevent mounting problems during the next boot.

Resolves: rhbz#754213
---
 fsset.py |   22 ++++++++++++++++++++--
 1 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/fsset.py b/fsset.py
index 9636003..64d948d 100644
--- a/fsset.py
+++ b/fsset.py
@@ -1297,6 +1297,20 @@ class AutoFileSystem(PsudoFileSystem):

fileSystemTypeRegister(AutoFileSystem())

+class unknownFileSystem(FileSystemType):
+    def __init__(self, fstype):
+        FileSystemType.__init__(self)
+        self.partedFileSystemType = None
+        self.formattable = 0
+        self.checked = 0
+        self.linuxnativefs = 0
+        self.supported = 0
+
+        self.name = fstype
+
+    def formatDevice(self, entry, progress, chroot='/'):
+        pass
+
 class BindFileSystem(PsudoFileSystem):
     def __init__(self):
         PsudoFileSystem.__init__(self, "bind")
@@ -1456,6 +1470,10 @@ class FileSystemSet:
                 options = entry.getOptions()
                 if entry.mountpoint == "/" and options is not None:
                     options = options.replace(",_netdev", ",_rnetdev")
+
+                # comment out lines with unknown filesystem
+                if isinstance(entry.fsystem, unknownFileSystem):
+                    device = "#" + device
                 fstab = fstab + format % (device, entry.mountpoint,
                                           entry.fsystem.getName(),
                                           options, entry.fsck,
@@ -2969,8 +2987,8 @@ def readFstab (anaconda):
             break
         # "none" is valid as an fs type for bind mounts (#151458)
         if fsystem is None and (string.find(fields[3], "bind") == -1):
-            continue
-        
+            fsystem = unknownFileSystem(fstotry[0])
+
         label = None
    if fields[0] == "none":
             device = Device()

[PATCH 2/2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[David Cantrell]

Comments and questions below.

On Tue, Jun 26, 2012 at 02:11:19PM +0200, Vratislav Podzimek wrote:
> When doing upgrade, there is no need to ignore the fstab lines with filesystem
> Anaconda does not support. We can leave the lines as they are and just comment
> them out to prevent mounting problems during the next boot.
> Resolves: rhbz#754213
>
> fsset.py |   22 ++++++++++++++++++++--
>  1 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/fsset.py b/fsset.py
> index 9636003..64d948d 100644
> --- a/fsset.py
> +++ b/fsset.py
> @@ -1297,6 +1297,20 @@ class AutoFileSystem(PsudoFileSystem):
>
> fileSystemTypeRegister(AutoFileSystem())
>
> +class unknownFileSystem(FileSystemType):
> def __init__(self, fstype):
>    FileSystemType.__init__(self)
>    self.partedFileSystemType = None
>    self.formattable = 0
>    self.checked = 0
>    self.linuxnativefs = 0
>    self.supported = 0
>    self.name = fstype
> def formatDevice(self, entry, progress, chroot='/'):
>    pass
>
> class BindFileSystem(PsudoFileSystem):
>      def __init__(self):
>          PsudoFileSystem.__init__(self, "bind")
> @@ -1456,6 +1470,10 @@ class FileSystemSet:
>                  options = entry.getOptions()
>                  if entry.mountpoint == "/" and options is not None:
>                      options = options.replace(",_netdev", ",_rnetdev")
>            # comment out lines with unknown filesystem
>            if isinstance(entry.fsystem, unknownFileSystem):
>                device = "#" + device

Using isinstance() isn't my favorite, but there is clearly precedent for it
in rhel5-branch.
>              fstab = fstab + format % (device, entry.mountpoint,
>                                        entry.fsystem.getName(),
>                                        options, entry.fsck,
>
> @@ -2969,8 +2987,8 @@ def readFstab (anaconda):
>              break
>          # "none" is valid as an fs type for bind mounts (#151458)
>          if fsystem is None and (string.find(fields[3], "bind") == -1):
>        continue
>        fsystem = unknownFileSystem(fstotry[0])

Why fstotry[0] here?
>      label = None
>
> if fields[0] == "none":
>              device = Device()

[PATCH 2/2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[Vratislav Podzimek]

On Tue, 2012-06-26 at 09:34 -0400, David Cantrell wrote:
> Comments and questions below.
>
> On Tue, Jun 26, 2012 at 02:11:19PM +0200, Vratislav Podzimek wrote:
> > When doing upgrade, there is no need to ignore the fstab lines with filesystem
> > Anaconda does not support. We can leave the lines as they are and just comment
> > them out to prevent mounting problems during the next boot.
> > Resolves: rhbz#754213
> >
> > fsset.py |   22 ++++++++++++++++++++--
> >  1 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/fsset.py b/fsset.py
> > index 9636003..64d948d 100644
> > --- a/fsset.py
> > +++ b/fsset.py
> > @@ -1297,6 +1297,20 @@ class AutoFileSystem(PsudoFileSystem):
> >
> > fileSystemTypeRegister(AutoFileSystem())
> >
> > +class unknownFileSystem(FileSystemType):
> > def __init__(self, fstype):
> >    FileSystemType.__init__(self)
> >    self.partedFileSystemType = None
> >    self.formattable = 0
> >    self.checked = 0
> >    self.linuxnativefs = 0
> >    self.supported = 0
> >    self.name = fstype
> > def formatDevice(self, entry, progress, chroot='/'):
> >    pass
> >
> > class BindFileSystem(PsudoFileSystem):
> >      def __init__(self):
> >          PsudoFileSystem.__init__(self, "bind")
> > @@ -1456,6 +1470,10 @@ class FileSystemSet:
> >                  options = entry.getOptions()
> >                  if entry.mountpoint == "/" and options is not None:
> >                      options = options.replace(",_netdev", ",_rnetdev")
> >            # comment out lines with unknown filesystem
> >            if isinstance(entry.fsystem, unknownFileSystem):
> >                device = "#" + device
>
> Using isinstance() isn't my favorite, but there is clearly precedent for it
> in rhel5-branch.

Yeah, and I don't know about any simple way to replace it with anything
else here.
> >              fstab = fstab + format % (device, entry.mountpoint,
> >                                        entry.fsystem.getName(),
> >                                        options, entry.fsck,
> >
> > @@ -2969,8 +2987,8 @@ def readFstab (anaconda):
> >              break
> >          # "none" is valid as an fs type for bind mounts (#151458)
> >          if fsystem is None and (string.find(fields[3], "bind") == -1):
> >        continue
> >        fsystem = unknownFileSystem(fstotry[0])
>
> Why fstotry[0] here?

Because fstotry is a list here even if there is only one filesystem on
the fstab's line (only one item in the list).
> >      label = None
> >
> > if fields[0] == "none":
> >              device = Device()

[PATCH 2/2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[David Cantrell]

On Tue, Jun 26, 2012 at 03:40:49PM +0200, Vratislav Podzimek wrote:
> On Tue, 2012-06-26 at 09:34 -0400, David Cantrell wrote:
> > Comments and questions below.
> >
> > On Tue, Jun 26, 2012 at 02:11:19PM +0200, Vratislav Podzimek wrote:
> > > When doing upgrade, there is no need to ignore the fstab lines with filesystem
> > > Anaconda does not support. We can leave the lines as they are and just comment
> > > them out to prevent mounting problems during the next boot.
> > > Resolves: rhbz#754213
> > >
> > > fsset.py |   22 ++++++++++++++++++++--
> > >  1 files changed, 20 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fsset.py b/fsset.py
> > > index 9636003..64d948d 100644
> > > --- a/fsset.py
> > > +++ b/fsset.py
> > > @@ -1297,6 +1297,20 @@ class AutoFileSystem(PsudoFileSystem):
> > >
> > > fileSystemTypeRegister(AutoFileSystem())
> > >
> > > +class unknownFileSystem(FileSystemType):
> > > def __init__(self, fstype):
> > >    FileSystemType.__init__(self)
> > >    self.partedFileSystemType = None
> > >    self.formattable = 0
> > >    self.checked = 0
> > >    self.linuxnativefs = 0
> > >    self.supported = 0
> > >    self.name = fstype
> > > def formatDevice(self, entry, progress, chroot='/'):
> > >    pass
> > >
> > > class BindFileSystem(PsudoFileSystem):
> > >      def __init__(self):
> > >          PsudoFileSystem.__init__(self, "bind")
> > > @@ -1456,6 +1470,10 @@ class FileSystemSet:
> > >                  options = entry.getOptions()
> > >                  if entry.mountpoint == "/" and options is not None:
> > >                      options = options.replace(",_netdev", ",_rnetdev")
> > >            # comment out lines with unknown filesystem
> > >            if isinstance(entry.fsystem, unknownFileSystem):
> > >                device = "#" + device
> >
> > Using isinstance() isn't my favorite, but there is clearly precedent for it
> > in rhel5-branch.
>
> Yeah, and I don't know about any simple way to replace it with anything
> else here.

It's ok for rhel5-branch.
> > >              fstab = fstab + format % (device, entry.mountpoint,
> > >                                        entry.fsystem.getName(),
> > >                                        options, entry.fsck,
> > >
> > > @@ -2969,8 +2987,8 @@ def readFstab (anaconda):
> > >              break
> > >          # "none" is valid as an fs type for bind mounts (#151458)
> > >          if fsystem is None and (string.find(fields[3], "bind") == -1):
> > >        continue
> > >        fsystem = unknownFileSystem(fstotry[0])
> >
> > Why fstotry[0] here?
>
> Because fstotry is a list here even if there is only one filesystem on
> the fstab's line (only one item in the list).

Is this going to work for lines with the fs field containing 'ext2,ext3' or
something like that?  And if it's broken, we're ending up with a comment out
with missing data.

Maybe a better approach is to let readFstab do its thing and then for lines
that would have been thrown out, stuff those lines as-is in to a list which
we then read from later and place in the newly generated /etc/fstab but with
each entry prefixed with '#' so they are commented out.

But maybe I'm misinterpreting what's happening here.

[PATCH 2/2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[Vratislav Podzimek]

On Tue, 2012-06-26 at 15:07 -0400, David Cantrell wrote:
> On Tue, Jun 26, 2012 at 03:40:49PM +0200, Vratislav Podzimek wrote:
> > On Tue, 2012-06-26 at 09:34 -0400, David Cantrell wrote:
> > > Comments and questions below.
> > >
> > > On Tue, Jun 26, 2012 at 02:11:19PM +0200, Vratislav Podzimek wrote:
> > > > When doing upgrade, there is no need to ignore the fstab lines with filesystem
> > > > Anaconda does not support. We can leave the lines as they are and just comment
> > > > them out to prevent mounting problems during the next boot.
> > > > Resolves: rhbz#754213
> > > >
> > > > fsset.py |   22 ++++++++++++++++++++--
> > > >  1 files changed, 20 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/fsset.py b/fsset.py
> > > > index 9636003..64d948d 100644
> > > > --- a/fsset.py
> > > > +++ b/fsset.py
> > > > @@ -1297,6 +1297,20 @@ class AutoFileSystem(PsudoFileSystem):
> > > >
> > > > fileSystemTypeRegister(AutoFileSystem())
> > > >
> > > > +class unknownFileSystem(FileSystemType):
> > > > def __init__(self, fstype):
> > > >    FileSystemType.__init__(self)
> > > >    self.partedFileSystemType = None
> > > >    self.formattable = 0
> > > >    self.checked = 0
> > > >    self.linuxnativefs = 0
> > > >    self.supported = 0
> > > >    self.name = fstype
> > > > def formatDevice(self, entry, progress, chroot='/'):
> > > >    pass
> > > >
> > > > class BindFileSystem(PsudoFileSystem):
> > > >      def __init__(self):
> > > >          PsudoFileSystem.__init__(self, "bind")
> > > > @@ -1456,6 +1470,10 @@ class FileSystemSet:
> > > >                  options = entry.getOptions()
> > > >                  if entry.mountpoint == "/" and options is not None:
> > > >                      options = options.replace(",_netdev", ",_rnetdev")
> > > >            # comment out lines with unknown filesystem
> > > >            if isinstance(entry.fsystem, unknownFileSystem):
> > > >                device = "#" + device
> > >
> > > Using isinstance() isn't my favorite, but there is clearly precedent for it
> > > in rhel5-branch.
> >
> > Yeah, and I don't know about any simple way to replace it with anything
> > else here.
>
> It's ok for rhel5-branch.
> > > >              fstab = fstab + format % (device, entry.mountpoint,
> > > >                                        entry.fsystem.getName(),
> > > >                                        options, entry.fsck,
> > > >
> > > > @@ -2969,8 +2987,8 @@ def readFstab (anaconda):
> > > >              break
> > > >          # "none" is valid as an fs type for bind mounts (#151458)
> > > >          if fsystem is None and (string.find(fields[3], "bind") == -1):
> > > >        continue
> > > >        fsystem = unknownFileSystem(fstotry[0])
> > >
> > > Why fstotry[0] here?
> >
> > Because fstotry is a list here even if there is only one filesystem on
> > the fstab's line (only one item in the list).
>
> Is this going to work for lines with the fs field containing 'ext2,ext3' or
> something like that?  And if it's broken, we're ending up with a comment out
> with missing data.

Nothing should change for such lines, because fsystem is not None for
them (the first one we support from the comma-separated list is chosen).
> Maybe a better approach is to let readFstab do its thing and then for lines
> that would have been thrown out, stuff those lines as-is in to a list which
> we then read from later and place in the newly generated /etc/fstab but with
> each entry prefixed with '#' so they are commented out.

That would be possible, but it would probably need more code. On the
other hand such lines would not end up in the fsset as some weird
unknown filesystems, which should be safer. I'll rework this patch.

[PATCHv2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[Vratislav Podzimek]

When doing upgrade, there is no need to ignore the fstab lines with filesystem
Anaconda does not support. We can leave the lines as they are and just comment
them out to prevent mounting problems during the next boot.

Resolves: rhbz#754213
---
 fsset.py   |   23 +++++++++++++++++++++++
 upgrade.py |    3 +--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/fsset.py b/fsset.py
index 9636003..85f6623 100644
--- a/fsset.py
+++ b/fsset.py
@@ -30,6 +30,7 @@ import partedUtils
 import raid
 import lvm
 import types
+import copy
 from flags import flags

import bootyutil
@@ -1316,6 +1317,7 @@ class FileSystemSet:
         self.reset()
         self.volumesCreated = 0
         self.anaconda = anaconda
+        self.unknownFSlines = list()

def isActive(self):
         return self.mountcount != 0
@@ -1431,8 +1433,21 @@ class FileSystemSet:
         new = FileSystemSet(self.anaconda)
         for entry in self.entries:
             new.add (entry)
+        new.unknownFSlines = copy.copy(self.unknownFSlines)
         return new

+    def extend (self, fsset):
+        """
+        Extends the entry set and the list of unknownFSlines with values
+        from a given fsset.
+
+        """
+
+        for entry in fsset.entries:
+            self.add(entry)
+
+        self.unknownFSlines.extend(fsset.unknownFSlines)
+
     def fstab (self):
         format = "%-23s %-23s %-7s %-15s %d %d\n"
         fstab = ""
@@ -1460,6 +1475,11 @@ class FileSystemSet:
                                           entry.fsystem.getName(),
                                           options, entry.fsck,
                                           entry.order)
+
+        # append commented out lines with unknown (not supported) filesystems
+        for line in self.unknownFSlines:
+            fstab += "#" + line
+
         return fstab

def mtab (self):
@@ -2969,6 +2989,9 @@ def readFstab (anaconda):
             break
         # "none" is valid as an fs type for bind mounts (#151458)
         if fsystem is None and (string.find(fields[3], "bind") == -1):
+            # add the line to the list of lines that will be commented out in
+            # the new fstab
+            fsset.unknownFSlines.append(line)
             continue

label = None
diff --git a/upgrade.py b/upgrade.py
index 5beb8a2..f3e243c 100644
--- a/upgrade.py
+++ b/upgrade.py
@@ -198,8 +198,7 @@ def mountRootPartition(anaconda, rootInfo, oldfsset, allowDirty = 0,

oldfsset.reset()
     newfsset = readFstab(anaconda)
-    for entry in newfsset.entries:
-        oldfsset.add(entry)
+    oldfsset.extend(newfsset)

isys.umount(anaconda.rootPath)

[PATCHv2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[Martin Gracik]

----- Original Message -----
> When doing upgrade, there is no need to ignore the fstab lines with
> filesystem
> Anaconda does not support. We can leave the lines as they are and
> just comment
> them out to prevent mounting problems during the next boot.
> Resolves: rhbz#754213
>
> fsset.py   |   23 +++++++++++++++++++++++
>  upgrade.py |    3 +--
>  2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/fsset.py b/fsset.py
> index 9636003..85f6623 100644
> --- a/fsset.py
> +++ b/fsset.py
> @@ -30,6 +30,7 @@ import partedUtils
>  import raid
>  import lvm
>  import types
> +import copy
>  from flags import flags
>
> import bootyutil
> @@ -1316,6 +1317,7 @@ class FileSystemSet:
>          self.reset()
>          self.volumesCreated = 0
>          self.anaconda = anaconda
>    self.unknownFSlines = list()
>
> def isActive(self):
>        return self.mountcount != 0
>
> @@ -1431,8 +1433,21 @@ class FileSystemSet:
>          new = FileSystemSet(self.anaconda)
>          for entry in self.entries:
>              new.add (entry)
>    new.unknownFSlines = copy.copy(self.unknownFSlines)

Aren't the members of self.unknownFSlines list just strings? Is the copy.copy necessary? Wouldn't plain self.unknownFSlines[:] be enough?
>      return new
>
> def extend (self, fsset):
>    """
>    Extends the entry set and the list of unknownFSlines with
>
> values
>    from a given fsset.
>    """
>    for entry in fsset.entries:
>        self.add(entry)
>    self.unknownFSlines.extend(fsset.unknownFSlines)
> def fstab (self):
>        format = "%-23s %-23s %-7s %-15s %d %d\n"
>        fstab = ""
>
> @@ -1460,6 +1475,11 @@ class FileSystemSet:
>                                            entry.fsystem.getName(),
>                                            options, entry.fsck,
>                                            entry.order)
>    # append commented out lines with unknown (not supported)
>
> filesystems
>    for line in self.unknownFSlines:
>        fstab += "#" + line
>    return fstab
>
> def mtab (self):
>
> @@ -2969,6 +2989,9 @@ def readFstab (anaconda):
>              break
>          # "none" is valid as an fs type for bind mounts (#151458)
>          if fsystem is None and (string.find(fields[3], "bind") ==
>          -1):
>        # add the line to the list of lines that will be
>
> commented out in
>        # the new fstab
>        fsset.unknownFSlines.append(line)
>        continue
>
>    label = None
>
> diff --git a/upgrade.py b/upgrade.py
> index 5beb8a2..f3e243c 100644
> --- a/upgrade.py
> +++ b/upgrade.py
> @@ -198,8 +198,7 @@ def mountRootPartition(anaconda, rootInfo,
> oldfsset, allowDirty = 0,
>  oldfsset.reset()
>  newfsset = readFstab(anaconda)
> for entry in newfsset.entries:
>    oldfsset.add(entry)
>
> oldfsset.extend(newfsset)
>
> isys.umount(anaconda.rootPath)
>
> --
> 1.7.4.4
>
> anaconda-patches mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/anaconda-patches

[PATCHv2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[Vratislav Podzimek]

On Thu, 2012-07-12 at 10:11 -0400, Martin Gracik wrote:
> ----- Original Message -----
> > When doing upgrade, there is no need to ignore the fstab lines with
> > filesystem
> > Anaconda does not support. We can leave the lines as they are and
> > just comment
> > them out to prevent mounting problems during the next boot.
> > Resolves: rhbz#754213
> >
> > fsset.py   |   23 +++++++++++++++++++++++
> >  upgrade.py |    3 +--
> >  2 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/fsset.py b/fsset.py
> > index 9636003..85f6623 100644
> > --- a/fsset.py
> > +++ b/fsset.py
> > @@ -30,6 +30,7 @@ import partedUtils
> >  import raid
> >  import lvm
> >  import types
> > +import copy
> >  from flags import flags
> >
> > import bootyutil
> > @@ -1316,6 +1317,7 @@ class FileSystemSet:
> >          self.reset()
> >          self.volumesCreated = 0
> >          self.anaconda = anaconda
> >    self.unknownFSlines = list()
> >
> > def isActive(self):
> >        return self.mountcount != 0
> >
> > @@ -1431,8 +1433,21 @@ class FileSystemSet:
> >          new = FileSystemSet(self.anaconda)
> >          for entry in self.entries:
> >              new.add (entry)
> >    new.unknownFSlines = copy.copy(self.unknownFSlines)
>
> Aren't the members of self.unknownFSlines list just strings? Is the copy.copy necessary? Wouldn't plain self.unknownFSlines[:] be enough?

copy.copy() seemed better readable to me. In the discussion on IRC
Martin suggested the one more way to do the copy:

new.unknownFSlines = list(self.unknownFSlines)

that would work also for tuples and generators. Any preferences? I will
change this to one of the methods not needing the import before pushing.
Does this patch otherwise look good?
> >      return new
> >
> > def extend (self, fsset):
> >    """
> >    Extends the entry set and the list of unknownFSlines with
> >
> > values
> >    from a given fsset.
> >    """
> >    for entry in fsset.entries:
> >        self.add(entry)
> >    self.unknownFSlines.extend(fsset.unknownFSlines)
> > def fstab (self):
> >        format = "%-23s %-23s %-7s %-15s %d %d\n"
> >        fstab = ""
> >
> > @@ -1460,6 +1475,11 @@ class FileSystemSet:
> >                                            entry.fsystem.getName(),
> >                                            options, entry.fsck,
> >                                            entry.order)
> >    # append commented out lines with unknown (not supported)
> >
> > filesystems
> >    for line in self.unknownFSlines:
> >        fstab += "#" + line
> >    return fstab
> >
> > def mtab (self):
> >
> > @@ -2969,6 +2989,9 @@ def readFstab (anaconda):
> >              break
> >          # "none" is valid as an fs type for bind mounts (#151458)
> >          if fsystem is None and (string.find(fields[3], "bind") ==
> >          -1):
> >        # add the line to the list of lines that will be
> >
> > commented out in
> >        # the new fstab
> >        fsset.unknownFSlines.append(line)
> >        continue
> >
> >    label = None
> >
> > diff --git a/upgrade.py b/upgrade.py
> > index 5beb8a2..f3e243c 100644
> > --- a/upgrade.py
> > +++ b/upgrade.py
> > @@ -198,8 +198,7 @@ def mountRootPartition(anaconda, rootInfo,
> > oldfsset, allowDirty = 0,
> >  oldfsset.reset()
> >  newfsset = readFstab(anaconda)
> > for entry in newfsset.entries:
> >    oldfsset.add(entry)
> >
> > oldfsset.extend(newfsset)
> >
> > isys.umount(anaconda.rootPath)
> >
> > --
> > 1.7.4.4
> >
> > anaconda-patches mailing list
> > [e-mail]
> > https://fedorahosted.org/mailman/listinfo/anaconda-patches
>
> anaconda-patches mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/anaconda-patches

[PATCHv2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[Brian C. Lane]

On Fri, Jul 13, 2012 at 10:38:26AM +0200, Vratislav Podzimek wrote:
> On Thu, 2012-07-12 at 10:11 -0400, Martin Gracik wrote:
> > >    new.unknownFSlines = copy.copy(self.unknownFSlines)
> >
> > Aren't the members of self.unknownFSlines list just strings? Is the copy.copy necessary? Wouldn't plain self.unknownFSlines[:] be enough?
>
> copy.copy() seemed better readable to me. In the discussion on IRC
> Martin suggested the one more way to do the copy:
>
> new.unknownFSlines = list(self.unknownFSlines)
>
> that would work also for tuples and generators. Any preferences? I will
> change this to one of the methods not needing the import before pushing.
> Does this patch otherwise look good?

Why do we need to handle tuples and generators? I prefer [:] unless
there is a compelling reason to use something else.

The rest of it looks good to me. I just think copy.copy is overkill for
a list of strings.

[PATCHv2] Comment out the fstab lines with unknown filesystem rather then ignoring them

[Martin Gracik]

----- Original Message -----
> On Fri, Jul 13, 2012 at 10:38:26AM +0200, Vratislav Podzimek wrote:
> > On Thu, 2012-07-12 at 10:11 -0400, Martin Gracik wrote:
> > > >    new.unknownFSlines = copy.copy(self.unknownFSlines)
> > >
> > > Aren't the members of self.unknownFSlines list just strings? Is
> > > the copy.copy necessary? Wouldn't plain self.unknownFSlines[:]
> > > be enough?
> >
> > copy.copy() seemed better readable to me. In the discussion on IRC
> > Martin suggested the one more way to do the copy:
> >
> > new.unknownFSlines = list(self.unknownFSlines)
> >
> > that would work also for tuples and generators. Any preferences? I
> > will
> > change this to one of the methods not needing the import before
> > pushing.
> > Does this patch otherwise look good?
>
> Why do we need to handle tuples and generators? I prefer [:] unless
> there is a compelling reason to use something else.

We don't. But the list() method is supposed to be "a better way to copy list" instead of [:], which is a bit cryptic.
But I agree that [:] is a commonly used idiom for making a copy of a list, I use it all the time.

http://henry.precheur.org/python/copy_list
> The rest of it looks good to me. I just think copy.copy is overkill
> for
> a list of strings.
>
> --
> Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA
> (PST8PDT)
>
> anaconda-patches mailing list
> [e-mail]
> https://fedorahosted.org/mailman/listinfo/anaconda-patches