Bug 350539

Please attach the y2log with the testcase.

Created attachment 189123 [details]
yast2logs

This should cover the incident.

Created attachment 189129 [details]
patch for /usr/share/YaST2/modules/OSRFstab.ycp

Please try patching  your /usr/share/YaST2/modules/OSRFstab.ycp. Than call "ycpc -c /usr/share/YaST2/modules/OSRFstab.ycp" and try again.

workstation6l:/home/antonyarendt # cp /usr/share/YaST2/modules/OSRFstab.ycp /usr/share/YaST2/modules/OSRFstab.ycp.bak
workstation6l:/home/antonyarendt # patch /usr/share/YaST2/modules/OSRFstab.ycp Download/bug-350539_OSRFstab.diff
patching file /usr/share/YaST2/modules/OSRFstab.ycp
workstation6l:/home/antonyarendt # ycpc -c /usr/share/YaST2/modules/OSRFstab.ycpcompiling to '/usr/share/YaST2/modules/OSRFstab.ybc'
parsing '/usr/share/YaST2/modules/OSRFstab.ycp'
modules/OSRFstab.ycp:871  [Parser] Undeclared identifier 'partition'
Error

Maybe I'm missing something here, I am also a _very_ casual patcher ;-)

Created attachment 189136 [details]
patch for /usr/share/YaST2/modules/OSRFstab.ycp

No, it's my fault, sorry. Hopefully this patch is correct.

Or just replace that "partition" with "part" on the line 871 of OSRFstab.ycp and run ycpc again.

OK, now it works.

This is looking even more funny. It suggests an fstab entry for /dev/sdb1 and devpts (whatever that may be) and then rejects two existing entries, while offering the same two entries as replacement. ( Provided the parts of the entries not visible also match.)

The visibility issue should be fixed as well I think.

Created attachment 189144 [details]
Screenshot for fstab suggestion #1

Created attachment 189145 [details]
Screenshot for fstab suggestion #2

Created attachment 189146 [details]
yast2logs

Created attachment 189173 [details]
patch for /usr/share/YaST2/modules/OSRFstab.ycp

Please patch again your OSRFstab.ycp, run ycpc, try again and attach fresh logs. This time I'm only adding some debug output to see what's going on.

Thanks for testing!

:-(

# patch /usr/share/YaST2/modules/OSRFstab.ycp bug-350539-1_OSRFstab.diff
patching file /usr/share/YaST2/modules/OSRFstab.ycp
Hunk #2 FAILED at 870.
1 out of 2 hunks FAILED -- saving rejects to file /usr/share/YaST2/modules/OSRFstab.ycp.rej

Created attachment 189197 [details]
/usr/share/YaST2/modules/OSRFstab.ycp.rej

Created attachment 189312 [details]
patch for /usr/share/YaST2/modules/OSRFstab.ycp

The patch was against the original (unpatched) code. Try this one, hopefully it will work.

OK, now it runs, I thought I had reset to the original file from backup :-( never mind.

It now suggests /media as mountpoint for /dev/sdb1, I still don't see why it suggests putting up an fstab entry for a removable drive,please explain.

In the second step I maximized display it now is possible to see some difference in the mount points listed. Although still some details remain invisible. I think this window needs a slider to enable visibility of the complete line.

Obviously it complains about the user space ntfs ( which is favoured by install ) and suggests the kernel ntfs file system instead.

This should be checked and made identical for install and repair. 

Created attachment 189322 [details]
Screenshot for fstab suggestion #1

Created attachment 189323 [details]
Screenshot for fstab suggestion #2

Created attachment 189324 [details]
yast2logs

(In reply to comment #14 from Casual J. Programmer)

> I think this window needs a slider to enable visibility of the
> complete line.

I agree.

> Obviously it complains about the user space ntfs ( which is favoured by install
> ) and suggests the kernel ntfs file system instead.

Thomas, why is this different? Technically, I call FileSystems::GetMountString with `ntfs parameter (= value of "detected_fs" key) and it returns the default value ("ntfs") instead of "ntfs-3g". 

The second issue is with mount options: FileSystems::CheckFstabOptions returns "fmask=133,dmask=022,locale=en_US.UTF-8" in the "unknown_options" entry. I assume they were generated during instalation, so I wonder why are they unknown now?

@ Jiří­ Suchomel

Can you please eludicate on this topic from #14 also ?

"It now suggests /media as mountpoint for /dev/sdb1, I still don't see why it
suggests putting up an fstab entry for a removable drive,please explain."

I was under the impression only permanent devices go into fstab.

(In reply to comment #19 from Casual J. Programmer)

> Can you please eludicate on this topic from #14 also ?
> 
> "It now suggests /media as mountpoint for /dev/sdb1, I still don't see why it
> suggests putting up an fstab entry for a removable drive,please explain."
> 
> I was under the impression only permanent devices go into fstab.

I'll look into this as well, I hope after other issues are done.

Thomas, additonal question about the removable devices: how could I know that a device is removable? For example, by usb stick gives me this map (result of Storage::GetPartition I think):

$[
     "detected_fs":`vfat,
     "dev_name":"/dev/sdd1",
     "device":"/dev/sdd1",
     "found":"dev",
     "fsid":11,
     "fstype":"Win95 FAT32",
     "label":"CORSAIR",
     "mount":"/media/CORSAIR",
     "mountby":`id,
     "name":"sdd1",
     "nr":1,
     "region":[
                0,
                123
     ],
     "size_k":987966,
     "type":`primary,
     "udev_id":[
                 "usb-Corsair_Flash_Voyager_AA12300000001023-0:0-part1"
     ],
     "udev_path":"pci-0000:00:1d.7-usb-0:5:1.0-scsi-0:0:0:0-part1",
     "used_fs":`vfat,
     "uuid":"2C9E-7178"
]

And about that entry for devpts (see comment 6), what do you think, it probably shouldn't be offered to fstab... ?

@ Jiří­ Suchomel
Thank you on #20

re #21
Not sure if this is what you want:

hwinfo --disk | grep Features
  Features: Hotpluggable
  Features: Hotpluggable

> re #21
> Not sure if this is what you want:
> 
> hwinfo --disk | grep Features

That's why I'm asking Thomas Fehr to provide ycp API :-)

ad comment #18:
- Due to the added ntfs-3g driver now there are more than on possibility to
  mount ntfs filesystems (both "mount -t ntfs" and "mount -t ntfs-3g" should
  work). I can change GetMountString to return ntfs-3g if you prefer that.
- The fact that some of the options show up under "Other Options" is because
  there are no UI-fields to set these options, all options without special
  UI fields show up under "Other Options", this is not new and was similar
  for older ntfs and vfat fstab options. 

ad comment #21:
The property of hotplugability is not a property of a partition, but of a disk.
So the key is contained in the map describing the disk, not a partition of
the disk. You can get the map of a certain disk with e.g.

map disk = Storage::GetDisk( Storage::GetTargetMap(), "/dev/sda" );
boolean pluggable = disk["hotpluggable"]:false;

(In reply to comment #24 from Thomas Fehr)
> ad comment #18:
> - Due to the added ntfs-3g driver now there are more than on possibility to
>   mount ntfs filesystems (both "mount -t ntfs" and "mount -t ntfs-3g" should
>   work). I can change GetMountString to return ntfs-3g if you prefer that.

My problem with that GetMountString call is that it actually returns the default value, passed as seconf parameter. According to the function code, it looks like it _should_ return ntfs-3g so I wonder what's wrong here?

> - The fact that some of the options show up under "Other Options" is because
>   there are no UI-fields to set these options, all options without special
>   UI fields show up under "Other Options", this is not new and was similar
>   for older ntfs and vfat fstab options. 

I only need the way to verify if current settings in /etc/fstab are somehow correct. That's why I'm using FileSystems::CheckFstabOptions and report an error where the key "all_known" in the return map is set to false. If this case is only that you offer no configuration for extra options but they are actually at correct place, please either change the return value of CheckFstabOptions or giv e me other way to verify such fact.

> ad comment #21:
> The property of hotplugability is not a property of a partition, but of a disk.
> So the key is contained in the map describing the disk, not a partition of
> the disk. You can get the map of a certain disk with e.g.
> 
> map disk = Storage::GetDisk( Storage::GetTargetMap(), "/dev/sda" );
> boolean pluggable = disk["hotpluggable"]:false;

In the time when I need this information, I only ave the data shown in comment 21. But the map Storage::GetDisk (Storage::GetTargetMap(), part["device"]:"") (where "device" is /dev/sdd1) doesn't have hotpluggable key at all. I'm testing it on 10.3 is it some new feature?

> My problem with that GetMountString call is that it actually returns the
> default value, passed as seconf parameter. According to the function code, it
> looks like it _should_ return ntfs-3g so I wonder what's wrong here?

Just try with newest SVN code, it should return "ntfs-3g" now. If not I need
y2log file.

> I only need the way to verify if current settings in /etc/fstab are somehow
> correct. That's why I'm using FileSystems::CheckFstabOptions and report an
> error where the key "all_known" in the return map is set to false. If this case
> is only that you offer no configuration for extra options but they are actually
> at correct place, please either change the return value of CheckFstabOptions or
> give me other way to verify such fact.

Ah, now I see. Forgot to add "locale=", "fmask=", "dmask=" to FstabOptionRegex.
Please retry with newest SVN version of FileSystems.ycp.

> In the time when I need this information, I only ave the data shown in comment
> 21. But the map Storage::GetDisk (Storage::GetTargetMap(), part["device"]:"")
> (where "device" is /dev/sdd1) doesn't have hotpluggable key at all. I'm testing
> it on 10.3 is it some new feature?

Calling GetDisk with a partition as device argument will of course not do 
what you want since the device name of the disk is "/dev/sdd", not "/dev/sdd1". 
If you want to use a partition device name you need to call Storage::GetDiskPartititon which accepts partition devices and searches and
returns the corresponding disk map from Storage
BTW: The "hotpluggable" key is only present if it is true.

I just fixed a problem propagating "hotpluggable" property to target map.
Please use newest SVN version for testing.

Sorry forgot to mention that last changes module was StorageDevices.ycp

Thanks for the help! I just tested the change in StorageDevices.ycp, it works. I do not have a testing environment to test the change of that parameters, so I hope it will work in next alpha.

There was one question remaining: for now-unknown reasons, yast2-repair offers creationg a mount point for devpts (see comment 6). What do you think of it, it probably shouldn't be offered there... ?

I have some rudimentary tests for some aspects of fstab handling code.
(see ~fehr/src/ycp/CheckFstabOptions.ycp). At least I checked that it now
accepts new ntfs-3g options and GetMountString correctly returns "ntfs-3g" 
instead of "ntfs".

As far as I see it, the option to create a mount point for devpts is not needed
in yast2-repair (no idea why it is there either). 

The mount point for devpts is /dev/pts and this is mounted automatically in 
/etc/init.d/boot.localfs. So there is no influence of an entry regarding devpts contained in /etc/fstab. No idea why this entry is still added to fstab. 
Probably just someone forgot that it is created by yast2-storage at all.

Fixed in yast2-repair-2.16.3