Re: [hatari-devel] Duplicate drive image file issue

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Hi Eero,

Yes, your patches appear to work. I'd like to suggest to be a bit more
precise in the error message. The message should not just say that locking
failed, but it should say what this means in practice, i.e. the respective
drive will not be available.

Best regards

Uwe

> Hi Uwe,
> 
> On 11/24/20 6:45 PM, Uwe Seimet wrote:
> > Before applying the patch I wanted to check the current behavior, and it
> > looks as if something else is (also) wrong: In TT mode when I use the same
> > image for IDE HD 0 and ACSI HD 0 there is no warning. But if I use the same
> > image for ACSI HD 0 and SCSI ID 0 there is a warning about Hatari not being
> > able to lock the file.
> 
> My patchset should have fixed that too.
> 
> Earlier IDE code printed that error only to
> console, my patch changes it to be an alert
> dialog, same as with ACSI/SCSI.
> 
> 
> > This is inconsistent, because whenever I try to use the same image more than
> > once there should be a warning, irrespective of the hard disk interface.
> > 
> > Maybe you can have a look at this first, your patches might change based on
> > this observation.
> 
> Attached is slightly updated patchset.
> 
> (I've fixed few typos in the messages and
> improved commit descriptions.)
> 
> Could you verify whether they fix both of
> the issues you noticed?
> 
> (I'm hoping to include them to Hatari v2.3.1
> bugfix release.)
> 
> 
> 	- Eero
> 
> 
> >> Hi Uwe,
> >>
> >> I've tested the attached patch(es) to fix
> >> the issue you found.
> >>
> >> Could you verify?
> >>
> >> (I think I'll push it only after release though.)
> >>
> >>
> >> 	- Eero
> >>
> >> On 10/30/20 9:55 AM, Uwe Seimet wrote:
> >>> Any news on this?
> >>>
> >>> Best regards
> >>>
> >>> Uwe
> >>>>
> >>>> No, I'm afraid nothing has changed, the issue is still there.
> >>>>
> >>>> Best regards
> >>>>
> >>>> Uwe
> >>   >>
> >>>>> On 11/3/19 9:33 PM, Uwe Seimet wrote:> Hi Roger,
> >>>>>>> On 3 Nov 2019 at 8:17, Uwe Seimet wrote:
> >>>>>>>>
> >>>>>>>> For IDE HD 0 I have configured a drive image to be used. When I select
> >>>>>>>> the same image file for ACSI HD 0 and reset Hatari, Hatari reports
> >>>>>>>> "Locking HD file for writing failed" and TOS only sees IDE HD 0, but not
> >>>>>>>> HD ACSI 0. So far, so good.
> >>>>>    >>>
> >>>>>>>> Now I eject the file for IDE HD 0, so that the image file is configured
> >>>>>>>> for ACSI HD 0 only. After rebooting there is no HD available for TOS
> >>>>>>>> at all, even though ACSI HD 0 is the only drive configured with an image
> >>>>>>>> file. Looks as if Hatari has not noticed that there is no image file
> >>>>>>>> conflict anymore but still ignores the (only) assigned image.
> >>>>>>>>
> >>>>>>> Are you sure this is not a byte-swapping problem?  My ACSI image and IDE image
> >>>>>>> are byte-swapped with respect to each other.
> >>>>>    >
> >>>>>    > Yes, I'm sure, because if I avoid the locking issue by applying the
> >>>>>    > change (i.e. using the image file for ACSI instead of IDE) in a single
> >>>>>    > step in the UI, everything is fine.
> >>>>>
> >>>>> I think I see the problem.  In:
> >>>>> 	change.c::Change_CopyChangedParamsToConfiguration()
> >>>>>
> >>>>> UnInit for IDE & ACSI/SCSI drives is called only if the status of drives
> >>>>> is changed *and* at least on of the changed drives is still used,
> >>>>> i.e. they're unlocked only if IDE or ACSI needs to re-initialized.
> >>>>>
> >>>>> Attached quick patch does unInit separately for them.  Does that fix
> >>>>> the problem?
> > 
> >> >From 392c4bc4d1de15bd0d0ba9c29ff41704aa703100 Mon Sep 17 00:00:00 2001
> >> From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
> >> Date: Sun, 22 Nov 2020 00:04:20 +0200
> >> Subject: [PATCH 1/3] Tell user which HD type (interface) is in question on HD
> >>   errors
> >>
> >> E.g. if user selects same HD file for multiple interfaces, it's
> >> impossible for user to know on which interface the file got rejected
> >> (due to file lock failing) unless Hatari tells it.
> >> ---
> >>   src/hdc.c          | 33 ++++++++++++++++++---------------
> >>   src/ide.c          |  4 ++--
> >>   src/includes/hdc.h |  4 ++--
> >>   src/ncr5380.c      |  2 +-
> >>   4 files changed, 23 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/src/hdc.c b/src/hdc.c
> >> index f0c728ce..509e6a36 100644
> >> --- a/src/hdc.c
> >> +++ b/src/hdc.c
> >> @@ -774,7 +774,7 @@ int HDC_PartitionCount(FILE *fp, const Uint64 tracelevel, int *pIsByteSwapped)
> >>    * Check file size for sane values (non-zero, multiple of 512),
> >>    * and return the size
> >>    */
> >> -off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize)
> >> +off_t HDC_CheckAndGetSize(const char *hdtype, const char *filename, unsigned long blockSize)
> >>   {
> >>   	off_t filesize;
> >>   	char shortname[48];
> >> @@ -784,8 +784,8 @@ off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize)
> >>   	filesize = File_Length(filename);
> >>   	if (filesize < 0)
> >>   	{
> >> -		Log_AlertDlg(LOG_ERROR, "Unable to get size of HD image file\n'%s'!",
> >> -		             shortname);
> >> +		Log_AlertDlg(LOG_ERROR, "Unable to get size of %s HD image file\n'%s'!",
> >> +		             hdtype, shortname);
> >>   		if (sizeof(off_t) < 8)
> >>   		{
> >>   			Log_Printf(LOG_ERROR, "Note: This version of Hatari has been built"
> >> @@ -796,16 +796,16 @@ off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize)
> >>   	}
> >>   	if (filesize == 0)
> >>   	{
> >> -		Log_AlertDlg(LOG_ERROR, "Can not use HD image file\n'%s'\n"
> >> +		Log_AlertDlg(LOG_ERROR, "Can not use %s HD image file\n'%s'\n"
> >>   		                        "since the file is empty.",
> >> -		             shortname);
> >> +		             hdtype, shortname);
> >>   		return -EINVAL;
> >>   	}
> >>   	if ((filesize & (blockSize - 1)) != 0)
> >>   	{
> >> -		Log_AlertDlg(LOG_ERROR, "Can not use the hard disk image file\n"
> >> +		Log_AlertDlg(LOG_ERROR, "Can not use the %s HD image file\n"
> >>   		                        "'%s'\nsince its size is not a multiple"
> >> -		                        " of %ld.", shortname, blockSize);
> >> +		                        " of %ld.", hdtype, shortname, blockSize);
> >>   		return -EINVAL;
> >>   	}
> >>   
> >> @@ -815,16 +815,16 @@ off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize)
> >>   /**
> >>    * Open a disk image file
> >>    */
> >> -int HDC_InitDevice(SCSI_DEV *dev, char *filename, unsigned long blockSize)
> >> +int HDC_InitDevice(const char *hdtype, SCSI_DEV *dev, char *filename, unsigned long blockSize)
> >>   {
> >>   	off_t filesize;
> >>   	FILE *fp;
> >>   
> >>   	dev->enabled = false;
> >> -	Log_Printf(LOG_INFO, "Mounting hard drive image '%s'\n", filename);
> >> +	Log_Printf(LOG_INFO, "Mounting %s HD image '%s'\n", hdtype, filename);
> >>   
> >>   	/* Check size for sanity */
> >> -	filesize = HDC_CheckAndGetSize(filename, blockSize);
> >> +	filesize = HDC_CheckAndGetSize(hdtype, filename, blockSize);
> >>   	if (filesize < 0)
> >>   		return filesize;
> >>   
> >> @@ -832,14 +832,17 @@ int HDC_InitDevice(SCSI_DEV *dev, char *filename, unsigned long blockSize)
> >>   	{
> >>   		if (!(fp = fopen(filename, "rb")))
> >>   		{
> >> -			Log_AlertDlg(LOG_ERROR, "Cannot open HD file for reading\n'%s'!\n", filename);
> >> +			Log_AlertDlg(LOG_ERROR, "Cannot open %s HD file for reading\n'%s'!\n",
> >> +				     hdtype, filename);
> >>   			return -ENOENT;
> >>   		}
> >> -		Log_AlertDlg(LOG_WARN, "HD file is read-only, no writes will go through\n'%s'.\n", filename);
> >> +		Log_AlertDlg(LOG_WARN, "%s HD file is read-only, no writes will go through\n'%s'.\n",
> >> +			     hdtype, filename);
> >>   	}
> >>   	else if (!File_Lock(fp))
> >>   	{
> >> -		Log_AlertDlg(LOG_ERROR, "Locking HD file for writing failed\n'%s'!\n", filename);
> >> +		Log_AlertDlg(LOG_ERROR, "Locking %s HD file for writing failed\n'%s'!\n",
> >> +			     hdtype, filename);
> >>   		fclose(fp);
> >>   		return -ENOLCK;
> >>   	}
> >> @@ -875,7 +878,7 @@ bool HDC_Init(void)
> >>   	{
> >>   		if (!ConfigureParams.Acsi[i].bUseDevice)
> >>   			continue;
> >> -		if (HDC_InitDevice(&AcsiBus.devs[i], ConfigureParams.Acsi[i].sDeviceFile, ConfigureParams.Acsi[i].nBlockSize) == 0)
> >> +		if (HDC_InitDevice("ACSI", &AcsiBus.devs[i], ConfigureParams.Acsi[i].sDeviceFile, ConfigureParams.Acsi[i].nBlockSize) == 0)
> >>   			nAcsiPartitions += HDC_PartitionCount(AcsiBus.devs[i].image_file, TRACE_SCSI_CMD, NULL);
> >>   	}
> >>   	/* set total number of partitions */
> >> @@ -1027,7 +1030,7 @@ static void Acsi_DmaTransfer(void)
> >>   			int wlen = fwrite(&STRam[nDmaAddr], 1, AcsiBus.data_len, AcsiBus.dmawrite_to_fh);
> >>   			if (wlen != AcsiBus.data_len)
> >>   			{
> >> -				Log_Printf(LOG_ERROR, "Could not write all bytes to hard disk image.\n");
> >> +				Log_Printf(LOG_ERROR, "Could not write all bytes to ACSI HD image.\n");
> >>   				AcsiBus.status = HD_STATUS_ERROR;
> >>   			}
> >>   #endif
> >> diff --git a/src/ide.c b/src/ide.c
> >> index b1955d21..55a75710 100644
> >> --- a/src/ide.c
> >> +++ b/src/ide.c
> >> @@ -560,7 +560,7 @@ static int bdrv_open(BlockDriverState *bs, const char *filename, unsigned long b
> >>   	Log_Printf(LOG_INFO, "Mounting IDE hard drive image %s\n", filename);
> >>   
> >>   	bs->read_only = 0;
> >> -	bs->file_size = HDC_CheckAndGetSize(filename, blockSize);
> >> +	bs->file_size = HDC_CheckAndGetSize("IDE", filename, blockSize);
> >>   	if (bs->file_size <= 0)
> >>   		return -1;
> >>   	if (bs->file_size < 2 * 16 * 63 * bs->sector_size)
> >> @@ -581,7 +581,7 @@ static int bdrv_open(BlockDriverState *bs, const char *filename, unsigned long b
> >>   	}
> >>   	else if (!File_Lock(bs->fhndl))
> >>   	{
> >> -		Log_Printf(LOG_ERROR, "Cannot lock HD file for writing!\n");
> >> +		Log_Printf(LOG_ERROR, "Cannot lock IDE HD file for writing!\n");
> >>   		fclose(bs->fhndl);
> >>   		bs->fhndl = NULL;
> >>   	}
> >> diff --git a/src/includes/hdc.h b/src/includes/hdc.h
> >> index df4445dc..201aa380 100644
> >> --- a/src/includes/hdc.h
> >> +++ b/src/includes/hdc.h
> >> @@ -94,12 +94,12 @@ extern bool bAcsiEmuOn;
> >>    */
> >>   extern bool HDC_Init(void);
> >>   extern void HDC_UnInit(void);
> >> -extern int HDC_InitDevice(SCSI_DEV *dev, char *filename, unsigned long blockSize);
> >> +extern int HDC_InitDevice(const char *hdtype, SCSI_DEV *dev, char *filename, unsigned long blockSize);
> >>   extern void HDC_ResetCommandStatus(void);
> >>   extern short int HDC_ReadCommandByte(int addr);
> >>   extern void HDC_WriteCommandByte(int addr, Uint8 byte);
> >>   extern int HDC_PartitionCount(FILE *fp, const Uint64 tracelevel, int *pIsByteSwapped);
> >> -extern off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize);
> >> +extern off_t HDC_CheckAndGetSize(const char *hdtype, const char *filename, unsigned long blockSize);
> >>   extern bool HDC_WriteCommandPacket(SCSI_CTRLR *ctr, Uint8 b);
> >>   extern void HDC_DmaTransfer(void);
> >>   
> >> diff --git a/src/ncr5380.c b/src/ncr5380.c
> >> index 44e6eaf7..aa8ec992 100644
> >> --- a/src/ncr5380.c
> >> +++ b/src/ncr5380.c
> >> @@ -1036,7 +1036,7 @@ bool Ncr5380_Init(void)
> >>   	{
> >>   		if (!ConfigureParams.Scsi[i].bUseDevice)
> >>   			continue;
> >> -		if (HDC_InitDevice(&ScsiBus.devs[i], ConfigureParams.Scsi[i].sDeviceFile, ConfigureParams.Scsi[i].nBlockSize) == 0)
> >> +		if (HDC_InitDevice("SCSI", &ScsiBus.devs[i], ConfigureParams.Scsi[i].sDeviceFile, ConfigureParams.Scsi[i].nBlockSize) == 0)
> >>   			nScsiPartitions += HDC_PartitionCount(ScsiBus.devs[i].image_file, TRACE_SCSI_CMD, NULL);
> >>   	}
> >>   	nNumDrives += nScsiPartitions;
> >> -- 
> >> 2.20.1
> >>
> > 
> >> >From 023f8f20c01c81f1205b18dd0502ad28214d63d4 Mon Sep 17 00:00:00 2001
> >> From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
> >> Date: Sun, 22 Nov 2020 01:10:57 +0200
> >> Subject: [PATCH 3/3] Treat IDE HD image errors similarly to ACSI/SCSI errors
> >>
> >> ---
> >>   src/ide.c | 9 ++++++++-
> >>   1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/ide.c b/src/ide.c
> >> index c66ebcc5..44dc770b 100644
> >> --- a/src/ide.c
> >> +++ b/src/ide.c
> >> @@ -576,14 +576,21 @@ static int bdrv_open(BlockDriverState *bs, const char *filename, unsigned long b
> >>   		/* Maybe the file is read-only? */
> >>   		bs->fhndl = fopen(filename, "rb");
> >>   		if (!bs->fhndl)
> >> +		{
> >>   			perror("bdrv_open");
> >> +			Log_AlertDlg(LOG_WARN, "IDE HD file open failed\n'%s'.\n", filename);
> >> +			return -1;
> >> +		}
> >> +		Log_AlertDlg(LOG_WARN, "IDE HD file is read-only, no writes will go through\n'%s'.\n",
> >> +			     filename);
> >>   		bs->read_only = 1;
> >>   	}
> >>   	else if (!File_Lock(bs->fhndl))
> >>   	{
> >> -		Log_Printf(LOG_ERROR, "Cannot lock IDE HD file for writing!\n");
> >> +		Log_AlertDlg(LOG_ERROR, "Locking ID HD file for writing failed\n'%s'!\n", filename);
> >>   		fclose(bs->fhndl);
> >>   		bs->fhndl = NULL;
> >> +		return -1;
> >>   	}
> >>   
> >>   	/* call the change callback */
> >> -- 
> >> 2.20.1
> >>
> > 
> >> >From 033e33310ec78f0390d151a5bbffa1ff1593704a Mon Sep 17 00:00:00 2001
> >> From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
> >> Date: Sun, 22 Nov 2020 00:48:38 +0200
> >> Subject: [PATCH 2/3] Disable failing HD images
> >>
> >> Fixes following issue:
> >> * User starts hatari with IDE: hatari --ide-master disk.img
> >> * User selects same image also for another interface in GUI,
> >>    e.g. for ACSI, and proceeds
> >>    -> Hatari ACSI file locking fails, and it's not mounted
> >> * User ejects IDE image from GUI, leaves ACSI image as-is and proceeds
> >>
> >> => Hatari won't (re-)initialize ACSI as its config did not change, and
> >>     as result ACSI drive won't be mounted to emulation, although it's
> >>     still visible in the GUI as being mounted.
> >>
> >> After the change, ACSI image name isn't anymore shown in the GUI after
> >> failure, as it's now disabled.  When user browses it again, he can
> >> just OK the old name, one doesn't need to search for it.
> >> ---
> >>   src/hdc.c     | 2 ++
> >>   src/ide.c     | 6 +++++-
> >>   src/ncr5380.c | 2 ++
> >>   3 files changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/hdc.c b/src/hdc.c
> >> index 509e6a36..8989add2 100644
> >> --- a/src/hdc.c
> >> +++ b/src/hdc.c
> >> @@ -880,6 +880,8 @@ bool HDC_Init(void)
> >>   			continue;
> >>   		if (HDC_InitDevice("ACSI", &AcsiBus.devs[i], ConfigureParams.Acsi[i].sDeviceFile, ConfigureParams.Acsi[i].nBlockSize) == 0)
> >>   			nAcsiPartitions += HDC_PartitionCount(AcsiBus.devs[i].image_file, TRACE_SCSI_CMD, NULL);
> >> +		else
> >> +			ConfigureParams.Acsi[i].bUseDevice = false;
> >>   	}
> >>   	/* set total number of partitions */
> >>   	nNumDrives += nAcsiPartitions;
> >> diff --git a/src/ide.c b/src/ide.c
> >> index 55a75710..c66ebcc5 100644
> >> --- a/src/ide.c
> >> +++ b/src/ide.c
> >> @@ -2733,7 +2733,11 @@ void Ide_Init(void)
> >>   		if (ConfigureParams.Ide[i].bUseDevice)
> >>   		{
> >>   			int is_byteswap;
> >> -			bdrv_open(hd_table[i], ConfigureParams.Ide[i].sDeviceFile, ConfigureParams.Ide[i].nBlockSize, 0);
> >> +			if (bdrv_open(hd_table[i], ConfigureParams.Ide[i].sDeviceFile, ConfigureParams.Ide[i].nBlockSize, 0) < 0)
> >> +			{
> >> +				ConfigureParams.Ide[i].bUseDevice = false;
> >> +				continue;
> >> +			}
> >>   			nIDEPartitions += HDC_PartitionCount(hd_table[i]->fhndl, TRACE_IDE, &is_byteswap);
> >>   			/* Our IDE implementation is little endian by default,
> >>   			 * so we need to byteswap if the image is not swapped! */
> >> diff --git a/src/ncr5380.c b/src/ncr5380.c
> >> index aa8ec992..4c6b7f39 100644
> >> --- a/src/ncr5380.c
> >> +++ b/src/ncr5380.c
> >> @@ -1038,6 +1038,8 @@ bool Ncr5380_Init(void)
> >>   			continue;
> >>   		if (HDC_InitDevice("SCSI", &ScsiBus.devs[i], ConfigureParams.Scsi[i].sDeviceFile, ConfigureParams.Scsi[i].nBlockSize) == 0)
> >>   			nScsiPartitions += HDC_PartitionCount(ScsiBus.devs[i].image_file, TRACE_SCSI_CMD, NULL);
> >> +		else
> >> +			ConfigureParams.Scsi[i].bUseDevice = false;
> >>   	}
> >>   	nNumDrives += nScsiPartitions;
> >>   	if (nScsiPartitions)
> >> -- 
> >> 2.20.1
> >>
> > 
> > 
> > 

> >From 5296d0123b4b0822d5563cf28fbd2d64122b9c21 Mon Sep 17 00:00:00 2001
> From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
> Date: Sun, 22 Nov 2020 00:48:38 +0200
> Subject: [PATCH 3/3] Disable failing HD images
> 
> Fixes following issue:
> * User starts hatari with IDE: hatari --ide-master disk.img
> * User selects same image also for another interface in GUI,
>   e.g. for ACSI, and proceeds
>   -> Hatari ACSI file locking fails, and it's not mounted
> * User ejects IDE image from GUI, leaves ACSI image as-is and proceeds
> 
> => Hatari won't (re-)initialize ACSI as its config did not change, and
>    as result ACSI drive won't be mounted to emulation, although it's
>    still visible in the GUI as being mounted.
> 
> After the change, ACSI image name isn't anymore shown in the GUI after
> failure, as it's now disabled.  When user browses it again, he can
> just OK the old name, one doesn't need to search for it.
> ---
>  src/hdc.c     | 2 ++
>  src/ide.c     | 6 +++++-
>  src/ncr5380.c | 2 ++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/hdc.c b/src/hdc.c
> index edf9421a..e6f3488c 100644
> --- a/src/hdc.c
> +++ b/src/hdc.c
> @@ -883,6 +883,8 @@ bool HDC_Init(void)
>  			nAcsiPartitions += HDC_PartitionCount(AcsiBus.devs[i].image_file, TRACE_SCSI_CMD, NULL);
>  			bAcsiEmuOn = true;
>  		}
> +		else
> +			ConfigureParams.Acsi[i].bUseDevice = false;
>  	}
>  	/* set total number of partitions */
>  	nNumDrives += nAcsiPartitions;
> diff --git a/src/ide.c b/src/ide.c
> index 61d4cf69..1563fd89 100644
> --- a/src/ide.c
> +++ b/src/ide.c
> @@ -2740,7 +2740,11 @@ void Ide_Init(void)
>  		if (ConfigureParams.Ide[i].bUseDevice)
>  		{
>  			int is_byteswap;
> -			bdrv_open(hd_table[i], ConfigureParams.Ide[i].sDeviceFile, ConfigureParams.Ide[i].nBlockSize, 0);
> +			if (bdrv_open(hd_table[i], ConfigureParams.Ide[i].sDeviceFile, ConfigureParams.Ide[i].nBlockSize, 0) < 0)
> +			{
> +				ConfigureParams.Ide[i].bUseDevice = false;
> +				continue;
> +			}
>  			nIDEPartitions += HDC_PartitionCount(hd_table[i]->fhndl, TRACE_IDE, &is_byteswap);
>  			/* Our IDE implementation is little endian by default,
>  			 * so we need to byteswap if the image is not swapped! */
> diff --git a/src/ncr5380.c b/src/ncr5380.c
> index 4d901ff6..f36c3de5 100644
> --- a/src/ncr5380.c
> +++ b/src/ncr5380.c
> @@ -1041,6 +1041,8 @@ bool Ncr5380_Init(void)
>  			nScsiPartitions += HDC_PartitionCount(ScsiBus.devs[i].image_file, TRACE_SCSI_CMD, NULL);
>  			bScsiEmuOn = true;
>  		}
> +		else
> +			ConfigureParams.Scsi[i].bUseDevice = false;
>  	}
>  	nNumDrives += nScsiPartitions;
>  #endif
> -- 
> 2.20.1
> 

> >From 3846b08206a59471169f799824483ec4d28c7af9 Mon Sep 17 00:00:00 2001
> From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
> Date: Sun, 22 Nov 2020 01:10:57 +0200
> Subject: [PATCH 2/3] Fail IDE on IDE disk open errors and tell user about it
> 
> Makes IDE image opening related errors work similarly to how they're
> handled with ACSI/SCSI.
> ---
>  src/ide.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ide.c b/src/ide.c
> index 55a75710..61d4cf69 100644
> --- a/src/ide.c
> +++ b/src/ide.c
> @@ -576,14 +576,21 @@ static int bdrv_open(BlockDriverState *bs, const char *filename, unsigned long b
>  		/* Maybe the file is read-only? */
>  		bs->fhndl = fopen(filename, "rb");
>  		if (!bs->fhndl)
> +		{
>  			perror("bdrv_open");
> +			Log_AlertDlg(LOG_ERROR, "Cannot open IDE HD for reading\n'%s'.\n", filename);
> +			return -1;
> +		}
> +		Log_AlertDlg(LOG_WARN, "IDE HD file is read-only, no writes will go through\n'%s'.\n",
> +			     filename);
>  		bs->read_only = 1;
>  	}
>  	else if (!File_Lock(bs->fhndl))
>  	{
> -		Log_Printf(LOG_ERROR, "Cannot lock IDE HD file for writing!\n");
> +		Log_AlertDlg(LOG_ERROR, "Locking IDE HD file for writing failed\n'%s'!\n", filename);
>  		fclose(bs->fhndl);
>  		bs->fhndl = NULL;
> +		return -1;
>  	}
>  
>  	/* call the change callback */
> -- 
> 2.20.1
> 

> >From fc8fa9344671e1a8e769e722f3c4f39a76bd84bc Mon Sep 17 00:00:00 2001
> From: Eero Tamminen <oak@xxxxxxxxxxxxxx>
> Date: Sun, 22 Nov 2020 00:04:20 +0200
> Subject: [PATCH 1/3] Tell user which HD type (interface) is in question on HD
>  errors
> 
> E.g. if user selects same HD file for multiple interfaces, it's
> impossible for user to know on which interface the file got rejected
> (due to file lock failing) unless Hatari tells it.
> ---
>  src/hdc.c          | 33 ++++++++++++++++++---------------
>  src/ide.c          |  4 ++--
>  src/includes/hdc.h |  4 ++--
>  src/ncr5380.c      |  2 +-
>  4 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/src/hdc.c b/src/hdc.c
> index 64766e57..edf9421a 100644
> --- a/src/hdc.c
> +++ b/src/hdc.c
> @@ -774,7 +774,7 @@ int HDC_PartitionCount(FILE *fp, const Uint64 tracelevel, int *pIsByteSwapped)
>   * Check file size for sane values (non-zero, multiple of 512),
>   * and return the size
>   */
> -off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize)
> +off_t HDC_CheckAndGetSize(const char *hdtype, const char *filename, unsigned long blockSize)
>  {
>  	off_t filesize;
>  	char shortname[48];
> @@ -784,8 +784,8 @@ off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize)
>  	filesize = File_Length(filename);
>  	if (filesize < 0)
>  	{
> -		Log_AlertDlg(LOG_ERROR, "Unable to get size of HD image file\n'%s'!",
> -		             shortname);
> +		Log_AlertDlg(LOG_ERROR, "Unable to get size of %s HD image file\n'%s'!",
> +		             hdtype, shortname);
>  		if (sizeof(off_t) < 8)
>  		{
>  			Log_Printf(LOG_ERROR, "Note: This version of Hatari has been built"
> @@ -796,16 +796,16 @@ off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize)
>  	}
>  	if (filesize == 0)
>  	{
> -		Log_AlertDlg(LOG_ERROR, "Can not use HD image file\n'%s'\n"
> +		Log_AlertDlg(LOG_ERROR, "Can not use %s HD image file\n'%s'\n"
>  		                        "since the file is empty.",
> -		             shortname);
> +		             hdtype, shortname);
>  		return -EINVAL;
>  	}
>  	if ((filesize & (blockSize - 1)) != 0)
>  	{
> -		Log_AlertDlg(LOG_ERROR, "Can not use the hard disk image file\n"
> +		Log_AlertDlg(LOG_ERROR, "Can not use the %s HD image file\n"
>  		                        "'%s'\nsince its size is not a multiple"
> -		                        " of %ld.", shortname, blockSize);
> +		                        " of %ld.", hdtype, shortname, blockSize);
>  		return -EINVAL;
>  	}
>  
> @@ -815,16 +815,16 @@ off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize)
>  /**
>   * Open a disk image file
>   */
> -int HDC_InitDevice(SCSI_DEV *dev, char *filename, unsigned long blockSize)
> +int HDC_InitDevice(const char *hdtype, SCSI_DEV *dev, char *filename, unsigned long blockSize)
>  {
>  	off_t filesize;
>  	FILE *fp;
>  
>  	dev->enabled = false;
> -	Log_Printf(LOG_INFO, "Mounting hard drive image '%s'\n", filename);
> +	Log_Printf(LOG_INFO, "Mounting %s HD image '%s'\n", hdtype, filename);
>  
>  	/* Check size for sanity */
> -	filesize = HDC_CheckAndGetSize(filename, blockSize);
> +	filesize = HDC_CheckAndGetSize(hdtype, filename, blockSize);
>  	if (filesize < 0)
>  		return filesize;
>  
> @@ -832,14 +832,17 @@ int HDC_InitDevice(SCSI_DEV *dev, char *filename, unsigned long blockSize)
>  	{
>  		if (!(fp = fopen(filename, "rb")))
>  		{
> -			Log_AlertDlg(LOG_ERROR, "Cannot open HD file for reading\n'%s'!\n", filename);
> +			Log_AlertDlg(LOG_ERROR, "Cannot open %s HD file for reading\n'%s'!\n",
> +				     hdtype, filename);
>  			return -ENOENT;
>  		}
> -		Log_AlertDlg(LOG_WARN, "HD file is read-only, no writes will go through\n'%s'.\n", filename);
> +		Log_AlertDlg(LOG_WARN, "%s HD file is read-only, no writes will go through\n'%s'.\n",
> +			     hdtype, filename);
>  	}
>  	else if (!File_Lock(fp))
>  	{
> -		Log_AlertDlg(LOG_ERROR, "Locking HD file for writing failed\n'%s'!\n", filename);
> +		Log_AlertDlg(LOG_ERROR, "Locking %s HD file for writing failed\n'%s'!\n",
> +			     hdtype, filename);
>  		fclose(fp);
>  		return -ENOLCK;
>  	}
> @@ -875,7 +878,7 @@ bool HDC_Init(void)
>  	{
>  		if (!ConfigureParams.Acsi[i].bUseDevice)
>  			continue;
> -		if (HDC_InitDevice(&AcsiBus.devs[i], ConfigureParams.Acsi[i].sDeviceFile, ConfigureParams.Acsi[i].nBlockSize) == 0)
> +		if (HDC_InitDevice("ACSI", &AcsiBus.devs[i], ConfigureParams.Acsi[i].sDeviceFile, ConfigureParams.Acsi[i].nBlockSize) == 0)
>  		{
>  			nAcsiPartitions += HDC_PartitionCount(AcsiBus.devs[i].image_file, TRACE_SCSI_CMD, NULL);
>  			bAcsiEmuOn = true;
> @@ -1028,7 +1031,7 @@ static void Acsi_DmaTransfer(void)
>  			int wlen = fwrite(&STRam[nDmaAddr], 1, AcsiBus.data_len, AcsiBus.dmawrite_to_fh);
>  			if (wlen != AcsiBus.data_len)
>  			{
> -				Log_Printf(LOG_ERROR, "Could not write all bytes to hard disk image.\n");
> +				Log_Printf(LOG_ERROR, "Could not write all bytes to ACSI HD image.\n");
>  				AcsiBus.status = HD_STATUS_ERROR;
>  			}
>  #endif
> diff --git a/src/ide.c b/src/ide.c
> index b1955d21..55a75710 100644
> --- a/src/ide.c
> +++ b/src/ide.c
> @@ -560,7 +560,7 @@ static int bdrv_open(BlockDriverState *bs, const char *filename, unsigned long b
>  	Log_Printf(LOG_INFO, "Mounting IDE hard drive image %s\n", filename);
>  
>  	bs->read_only = 0;
> -	bs->file_size = HDC_CheckAndGetSize(filename, blockSize);
> +	bs->file_size = HDC_CheckAndGetSize("IDE", filename, blockSize);
>  	if (bs->file_size <= 0)
>  		return -1;
>  	if (bs->file_size < 2 * 16 * 63 * bs->sector_size)
> @@ -581,7 +581,7 @@ static int bdrv_open(BlockDriverState *bs, const char *filename, unsigned long b
>  	}
>  	else if (!File_Lock(bs->fhndl))
>  	{
> -		Log_Printf(LOG_ERROR, "Cannot lock HD file for writing!\n");
> +		Log_Printf(LOG_ERROR, "Cannot lock IDE HD file for writing!\n");
>  		fclose(bs->fhndl);
>  		bs->fhndl = NULL;
>  	}
> diff --git a/src/includes/hdc.h b/src/includes/hdc.h
> index df4445dc..201aa380 100644
> --- a/src/includes/hdc.h
> +++ b/src/includes/hdc.h
> @@ -94,12 +94,12 @@ extern bool bAcsiEmuOn;
>   */
>  extern bool HDC_Init(void);
>  extern void HDC_UnInit(void);
> -extern int HDC_InitDevice(SCSI_DEV *dev, char *filename, unsigned long blockSize);
> +extern int HDC_InitDevice(const char *hdtype, SCSI_DEV *dev, char *filename, unsigned long blockSize);
>  extern void HDC_ResetCommandStatus(void);
>  extern short int HDC_ReadCommandByte(int addr);
>  extern void HDC_WriteCommandByte(int addr, Uint8 byte);
>  extern int HDC_PartitionCount(FILE *fp, const Uint64 tracelevel, int *pIsByteSwapped);
> -extern off_t HDC_CheckAndGetSize(const char *filename, unsigned long blockSize);
> +extern off_t HDC_CheckAndGetSize(const char *hdtype, const char *filename, unsigned long blockSize);
>  extern bool HDC_WriteCommandPacket(SCSI_CTRLR *ctr, Uint8 b);
>  extern void HDC_DmaTransfer(void);
>  
> diff --git a/src/ncr5380.c b/src/ncr5380.c
> index b43eed3e..4d901ff6 100644
> --- a/src/ncr5380.c
> +++ b/src/ncr5380.c
> @@ -1036,7 +1036,7 @@ bool Ncr5380_Init(void)
>  	{
>  		if (!ConfigureParams.Scsi[i].bUseDevice)
>  			continue;
> -		if (HDC_InitDevice(&ScsiBus.devs[i], ConfigureParams.Scsi[i].sDeviceFile, ConfigureParams.Scsi[i].nBlockSize) == 0)
> +		if (HDC_InitDevice("SCSI", &ScsiBus.devs[i], ConfigureParams.Scsi[i].sDeviceFile, ConfigureParams.Scsi[i].nBlockSize) == 0)
>  		{
>  			nScsiPartitions += HDC_PartitionCount(ScsiBus.devs[i].image_file, TRACE_SCSI_CMD, NULL);
>  			bScsiEmuOn = true;
> -- 
> 2.20.1
> 




Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/