diff mbox

[2/3] Switch to bit-flags based read-only drive option implementation

Message ID 4B4F2B32.6030704@redhat.com
State New
Headers show

Commit Message

Naphtali Sprei Jan. 14, 2010, 2:33 p.m. UTC 
Clean-up a little bit the RW related bits of BDRV_O_FLAGS.
BDRV_O_RDONLY gone (and so is BDRV_O_ACCESS). Default value for bdrv_flags (0/zero) is READ-ONLY.
Need to explicitly request READ-WRITE.

Instead of using the field 'readonly' of the BlockDriverState struct for passing the request,
pass the request in the flags parameter to the function.

Signed-off-by: Naphtali Sprei <nsprei@redhat.com>
---
 block.c           |   33 +++++++++++++++++----------------
 block.h           |    4 ++--
 block/raw-posix.c |    2 +-
 block/raw-win32.c |    4 ++--
 block/vmdk.c      |    9 +++++----
 hw/xen_disk.c     |    2 +-
 monitor.c         |    2 +-
 qemu-img.c        |   10 ++++++----
 qemu-io.c         |   14 ++++++--------
 qemu-nbd.c        |    2 +-
 qemu-options.hx   |    2 +-
 vl.c              |   14 +++++++++-----
 12 files changed, 52 insertions(+), 46 deletions(-)

Comments

Kevin Wolf Jan. 14, 2010, 3:29 p.m. UTC | #1 
Am 14.01.2010 15:33, schrieb Naphtali Sprei:
> 
> Clean-up a little bit the RW related bits of BDRV_O_FLAGS.
> BDRV_O_RDONLY gone (and so is BDRV_O_ACCESS). Default value for bdrv_flags (0/zero) is READ-ONLY.
> Need to explicitly request READ-WRITE.
> 
> Instead of using the field 'readonly' of the BlockDriverState struct for passing the request,
> pass the request in the flags parameter to the function.
> 
> Signed-off-by: Naphtali Sprei <nsprei@redhat.com>

I think this is a good change in general. Note that the block.c part is
going to conflict with Christoph's cleanup patch from yesterday. You'll
probably want to rebase on it.

> diff --git a/block.h b/block.h
> index bee9ec5..a37a605 100644
> --- a/block.h
> +++ b/block.h
> @@ -27,9 +27,7 @@ typedef struct QEMUSnapshotInfo {
>      uint64_t vm_clock_nsec; /* VM clock relative to boot */
>  } QEMUSnapshotInfo;
>  
> -#define BDRV_O_RDONLY      0x0000
>  #define BDRV_O_RDWR        0x0002
> -#define BDRV_O_ACCESS      0x0003
>  #define BDRV_O_CREAT       0x0004 /* create an empty file */
>  #define BDRV_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
>  #define BDRV_O_FILE        0x0010 /* open as a raw file (do not try to
> @@ -42,6 +40,8 @@ typedef struct QEMUSnapshotInfo {
>  #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
>  
>  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)
> +#define BDRV_O_DEFAULT_OPEN (BDRV_O_RDWR)
> +#define BDRV_FLAGS_IS_RO(flags) ((flags & BDRV_O_RDWR) == 0)

I think this introduces an inconsistency: All other flags are checked
without such a macro. I think we should have it for all of them or for
none (I think I'd prefer none).

> diff --git a/block/vmdk.c b/block/vmdk.c
> index 4e48622..7f866f9 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -357,7 +357,7 @@ static int vmdk_parent_open(BlockDriverState *bs, const char * filename)
>              return -1;
>          }
>          parent_open = 1;
> -        if (bdrv_open(bs->backing_hd, parent_img_name, BDRV_O_RDONLY) < 0)
> +        if (bdrv_open(bs->backing_hd, parent_img_name, 0) < 0)
>              goto failure;
>          parent_open = 0;
>      }
> @@ -371,9 +371,10 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags)
>      uint32_t magic;
>      int l1_size, i, ret;
>  
> -    if (parent_open)
> -        // Parent must be opened as RO.
> -        flags = BDRV_O_RDONLY;
> +    if (parent_open) {
> +        /* Parent must be opened as RO, turn off RDWR bit. */
> +        flags &= ~BDRV_O_RDWR;
> +    }

I'm not familiar with the VMDK code, but to retain current behaviour
you'd need to use flags = 0. Now I'm not sure if you're introducing or
fixing a bug with your change (or maybe both at the same time).

>      ret = bdrv_file_open(&s->hd, filename, flags);
>      if (ret < 0)
> diff --git a/hw/xen_disk.c b/hw/xen_disk.c
> index 5c55251..beadf90 100644
> --- a/hw/xen_disk.c
> +++ b/hw/xen_disk.c
> @@ -613,7 +613,7 @@ static int blk_init(struct XenDevice *xendev)
>  	qflags = BDRV_O_RDWR;
>      } else {
>  	mode   = O_RDONLY;
> -	qflags = BDRV_O_RDONLY;
> +	qflags = 0;
>  	info  |= VDISK_READONLY;
>      }
>  
> diff --git a/monitor.c b/monitor.c
> index b824e7c..0f2bdca 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -916,7 +916,7 @@ static void do_change_block(Monitor *mon, const char *device,
>      }
>      if (eject_device(mon, bs, 0) < 0)
>          return;
> -    bdrv_open2(bs, filename, 0, drv);
> +    bdrv_open2(bs, filename, BDRV_O_DEFAULT_OPEN, drv);
>      monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
>  }
>  
> diff --git a/qemu-img.c b/qemu-img.c
> index 48b9315..ccfdc30 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -204,7 +204,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
>      } else {
>          drv = NULL;
>      }
> -    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
> +    if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_DEFAULT_OPEN, drv) < 0) {
>          error("Could not open '%s'", filename);
>      }
>      if (bdrv_is_encrypted(bs)) {
> @@ -468,7 +468,7 @@ static int img_commit(int argc, char **argv)
>      } else {
>          drv = NULL;
>      }
> -    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
> +    if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_RDWR, drv) < 0) {

Is there a reason why you chose to add DEFAULT_OPEN in one place and
RDWR in the other one?

> diff --git a/qemu-io.c b/qemu-io.c
> index 1c19b92..b159bc9 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -1359,10 +1359,9 @@ open_f(int argc, char **argv)
>  		}
>  	}
>  
> -	if (readonly)
> -		flags |= BDRV_O_RDONLY;
> -	else
> -		flags |= BDRV_O_RDWR;
> +	if (!readonly) {
> +            flags |= BDRV_O_RDWR;
> +        }

Indentation looks wrong.

>  
>  	if (optind != argc - 1)
>  		return command_usage(&open_cmd);
> @@ -1506,10 +1505,9 @@ int main(int argc, char **argv)
>  	add_check_command(init_check_command);
>  
>  	/* open the device */
> -	if (readonly)
> -		flags |= BDRV_O_RDONLY;
> -	else
> -		flags |= BDRV_O_RDWR;
> +	if (!readonly) {
> +            flags |= BDRV_O_RDWR;
> +        }

Same here.

>  
>  	if ((argc - optind) == 1)
>  		openfile(argv[optind], flags, growable);
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 6707ea5..0f0e2fc 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -213,7 +213,7 @@ int main(int argc, char **argv)
>      int opt_ind = 0;
>      int li;
>      char *end;
> -    int flags = 0;
> +    int flags = BDRV_O_DEFAULT_OPEN;
>      int partition = -1;
>      int ret;
>      int shared = 1;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index e2edd71..4617867 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -103,7 +103,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
>      "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
>      "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
>      "       [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
> -    "       [,addr=A][,id=name][,aio=threads|native]\n"
> +    "       [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
>      "                use 'file' as a drive image\n")
>  DEF("set", HAS_ARG, QEMU_OPTION_set,
>      "-set group.id.arg=value\n"
> diff --git a/vl.c b/vl.c
> index 4f19505..6573bb9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2210,6 +2210,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
>          *fatal_error = 0;
>          return NULL;
>      }
> +
>      bdrv_flags = 0;
>      if (snapshot) {
>          bdrv_flags |= BDRV_O_SNAPSHOT;

This hunk doesn't belong in the patch.

Kevin
diff mbox

Patch

diff --git a/block.c b/block.c
index 115e591..cbd72cc 100644
--- a/block.c
+++ b/block.c
@@ -310,7 +310,7 @@  static BlockDriver *find_image_format(const char *filename)
     if (drv && strcmp(drv->format_name, "vvfat") == 0)
         return drv;
 
-    ret = bdrv_file_open(&bs, filename, BDRV_O_RDONLY);
+    ret = bdrv_file_open(&bs, filename, 0);
     if (ret < 0)
         return NULL;
     ret = bdrv_pread(bs, 0, buf, sizeof(buf));
@@ -356,7 +356,7 @@  int bdrv_open(BlockDriverState *bs, const char *filename, int flags)
 int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
                BlockDriver *drv)
 {
-    int ret, open_flags, try_rw;
+    int ret, open_flags;
     char tmp_filename[PATH_MAX];
     char backing_filename[PATH_MAX];
 
@@ -444,21 +444,23 @@  int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
     if (flags & (BDRV_O_CACHE_WB|BDRV_O_NOCACHE))
         bs->enable_write_cache = 1;
 
-    /* Note: for compatibility, we open disk image files as RDWR, and
-       RDONLY as fallback */
-    try_rw = !bs->read_only || bs->is_temporary;
-    if (!(flags & BDRV_O_FILE))
-        open_flags = (try_rw ? BDRV_O_RDWR : 0) |
-            (flags & (BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
-    else
+    bs->read_only = BDRV_FLAGS_IS_RO(flags);
+    if (!(flags & BDRV_O_FILE)) {
+        open_flags = (flags & (BDRV_O_RDWR | BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
+        if (bs->is_temporary) { /* snapshot should be writeable */
+            open_flags |= BDRV_O_RDWR;
+        }
+    } else {
         open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
-    if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv))
+    }
+    if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
         ret = -ENOTSUP;
-    else
+    } else {
         ret = drv->bdrv_open(bs, filename, open_flags);
-    if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
-        ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
-        bs->read_only = 1;
+        if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
+            ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
+            bs->read_only = 1;
+        }
     }
     if (ret < 0) {
         qemu_free(bs->opaque);
@@ -481,14 +483,13 @@  int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
         /* if there is a backing file, use it */
         BlockDriver *back_drv = NULL;
         bs->backing_hd = bdrv_new("");
-        /* pass on read_only property to the backing_hd */
-        bs->backing_hd->read_only = bs->read_only;
         path_combine(backing_filename, sizeof(backing_filename),
                      filename, bs->backing_file);
         if (bs->backing_format[0] != '\0')
             back_drv = bdrv_find_format(bs->backing_format);
         ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags,
                          back_drv);
+        bs->backing_hd->read_only =  BDRV_FLAGS_IS_RO(open_flags);
         if (ret < 0) {
             bdrv_close(bs);
             return ret;
diff --git a/block.h b/block.h
index bee9ec5..a37a605 100644
--- a/block.h
+++ b/block.h
@@ -27,9 +27,7 @@  typedef struct QEMUSnapshotInfo {
     uint64_t vm_clock_nsec; /* VM clock relative to boot */
 } QEMUSnapshotInfo;
 
-#define BDRV_O_RDONLY      0x0000
 #define BDRV_O_RDWR        0x0002
-#define BDRV_O_ACCESS      0x0003
 #define BDRV_O_CREAT       0x0004 /* create an empty file */
 #define BDRV_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
 #define BDRV_O_FILE        0x0010 /* open as a raw file (do not try to
@@ -42,6 +40,8 @@  typedef struct QEMUSnapshotInfo {
 #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)
+#define BDRV_O_DEFAULT_OPEN (BDRV_O_RDWR)
+#define BDRV_FLAGS_IS_RO(flags) ((flags & BDRV_O_RDWR) == 0)
 
 #define BDRV_SECTOR_BITS   9
 #define BDRV_SECTOR_SIZE   (1 << BDRV_SECTOR_BITS)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 5a6a22b..b427211 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -138,7 +138,7 @@  static int raw_open_common(BlockDriverState *bs, const char *filename,
 
     s->open_flags = open_flags | O_BINARY;
     s->open_flags &= ~O_ACCMODE;
-    if ((bdrv_flags & BDRV_O_ACCESS) == BDRV_O_RDWR) {
+    if (bdrv_flags & BDRV_O_RDWR) {
         s->open_flags |= O_RDWR;
     } else {
         s->open_flags |= O_RDONLY;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 72acad5..01a6d25 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -81,7 +81,7 @@  static int raw_open(BlockDriverState *bs, const char *filename, int flags)
 
     s->type = FTYPE_FILE;
 
-    if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+    if (flags & BDRV_O_RDWR) {
         access_flags = GENERIC_READ | GENERIC_WRITE;
     } else {
         access_flags = GENERIC_READ;
@@ -337,7 +337,7 @@  static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
     }
     s->type = find_device_type(bs, filename);
 
-    if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+    if (flags & BDRV_O_RDWR) {
         access_flags = GENERIC_READ | GENERIC_WRITE;
     } else {
         access_flags = GENERIC_READ;
diff --git a/block/vmdk.c b/block/vmdk.c
index 4e48622..7f866f9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -357,7 +357,7 @@  static int vmdk_parent_open(BlockDriverState *bs, const char * filename)
             return -1;
         }
         parent_open = 1;
-        if (bdrv_open(bs->backing_hd, parent_img_name, BDRV_O_RDONLY) < 0)
+        if (bdrv_open(bs->backing_hd, parent_img_name, 0) < 0)
             goto failure;
         parent_open = 0;
     }
@@ -371,9 +371,10 @@  static int vmdk_open(BlockDriverState *bs, const char *filename, int flags)
     uint32_t magic;
     int l1_size, i, ret;
 
-    if (parent_open)
-        // Parent must be opened as RO.
-        flags = BDRV_O_RDONLY;
+    if (parent_open) {
+        /* Parent must be opened as RO, turn off RDWR bit. */
+        flags &= ~BDRV_O_RDWR;
+    }
 
     ret = bdrv_file_open(&s->hd, filename, flags);
     if (ret < 0)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 5c55251..beadf90 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -613,7 +613,7 @@  static int blk_init(struct XenDevice *xendev)
 	qflags = BDRV_O_RDWR;
     } else {
 	mode   = O_RDONLY;
-	qflags = BDRV_O_RDONLY;
+	qflags = 0;
 	info  |= VDISK_READONLY;
     }
 
diff --git a/monitor.c b/monitor.c
index b824e7c..0f2bdca 100644
--- a/monitor.c
+++ b/monitor.c
@@ -916,7 +916,7 @@  static void do_change_block(Monitor *mon, const char *device,
     }
     if (eject_device(mon, bs, 0) < 0)
         return;
-    bdrv_open2(bs, filename, 0, drv);
+    bdrv_open2(bs, filename, BDRV_O_DEFAULT_OPEN, drv);
     monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
 }
 
diff --git a/qemu-img.c b/qemu-img.c
index 48b9315..ccfdc30 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -204,7 +204,7 @@  static BlockDriverState *bdrv_new_open(const char *filename,
     } else {
         drv = NULL;
     }
-    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
+    if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_DEFAULT_OPEN, drv) < 0) {
         error("Could not open '%s'", filename);
     }
     if (bdrv_is_encrypted(bs)) {
@@ -468,7 +468,7 @@  static int img_commit(int argc, char **argv)
     } else {
         drv = NULL;
     }
