Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CA-384030 Ignore awkardly named images in ISO SRs #652

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

rdn32
Copy link
Contributor

@rdn32 rdn32 commented Oct 23, 2023

  • Use os.listdir rather than util.listdir for finding images in an SMB share. I don't really know why the latter exists, but I didn't want to get side-tracked by the question of whether it could be eliminated or (if not) how it should best handle non-ascii characters in filenames - especially as I doubt that's ever an issue outside of ISOSR.py.
  • Refactor the identification of images so that finding and filtering are in a single function.
  • When looking for image files in a mounted directory, ignore ones with problematic names. This covers non-utf-8 names (which will always be a problem, but that aren't likely to occur in this context), and names that have had their utf-8 encodings mangled by Python's filesystem encodings (which I'm anticipating will stop being a problem with Python 3.7 and later).
  • When an ISO SR is being created, look to see if there are image files with problematic names, but if there are create an alert message rather than refuse to create the SR.

Signed-off-by: Robin Newton robin.newton@cloud.com

    
* Use os.listdir rather than util.listdir for finding images in an SMB share.
  I don't really know why the latter exists, but I didn't want to get
  side-tracked by the question of whether it could be eliminated or (if not)
  how it should best handle non-ascii characters in filenames - especially as I
  doubt that's ever an issue outside of ISOSR.py.
* Refactor the identification of images so that finding and filtering are in a
  single function.
* When looking for image files in a mounted directory, ignore ones with
  problematic names. This covers non-utf-8 names (which will always be a
  problem, but that aren't likely to occur in this context), and names
  that have had their utf-8 encodings mangled by Python's filesystem
  encodings (which I'm anticipating will stop being a problem with Python
  3.7 and later).
* When an ISO SR is being created, look to see if there are image files
  with problematic names, but if there are create an alert message
  rather than refuse to create the SR.
    
Signed-off-by: Robin Newton <robin.newton@cloud.com>
"4",
"SR",
self.uuid,
"Ignored disk image file(s) due to"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should include the num_images_ignored in the message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to do that, but I didn't know how well that would play with message localization - given that XenCenter would presumably have to parse the message and then recreate a localized version of it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XC only supports English now, localization was removed

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless. Messages should be internationalized.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't worry so much about the message text itself because the UI can always provide an override for it, but if we want the UI to show the number too, it would be useful to include it in the message in a form that the UI can parse it easily. For example, there are some messages with more complex info/more parameters that include the message body as xml - but this may be too much here. A simpler idea would be to use this message: N ignored disk image file(s) due to file name encoding issues, so the UI expects that the first word of this message is always the number, so it can fish it out easily.

@MarkSymsCtx MarkSymsCtx merged commit 797e0dc into xapi-project:master Oct 26, 2023
2 checks passed
@rdn32 rdn32 deleted the private/rnewton/CA-384030 branch February 15, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants