Re: [Libcdio-devel] Re: Burned CD-RW and DVD+RW via libcdio MMC interface

[Rocky Bernstein]

On Sun, Dec 20, 2009 at 3:30 AM, Thomas Schmitt <address@hidden> wrote:

> ...
>
> They were produced by diff -puN.
> I rarely submit patches.
> Any other command preferred for generating them ?
>

The problem isn't the way it is generated, just the way it is transmitted.
Don't attach as in-line text. A file attachment preserves data integrity.
Or attach as file to the Savannah patch system.
https://savannah.gnu.org/patch/?group=libcdio


>
> > The question I
> > have is whether run_mmc_linux after the patch *now* does something
> > reasonable (or not incorrect) when e_direction parameter is
> > SCSI_MMC_DATA_NONE ?
>
> It works with all commands which i send out of
> ...
> So it seems wise to use them with commands which
> do not transfer payload according to SCSI specs.
>

So is it advisable to add a check to run_mmc_cmd_linux? Perhaps that the
length of i_buf is 0?


> growisofs uses above NONE directions as default
> if no READ or WRITE is set explicitely.
> libburn uses them too. No problems known.
>
>
> > Clearly opening read-only *when that is all you need* is
> > probably a good thing.
> ...
>
> How about a mode "MMC_RDWR" which opens for
> writing and uses READ(10) or READ(12) via
> mmc_run_cmd() for libcdio read functions ?
>

I've added that to gnu_linux.c. It has only been minimally tested.


>
> On Linux it should open O_RDWR|O_EXCL for
> getting hopefully exclusive access.
>

Ok. That changed. But again check over.

>
> > whether to use blocking or non-blocking mode.
>
> I too wonder whether O_NONBLOCK is needed.
> I inherited it with the Linux adapter of libburn
> and had no reason to disable it yet.
>
>
Time will tell, I guess.
...

An allocation of responsibilities could be: ..
>

Sure, I think this is fine. In places where common code is shared between
libburn and libcdio, I'd like to see that be put out as a separate library
and used by both projects.
I don't really care if the library lives inside the version control of one
project or another; just that the part of it (without the other unneeded
stuff from the standpoint of the other project) is accessible on its own.
.

>
> To do for that:
>
> - libburnia develops a system adapter sg-libcdio
>  which can be chosen by ./configure. It will be
>  default on operating systems where libcdio is
>  available and libburn has no matching
>  sg-adapter.
>

Great! Thanks!


>
> - libcdio learns to handle BD and to recognize
>  emulated ISO 9660 multi session.
>

Sure that sounds reasonable. I don't know that we have someone devoted to
it.


>  (How is ISO 9660 multi-session support
>   anyway ? Something like on Linux
>     mount -o sbsector=...
>  )
>

I don't know.


>  (cd-info recognizes ISO 9660 on DVD+RW but not
>   on BD-RE or BD-ROM:
>     $ cd-info -i /dev/sr1 --dvd
>     ...
>     ++ WARN: error in ioctl CDROMREADTOCENTRY for lead-out: Input/output
> error
>   Can it be libcdio tries to handle the BD
>   as CD ?


Probably.


> Command READ TOC/PMA/ATIP is of very
>   limited use with BD. READ DISC INFORMATION
>   and READ TRACK INFORMATION work ok.
>  )
>
>
 Ok. Suggest a patch or make a change to git.

- libcdio learns to read ACL and xattr from
>  eventual AAIP entries in ISO 9660 images.
>

Sure, that's fine. Again it is more a matter of someone to work on ;-)


> ...
> For the sg-libcdio system adapter there is
> one important open point left:
>
> libburn needs a way to learn about the SCSI
> sense replies of the commands.
> At least on Linux and FreeBSD they are
> available. But libcdio does not forward them
> to its apps.
>

Yes, this is an omission or if you prefer, a bug.


> Yesterday i failed to implement a function
> which hands out the most recent sense reply.
> I have to find a place where to store it
> from  run_mmc_cmd_linux() which only has a
> _img_private_t and to later read it via the
> CdIo_t which owns that _img_private_t.
> There is a void pointer gap.
>
> I naively introduced in lib/driver/generic.h:
>
>  typedef struct {
>  ...
>    driver_return_code_t scsi_mmc_driver_ret;
>    unsigned char scsi_mmc_sense[18];
>  } generic_img_private_t;
>
> in lib/driver/gnu_linux.c
>
>    int i_rc = ioctl (p_env->gen.fd, CDROM_SEND_PACKET, &cgc);
>
> +   memcpy((void *) p_env->gen.scsi_mmc_sense, cgc.sense,
> +           sizeof(p_env->gen.scsi_mmc_sense));
>
> and in lib/driver/mmc.c
>
>  /** Obtain transport return code and SCSI sense of the most
>      recently performed MMC command.
>      @param sense  See SPC-3 4.5.3 Fixed format sense data
>                    SCSI error codes: sense[2]=Key, [12]=ASC, [13]=ASCQ
>      @return same value as with most recent MMC command
>  */
>  mmc_get_cmd_scsi_sense( const CdIo_t *p_cdio, unsigned char sense[18])
>  {
>    int len = 18;
>    unsigned char s;
>
>    if (!p_cdio) return DRIVER_OP_UNINIT;
>    memset(sense, 0, len);
>    s = p_cdio->env->gen.scsi_mmc_sense;
>    if(sizeof(s) < 18)
>      len = sizeof(s);
>    memcpy(sense, s, len);
>    return p_cdio->env->gen.scsi_mmc_driver_ret;
>  }
>
> But
>  s = p_cdio->env->gen.scsi_mmc_sense;
> yields in gcc
>  mmc.c:979: warning: dereferencing 'void *' pointer
>  mmc.c:979: error: request for member 'gen' in something
>                    not a structure or union
>
> So i would need some architectural advise here.
>

Use a cast or declare a variable of type generic_img_private_t on
p_cdio->env->gen. I think something like:
   generic_img_private_t *p_gen = p_cdio->env->gen.



>
> Shall one rather introduce a new
> mmc_run_cmd_sense() which performs a command and
> returns the sense bytes as parameter ?
>

That would be fine too. Seems a little cleaner, no?


>
> ------------------------------------------------
>
> Whew. Time for breakfast.
> I am sure i forgot some important topic.
>
>
> Have a nice day :)
>
> Thomas
>
>
>
>