-    if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
+    if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_RDWR, drv) < 0) {
         error("Could not open '%s'", filename);
     }
     ret = bdrv_commit(bs);
@@ -966,10 +966,11 @@  static int img_snapshot(int argc, char **argv)
     BlockDriverState *bs;
     QEMUSnapshotInfo sn;
     char *filename, *snapshot_name = NULL;
-    int c, ret;
+    int c, ret, bdrv_oflags;
     int action = 0;
     qemu_timeval tv;
 
+    bdrv_oflags = BDRV_O_RDWR;
     /* Parse commandline parameters */
     for(;;) {
         c = getopt(argc, argv, "la:c:d:h");
@@ -985,6 +986,7 @@  static int img_snapshot(int argc, char **argv)
                 return 0;
             }
             action = SNAPSHOT_LIST;
+            bdrv_oflags &= ~BDRV_O_RDWR; /* no need for RW */
             break;
         case 'a':
             if (action) {
@@ -1022,7 +1024,7 @@  static int img_snapshot(int argc, char **argv)
     if (!bs)
         error("Not enough memory");
 
-    if (bdrv_open2(bs, filename, 0, NULL) < 0) {
+    if (bdrv_open2(bs, filename, bdrv_oflags, NULL) < 0) {
         error("Could not open '%s'", filename);
     }
 
diff --git a/qemu-io.c b/qemu-io.c
index 1c19b92..b159bc9 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -1359,10 +1359,9 @@  open_f(int argc, char **argv)
 		}
 	}
 
