Re: svn commit: r368575 - head/stand/lua

[Toomas Soome via svn-src-all]

Yes, with pager or it is a bit useless.

Sent from my iPhone

> On 12. Dec 2020, at 16:10, Kyle Evans  wrote:
> 
> On Sat, Dec 12, 2020 at 1:35 AM Toomas Soome  wrote:
>> 
>> How about ’show-module-options’?
>> 
>> rgds,
>> toomas
>> 
> 
> I missed that one. My 4th is a bit rusty, but it looks like this
> should just be enumerating over config's local modules table and
> dumping everything in sight?
> 
> Thanks,
> 
> Kyle Evans
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r368575 - head/stand/lua

[Toomas Soome via svn-src-all]

How about ’show-module-options’? 

rgds,
toomas

> On 12. Dec 2020, at 07:57, Kyle Evans  wrote:
> 
> Author: kevans
> Date: Sat Dec 12 05:57:42 2020
> New Revision: 368575
> URL: https://svnweb.freebsd.org/changeset/base/368575
> 
> Log:
>  lualoader: provide module-manipulation commands
> 
>  Specifically, we have:
>  - enable-module
>  - disable-module
>  - toggle-module
> 
>  These can be used to add/remove modules to be loaded or force modules to be
>  loaded in spite of modules_blacklist. In the typical case, a user is
>  expected to use them to recover an issue happening due to a module directive
>  they've added to their loader.conf or because they discover that they've
>  under-specified what to load.
> 
>  MFC after:   1 week
> 
> Modified:
>  head/stand/lua/cli.lua
>  head/stand/lua/cli.lua.8
>  head/stand/lua/config.lua
>  head/stand/lua/config.lua.8
> 
> Modified: head/stand/lua/cli.lua
> ==
> --- head/stand/lua/cli.luaSat Dec 12 02:26:43 2020(r368574)
> +++ head/stand/lua/cli.luaSat Dec 12 05:57:42 2020(r368575)
> @@ -65,6 +65,14 @@ local function parseBootArgs(argv, with_kernel)
>   end
> end
> 
> +local function setModule(module, loading)
> + if loading and config.enableModule(module) then
> + print(module .. " will be loaded")
> + elseif not loading and config.disableModule(module) then
> + print(module .. " will not be loaded")
> + end
> +end
> +
> -- Declares a global function cli_execute that attempts to dispatch the
> -- arguments passed as a lua function. This gives lua a chance to intercept
> -- builtin CLI commands like "boot"
> @@ -132,6 +140,37 @@ end
> 
> cli['reload-conf'] = function()
>   config.reload()
> +end
> +
> +cli["enable-module"] = function(...)
> + local _, argv = cli.arguments(...)
> + if #argv == 0 then
> + print("usage error: enable-module module")
> + return
> + end
> +
> + setModule(argv[1], true)
> +end
> +
> +cli["disable-module"] = function(...)
> + local _, argv = cli.arguments(...)
> + if #argv == 0 then
> + print("usage error: disable-module module")
> + return
> + end
> +
> + setModule(argv[1], false)
> +end
> +
> +cli["toggle-module"] = function(...)
> + local _, argv = cli.arguments(...)
> + if #argv == 0 then
> + print("usage error: toggle-module module")
> + return
> + end
> +
> + local module = argv[1]
> + setModule(module, not config.isModuleEnabled(module))
> end
> 
> -- Used for splitting cli varargs into cmd_name and the rest of argv
> 
> Modified: head/stand/lua/cli.lua.8
> ==
> --- head/stand/lua/cli.lua.8  Sat Dec 12 02:26:43 2020(r368574)
> +++ head/stand/lua/cli.lua.8  Sat Dec 12 05:57:42 2020(r368575)
> @@ -26,7 +26,7 @@
> .\"
> .\" $FreeBSD$
> .\"
> -.Dd September 13, 2019
> +.Dd December 12, 2020
> .Dt CLI.LUA 8
> .Os
> .Sh NAME
> @@ -77,14 +77,26 @@ This function may be invoked by a user at the loader p
> .Ic foo .
> Arguments may be passed to it as usual, space-delimited.
> .Ss Default Commands
> -As of present, the
> +The
> .Nm
> -module by default provides commands for
> -.Ic autoboot ,
> -.Ic boot ,
> -.Ic boot-conf ,
> -and
> -.Ic reload-conf .
> +module provides the following default commands:
> +.Bl -bullet
> +.\"-width toggle-module -offset indent
> +.It
> +.Ic autoboot
> +.It
> +.Ic boot
> +.It
> +.Ic boot-conf
> +.It
> +.Ic reload-conf
> +.It
> +.Ic enable-module
> +.It
> +.Ic disable-module
> +.It
> +.Ic toggle-module
> +.El
> .Pp
> For
> .Ic autoboot ,
> @@ -103,6 +115,16 @@ The
> command will reload the configuration from disk.
> This is useful if you have manually changed currdev and would like to easily
> reload the configuration from the new device.
> +.Pp
> +The
> +.Ic enable-module ,
> +.Ic disable-module ,
> +and
> +.Ic toggle-module
> +commands manipulate the list of modules to be loaded along with the kernel.
> +Modules blacklisted are considered disabled by
> +.Ic toggle-module .
> +These commands will override any such restriction as needed.
> .Ss Exported Functions
> The following functions are exported from
> .Nm :
> 
> Modified: head/stand/lua/config.lua
> ==
> --- head/stand/lua/config.lua Sat Dec 12 02:26:43 2020(r368574)
> +++ head/stand/lua/config.lua Sat Dec 12 05:57:42 2020(r368575)
> @@ -312,7 +312,7 @@ local function loadModule(mod, silent)
>   for k, v in pairs(mod) do
>   if v.load ~= nil and v.load:lower() == "yes" then
>   local module_name = v.name or k
> - if blacklist[module_name] ~= nil then
> + if not v.force and blacklist[module_name] ~= nil then
>  

Re: svn commit: r366626 - head/sbin/reboot

[Toomas Soome via svn-src-all]

> On 11. Oct 2020, at 16:01, Alexey Dokuchaev  wrote:
> 
> On Sun, Oct 11, 2020 at 03:20:16PM +0300, Toomas Soome wrote:
>> Please note, the remove is done by rc script during the boot.
> 
> But not by the loader(8) as the page used to claim.  It confused me how to
> avoid the remove, and only later I've discovered with some relief that it
> is in fact not being removed, but only disabled (which IMHO is a lot more
> graceful and thus correct behavior).
> 
>> Also nextboot.conf not generic configuration file (such as loader.conf
>> or loader.conf.local), but the implementation specific file, part of
>> special feature.
>> 
>> That is, one should not assume the presence of nextboot.conf file, make
>> assumptions about its content, or perform manual edits on it.
> 
> Do we want it to be the second-class citizen like this?  Would it make
> better sense by documenting it more completely instead?
> 
> ./danfe

It is not really about being second-class citizen, it really is about if and 
how we can implement the feature. With UFS there is a limited write (write to 
existing, allocated disk blocks), with zfs there is no write to file system at 
all.

rgds,
toomas
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r366626 - head/sbin/reboot

[Toomas Soome via svn-src-all]

Please note, the remove is done by rc script during the boot. Also 
nextboot.conf not generic configuration file (such as loader.conf or 
loader.conf.local), but the implementation specific file, part of special 
feature. That is, one should not assume the presence of nextboot.conf file, 
make assumptions about its content, or perform manual edits on it.

Rgds,
Toomas

> On 11. Oct 2020, at 13:40, Alexey Dokuchaev  wrote:
> 
> Author: danfe (ports committer)
> Date: Sun Oct 11 10:40:11 2020
> New Revision: 366626
> URL: https://svnweb.freebsd.org/changeset/base/366626
> 
> Log:
>  The nextboot(8) manual page currently says that the loader(8) would delete
>  the /boot/nextboot.conf file or its contents which is 1) not the most user-
>  friendly way of working with custom configurations, and 2) simply not true
>  for both Forth and Lua implementations: they would not delete it, but just
>  change the setting to "NO", that is, disable it.
> 
>  While at it, add one missing serial (Oxford) comma and fix some bogus line
>  wraps along the way.
> 
>  Approved by:bcr (manpages)
>  Differential Revision:https://reviews.freebsd.org/D25971
> 
> Modified:
>  head/sbin/reboot/nextboot.8
> 
> Modified: head/sbin/reboot/nextboot.8
> ==
> --- head/sbin/reboot/nextboot.8Sun Oct 11 09:05:13 2020(r366625)
> +++ head/sbin/reboot/nextboot.8Sun Oct 11 10:40:11 2020(r366626)
> @@ -24,7 +24,7 @@
> .\"
> .\" $FreeBSD$
> .\"
> -.Dd September 19, 2020
> +.Dd October 11, 2020
> .Dt NEXTBOOT 8
> .Os
> .Sh NAME
> @@ -41,14 +41,14 @@
> .Sh DESCRIPTION
> The
> .Nm
> -utility allows specifying some combination of an alternate kernel, boot flags
> -and kernel environment for the
> -next time the machine is booted.
> +utility allows specifying some combination of an alternate kernel, boot
> +flags, and kernel environment for the next time the machine is booted.
> Once the
> .Xr loader 8
> -loads in the new kernel
> -information, it is deleted so in case the new kernel hangs the machine,
> -once it is rebooted, the machine will automatically revert to its previous
> +loads in the new kernel information from the
> +.Pa /boot/nextboot.conf
> +file, it is disabled so in case the new kernel hangs the machine, once
> +it is rebooted, the machine will automatically revert to its previous
> configuration.
> .Pp
> The options are as follows:
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r365643 - head/bin/cp

[Toomas Soome via svn-src-all]

> On 23. Sep 2020, at 00:50, Warner Losh  wrote:
> 
> I think it's a great leap sideways, but I've done cp /dev/null foo to clear 
> it out for 35 years now... It's why it feels like a workaround.
> 
> Though it is a legit optimization, no matter the feelings. As for clearer, 
> I'm less sure since then I have to remember what the : operator does.
> 
> Warner


you can always use /bin/true instead (unless its gnu;) or just echo.

rgds,
toomas

