From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:60255)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1d7O6Q-0007Hg-2G
	for guix-patches@gnu.org; Sun, 07 May 2017 11:28:07 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1d7O6M-0006Tj-U1
	for guix-patches@gnu.org; Sun, 07 May 2017 11:28:06 -0400
Received: from debbugs.gnu.org ([208.118.235.43]:34051)
	by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
	(Exim 4.71) (envelope-from <Debian-debbugs@debbugs.gnu.org>)
	id 1d7O6M-0006Tb-Qz
	for guix-patches@gnu.org; Sun, 07 May 2017 11:28:02 -0400
Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2)
	(envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1d7O6M-0008SI-GI
	for guix-patches@gnu.org; Sun, 07 May 2017 11:28:02 -0400
Subject: bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.
Resent-Message-ID: <handler.26815.B26815.149417082332440@debbugs.gnu.org>
Date: Sun, 7 May 2017 17:26:56 +0200
From: Danny Milosavljevic <dannym@scratchpost.org>
Message-ID: <20170507172656.55edb3e9@scratchpost.org>
In-Reply-To: <20170507143647.21036-2-mbakke@fastmail.com>
References: <20170507143647.21036-1-mbakke@fastmail.com>
	<20170507143647.21036-2-mbakke@fastmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
List-Id: <guix-patches.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-patches>,
	<mailto:guix-patches-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-patches/>
List-Post: <mailto:guix-patches@gnu.org>
List-Help: <mailto:guix-patches-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-patches>,
	<mailto:guix-patches-request@gnu.org?subject=subscribe>
Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org
Sender: "Guix-patches" <guix-patches-bounces+kyle=kyleam.com@gnu.org>
To: Marius Bakke <mbakke@fastmail.com>
Cc: 26815@debbugs.gnu.org

Hi Marius,

On Sun,  7 May 2017 16:36:46 +0200
Marius Bakke <mbakke@fastmail.com> wrote:
> +(define* (create-ext-file-system partition type
> +                                 #:key label)
> +  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
> +use that as the volume name."
> +  (format #t "creating ~a partition...\n" type)
> +  (apply system* (string-append "mkfs." type)
> +         "-F" partition
> +         (if label
> +             `("-L" ,label)
> +             '())))

Could you document that the result of the procedure is the system* status?  Also, is that wise?  I think it should instead do the error handling and (error ...) out on error.  It's longer but less surprising.

> +(define* (create-fat32-file-system partition
> +                                   #:key label)
> +  "Create a FAT32 filesystem on PARTITION, which must be at least 32 MiB long.
> +If LABEL is true, use that as volume name."

Same as above.

>  (define* (format-partition partition type
[...]
> +  (define format-procedure
> +    (cond
> +     ((string-prefix? "ext" type)
> +      (create-ext-file-system partition type #:label label))
> +     ((string-suffix? "fat" type)
> +      (create-fat32-file-system partition #:label label))
> +     (else #f)))

"format-procedure" is not actually the procedure, right? It's already the formatting-status ...

> +  (if format-procedure
> +      (match (status:exit-val format-procedure)
> +        (0 #t)
> +        (_ (error "Formatting partition failed.")))
> +      (error "Unsupported file system.")))

status:exit-val will fail when given #f, which it will get in the error case of format-procedure.

scheme@(guile-user)> (status:exit-val #f)
ERROR: In procedure status:exit-val:
ERROR: Wrong type (expecting exact integer): #f

> +      "nls_iso8859-1"                            ;for `mkfs.fat`, et.al

This adds nls_iso8859-1 unconditionally. OK.

Otherwise LGTM.