-	if (readonly)
-		flags |= BDRV_O_RDONLY;
-	else
-		flags |= BDRV_O_RDWR;
+	if (!readonly) {
+            flags |= BDRV_O_RDWR;
+        }
 
 	if (optind != argc - 1)
 		return command_usage(&open_cmd);
@@ -1506,10 +1505,9 @@  int main(int argc, char **argv)
 	add_check_command(init_check_command);
 
 	/* open the device */
-	if (readonly)
-		flags |= BDRV_O_RDONLY;
-	else
-		flags |= BDRV_O_RDWR;
+	if (!readonly) {
+            flags |= BDRV_O_RDWR;
+        }
 
 	if ((argc - optind) == 1)
 		openfile(argv[optind], flags, growable);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 6707ea5..0f0e2fc 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -213,7 +213,7 @@  int main(int argc, char **argv)
     int opt_ind = 0;
     int li;
     char *end;
-    int flags = 0;
+    int flags = BDRV_O_DEFAULT_OPEN;
     int partition = -1;
     int ret;
     int shared = 1;
diff --git a/qemu-options.hx b/qemu-options.hx
index e2edd71..4617867 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -103,7 +103,7 @@  DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
     "       [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
-    "       [,addr=A][,id=name][,aio=threads|native]\n"
+    "       [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
     "                use 'file' as a drive image\n")
 DEF("set", HAS_ARG, QEMU_OPTION_set,
     "-set group.id.arg=value\n"
diff --git a/vl.c b/vl.c
index 4f19505..6573bb9 100644
--- a/vl.c
+++ b/vl.c
@@ -2210,6 +2210,7 @@  DriveInfo *drive_init(QemuOpts *opts, void *opaque,
         *fatal_error = 0;
         return NULL;
     }
+
     bdrv_flags = 0;
     if (snapshot) {
         bdrv_flags |= BDRV_O_SNAPSHOT;
@@ -2227,16 +2228,19 @@  DriveInfo *drive_init(QemuOpts *opts, void *opaque,
     }
 
     if (ro == 1) {
-        if (type == IF_IDE) {
-            fprintf(stderr, "qemu: readonly flag not supported for drive with ide interface\n");
+        if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY) {
+            fprintf(stderr, "qemu: readonly flag not supported for drive with this interface\n");
             return NULL;
         }
-        (void)bdrv_set_read_only(dinfo->bdrv, 1);
     }
-
+    /* 
+     * cdrom is read-only. Set it now, after above interface checking
+     * since readonly attribute not explicitly required, so no error.
+     */
     if (media == MEDIA_CDROM) {
-        (void)bdrv_set_read_only(dinfo->bdrv, 1);
+        ro = 1;
     }
+    bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
 
     if (bdrv_open2(dinfo->bdrv, file, bdrv_flags, drv) < 0) {
         fprintf(stderr, "qemu: could not open disk image %s: %s\n",