> 
> On Tue, Sep 22, 2020 at 3:48 PM Alan Somers  > wrote:
> It doesn't feel like a workaround to me.  I think Kyle's version is clearer 
> than the original.
> 
> On Tue, Sep 22, 2020 at 3:45 PM Warner Losh  > wrote:
> 
> 
> On Tue, Sep 22, 2020 at 3:42 PM Kyle Evans  > wrote:
> cp is already fixed, people are still feeling the fallout of being
> within those revisions and needing to bootstrap their own cp. We can
> reduce the number of components these invocations rely on trivially to
> shell built-in mechanics, why not do so?
> 
> Fair point. I just bristle at putting workarounds in for valid /bin/sh syntax 
> because we opposed for a few days. so long as it's an unconditional clearing 
> of the file to be zero length, I'm OK with that.
> 
> Warner
>  
> On Tue, Sep 22, 2020 at 4:40 PM Warner Losh  > wrote:
> >
> > So why do we need a workaround at all? cp /dev/null has been fixed, and 
> > that's way more important to get right.
> >
> > I don't want to paper-over issues with this at all, though if we use the 
> > host's (now broken) cp, I suppose that might be OK in the short term. If 
> > that's the case, then maybe this is OK.
> >
> > Otherwise, I'd strongly prefer we fix cp.
> >
> > Warner
> >
> > On Tue, Sep 22, 2020 at 3:31 PM Alan Somers  > > wrote:
> >>
> >> +1.
> >>
> >> On Tue, Sep 22, 2020 at 3:27 PM Kyle Evans  >> > wrote:
> >>>
> >>> I'm running a build at the suggestion of mjg to confirm there aren't
> >>> any others hiding that can be converted, and I will commit after I've
> >>> verified that this is it.
> >>>
> >>> On Tue, Sep 22, 2020 at 4:02 PM Alan Somers  >>> > wrote:
> >>> >
> >>> > LGTM.
> >>> >
> >>> > On Tue, Sep 22, 2020 at 2:59 PM Kyle Evans  >>> > > wrote:
> >>> >>
> >>> >> Perhaps:
> >>> >>
> >>> >> diff --git a/stand/i386/zfsboot/Makefile b/stand/i386/zfsboot/Makefile
> >>> >> index ff315abc0ef..7e362b43a39 100644
> >>> >> --- a/stand/i386/zfsboot/Makefile
> >>> >> +++ b/stand/i386/zfsboot/Makefile
> >>> >> @@ -81,7 +81,7 @@ zfsboot.ld: zfsboot.ldr zfsboot.bin ${BTXKERN}
> >>> >> -o ${.TARGET} -P 1 zfsboot.bin
> >>> >>
> >>> >>  zfsboot.ldr:
> >>> >> -   cp /dev/null ${.TARGET}
> >>> >> +   :> ${.TARGET}
> >>> >>
> >>> >>  zfsboot.bin: zfsboot.out
> >>> >> ${OBJCOPY} -S -O binary zfsboot.out ${.TARGET}
> >>> >> diff --git a/stand/libsa/Makefile b/stand/libsa/Makefile
> >>> >> index effece9e01b..63cd46a9c54 100644
> >>> >> --- a/stand/libsa/Makefile
> >>> >> +++ b/stand/libsa/Makefile
> >>> >> @@ -122,7 +122,7 @@ beforedepend:
> >>> >> ln -sf ${SRCTOP}/include/arpa/inet.h arpa/inet.h; \
> >>> >> ln -sf ${SRCTOP}/include/arpa/tftp.h arpa/tftp.h; \
> >>> >> for i in _time.h _strings.h _string.h; do \
> >>> >> -   [ -f xlocale/$$i ] || cp /dev/null xlocale/$$i; \
> >>> >> +   [ -f xlocale/$$i ] || :> xlocale/$$i; \
> >>> >> done; \
> >>> >> for i in ${STAND_H_INC}; do \
> >>> >> ln -sf ${SASRC}/stand.h $$i; \
> >>> >>
> >>> >>
> >>> >> On Tue, Sep 22, 2020 at 3:58 PM Alan Somers  >>> >> > wrote:
> >>> >> >
> >>> >> > Looks like two places in stand.  Is there any reason why Mateusz's 
> >>> >> > suggestion wouldn't work?
> >>> >> >
> >>> >> > > rg -g Makefile 'cp.*/dev/null'
> >>> >> > stand/libsa/Makefile
> >>> >> > 125: [ -f xlocale/$$i ] || cp /dev/null xlocale/$$i; \
> >>> >> >
> >>> >> > stand/i386/zfsboot/Makefile
> >>> >> > 82: cp /dev/null ${.TARGET}
> >>> >> >
> >>> >> > On Tue, Sep 22, 2020 at 2:54 PM Mateusz Guzik  >>> >> > > wrote:
> >>> >> >>
> >>> >> >> Can we instead add a workaround to the build tree?
> >>> >> >>
> >>> >> >> Where is cp /dev/null coming from anyway? Perhaps this can be 
> >>> >> >> patched
> >>> >> >> to touch the target file.
> >>> >> >>
> >>> >> >> On 9/22/20, Alan Somers  >>> >> >> > wrote:
> >>> >> >> > On Tue, Sep 22, 2020 at 2:48 PM Kyle Evans  >>> >> >> > > wrote:
> >>> >> >> >
> >>> >> >> >> On Fri, Sep 11, 2020 at 3:49 PM Alan Somers  >>> >> >> >> > wrote:
> >>> >> >> >> >
> >>> >> >> >> > Author: asomers
> >>> >> >> >> > Date: Fri Sep 11 20:49:36 2020
> >>> >> >> >> > New Revision: 365643
> >>> >> >> >> > URL: 

Re: svn commit: r365249 - head

[Toomas Soome via svn-src-all]

> On 2. Sep 2020, at 18:53, Ryan Moeller  wrote:
> 
> 
> On 9/2/20 11:43 AM, Andriy Gapon wrote:
>> On 02/09/2020 18:23, Ryan Moeller wrote:
>>> On 9/2/20 10:28 AM, Warner Losh wrote:
 Author: imp
 Date: Wed Sep  2 14:28:54 2020
 New Revision: 365249
 URL: https://svnweb.freebsd.org/changeset/base/365249
 
 Log:
Add note about needing to manually import the zfs pools or update
/etc/rc.d due to the cache file moving to /etc.
 
 Modified:
head/UPDATING
 
 Modified: head/UPDATING
 ==
 --- head/UPDATINGWed Sep  2 12:57:34 2020(r365248)
 +++ head/UPDATINGWed Sep  2 14:28:54 2020(r365249)
 @@ -36,6 +36,10 @@ NOTE TO PEOPLE WHO THINK THAT FreeBSD 13.x IS SLOW:
   scenarios have been tested and fixed, but rebuilding kernels without
   rebuilding world may fail.
   +The ZFS cache file has moved from /boot to /etc to match the OpenZFS
 +upstream default. This means your zpool won't auto import until you
 +upgrade your /etc/rc.d files or you import them manually.
 +
   20200824:
   The resume code now notifies devd with the 'kernel' system
   rather than the old 'kern' subsystem to be consistent with
>>> Thanks, enough people seemed to be getting tripped up by this.
>> I think that this is a very useful note.
>> 
>> But I do not see a direct connection between the change of zpool.cache 
>> location
>> and the new ZFS's not automatically importing zpool.cache pools on boot.
>> 
> 
> True, the real reason is that the kernel module in OpenZFS does not 
> autoimport pools.
> Instead we explicitly "zpool import -a" in one of the ZFS rc scripts. I'll 
> amend the
> UPDATING message.
> 
> -Ryan


please note that  zpool import -a can be dangerous for multihomed setups.

rgds,
toomas


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r360836 - head/stand/libsa/zfs

[Toomas Soome via svn-src-all]

> On 9. May 2020, at 11:23, Ronald Klop  wrote:
> 
> On Sat, 09 May 2020 09:25:29 +0200, Toomas Soome  > wrote:
> 
>> 
>> 
>>> On 9. May 2020, at 09:57, Ronald Klop  wrote:
>>> 
>>> Hi Toomas,
>>> 
>>> Could this fix this issue 
>>> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=144234 ?
>>> 
>>> Regards,
>>> Ronald.
>> 
>> 
>> I doubt a bit unless you have GELI encryption or 4kn disk (which we can not 
>> boot with BIOS, only with UEFI). That issue was reported 2010 agains 9.0? is 
>> it still the case?
>> 
>> rgds,
>> toomas
> 
> 
> Clear answer. I don't use the computer I had this problem with anymore. (It 
> is in the attic somewhere,) And the problem disappeared for me in 2017 
> (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=144234#c33 
> ). But the 
> issue apparently happens for other people in 12.1 still as I read in the 
> replies to the issue.
> 
> Because of the bogus LBA numbers I suspected some memory corruption. But 
> never found further evidence for this.
> 
> Regards,
> Ronald.


Ok, We just need to check such errors case by case. We know pretty well how to 
debug those, even if the process can be time consuming.

rgds,
toomas

> 
> 
>>> 
>>> 
>>> On Sat, 09 May 2020 08:25:21 +0200, Toomas Soome  wrote:
>>> 
 Author: tsoome
 Date: Sat May  9 06:25:20 2020
 New Revision: 360836
 URL: https://svnweb.freebsd.org/changeset/base/360836
 
 Log:
 loader: vdev_read() can corrupt memory
 When reading less than sector size but from sector boundary,
 the vdev_read() will read full sector into the provided buffer
 and therefore corrupting memory past buffer end.
 MFC after: 2 days
 
 Modified:
 head/stand/libsa/zfs/zfs.c
 
 Modified: head/stand/libsa/zfs/zfs.c
 ==
 --- head/stand/libsa/zfs/zfs.c Sat May  9 05:04:02 2020
 (r360835)
 +++ head/stand/libsa/zfs/zfs.c Sat May  9 06:25:20 2020
 (r360836)
 @@ -418,7 +418,7 @@ vdev_read(vdev_t *vdev, void *priv, off_t offset, void
full_sec_size -= secsz;
/* Return of partial sector data requires a bounce buffer. */
 -  if ((head > 0) || do_tail_read) {
 +  if ((head > 0) || do_tail_read || bytes < secsz) {
bouncebuf = malloc(secsz);
if (bouncebuf == NULL) {
printf("vdev_read: out of memory\n");
 @@ -442,14 +442,28 @@ vdev_read(vdev_t *vdev, void *priv, off_t offset, 
 void
outbuf += min(secsz - head, bytes);
}
 -  /* Full data return from read sectors */
 +  /*
 +   * Full data return from read sectors.
 +   * Note, there is still corner case where we read
 +   * from sector boundary, but less than sector size, e.g. reading 512B
 +   * from 4k sector.
 +   */
if (full_sec_size > 0) {
 -  res = read(fd, outbuf, full_sec_size);
 -  if (res != full_sec_size) {
 -  ret = EIO;
 -  goto error;
 +  if (bytes < full_sec_size) {
 +  res = read(fd, bouncebuf, secsz);
 +  if (res != secsz) {
 +  ret = EIO;
 +  goto error;
 +  }
 +  memcpy(outbuf, bouncebuf, bytes);
 +  } else {
 +  res = read(fd, outbuf, full_sec_size);
 +  if (res != full_sec_size) {
 +  ret = EIO;
 +  goto error;
 +  }
 +  outbuf += full_sec_size;
}
 -  outbuf += full_sec_size;
}
/* Partial data return from last sector */
 ___
 svn-src-all@freebsd.org mailing list
 https://lists.freebsd.org/mailman/listinfo/svn-src-all
 To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r360836 - head/stand/libsa/zfs

[Toomas Soome via svn-src-all]

> On 9. May 2020, at 09:57, Ronald Klop  wrote:
> 
> Hi Toomas,
> 
> Could this fix this issue 
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=144234 ?
> 
> Regards,
> Ronald.


I doubt a bit unless you have GELI encryption or 4kn disk (which we can not 
boot with BIOS, only with UEFI). That issue was reported 2010 agains 9.0? is it 
still the case?

rgds,
toomas


> 
> 
> On Sat, 09 May 2020 08:25:21 +0200, Toomas Soome  wrote:
> 
>> Author: tsoome
>> Date: Sat May  9 06:25:20 2020
>> New Revision: 360836
>> URL: https://svnweb.freebsd.org/changeset/base/360836
>> 
>> Log:
>>  loader: vdev_read() can corrupt memory
>> When reading less than sector size but from sector boundary,
>>  the vdev_read() will read full sector into the provided buffer
>>  and therefore corrupting memory past buffer end.
>> MFC after:   2 days
>> 
>> Modified:
>>  head/stand/libsa/zfs/zfs.c
>> 
>> Modified: head/stand/libsa/zfs/zfs.c
>> ==
>> --- head/stand/libsa/zfs/zfs.c   Sat May  9 05:04:02 2020
>> (r360835)
>> +++ head/stand/libsa/zfs/zfs.c   Sat May  9 06:25:20 2020
>> (r360836)
>> @@ -418,7 +418,7 @@ vdev_read(vdev_t *vdev, void *priv, off_t offset, void
>>  full_sec_size -= secsz;
>>  /* Return of partial sector data requires a bounce buffer. */
>> -if ((head > 0) || do_tail_read) {
>> +if ((head > 0) || do_tail_read || bytes < secsz) {
>>  bouncebuf = malloc(secsz);
>>  if (bouncebuf == NULL) {
>>  printf("vdev_read: out of memory\n");
>> @@ -442,14 +442,28 @@ vdev_read(vdev_t *vdev, void *priv, off_t offset, void
>>  outbuf += min(secsz - head, bytes);
>>  }
>> -/* Full data return from read sectors */
>> +/*
>> + * Full data return from read sectors.
>> + * Note, there is still corner case where we read
>> + * from sector boundary, but less than sector size, e.g. reading 512B
>> + * from 4k sector.
>> + */
>>  if (full_sec_size > 0) {
>> -res = read(fd, outbuf, full_sec_size);
>> -if (res != full_sec_size) {
>> -ret = EIO;
>> -goto error;
>> +if (bytes < full_sec_size) {
>> +res = read(fd, bouncebuf, secsz);
>> +if (res != secsz) {
>> +ret = EIO;
>> +goto error;
>> +}
>> +memcpy(outbuf, bouncebuf, bytes);
>> +} else {
>> +res = read(fd, outbuf, full_sec_size);
>> +if (res != full_sec_size) {
>> +ret = EIO;
>> +goto error;
>> +}
>> +outbuf += full_sec_size;
>>  }
>> -outbuf += full_sec_size;
>>  }
>>  /* Partial data return from last sector */
>> ___
>> svn-src-all@freebsd.org mailing list
>> https://lists.freebsd.org/mailman/listinfo/svn-src-all
>> To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r358989 - in head/stand/efi: libefi loader loader/arch/arm loader/arch/arm64

[Toomas Soome via svn-src-all]

> On 29. Mar 2020, at 09:40, Ruslan Garipov  wrote:
> 
>>> 
>> 
>> That definitely can be the case. If you do not set any custom values, 
>> missing tem.* variables would confirm that.
>> 
>> Now, there is still an question, why in your system that allocation does 
>> fail?
> Is there a way I can help you to figure that out?  I'm afraid I will not
> have an access to my systems during the following week, but even so.

Hi!

Please test latest current:)

thanks,
toomas

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r359408 - head/stand/libsa/zfs

[Toomas Soome via svn-src-all]

> On 29. Mar 2020, at 00:04, Oliver Pinter  wrote:
> 
> 
> 
> On Saturday, March 28, 2020, Toomas Soome  > wrote:
> Author: tsoome
> Date: Sat Mar 28 21:50:27 2020
> New Revision: 359408
> URL: https://svnweb.freebsd.org/changeset/base/359408 
> 
> 
> Log:
>   loader: strdup name strings from dataset walker
> 
>   The removal of zfs scratch buffer did miss the fact the dataset
>   lookup was picking up the names from zap list.
> 
> Modified:
>   head/stand/libsa/zfs/zfs.c
> 
> Modified: head/stand/libsa/zfs/zfs.c
> ==
> --- head/stand/libsa/zfs/zfs.c  Sat Mar 28 21:47:44 2020(r359407)
> +++ head/stand/libsa/zfs/zfs.c  Sat Mar 28 21:50:27 2020(r359408)
> @@ -92,7 +92,7 @@ static intzfs_env_count;
>  SLIST_HEAD(zfs_be_list, zfs_be_entry) zfs_be_head = 
> SLIST_HEAD_INITIALIZER(zfs_be_head);
>  struct zfs_be_list *zfs_be_headp;
>  struct zfs_be_entry {
> -   const char *name;
> +   cha *name;
> 
> I think this will be "char *". 


Yes, I guess I must have (accidentally) removed the ‘r' after the build. The 
fix is already posted.

rgds,
toomas

>  
> SLIST_ENTRY(zfs_be_entry) entries;
>  } *zfs_be, *zfs_be_tmp;
> 
> @@ -906,6 +906,7 @@ zfs_bootenv_initial(const char *name)
> while (!SLIST_EMPTY(_be_head)) {
> zfs_be = SLIST_FIRST(_be_head);
> SLIST_REMOVE_HEAD(_be_head, entries);
> +   free(zfs_be->name);
> free(zfs_be);
> }
> 
> @@ -973,6 +974,7 @@ zfs_bootenv(const char *name)
> while (!SLIST_EMPTY(_be_head)) {
> zfs_be = SLIST_FIRST(_be_head);
> SLIST_REMOVE_HEAD(_be_head, entries);
> +   free(zfs_be->name);
> free(zfs_be);
> }
> 
> @@ -992,7 +994,11 @@ zfs_belist_add(const char *name, uint64_t value __unus
> if (zfs_be == NULL) {
> return (ENOMEM);
> }
> -   zfs_be->name = name;
> +   zfs_be->name = strdup(name);
> +   if (zfs_be->name == NULL) {
> +   free(zfs_be);
> +   return (ENOMEM);
> +   }
> SLIST_INSERT_HEAD(_be_head, zfs_be, entries);
> zfs_env_count++;
> 
> ___
> svn-src-h...@freebsd.org  mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-head 
> 
> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org 
> "

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r358989 - in head/stand/efi: libefi loader loader/arch/arm loader/arch/arm64

[Toomas Soome via svn-src-all]

> On 28. Mar 2020, at 08:07, Alexey Dokuchaev  wrote:
> 
> On Sat, Mar 14, 2020 at 06:36:03AM +, Toomas Soome wrote:
>> New Revision: 358989
>> URL: https://svnweb.freebsd.org/changeset/base/358989
>> 
>> Log:
>>  loader: add comconsole implementation on top of SIO protocol
>> 
>> [...]
>> --- /dev/null00:00:00 1970   (empty, because file is newly added)
>> +++ head/stand/efi/loader/efiserialio.c  Sat Mar 14 06:36:03 2020
>> (r358989)
>> @@ -0,0 +1,518 @@
>> +/*-
>> + * Copyright (c) 1998 Michael Smith (msm...@freebsd.org)
> 
> If this is the new code, then copyright date and maybe attribution
> seem wrong.  If this is the old code, why wasn't it repocopied?
> 
> ./danfe

It is based on libi386 version.

rgds,
toomas
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r358989 - in head/stand/efi: libefi loader loader/arch/arm loader/arch/arm64

[Toomas Soome via svn-src-all]

> On 27. Mar 2020, at 16:39, Ruslan Garipov  wrote:
> 
> On 3/14/2020 11:36 AM, Toomas Soome wrote:
>> Author: tsoome
>> Date: Sat Mar 14 06:36:03 2020
>> New Revision: 358989
>> URL: https://svnweb.freebsd.org/changeset/base/358989
>> 
>> Log:
>>  loader: add comconsole implementation on top of SIO protocol
>> 
>>  Provide comconsole on top of SIO for arm platforms (x86 does use bios 
>> version).
>> 
>> Added:
>>  head/stand/efi/loader/efiserialio.c   (contents, props changed)
>> Modified:
>>  head/stand/efi/libefi/efi_console.c
>>  head/stand/efi/loader/arch/arm/Makefile.inc
>>  head/stand/efi/loader/arch/arm64/Makefile.inc
>>  head/stand/efi/loader/conf.c
>>  head/stand/efi/loader/main.c
>> 
>> Modified: head/stand/efi/libefi/efi_console.c
>> ==
>> --- head/stand/efi/libefi/efi_console.c  Sat Mar 14 05:57:22 2020
>> (r358988)
>> +++ head/stand/efi/libefi/efi_console.c  Sat Mar 14 06:36:03 2020
>> (r358989)
>> @@ -377,9 +377,22 @@ efi_cons_respond(void *s __unused, const void *buf __u
>> {
>> }
>> 
>> +/*
>> + * Set up conin/conout/coninex to make sure we have input ready.
>> + */
>> static void
>> efi_cons_probe(struct console *cp)
>> {
>> +EFI_STATUS status;
>> +
>> +conout = ST->ConOut;
>> +conin = ST->ConIn;
>> +
>> +status = BS->OpenProtocol(ST->ConsoleInHandle, _input_ex_guid,
>> +(void **), IH, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>> +if (status != EFI_SUCCESS)
>> +coninex = NULL;
>> +
>>  cp->c_flags |= C_PRESENTIN | C_PRESENTOUT;
>> }
>> 
>> @@ -889,15 +902,7 @@ efi_cons_init(int arg)
>>  if (conin != NULL)
>>  return (0);
>> 
>> -conout = ST->ConOut;
>> -conin = ST->ConIn;
>> -
>>  conout->EnableCursor(conout, TRUE);
>> -status = BS->OpenProtocol(ST->ConsoleInHandle, _input_ex_guid,
>> -(void **), IH, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>> -if (status != EFI_SUCCESS)
>> -coninex = NULL;
>> -
>>  if (efi_cons_update_mode())
>>  return (0);
> Hello, Toomas!
> 
> I have to return to this revision once again.
> 
> Speaking in advance, the problem I'll describe isn't a fatal one.  I
> want to find a solution/root cause by myself... well, I had tried to do
> that but failed, therefore, I need some tips from you, if you have free
> time for that.
> 
> The loader started to ignore teken.fg_color after r358989.
> 
> I like to have green text on black console.  Therefore, I have this:
> 
> teken.fg_color="green"


Oh cool, at least it is useful for someone:)

I am sorry, yes this is my bug, I somehow missed the second probe and assumed 
we do probe only once.. And yes, your analysis is correct, the environment with 
callback should be treated carefully.

What probe must do is to set up conin/coninex so the efiserial can use 
workaround for buggy SIO, the rest is not that important.


> 
> in my /boot/loader.conf.  Before r358989 everything worked just like I
> wanted: not only vt(4) had green text on black, but the loader menu
> also.  After r358989 vt(4) still renders greeen text on black, but the
> loader doesn't.  It use default white on black.  The variable is
> assigned but doesn't affect the output:
> 
> OK show teken.fg_color
> green
> 
> That started to happen after the changes from above: when code from the
> efi_cons_init() was moved to the efi_cons_probe()
> (stand/efi/libefi/efi_console.c).  Therefore, if I revert those only
> changes, the loader starts to draw green text on black.
> 
> If I read the sources correctly, the cons_probe() function in
> stand/common/console.c calls both those functions.  Moreover, the
> efi_cons_probe() is called twice: first time when the cons_probe()
> probes all available consoles, and then when it searches an "online"
> console.  And then the cons_probe() calls the efi_cons_init().  I see
> nothing between those calls which may cause the loader to ignore
> teken.fg_color (or any other variable).
> 
> I believe that the efi_set_colors() function from
> stand/efi/libefi/efi_console.c is not call being the hook function for
> the teken.fg_color variable.  The efi_cons_update_mode() sets the
> efi_set_colors() as the callback for teken.fg_color, but it's never
> called.  The only reason for that, according to code of the env_setenv()
> in stand/libsa/environment.c, is that teken.fg_color was already created
> (without the hook function, of course) when the efi_cons_update_mode()
> tries to assign the efi_set_colors() hook.  Or, the
> efi_cons_update_mode() failed to allocate the buffer, and, therefore,
> didn't set teken.fg_color at all.  And later teken.fg_color is read from
> /boot/loader.conf.
> 
> One more evidence that efi_set_colors() is not called: setting
> teken.fg_color from the loader prompt to something incorrect:
> 
> OK set teken.fg_color=foobar
> 
> doesn't print error message "Allowed values are either ansi color name
> or 

Re: svn commit: r356693 - in head/stand: efi/libefi i386/libi386 libofw uboot/lib

[Toomas Soome via svn-src-all]

> On 13. Jan 2020, at 20:45, Ian Lepore  wrote:
> 
> On Mon, 2020-01-13 at 20:43 +0200, Toomas Soome wrote:
>>> On 13. Jan 2020, at 20:31, Ian Lepore  wrote:
>>> 
>>> On Mon, 2020-01-13 at 18:22 +, Toomas Soome wrote:
 Author: tsoome
 Date: Mon Jan 13 18:22:54 2020
 New Revision: 356693
 URL: https://svnweb.freebsd.org/changeset/base/356693
 
 Log:
 loader: allocate properly aligned buffer for network packet
 
 Use memalign(4, size) to ensure we have properly aligned buffer.
 
 MFC after: 2 weeks
 
 Modified:
 head/stand/efi/libefi/efinet.c
 head/stand/i386/libi386/pxe.c
 head/stand/libofw/ofw_net.c
 head/stand/uboot/lib/net.c
 
>>> 
>>> The malloc implementation in libstand already g'tees minimum
>>> alignment
>>> of 16 bytes on most arches, 64 bytes on arches that use u-boot (see
>>> libsa/zalloc_defs.h).  So how does this change anything?
>>> 
>> 
>> Hi!
>> 
>> Well, given the amount of knobs etc, it does not hurt to be explicit,
>> does it?
>> 
>> rgds,
>> toomas
> 
> I think it does hurt, because now it misleads you into thinking it's 4-
> byte aligned when it's actually 16 or 64.  (That's what made me reply
> at all, my first gut reaction to reading the commit message was "but 4
> is not at all the right alignment on many platforms").
> 

hm, I think you are right. time for backout.

thanks for bringing this into my attention,
toomas



___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r356693 - in head/stand: efi/libefi i386/libi386 libofw uboot/lib

[Toomas Soome via svn-src-all]

> On 13. Jan 2020, at 20:31, Ian Lepore  wrote:
> 
> On Mon, 2020-01-13 at 18:22 +, Toomas Soome wrote:
>> Author: tsoome
>> Date: Mon Jan 13 18:22:54 2020
>> New Revision: 356693
>> URL: https://svnweb.freebsd.org/changeset/base/356693
>> 
>> Log:
>>  loader: allocate properly aligned buffer for network packet
>> 
>>  Use memalign(4, size) to ensure we have properly aligned buffer.
>> 
>>  MFC after:  2 weeks
>> 
>> Modified:
>>  head/stand/efi/libefi/efinet.c
>>  head/stand/i386/libi386/pxe.c
>>  head/stand/libofw/ofw_net.c
>>  head/stand/uboot/lib/net.c
>> 
> 
> The malloc implementation in libstand already g'tees minimum alignment
> of 16 bytes on most arches, 64 bytes on arches that use u-boot (see
> libsa/zalloc_defs.h).  So how does this change anything?
> 

Hi!

Well, given the amount of knobs etc, it does not hurt to be explicit, does it?

rgds,
toomas
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r356031 - head/stand/i386/gptboot

[Toomas Soome via svn-src-all]

The only limit set by specification is:
A minimum of 16,384 bytes of space must be reserved for the GPT Partition Entry 
Array.

>From this size and array entry size we do get 128, but specification does not 
>limit the max number.

rgds,
toomas

> On 23. Dec 2019, at 00:33, Ian Lepore  wrote:
> 
> Author: ian
> Date: Sun Dec 22 22:33:22 2019
> New Revision: 356031
> URL: https://svnweb.freebsd.org/changeset/base/356031
> 
> Log:
>  In gptboot, don't assume a partition number is a single digit, 1-9.  GPT
>  partitions can have 128 partitions, so parse contiguous digits and then
>  validate that the number is between 1-128 inclusive.
> 
>  I'm not sure 128 is a hard limit in the GPT standard, but it's the common
>  number in use, and it's a better upper limit than 9.
> 
> Modified:
>  head/stand/i386/gptboot/gptboot.c
> 
> Modified: head/stand/i386/gptboot/gptboot.c
> ==
> --- head/stand/i386/gptboot/gptboot.c Sun Dec 22 22:10:20 2019
> (r356030)
> +++ head/stand/i386/gptboot/gptboot.c Sun Dec 22 22:33:22 2019
> (r356031)
> @@ -574,10 +574,12 @@ parse_cmds(char *cmdstr, int *dskupdated)
>   if (arg[1] != 'p' || gdsk.dsk.unit > 9)
>   return (-1);
>   arg += 2;
> - gdsk.dsk.part = *arg - '0';
> - if (gdsk.dsk.part < 1 || gdsk.dsk.part > 9)
> + j = 0;
> + while (*arg >= '0' && *arg <= '9')
> + j = j * 10 + *arg++ - '0';
> + gdsk.dsk.part = j;
> + if (gdsk.dsk.part < 1 || gdsk.dsk.part > 128)
>   return (-1);
> - arg++;
>   if (arg[0] != ')')
>   return (-1);
>   arg++;

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r354435 - head/stand/efi/libefi

[Toomas Soome via svn-src-all]

> On 7. Nov 2019, at 17:29, Warner Losh  wrote:
> 
> 
> 
> On Thu, Nov 7, 2019 at 4:17 AM Toomas Soome  > wrote:
> Author: tsoome
> Date: Thu Nov  7 11:17:03 2019
> New Revision: 354435
> URL: https://svnweb.freebsd.org/changeset/base/354435 
> 
> 
> Log:
>   loader: implement fallback efi_devpath_to_name()
> 
>   UEFI 1.10 on macs does not seem to provide devpath to name translation,
>   provide our own (limited) version, so we can get information about commmon
>   devices.
> 
> We specifically deleted our own version of this function (it was wrong in 
> many ways too) because we thought we could require a minimum UEFI 2.0 for 
> full functionality... This sort of function is a total pain to maintain.
> 
> I'd prefer the fallback was "you don't get this" rather than bloating the 
> code, especially for devices that we don't care about and will never care 
> about for booting...
> 
> Warner


I can see why, but then again, try to debug such system…  btw, that particular 
one I debug is MacBookPro11,4 with quad core i7….  not even that old hw anyhow.

I’d rather keep some bloat than spend 2-3 days to write code just to get any 
idea what is going on in the machine…

rgds,
toomas


>  
>   MFC after:1 week
> 
> Modified:
>   head/stand/efi/libefi/devpath.c
> 
> Modified: head/stand/efi/libefi/devpath.c
> ==
> --- head/stand/efi/libefi/devpath.c Thu Nov  7 07:21:45 2019
> (r354434)
> +++ head/stand/efi/libefi/devpath.c Thu Nov  7 11:17:03 2019
> (r354435)
> @@ -29,13 +29,16 @@ __FBSDID("$FreeBSD$");
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> 
>  static EFI_GUID ImageDevicePathGUID =
>  EFI_LOADED_IMAGE_DEVICE_PATH_PROTOCOL_GUID;
>  static EFI_GUID DevicePathGUID = DEVICE_PATH_PROTOCOL;
>  static EFI_GUID DevicePathToTextGUID = EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID;
>  static EFI_DEVICE_PATH_TO_TEXT_PROTOCOL *toTextProtocol;
> -static EFI_GUID DevicePathFromTextGUID = 
> EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL_GUID;
> +static EFI_GUID DevicePathFromTextGUID =
> +EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL_GUID;
>  static EFI_DEVICE_PATH_FROM_TEXT_PROTOCOL *fromTextProtocol;
> 
>  EFI_DEVICE_PATH *
> @@ -64,6 +67,427 @@ efi_lookup_devpath(EFI_HANDLE handle)
> return (devpath);
>  }
> 
> +static char *
> +efi_make_tail(char *suffix)
> +{
> +   char *tail;
> +
> +   tail = NULL;
> +   if (suffix != NULL)
> +   (void)asprintf(, "/%s", suffix);
> +   else
> +   tail = strdup("");
> +   return (tail);
> +}
> +
> +typedef struct {
> +   EFI_DEVICE_PATH Header;
> +   EFI_GUIDGuid;
> +   UINT8   VendorDefinedData[1];
> +} __packed VENDOR_DEVICE_PATH_WITH_DATA;
> +
> +static char *
> +efi_vendor_path(const char *type, VENDOR_DEVICE_PATH *node, char *suffix)
> +{
> +   uint32_t size = DevicePathNodeLength(>Header) - sizeof(*node);
> +   VENDOR_DEVICE_PATH_WITH_DATA *dp = (VENDOR_DEVICE_PATH_WITH_DATA 
> *)node;
> +   char *name, *tail, *head;
> +   char *uuid;
> +   int rv;
> +
> +   uuid_to_string((const uuid_t *)(void *)>Guid, , );
> +   if (rv != uuid_s_ok)
> +   return (NULL);
> +
> +   tail = efi_make_tail(suffix);
> +   rv = asprintf(, "%sVendor(%s)[%x:", type, uuid, size);
> +   free(uuid);
> +   if (rv < 0)
> +   return (NULL);
> +
> +   if (DevicePathNodeLength(>Header) > sizeof(*node)) {
> +   for (uint32_t i = 0; i < size; i++) {
> +   rv = asprintf(, "%s%02x", head,
> +   dp->VendorDefinedData[i]);
> +   if (rv < 0) {
> +   free(tail);
> +   free(head);
> +   return (NULL);
> +   }
> +   free(head);
> +   head = name;
> +   }
> +   }
> +
> +   if (asprintf(, "%s]%s", head, tail) < 0)
> +   name = NULL;
> +   free(head);
> +   free(tail);
> +   return (name);
> +}
> +
> +static char *
> +efi_hw_dev_path(EFI_DEVICE_PATH *node, char *suffix)
> +{
> +   uint8_t subtype = DevicePathSubType(node);
> +   char *name, *tail;
> +
> +   tail = efi_make_tail(suffix);
> +   switch (subtype) {
> +   case HW_PCI_DP:
> +   if (asprintf(, "Pci(%x,%x)%s",
> +   ((PCI_DEVICE_PATH *)node)->Function,
> +   ((PCI_DEVICE_PATH *)node)->Device, tail) < 0)
> +   name = NULL;
> +   break;
> +   case HW_PCCARD_DP:
> +   if (asprintf(, "PCCARD(%x)%s",
> +   ((PCCARD_DEVICE_PATH *)node)->FunctionNumber, tail) < 0)
> +   name = NULL;
> +   break;
> +   case HW_MEMMAP_DP:
> 

Re: svn commit: r354283 - in head: stand/libsa/zfs sys/cddl/boot/zfs

[Toomas Soome via svn-src-all]

> On 4. Nov 2019, at 03:42, Alexander Motin  wrote:
> 
> On 03.11.2019 20:19, Xin Li wrote:
>> On 2019-11-03 15:30, Ravi Pokala wrote:
>>> Uh
>>> 
>>> I've had a log device in my boot-pool for months, and have booted without 
>>> issue:
>>> 
>>>[threepio:~] rpokala% zpool status zroot
>>>  pool: zroot
>>> state: ONLINE
>>>  scan: scrub repaired 0 in 0 days 00:04:36 with 0 errors on Mon Oct 28 
>>> 03:10:59 2019
>>>config:
>>> 
>>>NAMESTATE READ WRITE CKSUM
>>>zroot   ONLINE   0 0 0
>>>  nvd1p4ONLINE   0 0 0
>>>logs
>>>  nvd0p1ONLINE   0 0 0
>>> 
>>>errors: No known data errors
>> 
>> 
>> This is not supported, and it's not trivial to support it, because in
>> order to support it, the bootloader would have to know how to replay
>> zilogs, which would add quite a lot of code to it.
> 
> The issue with ZIL not being replayed in case of read-only mount has
> nothing to do with the fact of SLOG device presence.  The issue is the
> same when ZIL resides on the main disks of the pool.  So while
> everything else you said is right, I see no any reason to ban pools with
> SLOG devices in this context.
> 

Yep, indeed. I got myself confused from the fact we return EIO for log device. 
So a bit more cleanup is in order.

However, the big question there is not about how recent update the boot files 
have. To read out the boot files, we need a bit of processing done and to 
understand if those structures are consistent or not. But as I wrote before, 
thats something to be investigated.

rgds,
toomas

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r354283 - in head: stand/libsa/zfs sys/cddl/boot/zfs

[Toomas Soome via svn-src-all]

> On 3. Nov 2019, at 19:42, Andriy Gapon  wrote:
> 
> On 03/11/2019 15:25, Toomas Soome wrote:
>> Author: tsoome
>> Date: Sun Nov  3 13:25:47 2019
>> New Revision: 354283
>> URL: https://svnweb.freebsd.org/changeset/base/354283
>> 
>> Log:
>>  loader: we do not support booting from pool with log device
>> 
>>  If pool has log device, stop there and tell about it.
> 
> Why?
> 

because it is not implemented? We already return EIO from vdev_probe() for LOG 
device. I mean I does not have to stay this way, but also we have some weird 
unexplained error cases… 

anyhow, it is trivial to undo the disable, and definitely open to ideas about 
handling the reading the log.

rgds,
toomas
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r354283 - in head: stand/libsa/zfs sys/cddl/boot/zfs

[Toomas Soome via svn-src-all]

> On 3. Nov 2019, at 22:39, Kevin Bowling  wrote:
> 
> I believe this is/was a common configuration, at least the few
> spinning disk based systems I have left have a slog.
> 

On boot pool? um. well, I’ll kick out that return then.

rgds,
toomas

> On Sun, Nov 3, 2019 at 10:55 AM Andriy Gapon  wrote:
>> 
>> On 03/11/2019 15:25, Toomas Soome wrote:
>>> Author: tsoome
>>> Date: Sun Nov  3 13:25:47 2019
>>> New Revision: 354283
>>> URL: https://svnweb.freebsd.org/changeset/base/354283
>>> 
>>> Log:
>>>  loader: we do not support booting from pool with log device
>>> 
>>>  If pool has log device, stop there and tell about it.
>> 
>> Why?
>> 
>>> Modified:
>>>  head/stand/libsa/zfs/zfs.c
>>>  head/stand/libsa/zfs/zfsimpl.c
>>>  head/sys/cddl/boot/zfs/zfsimpl.h
>> 
>> 
>> 
>> --
>> Andriy Gapon
>> ___
>> svn-src-h...@freebsd.org mailing list
>> https://lists.freebsd.org/mailman/listinfo/svn-src-head
>> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r353341 - head/cddl/contrib/opensolaris/cmd/zpool

[Toomas Soome via svn-src-all]

> On 9. Oct 2019, at 14:34, Andriy Gapon  wrote:
> 
> Author: avg
> Date: Wed Oct  9 11:34:16 2019
> New Revision: 353341
> URL: https://svnweb.freebsd.org/changeset/base/353341
> 
> Log:
>  zfs: document large_dnode feature
> 
>  The text is copied from illumos.
>  The conversion to mdoc is mine.
>  The FreeBSD boot warning is copied from large_block description.


We do support booting with large_dnode enabled:)

r323494 | tsoome | 2017-09-12 16:45:04 +0300 (Tue, 12 Sep 2017) | 7 lines

loader should support large_dnode

rgds,
toomas


> 
>  MFC after:   4 days
> 
> Modified:
>  head/cddl/contrib/opensolaris/cmd/zpool/zpool-features.7
> 
> Modified: head/cddl/contrib/opensolaris/cmd/zpool/zpool-features.7
> ==
> --- head/cddl/contrib/opensolaris/cmd/zpool/zpool-features.7  Wed Oct  9 
> 11:26:36 2019(r353340)
> +++ head/cddl/contrib/opensolaris/cmd/zpool/zpool-features.7  Wed Oct  9 
> 11:34:16 2019(r353341)
> @@ -527,6 +527,36 @@ Please note that booting from datasets that have recor
> supported by the
> .Fx
> boot loader.
> +.It Sy large_dnode
> +.Bl -column "READ\-ONLY COMPATIBLE" "org.zfsonlinux:large_dnode"
> +.It GUID Ta org.zfsonlinux:large_dnode
> +.It READ\-ONLY COMPATIBLE Ta no
> +.It DEPENDENCIES Ta extensible_dataset
> +.El
> +.Pp
> +The
> +.Sy large_dnode
> +feature allows the size of dnodes in a dataset to be set larger than 512B.
> +.Pp
> +This feature becomes
> +.Sy active
> +once a dataset contains an object with a dnode larger than 512B,
> +which occurs as a result of setting the
> +.Sy dnodesize
> +dataset property to a value other than
> +.Sy legacy .
> +The feature will return to being
> +.Sy enabled
> +once all filesystems that have ever contained a dnode larger than 512B are
> +destroyed.
> +Large dnodes allow more data to be stored in the bonus buffer, thus 
> potentially
> +improving performance by avoiding the use of spill blocks.
> +.Pp
> +Please note that booting from datasets that have dnodes larger than 512B is
> +.Em NOT
> +supported by the
> +.Fx
> +boot loader.
> .It Sy sha512
> .Bl -column "READ\-ONLY COMPATIBLE" "org.illumos:sha512"
> .It GUID Ta org.illumos:sha512

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r352421 - head/stand/libsa

[Toomas Soome via svn-src-all]

> On 17 Sep 2019, at 11:56, Konstantin Belousov  wrote:
> 
> On Mon, Sep 16, 2019 at 08:28:09PM +, Toomas Soome wrote:
>> Author: tsoome
>> Date: Mon Sep 16 20:28:08 2019
>> New Revision: 352421
>> URL: https://svnweb.freebsd.org/changeset/base/352421
>> 
>> Log:
>>  loader: Malloc(0) should return NULL.
>> 
>>  We really should not allocate anything with size 0.
> Why ?  This is quite unexpected from other environments, where
> malloc(0) returns unique object.
> 

Hiding bugs. And, we are not getting unique object, we are getting chunk of 
memory with payload length of 0 and the memory content depends on if head and 
tail guards are used - the chunk is with size at least MALLOCALIGN (16 or 64 
bytes) and the chunk is from random location of heap.

rgds,
toomas
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r350444 - head/stand/efi/loader

[Toomas Soome via svn-src-all]

hi!

efihttp is also using network, is it intentionally left untouched?

rgds,
toomas

> On 30 Jul 2019, at 17:54, Ruslan Bukin  wrote:
> 
> Author: br
> Date: Tue Jul 30 14:54:18 2019
> New Revision: 350444
> URL: https://svnweb.freebsd.org/changeset/base/350444
> 
> Log:
>  Fix EFI loader build when LOADER_NET_SUPPORT=no.
> 
>  Sponsored by:DARPA, AFRL
> 
> Modified:
>  head/stand/efi/loader/conf.c
> 
> Modified: head/stand/efi/loader/conf.c
> ==
> --- head/stand/efi/loader/conf.c  Tue Jul 30 14:21:00 2019
> (r350443)
> +++ head/stand/efi/loader/conf.c  Tue Jul 30 14:54:18 2019
> (r350444)
> @@ -40,7 +40,9 @@ struct devsw *devsw[] = {
>   _cddev,
>   _hddev,
>   _dev, /* ordering with efinet_dev matters */
> +#if defined(LOADER_NET_SUPPORT)
>   _dev,
> +#endif
>   _dev,
> #ifdef EFI_ZFS_BOOT
>   _dev,
> @@ -64,7 +66,9 @@ struct fs_ops *file_system[] = {
> };
> 
> struct netif_driver *netif_drivers[] = {
> +#if defined(LOADER_NET_SUPPORT)
>   ,
> +#endif
>   NULL
> };
> 
> 

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r347953 - head/cddl/contrib/opensolaris/cmd/zfs

[Toomas Soome via svn-src-all]

> On 18 May 2019, at 15:37, Alexey Dokuchaev  wrote:
> 
> On Sat, May 18, 2019 at 12:27:22PM +, Allan Jude wrote:
>> New Revision: 347953
>> URL: https://svnweb.freebsd.org/changeset/base/347953
>> 
>> Log:
>>  MFV/ZoL: `zfs userspace` ignored all unresolved UIDs after the first
>> 
>>  zfsonlinux/zfs@88cfff182432e4d1c24c877f33b47ee6cf109eee
>> 
>>  zfs_main: fix `zfs userspace` squashing unresolved entries
>> 
>> ...
>> @@ -2368,10 +2369,12 @@ us_compare(const void *larg, const void *rarg, void 
>> *u
>>  if (rv64 != lv64)
>>  rc = (rv64 < lv64) ? 1 : -1;
>>  } else {
>> -(void) nvlist_lookup_string(lnvl, propname,
>> -);
>> -(void) nvlist_lookup_string(rnvl, propname,
>> -);
>> +if ((nvlist_lookup_string(lnvl, propname,
>> +) == ENOENT) ||
>> +(nvlist_lookup_string(rnvl, propname,
>> +) == ENOENT)) {
>> +goto compare_nums;
>> +}
> 
> Another thing not to like about ZoL: their completely bogus code style
> and formatting practices (look at those ") == ENOENT").  If they
> are going to listen to us, can we at least try to convince them not to
> break existing, much FreeBSD-like formatting?
> 
> ./danfe
> 


Actually it is required to use the proper cstyle… 
https://github.com/zfsonlinux/zfs/blob/master/.github/CONTRIBUTING.md#style-guides
 


Therefore bogus style is bug.

rgds,
toomas

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r344569 - in head/cddl/contrib/opensolaris: cmd/zfs lib/libzfs/common

[Toomas Soome via svn-src-all]

> On 26 Feb 2019, at 23:02, Cy Schubert  wrote:
> 
> In message , Fatih Acar 
> writes:
>> This is a multi-part message in MIME format.
>> --6A54CE5B28D823DCB8C41577
>> Content-Type: text/plain; charset=utf-8
>> Content-Transfer-Encoding: 8bit
>> 
>> On 2/26/19 5:36 PM, Cy Schubert wrote:
>>> On February 26, 2019 8:11:31 AM PST, Baptiste Daroussin  
>> wrote:
 On Tue, Feb 26, 2019 at 05:04:11PM +0100, Baptiste Daroussin wrote:
> On Tue, Feb 26, 2019 at 07:48:27AM -0800, Cy Schubert wrote:
>> On February 26, 2019 12:18:35 AM PST, Baptiste Daroussin
  wrote:
>>> Author: bapt
>>> Date: Tue Feb 26 08:18:34 2019
>>> New Revision: 344569
>>> URL: https://svnweb.freebsd.org/changeset/base/344569
>>> 
>>> Log:
>>> Implement parallel mounting for ZFS filesystem
>>> 
>>> It was first implemented on Illumos and then ported to ZoL.
>>> This patch is a port to FreeBSD of the ZoL version.
>>> This patch also includes a fix for a race condition that was
 amended
>>> 
>>> With such patch Delphix has seen a huge decrease in latency of the
>>> mount phase
>>> (https://github.com/openzfs/openzfs/commit/a3f0e2b569 for
 details).
>>> With that current change Gandi has measured improvments that are
 on par
>>> with
>>> those reported by Delphix.
>>> 
>>> Zol commits incorporated:
>> 
> https://github.com/zfsonlinux/zfs/commit/a10d50f999511d304f910852c7825c70
>> c9c9e303
>> 
> https://github.com/zfsonlinux/zfs/commit/e63ac16d25fbe991a356489c86d40775
>> 67dfea21
>>> 
>>> Reviewed by:avg, sef
>>> Approved by:avg, sef
>>> Obtained from:  ZoL
>>> MFC after:  1 month
>>> Relnotes:   yes
>>> Sponsored by:   Gandi.net
>>> Differential Revision:  https://reviews.freebsd.org/D19098
>>> 
>>> Modified:
>>> head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c
>>> head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h
>>> head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c
>>> head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_impl.h
>>> head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c
>>> 
>>> Modified: head/cddl/contrib/opensolaris/cmd/zfs/zfs_main.c
>> 
> =
>> =
> [...]
>>> 
>> 
>> This broke my systems, many filesystems fail to mount causing
 nullfs late mounts to fail. No details now until tonight.
>> 
>> Suggest we back this out until it is properly tested.
>> 
> 
> What fails to mount? what message? can you provide Gandi folks more
 informations
> so they can fix?
> 
> I will revert if we cannot have a quick fix but let s give them a
 chance to fix
> first.
> 
 With the proper email in CC there is a better chance to reach at them
 :)
 
 Best regards,
 Bapt
>>> 
>>> Sorry about that. I'm terribly frustrated as this broke my mail gateway, ha
>> ving to fix it using juiceSSH on my phone on the bus. Ssh on the phone makes 
>> for a very grumpy Cy.
>>> 
>>> I did bring my personal laptop to work, so I'll try to help out testing thi
>> s at noon here and maybe look at it more. I'll help out any way I can.
>>> 
>>> 
>> 
>> Sorry about all this...
>> Could you try the attached patch, it should fix the issue. I don't
>> understand how this regression happened, it's not present in ZoL...
>> I'll check with Jack who worked on this when he's back from PTO.
>> 
>> Thanks.
>> 
>> -- 
>> Fatih ACAR
>> Gandi
>> fatih.a...@gandi.net
>> 
>> --6A54CE5B28D823DCB8C41577
>> Content-Type: text/x-patch;
>> name="mount.patch"
>> Content-Transfer-Encoding: quoted-printable
>> Content-Disposition: attachment;
>> filename="mount.patch"
>> 
>> Index: cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c
>> (revision 3=
>> 44590)
>> +++ cddl/contrib/opensolaris/lib/libzfs/common/libzfs_mount.c
>> (working co=
>> py)
>> @@ -1260,11 +1260,11 @@
>>  if (*a =3D=3D '\0')
>>  return (-1);
>>  if (*b =3D=3D '\0')
>> -return (-1);
>> +return (1);
>>  if (*a =3D=3D '/')
>>  return (-1);
>>  if (*b =3D=3D '/')
>> -return (-1);
>> +return (1);
>>  return (*a < *b ? -1 : *a > *b);
>>  }
>> =20
>> 
> 
> Thanks. This fixes this particular issue.
> 
> There is one other minor issues that should also be addressed. The mount 
> message
> at boot previously displayed:
> 
> Mounting 

Re: svn commit: r344316 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Toomas Soome via svn-src-all]

> On 20 Feb 2019, at 09:56, Alexey Dokuchaev  wrote:
> 
> On Tue, Feb 19, 2019 at 06:43:28PM -0500, Shawn Webb wrote:
>> At the risk of painting a bikeshed a lovely color of neon purple, I'm
>> curious about if/how these types of commits get merged upstream to
>> (OpenZFS|Illumos|ZFS On Linux|where ever ZFS upstream is now|I'm very
>> confused|is anyone else confused where upstream is?).
>> 
>> Who is upstream? Is work like this going to remain as a downstream
>> patch to ZFS? Or is FreeBSD going to work to upstream this type of
>> work?
> 
> I've always felt that we should've become upstream to everyone else
> the moment we knew Oracle would eat Sun (20 April 2009), and never
> understood why it didn't happen and now, ten years later, we're talking
> about ZFS on fucking Linux becoming our upstream.  Something'd got very
> wrong here and I'd like to know what and why.
> 
>> I hope my curiousity doesn't offend anyone. ;)
> 
> Not at all, I'm also confused and curious.
> 
> ./danfe
> 

The genuine lack of developers and development. If the updates do happen in ZoL 
(for zfs), it only means the developers find it easier to work there.

rgds,
toomas
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r344238 - head/stand/common

[Toomas Soome via svn-src-all]

> On 18 Feb 2019, at 18:25, Warner Losh  wrote:
> 
> 
> 
> On Mon, Feb 18, 2019 at 7:31 AM Ian Lepore  > wrote:
> On Mon, 2019-02-18 at 15:09 +0200, Toomas Soome wrote:
> > > On 18 Feb 2019, at 01:32, Ian Lepore  wrote:
> > > 
> > > Author: ian
> > > Date: Sun Feb 17 23:32:09 2019
> > > New Revision: 344238
> > > URL: https://svnweb.freebsd.org/changeset/base/344238 
> > > 
> > > 
> > > Log:
> > >  Restore loader(8)'s ability for lsdev to show partitions within a bsd 
> > > slice.
> > > 
> > >  I'm pretty sure this used to work at one time, perhaps long ago.  It has
> > >  been failing recently because if you call disk_open() with 
> > > dev->d_partition
> > >  set to -1 when d_slice refers to a bsd slice, it assumes you want it to
> > >  open the first partition within that slice.  When you then pass that open
> > >  dev instance to ptable_open(), it tries to read the start of the 'a'
> > >  partition and decides there is no recognizable partition type there.
> > > 
> > >  This restores the old functionality by resetting d_offset to the start
> > >  of the raw slice after disk_open() returns.  For good measure, 
> > > d_partition
> > >  is also set back to -1, although that doesn't currently affect anything.
> > > 
> > >  I would have preferred to make disk_open() avoid such rude assumptions 
> > > and
> > >  if you ask for partition -1 you get the raw slice.  But the commit 
> > > history
> > >  shows that someone already did that once (r239058), and had to revert it
> > >  (r239232), so I didn't even try to go down that road.
> > 
> > 
> > What was the reason for the revert? I still do think the current
> > disk_open() approach is not good because it does break the promise to
> > get MBR partition (see common/disk.h). 
> > 
> > Of course I can see the appeal for something like “boot disk0:” but
> > case like that should be solved by iterating partition table, and not
> > by making API to do wrong thing - if I did ask to for disk0s0: I
> > really would expect to get disk0s0: and not disk0s0a:
> > 
> > But anyhow, it would be good to understand the actual reasoning
> > behind that decision.
> > 
> 
> I have no idea. As is so often the case, the commit message for the
> revert said what the commit did ("revert to historic behavior") without
> any hint of why the change was made. One has to assume that it broke
> some working cases and people complained.
> 
> Part of the problem for the disk_open() "api" is that there is not even
> a comment block with some hints in it. I was thinking one potential
> solution might instead of using "if (partition < 0)" we could define a
> couple magical partition number values, PNUM_GETBEST = -1,
> PNUM_RAWSLICE = -2, that sort of thing. But it would require carefully
> combing through the existing code looking at all calls to disk_open()
> and all usage of disk_devdesc.d_partition.
> 
> I think that we should fix the disk_open() api. And then fix everything that 
> uses it to comply with the new rules. The current hodge-podge that we have 
> causes a number of issues for different environments that are only mostly 
> worked around. I've done some work in this area to make things more regular, 
> but it's still a dog's breakfast of almost-stackable devices that we overload 
> too many things on, resulting in the mis-mash we have today. When the whole 
> world was just MBR + BSD label, we could get away with it. But now that we 
> have GPT as well as GELI support and ZFS pool devices on top of disk devices, 
> the arrangements aren't working as well as could be desired.
> 
> Warner


I am looking on it too, but it has more traps than I was suspecting:)

rgds,
toomas

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r344238 - head/stand/common

[Toomas Soome via svn-src-all]

> On 18 Feb 2019, at 01:32, Ian Lepore  wrote:
> 
> Author: ian
> Date: Sun Feb 17 23:32:09 2019
> New Revision: 344238
> URL: https://svnweb.freebsd.org/changeset/base/344238
> 
> Log:
>  Restore loader(8)'s ability for lsdev to show partitions within a bsd slice.
> 
>  I'm pretty sure this used to work at one time, perhaps long ago.  It has
>  been failing recently because if you call disk_open() with dev->d_partition
>  set to -1 when d_slice refers to a bsd slice, it assumes you want it to
>  open the first partition within that slice.  When you then pass that open
>  dev instance to ptable_open(), it tries to read the start of the 'a'
>  partition and decides there is no recognizable partition type there.
> 
>  This restores the old functionality by resetting d_offset to the start
>  of the raw slice after disk_open() returns.  For good measure, d_partition
>  is also set back to -1, although that doesn't currently affect anything.
> 
>  I would have preferred to make disk_open() avoid such rude assumptions and
>  if you ask for partition -1 you get the raw slice.  But the commit history
>  shows that someone already did that once (r239058), and had to revert it
>  (r239232), so I didn't even try to go down that road.


What was the reason for the revert? I still do think the current disk_open() 
approach is not good because it does break the promise to get MBR partition 
(see common/disk.h). 

Of course I can see the appeal for something like “boot disk0:” but case like 
that should be solved by iterating partition table, and not by making API to do 
wrong thing - if I did ask to for disk0s0: I really would expect to get 
disk0s0: and not disk0s0a:

But anyhow, it would be good to understand the actual reasoning behind that 
decision.

rgds,
toomas

> 
> Modified:
>  head/stand/common/disk.c
> 
> Modified: head/stand/common/disk.c
> ==
> --- head/stand/common/disk.c  Sun Feb 17 20:25:07 2019(r344237)
> +++ head/stand/common/disk.c  Sun Feb 17 23:32:09 2019(r344238)
> @@ -133,6 +133,13 @@ ptable_print(void *arg, const char *pname, const struc
>   dev.d_partition = -1;
>   if (disk_open(, part->end - part->start + 1,
>   od->sectorsize) == 0) {
> + /*
> +  * disk_open() for partition -1 on a bsd slice assumes
> +  * you want the first bsd partition.  Reset things so
> +  * that we're looking at the start of the raw slice.
> +  */
> + dev.d_partition = -1;
> + dev.d_offset = part->start;
>   table = ptable_open(, part->end - part->start + 1,
>   od->sectorsize, ptblread);
>   if (table != NULL) {
> 

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r339959 - head/stand/i386/libi386

[Toomas Soome via svn-src-all]

I forgot to note imp, sorry…

rgds,
toomas

> On 31 Oct 2018, at 18:42, Toomas Soome  wrote:
> 
> Author: tsoome
> Date: Wed Oct 31 16:42:40 2018
> New Revision: 339959
> URL: https://svnweb.freebsd.org/changeset/base/339959
> 
> Log:
>  loader: issue edd probe before legacy ah=08 and detect no media
> 
>  while probing for drives, use int13 extended info before standard one and
>  provide workaround for case we are not getting needed information in case
>  of floppy drive.
> 
>  In case of INT13 errors, there are (at least) 3 error codes appearing in case
>  of missin media - 20h, 31h and 80h. Flag the no media and do not print an
>  error.
> 
>  Differential Revision:   https://reviews.freebsd.org/D17667
> 
> Modified:
>  head/stand/i386/libi386/biosdisk.c
> 
> Modified: head/stand/i386/libi386/biosdisk.c
> ==
> --- head/stand/i386/libi386/biosdisk.cWed Oct 31 16:17:45 2018
> (r339958)
> +++ head/stand/i386/libi386/biosdisk.cWed Oct 31 16:42:40 2018
> (r339959)
> @@ -43,7 +43,6 @@ __FBSDID("$FreeBSD$");
> #include 
> #include 
> #include 
> -#include 
> 
> #include 
> #include 
> @@ -81,8 +80,10 @@ static struct bdinfo
> #define   BD_MODEINT130x
> #define   BD_MODEEDD1 0x0001
> #define   BD_MODEEDD3 0x0002
> +#define  BD_MODEEDD  (BD_MODEEDD1 | BD_MODEEDD3)
> #define   BD_MODEMASK 0x0003
> #define   BD_FLOPPY   0x0004
> +#define  BD_NO_MEDIA 0x0008
>   int bd_type;/* BIOS 'drive type' (floppy only) */
>   uint16_tbd_sectorsize;  /* Sector size */
>   uint64_tbd_sectors; /* Disk size */
> @@ -188,60 +189,83 @@ bd_init(void)
> }
> 
> /*
> - * Try to detect a device supported by the legacy int13 BIOS
> + * Return EDD version or 0 if EDD is not supported on this drive.
>  */
> static int
> -bd_int13probe(struct bdinfo *bd)
> +bd_check_extensions(int unit)
> {
> - struct edd_params params;
> - int ret = 1;/* assume success */
> + /* Determine if we can use EDD with this device. */
> + v86.ctl = V86_FLAGS;
> + v86.addr = 0x13;
> + v86.eax = 0x4100;
> + v86.edx = unit;
> + v86.ebx = 0x55aa;
> + v86int();
> 
> + if (V86_CY(v86.efl) ||  /* carry set */
> + (v86.ebx & 0x) != 0xaa55)   /* signature */
> + return (0);
> +
> + /* extended disk access functions (AH=42h-44h,47h,48h) supported */
> + if ((v86.ecx & EDD_INTERFACE_FIXED_DISK) == 0)
> + return (0);
> +
> + return ((v86.eax >> 8) & 0xff);
> +}
> +
> +static void
> +bd_reset_disk(int unit)
> +{
> + /* reset disk */
>   v86.ctl = V86_FLAGS;
>   v86.addr = 0x13;
> + v86.eax = 0;
> + v86.edx = unit;
> + v86int();
> +}
> +
> +/*
> + * Read CHS info. Return 0 on success, error otherwise.
> + */
> +static int
> +bd_get_diskinfo_std(struct bdinfo *bd)
> +{
> + bzero(, sizeof(v86));
> + v86.ctl = V86_FLAGS;
> + v86.addr = 0x13;
>   v86.eax = 0x800;
>   v86.edx = bd->bd_unit;
>   v86int();
> 
> - /* Don't error out if we get bad sector number, try EDD as well */
> - if (V86_CY(v86.efl) ||  /* carry set */
> - (v86.edx & 0xff) <= (unsigned)(bd->bd_unit & 0x7f)) /* unit # bad */
> - return (0); /* skip device */
> + if (V86_CY(v86.efl) && ((v86.eax & 0xff00) != 0))
> + return ((v86.eax & 0xff00) >> 8);
> 
> - if ((v86.ecx & 0x3f) == 0)  /* absurd sector number */
> - ret = 0;/* set error */
> + /* return custom error on absurd sector number */
> + if ((v86.ecx & 0x3f) == 0)
> + return (0x60);
> 
> - /* Convert max cyl # -> # of cylinders */
>   bd->bd_cyl = ((v86.ecx & 0xc0) << 2) + ((v86.ecx & 0xff00) >> 8) + 1;
>   /* Convert max head # -> # of heads */
>   bd->bd_hds = ((v86.edx & 0xff00) >> 8) + 1;
>   bd->bd_sec = v86.ecx & 0x3f;
> - bd->bd_type = v86.ebx & 0xff;
> - bd->bd_flags |= BD_MODEINT13;
> + bd->bd_type = v86.ebx;
> + bd->bd_sectors = (uint64_t)bd->bd_cyl * bd->bd_hds * bd->bd_sec;
> 
> - /* Calculate sectors count from the geometry */
> - bd->bd_sectors = bd->bd_cyl * bd->bd_hds * bd->bd_sec;
> - bd->bd_sectorsize = BIOSDISK_SECSIZE;
> - DEBUG("unit 0x%x geometry %d/%d/%d", bd->bd_unit, bd->bd_cyl,
> - bd->bd_hds, bd->bd_sec);
> + return (0);
> +}
> 
> - /* Determine if we can use EDD with this device. */
> - v86.ctl = V86_FLAGS;
> - v86.addr = 0x13;
> - v86.eax = 0x4100;
> - v86.edx = bd->bd_unit;
> - v86.ebx = 0x55aa;
> - v86int();
> - if (V86_CY(v86.efl) ||  /* carry set */
> - (v86.ebx & 0x) != 0xaa55 || /* signature */
> - (v86.ecx & EDD_INTERFACE_FIXED_DISK) == 0)
> - return (ret);   /* return code from int13 AH=08 */
> +/*
> + * Read EDD info. Return 

Re: svn commit: r339673 - head/stand/libsa

[Toomas Soome via svn-src-all]

> On 24 Oct 2018, at 02:11, Konstantin Belousov  wrote:
> 
> Author: kib
> Date: Tue Oct 23 23:11:38 2018
> New Revision: 339673
> URL: https://svnweb.freebsd.org/changeset/base/339673
> 
> Log:
>  Fix stand/ build after r339671.
> 
>  ffs_subr.c requires calculate_crc32c() from libkern.  Unfortunately we
>  cannot just add libkern/crc32.c to libstand because crc32.o is already
>  compiled from contrib/zlib/crc32.c. Use the include trick to rename
>  the source.
> 
>  Note that libstand also provides crc32.c which seems to be unused.

It is used by part.c, by the GPT functions. However, if we have non-optional 
compile case for crc32, it is not hard to update part.c and drop the unused 
instance.

rgds,
toomas

> 
>  Reviewed by: imp
>  Sponsored by:The FreeBSD Foundation
>  Differential revision:   https://reviews.freebsd.org/D17677
> 
> Added:
>  head/stand/libsa/crc32_libkern.c   (contents, props changed)
> Modified:
>  head/stand/libsa/Makefile
> 
> Modified: head/stand/libsa/Makefile
> ==
> --- head/stand/libsa/Makefile Tue Oct 23 21:43:41 2018(r339672)
> +++ head/stand/libsa/Makefile Tue Oct 23 23:11:38 2018(r339673)
> @@ -155,9 +155,9 @@ SRCS+=ffs_subr.c ffs_tables.c
> 
> CFLAGS.bzipfs.c+= -I${SRCTOP}/contrib/bzip2
> 
> -# explicit_bzero
> +# explicit_bzero and calculate_crc32c
> .PATH: ${SYSDIR}/libkern
> -SRCS+=  explicit_bzero.c
> +SRCS+=  explicit_bzero.c crc32_libkern.c
> 
> # Maybe GELI
> .if ${MK_LOADER_GELI} == "yes"
> 
> Added: head/stand/libsa/crc32_libkern.c
> ==
> --- /dev/null 00:00:00 1970   (empty, because file is newly added)
> +++ head/stand/libsa/crc32_libkern.c  Tue Oct 23 23:11:38 2018
> (r339673)
> @@ -0,0 +1,3 @@
> +/* $FreeBSD$ */
> +
> +#include "../../sys/libkern/crc32.c"
> 

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r337878 - head/stand/i386/libi386

[Toomas Soome via svn-src-all]

> On 16 Aug 2018, at 16:03, Warner Losh  wrote:
> 
> 
> 
> On Thu, Aug 16, 2018 at 1:10 AM, Toomas Soome  > wrote:
> 
> 
> > On 16 Aug 2018, at 09:59, John Baldwin  wrote:
> > 
> > On 8/15/18 11:59 PM, Warner Losh wrote:
> >> 
> >> 
> >> On Wed, Aug 15, 2018 at 4:28 PM, Ian Lepore  >>   >> >> wrote:
> >> 
> >>On Wed, 2018-08-15 at 22:25 +, Toomas Soome wrote:
> >>> Author: tsoome
> >>> Date: Wed Aug 15 22:25:05 2018
> >>> New Revision: 337878
> >>> URL: https://svnweb.freebsd.org/changeset/base/337878 
> >>>  
> >>>  >>> >
> >>> 
> >>> Log:
> >>>   libi386: remove bd_read() and bd_write() wrappers
> >>>   
> >>>   Those wroappers are nice, but do not really add much value.
> >>> 
> >>> Modified:
> >>>   head/stand/i386/libi386/biosdisk.c
> >>> 
> >>> Modified: head/stand/i386/libi386/biosdisk.c
> >>> =
> >>> =
> >>> --- head/stand/i386/libi386/biosdisk.cWed Aug 15 21:47:03
> >>> 2018  (r337877)
> >>> +++ head/stand/i386/libi386/biosdisk.cWed Aug 15 22:25:05
> >>> 2018  (r337878)
> >>> @@ -94,10 +94,7 @@ static int nbdinfo = 0;
> >>>  
> >>>  static void bd_io_workaround(struct disk_devdesc *dev);
> >>>  
> >>> -static int bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks,
> >>> -caddr_t dest);
> >>> -static int bd_write(struct disk_devdesc *dev, daddr_t dblk, int
> >>> blks,
> >>> -caddr_t dest);
> >>> +static int bd_io(struct disk_devdesc *, daddr_t, int, caddr_t, int);
> >>>  static int bd_int13probe(struct bdinfo *bd);
> >>>  
> >>>  static int bd_init(void);
> >>> @@ -506,7 +503,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t
> >>> dblk, s
> >>>   case F_READ:
> >>>   DEBUG("read %d from %lld to %p", blks, dblk, buf);
> >>>  
> >>> - if (blks && (rc = bd_read(dev, dblk, blks, buf))) {
> >>> + if (blks && (rc = bd_io(dev, dblk, blks, buf, 0))) {
> >>>   /* Filter out floppy controller errors */
> >>>   if (BD(dev).bd_flags != BD_FLOPPY || rc !=
> >>> 0x20) {
> >>>   printf("read %d from %lld to %p,
> >>> error: 0x%x\n",
> >>> @@ -518,7 +515,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t
> >>> dblk, s
> >>>   case F_WRITE :
> >>>   DEBUG("write %d from %lld to %p", blks, dblk, buf);
> >>>  
> >>> - if (blks && bd_write(dev, dblk, blks, buf)) {
> >>> + if (blks && bd_io(dev, dblk, blks, buf, 1)) {
> >>>   DEBUG("write error");
> >>>   return (EIO);
> >>>   }
> >>> @@ -713,20 +710,6 @@ bd_io(struct disk_devdesc *dev, daddr_t dblk,
> >>> int blks
> >>>   }
> >>>  
> >>>   return (0);
> >>> -}
> >>> -
> >>> -static int
> >>> -bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t
> >>> dest)
> >>> -{
> >>> -
> >>> - return (bd_io(dev, dblk, blks, dest, 0));
> >>> -}
> >>> -
> >>> -static int
> >>> -bd_write(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t
> >>> dest)
> >>> -{
> >>> -
> >>> - return (bd_io(dev, dblk, blks, dest, 1));
> >>>  }
> >>>  
> >>>  /*
> >>> 
> >> 
> >>This would be a more satisfying change if there were something like
> >> 
> >> #define BD_RD 0
> >> #define BD_WR 1
> >> 
> >>so that it was clear at a glance what a bd_io() call is doing.
> >> 
> >> 
> >> I think that's a good idea...
> > 
> > Arguably the bd_read/write wrappers were even clearer (and there purpose
> > was readability in that case).
> 
> Yes thats true, but also will leave us in mercy of inlining etc.. anyhow, 
> *my* purpose is to get the line of changes done (to be able to perform IO 
> with >512 sector size, merge with bioscd.c and split up floppy, cd and hdd 
> cases, so the user can distinguish the devices.
> 
> OK, Now I'm confused. All this sounds like regression, apart from the >512 
> sector thing.
> 
> Warner

Oh sorry, it was bad wording I think - the split up in sense that currently the 
BIOS loader can not distinguish floppy from disk, so the user has to guess if 
diskX is floppy or disk -  so I want to make the device list for user very 
clear (fdX:, cdX:, diskX: as its already done in EFI version). The code should 
be consolidated (biosdisk + bioscd).

rgds,
toomas

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r337878 - head/stand/i386/libi386

[Toomas Soome via svn-src-all]

> On 16 Aug 2018, at 09:59, John Baldwin  wrote:
> 
> On 8/15/18 11:59 PM, Warner Losh wrote:
>> 
>> 
>> On Wed, Aug 15, 2018 at 4:28 PM, Ian Lepore > > wrote:
>> 
>>On Wed, 2018-08-15 at 22:25 +, Toomas Soome wrote:
>>> Author: tsoome
>>> Date: Wed Aug 15 22:25:05 2018
>>> New Revision: 337878
>>> URL: https://svnweb.freebsd.org/changeset/base/337878 
>>> 
>>> 
>>> Log:
>>>   libi386: remove bd_read() and bd_write() wrappers
>>>   
>>>   Those wroappers are nice, but do not really add much value.
>>> 
>>> Modified:
>>>   head/stand/i386/libi386/biosdisk.c
>>> 
>>> Modified: head/stand/i386/libi386/biosdisk.c
>>> =
>>> =
>>> --- head/stand/i386/libi386/biosdisk.cWed Aug 15 21:47:03
>>> 2018  (r337877)
>>> +++ head/stand/i386/libi386/biosdisk.cWed Aug 15 22:25:05
>>> 2018  (r337878)
>>> @@ -94,10 +94,7 @@ static int nbdinfo = 0;
>>>  
>>>  static void bd_io_workaround(struct disk_devdesc *dev);
>>>  
>>> -static int bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks,
>>> -caddr_t dest);
>>> -static int bd_write(struct disk_devdesc *dev, daddr_t dblk, int
>>> blks,
>>> -caddr_t dest);
>>> +static int bd_io(struct disk_devdesc *, daddr_t, int, caddr_t, int);
>>>  static int bd_int13probe(struct bdinfo *bd);
>>>  
>>>  static int bd_init(void);
>>> @@ -506,7 +503,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t
>>> dblk, s
>>>   case F_READ:
>>>   DEBUG("read %d from %lld to %p", blks, dblk, buf);
>>>  
>>> - if (blks && (rc = bd_read(dev, dblk, blks, buf))) {
>>> + if (blks && (rc = bd_io(dev, dblk, blks, buf, 0))) {
>>>   /* Filter out floppy controller errors */
>>>   if (BD(dev).bd_flags != BD_FLOPPY || rc !=
>>> 0x20) {
>>>   printf("read %d from %lld to %p,
>>> error: 0x%x\n",
>>> @@ -518,7 +515,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t
>>> dblk, s
>>>   case F_WRITE :
>>>   DEBUG("write %d from %lld to %p", blks, dblk, buf);
>>>  
>>> - if (blks && bd_write(dev, dblk, blks, buf)) {
>>> + if (blks && bd_io(dev, dblk, blks, buf, 1)) {
>>>   DEBUG("write error");
>>>   return (EIO);
>>>   }
>>> @@ -713,20 +710,6 @@ bd_io(struct disk_devdesc *dev, daddr_t dblk,
>>> int blks
>>>   }
>>>  
>>>   return (0);
>>> -}
>>> -
>>> -static int
>>> -bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t
>>> dest)
>>> -{
>>> -
>>> - return (bd_io(dev, dblk, blks, dest, 0));
>>> -}
>>> -
>>> -static int
>>> -bd_write(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t
>>> dest)
>>> -{
>>> -
>>> - return (bd_io(dev, dblk, blks, dest, 1));
>>>  }
>>>  
>>>  /*
>>> 
>> 
>>This would be a more satisfying change if there were something like
>> 
>> #define BD_RD 0
>> #define BD_WR 1
>> 
>>so that it was clear at a glance what a bd_io() call is doing.
>> 
>> 
>> I think that's a good idea...
> 
> Arguably the bd_read/write wrappers were even clearer (and there purpose
> was readability in that case).

Yes thats true, but also will leave us in mercy of inlining etc.. anyhow, *my* 
purpose is to get the line of changes done (to be able to perform IO with >512 
sector size, merge with bioscd.c and split up floppy, cd and hdd cases, so the 
user can distinguish the devices. 

rgds,
toomas

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r337271 - head/stand/i386/libi386

[Toomas Soome via svn-src-all]

> On 4 Aug 2018, at 11:54, Xin Li  wrote:
> 
> Hi, Cy,
> 
> On 8/3/18 12:11, Cy Schubert wrote:
>> Author: cy
>> Date: Fri Aug  3 19:11:00 2018
>> New Revision: 337271
>> URL: https://svnweb.freebsd.org/changeset/base/337271
>> 
>> Log:
>>  Some drives report a geometry that is inconsisetent with the total
>>  number of sectors reported through the BIOS. Cylinders * heads *
>>  sectors may not necessarily be equal to the total number of sectors
>>  reported through int13h function 48h.
>> 
>>  An example of this is when a Mediasonic HD3-U2B PATA to USB enclosure
>>  with a 80 GB disk is attached. Loader hangs at line 506 of
>>  stand/i386/libi386/biosdisk.c while attempting to read sectors beyond
>>  the end of the disk, sector 156906855. I discovered that the Mediasonic
>>  enclosure was reporting the disk with 9767 cylinders, 255 heads, 63
>>  sectors/track. That's 156906855 sectors. However camcontrol and
>>  Windows 10 both report report the disk having 156301488 sectors, not
>>  the calculated value. At line 280 biosdisk.c sets the sectors to the
>>  higher of either bd->bd_sectors or the total calculated at line 276
>>  (156906855) instead of the lower and correct value of 156301488 reported
>>  by int 13h 48h.
>> 
>>  This was tested on all three of my Mediasonic HD3-U2B PATA to USB
>>  enclosures.
>> 
>>  Instead of using the higher of bd_sectors (returned by int13h) or the
>>  calculated value, this patch uses the lower and safer of the values.
>> 
>>  Reviewed by:tsoome@
>>  Differential Revision:  https://reviews.freebsd.org/D16577
>> 
>> Modified:
>>  head/stand/i386/libi386/biosdisk.c
>> 
>> Modified: head/stand/i386/libi386/biosdisk.c
>> ==
>> --- head/stand/i386/libi386/biosdisk.c   Fri Aug  3 18:52:51 2018
>> (r337270)
>> +++ head/stand/i386/libi386/biosdisk.c   Fri Aug  3 19:11:00 2018
>> (r337271)
>> @@ -275,7 +275,7 @@ bd_int13probe(struct bdinfo *bd)
>> 
>>  total = (uint64_t)params.cylinders *
>>  params.heads * params.sectors_per_track;
>> -if (bd->bd_sectors < total)
>> +if (bd->bd_sectors > total)
>>  bd->bd_sectors = total;
>> 
>>  ret = 1;
>> 
> 
> This broke loader on my system, but I think your reasoning was valid so
> I took a deeper look and discovered that on my system, INT 13h, function
> 48h would give zeros in EDD parameters' CHS fields.  With that, the
> calculated CHS based total would be 0, and your change would cause
> bd_sectors be zeroed.
> 
> Could you please let me know if https://reviews.freebsd.org/D16588 makes
> sense to you?  (I'm not 100% certain if I have followed the code).  It
> allowed my Asrock C2750D4I based board to boot from ZFS.
> 

I have in mind something a bit different for some time, but haven’t had chance 
to complete it because I have no “weird” systems to validate the idea:D I’ll 
try to get a bit of time and post an phabricator soon.

rgds,
toomas


___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r337231 - head/stand/efi/loader

[Toomas Soome via svn-src-all]

> On 3 Aug 2018, at 11:32, Warner Losh  wrote:
> 
> Any reason why efipart_inithandles() should even return an error in this 
> case? Seems to cause us nothing but trouble so we have to filter some, but 
> not all errors which strikes me as a bad design. We should only return errors 
> for real errors, like not having enough memory for the arrays we need.
> 
> Warner

The current code does return the efi error translated to errno and does leave 
the filtering/notification for caller. I guess it is good idea to filter cases 
like no devices in-place, however, the fun is with out of memory — at that 
stage (we even haven't started to probe for disks, so we do not have even 
interpreter) it basically means loader.efi failure.

rgds,
toomas


> 
> On Fri, Aug 3, 2018 at 1:59 AM, Toomas Soome  > wrote:
> Author: tsoome
> Date: Fri Aug  3 07:59:29 2018
> New Revision: 337231
> URL: https://svnweb.freebsd.org/changeset/base/337231 
> 
> 
> Log:
>   loader.efi: clean up misleading noise from missing block devices
> 
>   If there are no block devices, there is no need to printout
>   error (ENOENT).
> 
>   In case of netboot, our image path has no block device, no need to make
>   noise about it.
> 
> Modified:
>   head/stand/efi/loader/main.c
> 
> Modified: head/stand/efi/loader/main.c
> ==
> --- head/stand/efi/loader/main.cFri Aug  3 02:51:37 2018
> (r337230)
> +++ head/stand/efi/loader/main.cFri Aug  3 07:59:29 2018
> (r337231)
> @@ -545,8 +545,6 @@ find_currdev(EFI_LOADED_IMAGE *img, bool do_bootmgr, b
> return (0);
> }
> }
> -   } else {
> -   printf("Can't find device by handle\n");
> }
> 
> /*
> @@ -862,9 +860,9 @@ main(int argc, CHAR16 *argv[])
>  * march through the device switch probing for things.
>  */
> i = efipart_inithandles();
> -   if (i != 0) {
> +   if (i != 0 && i != ENOENT) {
> printf("efipart_inithandles failed with ERRNO %d, expect "
> -   "failures", i);
> +   "failures\n", i);
> }
> 
> for (i = 0; devsw[i] != NULL; i++)
> 
> 

___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r336532 - in head/stand: . common i386 i386/loader i386/zfsboot i386/zfsloader sparc64 sparc64/loader sparc64/zfsloader

[Toomas Soome via svn-src-all]

> On 20 Jul 2018, at 11:00, Andriy Gapon  wrote:
> 
> On 20/07/2018 08:17, Warner Losh wrote:
>> Provide a symbolic link from zfsloader
>>  to loader so people who have not upgraded their boot blocks are not
>>  affected
> 
> I am not sure that ZFS boot blocks can handle symbolic links...
> I seem to recall that not a long time ago they lacked that ability.
> 

I did implement it. Just need to be sure if its backported when needed.

https://svnweb.freebsd.org/base?view=revision=r314112

rgds,
toomas
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Re: svn commit: r336017 - in head: include/rpcsvc lib/libutil libexec/rpc.rquotad sys/cddl/contrib/opensolaris/uts/common/fs/zfs usr.bin/quota

[Toomas Soome via svn-src-all]

Hi!

Unfortunately this patch is not quite correct regarding how you define the RPC 
interfaces.

The versioning in RPC is to specify the interface, much like versioning in 
shared library. 

If you have RPC program version 1, exposing functions get_quota() and 
get_active_quota(), and you want to expose new function, you should introduce 
new version.

The best example about this logic is in mount.x - there you can see how they 
have done 2 versions of the interface, expanding the list of exported functions.

The problem is, to create connection with server, you specify  pair, and if supported, you will get the corresponding handle. Now you 
can have situation with mixed new and old interfaces (sharing the same version 
number) but the exported functions are not the same.

rgds,
toomas

> On 6 Jul 2018, at 01:56, Sean Eric Fagan  wrote:
> 
> Author: sef
> Date: Thu Jul  5 22:56:13 2018
> New Revision: 336017
> URL: https://svnweb.freebsd.org/changeset/base/336017
> 
> Log:
>  This exposes ZFS user and group quotas via the normal
>  quatactl(2) mechanism.  (Read-only at this point, however.)
>  In particular, this is to allow rpc.rquotad query quotas
>  for NFS mounts, allowing users to see their quotas on the
>  hosts using the datasets.
> 
>  The changes specifically:
> 
>  * Add new RPC entry points for querying quotas.
>  * Changes the library routines to allow non-UFS quotas.
>  * Changes rquotad to check for quotas on mounted filesystems,
>  rather than being limited to entries in /etc/fstab
>  * Lastly, adds a VFS entry-point for ZFS to query quotas.
> 
>  Note that this makes one unavoidable behavioural change: if quotas
>  are enabled, then they can be queried, as opposed to the current
>  method of checking for quotas being specified in fstab.  (With
>  ZFS, if there are user or group quotas, they're used, always.)
> 
>  Reviewed by: delphij, mav
>  Approved by: mav
>  Sponsored by:iXsystems Inc
>  Differential Revision:   https://reviews.freebsd.org/D15886
> 
> Modified:
>  head/include/rpcsvc/rquota.x
>  head/lib/libutil/quotafile.c
>  head/libexec/rpc.rquotad/rquotad.c
>  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
>  head/usr.bin/quota/quota.c
> 
> Modified: head/include/rpcsvc/rquota.x
> ==
> --- head/include/rpcsvc/rquota.x  Thu Jul  5 21:38:54 2018
> (r336016)
> +++ head/include/rpcsvc/rquota.x  Thu Jul  5 22:56:13 2018
> (r336017)
> @@ -1,24 +1,55 @@
> +/* @(#)rquota.x  2.1 88/08/01 4.0 RPCSRC */
> +/* @(#)rquota.x 1.2 87/09/20 Copyr 1987 Sun Micro */
> +
> /*
>  * Remote quota protocol
>  * Requires unix authentication
>  */
> 
> #ifndef RPC_HDR
> -%#ifndef lint
> -%/*static char sccsid[] = "from: @(#)rquota.x 1.2 87/09/20 Copyr 1987 Sun 
> Micro";*/
> -%/*static char sccsid[] = "from: @(#)rquota.x2.1 88/08/01 4.0 
> RPCSRC";*/
> -%#endif /* not lint */
> %#include 
> %__FBSDID("$FreeBSD$");
> #endif
> 
> const RQ_PATHLEN = 1024;
> 
> +struct sq_dqblk {
> + unsigned int rq_bhardlimit; /* absolute limit on disk blks alloc */
> + unsigned int rq_bsoftlimit; /* preferred limit on disk blks */
> + unsigned int rq_curblocks;  /* current block count */
> + unsigned int rq_fhardlimit; /* absolute limit on allocated files */
> + unsigned int rq_fsoftlimit; /* preferred file limit */
> + unsigned int rq_curfiles;   /* current # allocated files */
> + unsigned int rq_btimeleft;  /* time left for excessive disk use */
> + unsigned int rq_ftimeleft;  /* time left for excessive files */
> +};
> +
> struct getquota_args {
>   string gqa_pathp;   /* path to filesystem of interest */
> - int gqa_uid;/* inquire about quota for uid */
> + int gqa_uid;/* Inquire about quota for uid */
> };
> 
> +struct setquota_args {
> + int sqa_qcmd;
> + string sqa_pathp;   /* path to filesystem of interest */
> + int sqa_id; /* Set quota for uid */
> + sq_dqblk sqa_dqblk;
> +};
> +
> +struct ext_getquota_args {
> + string gqa_pathp;   /* path to filesystem of interest */
> + int gqa_type;   /* Type of quota info is needed about */
> + int gqa_id; /* Inquire about quota for id */
> +};
> +
> +struct ext_setquota_args {
> + int sqa_qcmd;
> + string sqa_pathp;   /* path to filesystem of interest */
> + int sqa_id; /* Set quota for id */
> + int sqa_type;   /* Type of quota to set */
> + sq_dqblk sqa_dqblk;
> +};
> +
> /*
>  * remote quota structure
>  */
> @@ -37,7 +68,7 @@ struct rquota {
> 
> enum gqr_status {
>   Q_OK = 1,   /* quota returned */
> - Q_NOQUOTA = 2,  /* noquota for uid */
> + Q_NOQUOTA = 2,  /* noquota for uid */
>   Q_EPERM = 3 /* no permission to access